You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by GitBox <gi...@apache.org> on 2019/12/20 18:27:49 UTC
[GitHub] [sling-org-apache-sling-resourceresolver] Buuhuu opened a new pull
request #16: SLING-8946 non deterministic resource observation
Buuhuu opened a new pull request #16: SLING-8946 non deterministic resource observation
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/16
This change calls the same logic that is applied for ResourceProviderHandlers when a ResourceChangeListener is registered after a ResourceProvider got either activated or deactivated.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [sling-org-apache-sling-resourceresolver] cziegeler commented on
issue #16: SLING-8946 non deterministic resource observation
Posted by GitBox <gi...@apache.org>.
cziegeler commented on issue #16: SLING-8946 non deterministic resource observation
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/16#issuecomment-571152886
Long time since I last looked at this code, but I think the changes are good
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [sling-org-apache-sling-resourceresolver] Buuhuu commented on issue
#16: SLING-8946 non deterministic resource observation
Posted by GitBox <gi...@apache.org>.
Buuhuu commented on issue #16: SLING-8946 non deterministic resource observation
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/16#issuecomment-571666371
Great thanks. Can we continue the discussion about back-porting this to 1.5.x in the ticket?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [sling-org-apache-sling-resourceresolver] sonarcloud[bot] commented
on issue #16: SLING-8946 non deterministic resource observation
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on issue #16: SLING-8946 non deterministic resource observation
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/16#issuecomment-571989957
SonarCloud Quality Gate failed.
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=16&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=16&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=16&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=16&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=16&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=16&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=16&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=16&resolved=false&types=SECURITY_HOTSPOT) to review)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=16&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=16&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=16&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=16&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=16&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=16&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-resourceresolver&pullRequest=16&metric=new_duplicated_lines_density&view=list)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [sling-org-apache-sling-resourceresolver] rombert merged pull
request #16: SLING-8946 non deterministic resource observation
Posted by GitBox <gi...@apache.org>.
rombert merged pull request #16: SLING-8946 non deterministic resource observation
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/16
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [sling-org-apache-sling-resourceresolver] rombert commented on
issue #16: SLING-8946 non deterministic resource observation
Posted by GitBox <gi...@apache.org>.
rombert commented on issue #16: SLING-8946 non deterministic resource observation
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/16#issuecomment-571659204
Thanks @cziegeler . @Buuhuu - can you please squash all commits into a single one and force-push? I will then merge the PR.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [sling-org-apache-sling-resourceresolver] Buuhuu commented on a
change in pull request #16: SLING-8946 non deterministic resource observation
Posted by GitBox <gi...@apache.org>.
Buuhuu commented on a change in pull request #16: SLING-8946 non deterministic resource observation
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/16#discussion_r363835997
##########
File path: src/test/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderTrackerTest.java
##########
@@ -240,6 +248,62 @@ public void providerRemoved(String name, String pid, boolean stateful, boolean u
assertThat(tracker.getResourceProviderStorage().getAllHandlers().size(), equalTo(0));
}
+ /**
+ * This test verifies that shadowing of Resource observation is deterministic when ResourceProviders get registered and unregistered,
+ * meaning it is independent of the order in which those events happen.
+ * <p>
+ * It does so by
+ * 1) registering a ResourceProvider A on a deeper path then root (shadowing root)
+ * 2) registering a ResourceProvider B on root
+ * 3) unregistering the ResourceProvider A
+ * 4) and registering the ResoucreProvider A
+ * <p>
+ * This guarantees in both cases (A before B and B before A) the same excludes are applied in the ObservationReporter.
+ *
+ * @throws InvalidSyntaxException
+ */
+ @Test
+ public void testDeterministicObservationShadowing() throws InvalidSyntaxException {
+ final ResourceProviderTracker tracker = new ResourceProviderTracker();
+ final Map<String, List<String>> excludeSets = new HashMap<>();
+
+ tracker.activate(context.bundleContext(), eventAdmin, null);
+ tracker.setObservationReporterGenerator(new SimpleObservationReporterGenerator(new DoNothingObservationReporter()) {
+ @Override
+ public ObservationReporter create(Path path, PathSet excludes) {
+ List<String> excludeSetsPerPath = excludeSets.get(path.getPath());
+ if (excludeSetsPerPath == null) {
+ excludeSetsPerPath = new ArrayList<>(1);
+ excludeSets.put(path.getPath(), excludeSetsPerPath);
+ }
+
+ excludeSetsPerPath.clear();
+ for (Iterator<Path> excludesIt = excludes.iterator(); excludesIt.hasNext(); ) {
+ excludeSetsPerPath.add(excludesIt.next().getPath());
+ }
+
+ return super.create(path, excludes);
+ }
+ });
+
+ ResourceProvider rp = mock(ResourceProvider.class);
+ ResourceProviderInfo info;
+ // register on RP on /content/B2C, empty exclude set expected
Review comment:
I will remove those project specifics
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [sling-org-apache-sling-resourceresolver] rombert commented on
issue #16: SLING-8946 non deterministic resource observation
Posted by GitBox <gi...@apache.org>.
rombert commented on issue #16: SLING-8946 non deterministic resource observation
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/16#issuecomment-571104112
I think @cziegeler would be the best to review this.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [sling-org-apache-sling-resourceresolver] Buuhuu commented on issue
#16: SLING-8946 non deterministic resource observation
Posted by GitBox <gi...@apache.org>.
Buuhuu commented on issue #16: SLING-8946 non deterministic resource observation
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/16#issuecomment-571075921
@rombert @kwin may either of you have a look at this?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services