You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by "joerghoh (via GitHub)" <gi...@apache.org> on 2025/07/23 14:21:47 UTC
[PR] JCRVLT-810 skip filter validation [jackrabbit-filevault]
joerghoh opened a new pull request, #378:
URL: https://github.com/apache/jackrabbit-filevault/pull/378
When controlling both package creation, transport and import checking the filters in import can be time-consuming activity. For that reason it should be possible to skip the validation of the filters, as it will always match. In my test case this reduced the package import time by about 25%.
Right now it only skips the validation of properties, but we could think about skipping the validation of node paths as well. Given that in my case we had around 10 properties per node in average, skipping the filters for adding nodes will just add some small benefit on top.
--
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
Re: [PR] JCRVLT-810 skip filter validation [jackrabbit-filevault]
Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh commented on code in PR #378:
URL: https://github.com/apache/jackrabbit-filevault/pull/378#discussion_r2336951360
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultWorkspaceFilter.java:
##########
@@ -241,6 +241,10 @@ public void setSkipFilterChecksOnImport(boolean skipFilterChecksOnImport) {
this.skipFilterChecksOnImport = skipFilterChecksOnImport;
}
+ public boolean getSkipFilterChecksOnImport() {
Review Comment:
I went through and changed them ... a bit less changes than I actually anticipated.
--
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
Re: [PR] JCRVLT-810 skip filter validation [jackrabbit-filevault]
Posted by "kwin (via GitHub)" <gi...@apache.org>.
kwin commented on code in PR #378:
URL: https://github.com/apache/jackrabbit-filevault/pull/378#discussion_r2330812991
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultWorkspaceFilter.java:
##########
@@ -241,6 +241,10 @@ public void setSkipFilterChecksOnImport(boolean skipFilterChecksOnImport) {
this.skipFilterChecksOnImport = skipFilterChecksOnImport;
}
+ public boolean getSkipFilterChecksOnImport() {
Review Comment:
I see these methods only being used once each....
--
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
Re: [PR] JCRVLT-810 skip filter validation [jackrabbit-filevault]
Posted by "kwin (via GitHub)" <gi...@apache.org>.
kwin commented on code in PR #378:
URL: https://github.com/apache/jackrabbit-filevault/pull/378#discussion_r2337078789
##########
vault-core-it/vault-core-integration-tests/src/main/java/org/apache/jackrabbit/vault/packaging/integration/ImportIT.java:
##########
@@ -445,4 +445,55 @@ public void testEnhancedFileAggregatePackageWithIntermediateSaves() throws IOExc
assertPropertyExists("/testroot/tika/config.xml/jcr:content/jcr:data");
assertProperty("/testroot/tika/config.xml/jcr:content/jcr:mimeType", "text/xml");
}
+
+ @Test
+ public void testSkipFilterChecksOnImport_disabled() throws Exception {
+ ImportOptions opts = getDefaultOptions();
+ Importer importer = new Importer(opts);
+ try (Archive archive = getFileArchive("/test-packages/outside-filters.zip")) {
+ archive.open(true);
+ importer.run(archive, admin.getRootNode());
+ admin.save();
+ }
+ assertNodeExists("/tmp/foo");
+ assertPropertyMissing("/tmp/foo/bar/tobi/excludeddProp");
Review Comment:
Please add some assertions what happens to nodes/properties below filter roots not contained in the package.
--
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
Re: [PR] JCRVLT-810 skip filter validation [jackrabbit-filevault]
Posted by "kwin (via GitHub)" <gi...@apache.org>.
kwin commented on code in PR #378:
URL: https://github.com/apache/jackrabbit-filevault/pull/378#discussion_r2330814365
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/ImportOptions.java:
##########
@@ -480,6 +484,26 @@ public void setIdConflictPolicy(@NotNull IdConflictPolicy idConflictPolicy) {
this.idConflictPolicy = idConflictPolicy;
}
+ /**
+ * Allows to skip filter checks when importing packages; you can set this
+ * to {{code true}} if you are sure that there this package contains no content
+ * which would be rejected by filters.
+ * This only affects checks against items in the package, but not existing items in
+ * the repository.
+ * @param skipFilterChecksOnImport if true skip the checks
+ */
+ public void setSkipFilterChecksOnImport (boolean skipFilterChecksOnImport) {
+ this.skipFilterChecksOnImport = skipFilterChecksOnImport;
+ }
+
+ /**
+ * get the filter check skip policy
+ * @return true if the filter check is skipped
+ */
+ public boolean getskipFilterChecksOnImport() {
Review Comment:
```suggestion
public boolean getSkipFilterChecksOnImport() {
```
--
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
Re: [PR] JCRVLT-810 skip filter validation [jackrabbit-filevault]
Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh commented on code in PR #378:
URL: https://github.com/apache/jackrabbit-filevault/pull/378#discussion_r2333339660
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultWorkspaceFilter.java:
##########
@@ -241,6 +241,10 @@ public void setSkipFilterChecksOnImport(boolean skipFilterChecksOnImport) {
this.skipFilterChecksOnImport = skipFilterChecksOnImport;
}
+ public boolean getSkipFilterChecksOnImport() {
Review Comment:
I tried to backtrack from the constructor of the DocViewImporter, which methods would need to be changed, and the list is getting long; where annotated with ```...``` I stopped (line numbers might not be 100% accurate, as my eclipse behaves sometimes a bit strange (https://github.com/eclipse-platform/eclipse.platform.ui/issues/3039).
But it shows, that passing this parameter through the entire call chain(s) would be quite an effort, and for that reason I augmented the DefaultWorkspaceFilter.
```
DocViewImporter.java (l249)
-> AbstractArtifactHandler:importDocView (l170)
-> AbstractArtifactHandler:importDocView (l166)
-> FileArtifactHandler:accept (l231)
-> ...
-> FileArtifactHandler:importDocView (l236)
-> FileArtifactHandler::accept (l231) -> see above
-> GenericArtifactHandler:accept (l88)
-> AbstractArtifactHandler:accept (l126)
-> ...
-> AbstractArtifactHandler:accept (l136)
-> ...
-> Importer:commit (l1085)
-> Importer:commit (l1058)
-> Importer:commit (recursion)
-> Importer:run (l537)
-> ...
```
--
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
Re: [PR] JCRVLT-810 skip filter validation [jackrabbit-filevault]
Posted by "kwin (via GitHub)" <gi...@apache.org>.
kwin commented on code in PR #378:
URL: https://github.com/apache/jackrabbit-filevault/pull/378#discussion_r2336988382
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/ImportOptions.java:
##########
@@ -486,7 +486,7 @@ public void setIdConflictPolicy(@NotNull IdConflictPolicy idConflictPolicy) {
/**
* Allows to skip filter checks when importing packages; you can set this
- * to {{code true}} if you are sure that there this package contains no content
+ * to {{code true}} if you are sure that this package contains no content
* which would be rejected by filters.
* This only affects checks against items in the package, but not existing items in
Review Comment:
```suggestion
* This only affects items in the package, but not existing items in
```
--
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
Re: [PR] JCRVLT-810 skip filter validation [jackrabbit-filevault]
Posted by "kwin (via GitHub)" <gi...@apache.org>.
kwin commented on code in PR #378:
URL: https://github.com/apache/jackrabbit-filevault/pull/378#discussion_r2333858140
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultWorkspaceFilter.java:
##########
@@ -241,6 +241,10 @@ public void setSkipFilterChecksOnImport(boolean skipFilterChecksOnImport) {
this.skipFilterChecksOnImport = skipFilterChecksOnImport;
}
+ public boolean getSkipFilterChecksOnImport() {
Review Comment:
But both FileArtifactHandler:accept and GenericArtifactHandler:accept have access to the ImportOptions which expose that already, so it needs to be passed along DocViewImporter, AbstractArtifactHandler:importDocView and FileArtifactHandler:importDocView which seems acceptable to me. Right now it really is duplicate in ImporterOptions and WorkspaceFilter.
--
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
Re: [PR] JCRVLT-810 skip filter validation [jackrabbit-filevault]
Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh commented on PR #378:
URL: https://github.com/apache/jackrabbit-filevault/pull/378#issuecomment-3275367930
Right now this does only affect the case when properties are added to a new node or updated on an existing node, the codepath for the deletion of properties is not affected. So for that I don't see anything which changes the result of a package installation.
Also it is only supposed to work under the assumption, that you control both the creation and the consumption of the package, and that in both cases the same filter rules are used. In other words: when matching the content of the package to the filters, the result is expected to be always true.
--
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
Re: [PR] JCRVLT-810 skip filter validation [jackrabbit-filevault]
Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh commented on code in PR #378:
URL: https://github.com/apache/jackrabbit-filevault/pull/378#discussion_r2249918541
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultWorkspaceFilter.java:
##########
@@ -241,6 +241,10 @@ public void setSkipFilterChecksOnImport(boolean skipFilterChecksOnImport) {
this.skipFilterChecksOnImport = skipFilterChecksOnImport;
}
+ public boolean getSkipFilterChecksOnImport() {
Review Comment:
``ImportOptions`` already has such a method; i clarified the javadoc as recommended
--
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
Re: [PR] JCRVLT-810 skip filter validation [jackrabbit-filevault]
Posted by "kwin (via GitHub)" <gi...@apache.org>.
kwin commented on code in PR #378:
URL: https://github.com/apache/jackrabbit-filevault/pull/378#discussion_r2225876166
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultWorkspaceFilter.java:
##########
@@ -264,6 +273,11 @@ public boolean covers(String path) {
@Override
public boolean includesProperty(String propertyPath) {
+ if (skipFilterChecksOnImport) {
+ // skip the filter checks if requested; assume that the package only includes content
+ // which matches the filters.
+ return true;
Review Comment:
But isn’t this also called for properties contained in existing nodes but not contained in the package? That would make using this flag break merge properties
--
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
Re: [PR] JCRVLT-810 skip filter validation [jackrabbit-filevault]
Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh commented on code in PR #378:
URL: https://github.com/apache/jackrabbit-filevault/pull/378#discussion_r2239679532
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultWorkspaceFilter.java:
##########
@@ -264,6 +273,11 @@ public boolean covers(String path) {
@Override
public boolean includesProperty(String propertyPath) {
+ if (skipFilterChecksOnImport) {
+ // skip the filter checks if requested; assume that the package only includes content
+ // which matches the filters.
+ return true;
Review Comment:
You are right, skipping the checks here is too broad.
--
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
Re: [PR] JCRVLT-810 skip filter validation [jackrabbit-filevault]
Posted by "kwin (via GitHub)" <gi...@apache.org>.
kwin commented on PR #378:
URL: https://github.com/apache/jackrabbit-filevault/pull/378#issuecomment-3275249412
Looks much better now, I think long term it makes sense to pass the full `ImportOptions` object to the `ArtifactHandler`s and also use that as argument for the `DocViewImporter` constructor, however that can be done outside this PR.
I am still wondering how the skipFilterChecksOnImport affect
a) node/property deletions
b) [direct data serializations](https://jackrabbit.apache.org/filevault/vaultfs.html#Serialization) of files/folders
--
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
Re: [PR] JCRVLT-810 skip filter validation [jackrabbit-filevault]
Posted by "kwin (via GitHub)" <gi...@apache.org>.
kwin commented on code in PR #378:
URL: https://github.com/apache/jackrabbit-filevault/pull/378#discussion_r2337078789
##########
vault-core-it/vault-core-integration-tests/src/main/java/org/apache/jackrabbit/vault/packaging/integration/ImportIT.java:
##########
@@ -445,4 +445,55 @@ public void testEnhancedFileAggregatePackageWithIntermediateSaves() throws IOExc
assertPropertyExists("/testroot/tika/config.xml/jcr:content/jcr:data");
assertProperty("/testroot/tika/config.xml/jcr:content/jcr:mimeType", "text/xml");
}
+
+ @Test
+ public void testSkipFilterChecksOnImport_disabled() throws Exception {
+ ImportOptions opts = getDefaultOptions();
+ Importer importer = new Importer(opts);
+ try (Archive archive = getFileArchive("/test-packages/outside-filters.zip")) {
+ archive.open(true);
+ importer.run(archive, admin.getRootNode());
+ admin.save();
+ }
+ assertNodeExists("/tmp/foo");
+ assertPropertyMissing("/tmp/foo/bar/tobi/excludeddProp");
Review Comment:
Please add some assertions what happens to nodes below filter roots not contained in the package.
--
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
Re: [PR] JCRVLT-810 skip filter validation [jackrabbit-filevault]
Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh commented on PR #378:
URL: https://github.com/apache/jackrabbit-filevault/pull/378#issuecomment-3132321808
I moved the skip logic out of the workspace filter class, and into the DocViewImporter.
--
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
Re: [PR] JCRVLT-810 skip filter validation [jackrabbit-filevault]
Posted by "kwin (via GitHub)" <gi...@apache.org>.
kwin commented on code in PR #378:
URL: https://github.com/apache/jackrabbit-filevault/pull/378#discussion_r2330445741
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultWorkspaceFilter.java:
##########
@@ -241,6 +241,10 @@ public void setSkipFilterChecksOnImport(boolean skipFilterChecksOnImport) {
this.skipFilterChecksOnImport = skipFilterChecksOnImport;
}
+ public boolean getSkipFilterChecksOnImport() {
Review Comment:
Those methods should not be added to DefaultWorkspaceFilter either. Rather pass that as dedicated boolean flag to the importer.
--
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
Re: [PR] JCRVLT-810 skip filter validation [jackrabbit-filevault]
Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh commented on code in PR #378:
URL: https://github.com/apache/jackrabbit-filevault/pull/378#discussion_r2336969026
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewImporter.java:
##########
@@ -225,6 +226,11 @@ public class DocViewImporter implements DocViewParserHandler {
*/
private final boolean isSnsSupported;
+ /**
+ * set true if filter checks can be skipped on import
+ */
+ private boolean skipFilterChecksOnImport = false;
Review Comment:
done
--
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
Re: [PR] JCRVLT-810 skip filter validation [jackrabbit-filevault]
Posted by "kwin (via GitHub)" <gi...@apache.org>.
kwin commented on code in PR #378:
URL: https://github.com/apache/jackrabbit-filevault/pull/378#discussion_r2336953981
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/ImportOptions.java:
##########
@@ -480,6 +484,26 @@ public void setIdConflictPolicy(@NotNull IdConflictPolicy idConflictPolicy) {
this.idConflictPolicy = idConflictPolicy;
}
+ /**
+ * Allows to skip filter checks when importing packages; you can set this
+ * to {{code true}} if you are sure that there this package contains no content
Review Comment:
```suggestion
* to {{code true}} if you are sure that this package contains no content
```
--
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
Re: [PR] JCRVLT-810 skip filter validation [jackrabbit-filevault]
Posted by "kwin (via GitHub)" <gi...@apache.org>.
kwin commented on code in PR #378:
URL: https://github.com/apache/jackrabbit-filevault/pull/378#discussion_r2240744982
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultWorkspaceFilter.java:
##########
@@ -241,6 +241,10 @@ public void setSkipFilterChecksOnImport(boolean skipFilterChecksOnImport) {
this.skipFilterChecksOnImport = skipFilterChecksOnImport;
}
+ public boolean getSkipFilterChecksOnImport() {
Review Comment:
If this only affects import I would rather move it to `ImportOptions` and also clarify that this only affects checks against package items not about existing repo items.
--
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
Re: [PR] JCRVLT-810 skip filter validation [jackrabbit-filevault]
Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh commented on code in PR #378:
URL: https://github.com/apache/jackrabbit-filevault/pull/378#discussion_r2330774493
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultWorkspaceFilter.java:
##########
@@ -241,6 +241,10 @@ public void setSkipFilterChecksOnImport(boolean skipFilterChecksOnImport) {
this.skipFilterChecksOnImport = skipFilterChecksOnImport;
}
+ public boolean getSkipFilterChecksOnImport() {
Review Comment:
you mean extending the (already quite long) constructor of the ``DocViewImporter`` by this flag? That would make the patch much more complicated, as I would need to pass through that parameter through a series of other methods.
--
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
Re: [PR] JCRVLT-810 skip filter validation [jackrabbit-filevault]
Posted by "kwin (via GitHub)" <gi...@apache.org>.
kwin commented on code in PR #378:
URL: https://github.com/apache/jackrabbit-filevault/pull/378#discussion_r2330447527
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewImporter.java:
##########
@@ -225,6 +226,11 @@ public class DocViewImporter implements DocViewParserHandler {
*/
private final boolean isSnsSupported;
+ /**
+ * set true if filter checks can be skipped on import
+ */
+ private boolean skipFilterChecksOnImport = false;
Review Comment:
Please pass as dedicated boolean argument to the constructor rather than getting it from the filter impl.
--
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
Re: [PR] JCRVLT-810 skip filter validation [jackrabbit-filevault]
Posted by "kwin (via GitHub)" <gi...@apache.org>.
kwin commented on code in PR #378:
URL: https://github.com/apache/jackrabbit-filevault/pull/378#discussion_r2325633131
##########
vault-core-it/vault-core-integration-tests/src/main/resources/test-packages/tmp-content-outside-filters.zip:
##########
Review Comment:
Let's use non binary packages (i.e. exploded ones, as can be seen e.g. in https://github.com/apache/jackrabbit-filevault/tree/master/vault-core-it/vault-core-integration-tests/src/main/resources/test-packages/import_modes_test_filehandler.zip)
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultWorkspaceFilter.java:
##########
@@ -241,6 +241,10 @@ public void setSkipFilterChecksOnImport(boolean skipFilterChecksOnImport) {
this.skipFilterChecksOnImport = skipFilterChecksOnImport;
}
+ public boolean getSkipFilterChecksOnImport() {
Review Comment:
No need to add new methods/fields to WorkspaceFilter then, just use the one from ImportOptions directly (i.e. pass as boolean argument to `DocViewImporter`
--
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
Re: [PR] JCRVLT-810 skip filter validation [jackrabbit-filevault]
Posted by "kwin (via GitHub)" <gi...@apache.org>.
kwin commented on code in PR #378:
URL: https://github.com/apache/jackrabbit-filevault/pull/378#discussion_r2333860926
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultWorkspaceFilter.java:
##########
@@ -241,6 +241,10 @@ public void setSkipFilterChecksOnImport(boolean skipFilterChecksOnImport) {
this.skipFilterChecksOnImport = skipFilterChecksOnImport;
}
+ public boolean getSkipFilterChecksOnImport() {
Review Comment:
I can try to take a stab at it.
--
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
Re: [PR] JCRVLT-810 skip filter validation [jackrabbit-filevault]
Posted by "kwin (via GitHub)" <gi...@apache.org>.
kwin commented on code in PR #378:
URL: https://github.com/apache/jackrabbit-filevault/pull/378#discussion_r2336988382
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/ImportOptions.java:
##########
@@ -486,7 +486,7 @@ public void setIdConflictPolicy(@NotNull IdConflictPolicy idConflictPolicy) {
/**
* Allows to skip filter checks when importing packages; you can set this
- * to {{code true}} if you are sure that there this package contains no content
+ * to {{code true}} if you are sure that this package contains no content
* which would be rejected by filters.
* This only affects checks against items in the package, but not existing items in
Review Comment:
```suggestion
* This only affects items in the package, but not existing items in
```
--
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
Re: [PR] JCRVLT-810 skip filter validation [jackrabbit-filevault]
Posted by "kwin (via GitHub)" <gi...@apache.org>.
kwin commented on PR #378:
URL: https://github.com/apache/jackrabbit-filevault/pull/378#issuecomment-3275384717
Would be nice to cover that in a test case as well. Not only that deletion of properties no longer contained in the package still works but also that nodes below filter roots not contained in the package are still removed. Also this makes the name `skipFilterChecksOnImport` a bit misleading as actually the filter check is only skipped in very few cases during import.
--
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
Re: [PR] JCRVLT-810 skip filter validation [jackrabbit-filevault]
Posted by "kwin (via GitHub)" <gi...@apache.org>.
kwin commented on code in PR #378:
URL: https://github.com/apache/jackrabbit-filevault/pull/378#discussion_r2225876166
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultWorkspaceFilter.java:
##########
@@ -264,6 +273,11 @@ public boolean covers(String path) {
@Override
public boolean includesProperty(String propertyPath) {
+ if (skipFilterChecksOnImport) {
+ // skip the filter checks if requested; assume that the package only includes content
+ // which matches the filters.
+ return true;
Review Comment:
But isn’t this also called for properties contained in existing nodes but not in contained in the package? That would make using this flag break merge properties
--
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