Skip to content

JCRVLT-637#228

Merged
kwin merged 4 commits intomasterfrom
pr-227
Jun 17, 2022
Merged

JCRVLT-637#228
kwin merged 4 commits intomasterfrom
pr-227

Conversation

@kwin
Copy link
Copy Markdown
Member

@kwin kwin commented Jun 15, 2022

No description provided.

buuhuu and others added 3 commits June 13, 2022 21:10
@buuhuu
JCRVLT-637: fix support for non-docview files with additional properties
eee25a4 
The DocumentViewParserValidator returns a binary file back to the ValidationExecutor as node path with a line number of 0.
The ValidationExecutor considers this state as docview xml case.

With this change the ValidationExecutor checks the returned nodePaths and lineNubmers and if only one nodePath (the file) is returned with lineNumber 0, it handles it the same way as if nothing would have been returned (existing logic).
@buuhuu
JCRVLT-637: update integration tests
1967dc6
@kwin
JCRVLT-637 only determine path of non-docview xml once
eceb952 
clarify javadoc of GenericJcrDataValidator
@kwin
Copy link
Copy Markdown
Member Author

kwin commented Jun 15, 2022

@buuhuu Please have a look, I simplified your approach a bit.

@kwin kwin mentioned this pull request Jun 15, 2022
Closed
@buuhuu
Copy link
Copy Markdown
Member

buuhuu commented Jun 15, 2022 •
edited
Loading

Yes, as said earlier, this should work

We could also remove this line, but there is a unit test that validates this behaviour of the DocumentViewParserValidator so I assume that would be a breaking change.

With that you can remove testGenericJcrDataWithBinaryFileDetected() as this should now be covered by testGenericJcrData() already.

I don't understand how these changes are related
https://github.com/apache/jackrabbit-filevault/pull/228/files#diff-d467e47245ea852f2e8ae6cc036e413bc2098a0e5be790a56eb238e3a17f3b41R375-R383

@kwin
Copy link
Copy Markdown
Member Author

kwin commented Jun 15, 2022 •
edited
Loading

With that you can remove testGenericJcrDataWithBinaryFileDetected() as this should now be covered by testGenericJcrData() already.

Right, gonna remove the former.

I don't understand how these changes are related
https://github.com/apache/jackrabbit-filevault/pull/228/files#diff-d467e47245ea852f2e8ae6cc036e413bc2098a0e5be790a56eb238e3a17f3b41R375-R383

The problem was that testGenericJcrDataWithBinaryFileDetected() was never failing for me when called from the IDE, as in that case the validators genericjcrdataid2 and jackrabbit-docviewparser had the wrong order (the latter must always come first). As the new map originally didn't retain any order, this fix was necessary to make the test and probably also the validation at runtime always behave the same (irrespective of underlying Map implementations).

@kwin kwin merged commit 4571118 into master Jun 17, 2022
@kwin kwin deleted the pr-227 branch June 17, 2022 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kwin @buuhuu