You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by GitBox <gi...@apache.org> on 2022/06/13 19:14:02 UTC

[GitHub] [jackrabbit-filevault] Buuhuu opened a new pull request, #227: JCRVLT-637: fix support for non-docview files with additional properties

Buuhuu opened a new pull request, #227:
URL: https://github.com/apache/jackrabbit-filevault/pull/227

   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).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [jackrabbit-filevault] kwin closed pull request #227: JCRVLT-637: fix support for non-docview files with additional properties

Posted by GitBox <gi...@apache.org>.
kwin closed pull request #227: JCRVLT-637: fix support for non-docview files with additional properties
URL: https://github.com/apache/jackrabbit-filevault/pull/227


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [jackrabbit-filevault] kwin commented on pull request #227: JCRVLT-637: fix support for non-docview files with additional properties

Posted by GitBox <gi...@apache.org>.
kwin commented on PR #227:
URL: https://github.com/apache/jackrabbit-filevault/pull/227#issuecomment-1154973372

   @Buuhuu Thanks for the report and the PR. While I am still trying to understand it would it be possible to enhance this PR by an additional IT (with the example from the JIRA description), the unit test is very artificial.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [jackrabbit-filevault] Buuhuu commented on pull request #227: JCRVLT-637: fix support for non-docview files with additional properties

Posted by GitBox <gi...@apache.org>.
Buuhuu commented on PR #227:
URL: https://github.com/apache/jackrabbit-filevault/pull/227#issuecomment-1156124727

   Sure. 
   
   Let me explain the issue in more detail. In [validateGenericJcrData](https://github.com/apache/jackrabbit-filevault/blob/master/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/ValidationExecutor.java#L313-L319) the `ValidationExecutor` considers a `filePath` as non-docview file (`isDocViewXml = false`) only when the `GenericJcrDataValidator` implementation returns with an empty `nodePathsAndLineNumbers` list. The [DocumentViewParserValidator](https://github.com/apache/jackrabbit-filevault/blob/jackrabbit-filevault-3.6.0/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/impl/DocumentViewParserValidator.java#L105) however, does not return an empty list in this case but adds the filePath to the list itself (duplicated logic). This causes the [NodeTypeValidator](https://github.com/apache/jackrabbit-filevault/blob/jackrabbit-filevault-3.6.0/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/impl/nodetype/NodeTypeValid
 ator.java#L342) to skip the file in this stage, and so the `jcr:data` attribute is missing on the `nt:resource` node. 
   
   We could also remove this [line](https://github.com/apache/jackrabbit-filevault/blob/jackrabbit-filevault-3.6.0/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/impl/DocumentViewParserValidator.java#L105), but there is a unit test that validates this behaviour of the DocumentViewParserValidator so I assume that would be a breaking change. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [jackrabbit-filevault] kwin commented on pull request #227: JCRVLT-637: fix support for non-docview files with additional properties

Posted by GitBox <gi...@apache.org>.
kwin commented on PR #227:
URL: https://github.com/apache/jackrabbit-filevault/pull/227#issuecomment-1156562526

   Superseded by #228 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org