Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX - PDF module, various processing errors #322

Merged
merged 21 commits into from Mar 29, 2018

Conversation

carlwilson
Copy link
Member

@carlwilson carlwilson commented Mar 19, 2018

- fixed spacing errors; and
- merged test data and utils.
- added basic JavaDoc to `TestUtils`;
- added extra convenience method for file testing; and
- fixed whitespace issues.
- now uses `TestUtils`; and
- basic JavaDoc.
- added test suite to run individual aspect tests; and
- tests re-named by PDF Feature, e.g. `HeaderTests`.
- added complete set of iPres corpus header tests to resource folder;
- added full set of iPres corpus header unit tests;
- changed name of minimal valid test file to T00_000_minimal-valid.pdf;
- moved header test files to dedicated folder;
- fixed whitespace issues; and
- JavaDoc for tests.
- added all iPres corpus document catalog test cases;
- unit tests for document catalog iPres Corpus resources;
- fixed duplicate error message for document catalog type; and
- moved document catalog test cases to specific folder.
- added explicit catch for missing pages dictionary lookup; and
- amended test message value.
- added \Type key value check for Pages Dictionary;
- added iPres corpus test files for page tree conditions to test resources;
- unit tests for page tree conditions.
- added MessageConstants for page dict issues;
- factored out type check for PDF dictionary;
- changed message constant ERR_DOC_CAT_TYPE_NO_CAT to ERR_DOC_CAT_TYPE_INVALID; and
- dedicated folder for page tree test resources.
- added catch for missing document page node.
- included page tree tests in PDF Module test suite class;
- corrected date for RC PDF Module release; and
- fixed spacing issues.
@carlwilson carlwilson added this to the Release v1.20 milestone Mar 19, 2018
@carlwilson carlwilson self-assigned this Mar 19, 2018
@codecov
Copy link

codecov bot commented Mar 19, 2018

Codecov Report

Merging #322 into integration will increase coverage by 0.03%.
The diff coverage is 84%.

Impacted file tree graph

@@                Coverage Diff                @@
##             integration     #322      +/-   ##
=================================================
+ Coverage          44.45%   44.49%   +0.03%     
- Complexity          3415     3419       +4     
=================================================
  Files                368      368              
  Lines              30318    30330      +12     
  Branches            5994     5995       +1     
=================================================
+ Hits               13478    13495      +17     
+ Misses             14430    14426       -4     
+ Partials            2410     2409       -1
Impacted Files Coverage Δ Complexity Δ
...du/harvard/hul/ois/jhove/module/pdf/PdfHeader.java 68.96% <ø> (+1.72%) 12 <0> (+1) ⬆️
...ard/hul/ois/jhove/module/pdf/MessageConstants.java 0% <ø> (ø) 0 <0> (ø) ⬇️
...harvard/hul/ois/jhove/module/pdf/PageTreeNode.java 79.2% <100%> (+4.46%) 21 <0> (+1) ⬆️
...va/edu/harvard/hul/ois/jhove/module/PdfModule.java 70.96% <82.6%> (+0.14%) 292 <3> (+2) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6304ee3...de65442. Read the comment docs.

@@ -0,0 +1,184 @@
/**
*
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty Javadoc.


/**
*
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty Javadoc.

try {
minorVersion = getMinorVersion(this.versionString);
} catch (NumberFormatException nfe) {
// TODO : Do nothing for now, the version number is still invalid.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe remove the TODO here since there's no instructions on what needs to be done differently in the future, and it's a special keyword for many IDEs and programmers to indicate such.

public static final String PDF_VER1_HEADER_PREFIX = "PDF-1."; //$NON-NLS-1$
public static final String PDF_SIG_HEADER = "%" + PDF_VER1_HEADER_PREFIX; //$NON-NLS-1$
public static final String POSTSCRIPT_HEADER_PREFIX = "!PS-Adobe-"; //$NON-NLS-1$
public static final int MAX_VALID_MAJOR_VERSION = 7;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAX_VALID_MAJOR_VERSION looks like it should be MAX_VALID_MINOR_VERSION.

@@ -0,0 +1,166 @@
/**
*
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty Javadoc.

…at-and-pagedict

- PDF Module and test classes conflicts; and
- test script conflicts.
@carlwilson carlwilson merged commit 9480d35 into integration Mar 29, 2018
@carlwilson carlwilson deleted the fix/pdf-head-cat-and-pagedict branch March 29, 2018 04:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment