FIX - PDF minor version number checking. #317
Conversation
- added test for invalid minor version number; - added MessageConstant for invalid minor version; - bumped PDF Module release details for release candidate; - refactored `PdfHeader` class; - JavaDoc for `PdfHeader`; - unit tests and test resources for `PdfHeader`; - added JHOVE RC PDF Module details to baseline; and - improved result logging for Travis tests. Closes #207
Codecov Report
@@ Coverage Diff @@
## integration #317 +/- ##
=================================================
+ Coverage 43.47% 43.52% +0.05%
- Complexity 3308 3321 +13
=================================================
Files 366 367 +1
Lines 30123 30148 +25
Branches 5959 5962 +3
=================================================
+ Hits 13096 13122 +26
+ Misses 14630 14627 -3
- Partials 2397 2399 +2
Continue to review full report at Codecov.
|
| _pdfACompliant = false; | ||
| } | ||
| return true; | ||
| if (!header.isVersionValid()) { |
There was a problem hiding this comment.
Mixed indentation white-space.
| @@ -0,0 +1,184 @@ | |||
| /** | |||
| * | |||
| */ | |||
| try { | ||
| minorVersion = getMinorVersion(this.versionString); | ||
| } catch (NumberFormatException nfe) { | ||
| // TODO : Do nothing for now, the version number is still invalid. |
There was a problem hiding this comment.
The aim is to not break existing behaviour. This ensures that this case is handled as before as the minor version number is set too large and false is returned. I've left the TODO as I'm intending to polish this a little more, but it needs a new error messages and is outside of the PR scope.
There was a problem hiding this comment.
I wonder if in the long term view it is as easy as thinking about the minor version not being set too large, especially with PDF 2.0 now available. Couldn't it be instead checking the versionString against a list of currently available versions? So, as of today: 1.0, 1.1, 1.2, 1.3, ,1.4, 1.5., 1.6, 1.7, 2.0 ?
Downside is that the list of available versions would have to be updated everytime a new version becomes available - but if the code instead states that the max correct minor version is 7, which is true for PDF 1.x, it would have to still be updated regarding the limitation of minor version for 2.x as there currently only is 2.0 (and this would again have to be updated once 2.1 becomes available).
Does that make any sense?
| @@ -0,0 +1,101 @@ | |||
| /** | |||
| * | |||
| */ | |||
- replaced inconistent tabs with spaces; and - removed empty JavaDoc headers. Thanks to @david-russo for review. Fixed the header JavaDoc generation on my IDE now also.
PdfHeaderclass;PdfHeader;PdfHeader;Closes PDF version checking not correct #207