You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by "catalinadumitruu (via GitHub)" <gi...@apache.org> on 2023/02/16 07:34:33 UTC

[GitHub] [sling-org-apache-sling-testing-jcr-mock] catalinadumitruu opened a new pull request, #10: Add implementation for addMixin method

catalinadumitruu opened a new pull request, #10:
URL: https://github.com/apache/sling-org-apache-sling-testing-jcr-mock/pull/10

   Add implementation for method `addMixin()` according to developer documentation:
   
   > Adds the mixin node type named mixinName to this node. If this node is already of type mixinName (either due to a previously added mixin or due to its primary type, through inheritance) then this method has no effect. Otherwise mixinName is added to this node's jcr:mixinTypes property.


-- 
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@sling.apache.org

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


[GitHub] [sling-org-apache-sling-testing-jcr-mock] sonarcloud[bot] commented on pull request #10: Add implementation for addMixin method

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #10:
URL: https://github.com/apache/sling-org-apache-sling-testing-jcr-mock/pull/10#issuecomment-1438010172

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=CODE_SMELL)
   
   [![93.2%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '93.2%')](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&metric=new_coverage&view=list) [93.2% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&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.

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

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


[GitHub] [sling-org-apache-sling-testing-jcr-mock] joerghoh commented on pull request #10: Add implementation for addMixin method

Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh commented on PR #10:
URL: https://github.com/apache/sling-org-apache-sling-testing-jcr-mock/pull/10#issuecomment-1436915417

   @stefanseifert are you good with it, too? I would then merge and initiate a release.


-- 
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@sling.apache.org

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


[GitHub] [sling-org-apache-sling-testing-jcr-mock] stefanseifert commented on a diff in pull request #10: Add implementation for addMixin method

Posted by "stefanseifert (via GitHub)" <gi...@apache.org>.
stefanseifert commented on code in PR #10:
URL: https://github.com/apache/sling-org-apache-sling-testing-jcr-mock/pull/10#discussion_r1111981040


##########
src/main/java/org/apache/sling/testing/mock/jcr/MockNodeType.java:
##########
@@ -54,6 +54,12 @@ public boolean hasOrderableChildNodes() {
         // support only well-known built-in node type
         return StringUtils.equals(getName(), JcrConstants.NT_UNSTRUCTURED);
     }
+    
+        @Override
+    public boolean isMixin() {
+        // if it has at least 1 supertype -> isMixin
+       return (getDeclaredSupertypes().length > 0);

Review Comment:
   this method is not covered by unit tests - and it will fail for sure, as it references getDeclaredSupertypes which throws an UnsupportedOperationException.



-- 
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@sling.apache.org

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


[GitHub] [sling-org-apache-sling-testing-jcr-mock] catalinadumitruu commented on a diff in pull request #10: Add implementation for addMixin method

Posted by "catalinadumitruu (via GitHub)" <gi...@apache.org>.
catalinadumitruu commented on code in PR #10:
URL: https://github.com/apache/sling-org-apache-sling-testing-jcr-mock/pull/10#discussion_r1112053041


##########
src/main/java/org/apache/sling/testing/mock/jcr/MockNodeType.java:
##########
@@ -54,6 +54,12 @@ public boolean hasOrderableChildNodes() {
         // support only well-known built-in node type
         return StringUtils.equals(getName(), JcrConstants.NT_UNSTRUCTURED);
     }
+    
+        @Override
+    public boolean isMixin() {
+        // if it has at least 1 supertype -> isMixin
+       return (getDeclaredSupertypes().length > 0);

Review Comment:
   Also, I've refactored the code using streams
   



-- 
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@sling.apache.org

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


[GitHub] [sling-org-apache-sling-testing-jcr-mock] catalinadumitruu commented on pull request #10: Add implementation for addMixin method

Posted by "catalinadumitruu (via GitHub)" <gi...@apache.org>.
catalinadumitruu commented on PR #10:
URL: https://github.com/apache/sling-org-apache-sling-testing-jcr-mock/pull/10#issuecomment-1432936756

   @joerghoh my goal was to do a dummy implementation of this method as I need it in one of my tests.
   Due to the error thrown before my tests were failing. 
   


-- 
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@sling.apache.org

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


[GitHub] [sling-org-apache-sling-testing-jcr-mock] catalinadumitruu commented on a diff in pull request #10: Add implementation for addMixin method

Posted by "catalinadumitruu (via GitHub)" <gi...@apache.org>.
catalinadumitruu commented on code in PR #10:
URL: https://github.com/apache/sling-org-apache-sling-testing-jcr-mock/pull/10#discussion_r1113492026


##########
src/main/java/org/apache/sling/testing/mock/jcr/MockNode.java:
##########
@@ -368,8 +372,26 @@ public boolean equals(Object obj) {
 
     @Override
     public NodeType[] getMixinNodeTypes() throws RepositoryException {
-        // we have no real mixin support - just assume no mixin nodetypes are set
-        return new NodeType[0];
+        Value[] mixinNames = getProperty(JcrConstants.JCR_MIXINTYPES).getValues();
+
+        return Arrays.stream(mixinNames)
+                .map(value -> {
+                    try {
+                        return value.getString();
+                    } catch (RepositoryException e) {
+                        return new NodeType[0];

Review Comment:
   I more inclined to return an empty array because it is a mock implementation and it is safer to not return null and cause errors, but it is your call :)



-- 
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@sling.apache.org

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


[GitHub] [sling-org-apache-sling-testing-jcr-mock] stefanseifert commented on a diff in pull request #10: Add implementation for addMixin method

Posted by "stefanseifert (via GitHub)" <gi...@apache.org>.
stefanseifert commented on code in PR #10:
URL: https://github.com/apache/sling-org-apache-sling-testing-jcr-mock/pull/10#discussion_r1112205272


##########
src/main/java/org/apache/sling/testing/mock/jcr/MockNodeType.java:
##########
@@ -54,6 +54,12 @@ public boolean hasOrderableChildNodes() {
         // support only well-known built-in node type
         return StringUtils.equals(getName(), JcrConstants.NT_UNSTRUCTURED);
     }
+    
+        @Override
+    public boolean isMixin() {
+        // if it has at least 1 supertype -> isMixin
+       return (getDeclaredSupertypes().length > 0);

Review Comment:
   from my pov it's fine to move isMixin back to unsupported methods and remove the implementation. it would require a bit more rework to make this working.



-- 
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@sling.apache.org

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


[GitHub] [sling-org-apache-sling-testing-jcr-mock] stefanseifert commented on pull request #10: Add implementation for addMixin method

Posted by "stefanseifert (via GitHub)" <gi...@apache.org>.
stefanseifert commented on PR #10:
URL: https://github.com/apache/sling-org-apache-sling-testing-jcr-mock/pull/10#issuecomment-1432948272

   yes, that's a good starting point. but although jcr-mock mocks only a subset of functionality, we try to mock methods closely related together to get a somewhat consistent behavior. can you have a look if you find an easy we to also cover the methods mentioned by jörg, and also removeMixin?
   
   also, make sure it's possible to add multiple mixins, and not only one.


-- 
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@sling.apache.org

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


[GitHub] [sling-org-apache-sling-testing-jcr-mock] sonarcloud[bot] commented on pull request #10: Add implementation for addMixin method

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #10:
URL: https://github.com/apache/sling-org-apache-sling-testing-jcr-mock/pull/10#issuecomment-1439586033

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=CODE_SMELL)
   
   [![91.3%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '91.3%')](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&metric=new_coverage&view=list) [91.3% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&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.

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

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


[GitHub] [sling-org-apache-sling-testing-jcr-mock] sdumitriu commented on a diff in pull request #10: Add implementation for addMixin method

Posted by "sdumitriu (via GitHub)" <gi...@apache.org>.
sdumitriu commented on code in PR #10:
URL: https://github.com/apache/sling-org-apache-sling-testing-jcr-mock/pull/10#discussion_r1108686107


##########
src/main/java/org/apache/sling/testing/mock/jcr/MockNode.java:
##########
@@ -394,6 +394,17 @@ public void setPrimaryType(final String primaryNodeTypeName) throws RepositoryEx
             throw new NoSuchNodeTypeException("Not accepting blank node types");
         }
     }
+    
+    @Override
+    public void addMixin(final String mixinName) throws RepositoryException {
+        if (StringUtils.isNotBlank(mixinName)) {
+            if(!this.hasProperty(JcrConstants.JCR_MIXINTYPES)) {

Review Comment:
   This is incomplete. From the spec:
   
   > If this node is already of type mixinName...
   
   This doesn't mean "if the node already has a mixin property", but that among the values there's this one, so a better approach would be something like:
   
   ```
   if (! has property)
      set property to an array with just this value
   else if (! this value among the array of values for the property)
     set property to an array consisting of the original array of values + this new value
   ```
   
   This is still not complete, since a mixin may be inherited from the primary type or another mixin, but short of fully implementing node type management, the simple "is it among the listed mixins" check above should be enough for mocks.



-- 
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@sling.apache.org

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


[GitHub] [sling-org-apache-sling-testing-jcr-mock] joerghoh commented on pull request #10: Add implementation for addMixin method

Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh commented on PR #10:
URL: https://github.com/apache/sling-org-apache-sling-testing-jcr-mock/pull/10#issuecomment-1434878654

   To complete a somehow working mixin support, I would recommend to implement the ```canAddMixin(String)``` method as well (in the first approach it can always return ```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@sling.apache.org

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


[GitHub] [sling-org-apache-sling-testing-jcr-mock] sonarcloud[bot] commented on pull request #10: Add implementation for addMixin method

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #10:
URL: https://github.com/apache/sling-org-apache-sling-testing-jcr-mock/pull/10#issuecomment-1437231362

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=CODE_SMELL)
   
   [![76.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '76.0%')](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&metric=new_coverage&view=list) [76.0% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&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.

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

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


[GitHub] [sling-org-apache-sling-testing-jcr-mock] sonarcloud[bot] commented on pull request #10: Add implementation for addMixin method

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #10:
URL: https://github.com/apache/sling-org-apache-sling-testing-jcr-mock/pull/10#issuecomment-1433691888

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=CODE_SMELL)
   
   [![90.9%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '90.9%')](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&metric=new_coverage&view=list) [90.9% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&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.

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

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


[GitHub] [sling-org-apache-sling-testing-jcr-mock] stefanseifert merged pull request #10: Add implementation for addMixin method

Posted by "stefanseifert (via GitHub)" <gi...@apache.org>.
stefanseifert merged PR #10:
URL: https://github.com/apache/sling-org-apache-sling-testing-jcr-mock/pull/10


-- 
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@sling.apache.org

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


[GitHub] [sling-org-apache-sling-testing-jcr-mock] stefanseifert commented on a diff in pull request #10: Add implementation for addMixin method

Posted by "stefanseifert (via GitHub)" <gi...@apache.org>.
stefanseifert commented on code in PR #10:
URL: https://github.com/apache/sling-org-apache-sling-testing-jcr-mock/pull/10#discussion_r1113416143


##########
src/main/java/org/apache/sling/testing/mock/jcr/MockNode.java:
##########
@@ -368,8 +372,26 @@ public boolean equals(Object obj) {
 
     @Override
     public NodeType[] getMixinNodeTypes() throws RepositoryException {
-        // we have no real mixin support - just assume no mixin nodetypes are set
-        return new NodeType[0];
+        Value[] mixinNames = getProperty(JcrConstants.JCR_MIXINTYPES).getValues();
+
+        return Arrays.stream(mixinNames)
+                .map(value -> {
+                    try {
+                        return value.getString();
+                    } catch (RepositoryException e) {
+                        return new NodeType[0];

Review Comment:
   i assume this should return null in case of error - to be filtered out in the next step?



##########
src/main/java/org/apache/sling/testing/mock/jcr/MockNode.java:
##########
@@ -368,8 +372,26 @@ public boolean equals(Object obj) {
 
     @Override
     public NodeType[] getMixinNodeTypes() throws RepositoryException {
-        // we have no real mixin support - just assume no mixin nodetypes are set
-        return new NodeType[0];
+        Value[] mixinNames = getProperty(JcrConstants.JCR_MIXINTYPES).getValues();
+
+        return Arrays.stream(mixinNames)
+                .map(value -> {
+                    try {
+                        return value.getString();
+                    } catch (RepositoryException e) {
+                        return new NodeType[0];
+                    }
+                })
+                .filter(Objects::nonNull)
+                .map(name -> {
+                    try {
+                        return getSession().getWorkspace().getNodeTypeManager().getNodeType(name.toString());
+                    } catch (RepositoryException e) {
+                        return new NodeType[0];

Review Comment:
   i assume this should return null in case of error - to be filtered out in the next step?



##########
src/test/java/org/apache/sling/testing/mock/jcr/MockNodeTest.java:
##########
@@ -18,32 +18,22 @@
  */
 package org.apache.sling.testing.mock.jcr;
 
-import static org.junit.Assert.assertArrayEquals;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-
 import java.util.HashMap;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 
-import javax.jcr.ItemNotFoundException;
-import javax.jcr.Node;
-import javax.jcr.NodeIterator;
-import javax.jcr.Property;
-import javax.jcr.PropertyIterator;
-import javax.jcr.RepositoryException;
-import javax.jcr.Session;
+import javax.jcr.*;
 import javax.jcr.nodetype.NoSuchNodeTypeException;
+import javax.jcr.nodetype.NodeType;

Review Comment:
   cosmetic: please remove unused 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@sling.apache.org

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


[GitHub] [sling-org-apache-sling-testing-jcr-mock] sonarcloud[bot] commented on pull request #10: Add implementation for addMixin method

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #10:
URL: https://github.com/apache/sling-org-apache-sling-testing-jcr-mock/pull/10#issuecomment-1432649853

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=CODE_SMELL)
   
   [![66.7%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '66.7%')](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&metric=new_coverage&view=list) [66.7% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&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.

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

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


[GitHub] [sling-org-apache-sling-testing-jcr-mock] catalinadumitruu commented on pull request #10: Add implementation for addMixin method

Posted by "catalinadumitruu (via GitHub)" <gi...@apache.org>.
catalinadumitruu commented on PR #10:
URL: https://github.com/apache/sling-org-apache-sling-testing-jcr-mock/pull/10#issuecomment-1433547937

   I implemented `addMixin` and `removeMixin`.
   For `getMixinNodeTypes` I couldn't see a clear approach, without creating something too complex. 


-- 
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@sling.apache.org

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


[GitHub] [sling-org-apache-sling-testing-jcr-mock] sonarcloud[bot] commented on pull request #10: Add implementation for addMixin method

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #10:
URL: https://github.com/apache/sling-org-apache-sling-testing-jcr-mock/pull/10#issuecomment-1437363181

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=CODE_SMELL)
   
   [![87.2%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '87.2%')](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&metric=new_coverage&view=list) [87.2% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&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.

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

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


[GitHub] [sling-org-apache-sling-testing-jcr-mock] catalinadumitruu commented on a diff in pull request #10: Add implementation for addMixin method

Posted by "catalinadumitruu (via GitHub)" <gi...@apache.org>.
catalinadumitruu commented on code in PR #10:
URL: https://github.com/apache/sling-org-apache-sling-testing-jcr-mock/pull/10#discussion_r1112052625


##########
src/main/java/org/apache/sling/testing/mock/jcr/MockNodeType.java:
##########
@@ -54,6 +54,12 @@ public boolean hasOrderableChildNodes() {
         // support only well-known built-in node type
         return StringUtils.equals(getName(), JcrConstants.NT_UNSTRUCTURED);
     }
+    
+        @Override
+    public boolean isMixin() {
+        // if it has at least 1 supertype -> isMixin
+       return (getDeclaredSupertypes().length > 0);

Review Comment:
   What is the desired approach here? 
   If method `isMixin` is not critical I can undo the changes or hardcode to always return true.
   Implementing `getDeclaredSupertypes` implies implementing at least 2 other methods `getDefinition` and `getDeclaredSupertypeNames` and maybe more



-- 
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@sling.apache.org

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


[GitHub] [sling-org-apache-sling-testing-jcr-mock] sonarcloud[bot] commented on pull request #10: Add implementation for addMixin method

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #10:
URL: https://github.com/apache/sling-org-apache-sling-testing-jcr-mock/pull/10#issuecomment-1435113444

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=CODE_SMELL)
   
   [![83.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '83.0%')](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&metric=new_coverage&view=list) [83.0% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&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.

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

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


[GitHub] [sling-org-apache-sling-testing-jcr-mock] joerghoh commented on pull request #10: Add implementation for addMixin method

Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh commented on PR #10:
URL: https://github.com/apache/sling-org-apache-sling-testing-jcr-mock/pull/10#issuecomment-1432683716

   @catalinadumitruu what is your goal with this PR? Do you want to add full mixin support? Because in my view just implementing ```addMixin``` does not help much, if the other methods (like ```getMixinNodeTypes``` or ```isNodeType``` are not implemented as well.


-- 
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@sling.apache.org

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


[GitHub] [sling-org-apache-sling-testing-jcr-mock] catalinadumitruu commented on a diff in pull request #10: Add implementation for addMixin method

Posted by "catalinadumitruu (via GitHub)" <gi...@apache.org>.
catalinadumitruu commented on code in PR #10:
URL: https://github.com/apache/sling-org-apache-sling-testing-jcr-mock/pull/10#discussion_r1113484408


##########
src/main/java/org/apache/sling/testing/mock/jcr/MockNode.java:
##########
@@ -368,8 +372,26 @@ public boolean equals(Object obj) {
 
     @Override
     public NodeType[] getMixinNodeTypes() throws RepositoryException {
-        // we have no real mixin support - just assume no mixin nodetypes are set
-        return new NodeType[0];
+        Value[] mixinNames = getProperty(JcrConstants.JCR_MIXINTYPES).getValues();
+
+        return Arrays.stream(mixinNames)
+                .map(value -> {
+                    try {
+                        return value.getString();
+                    } catch (RepositoryException e) {
+                        return new NodeType[0];
+                    }
+                })
+                .filter(Objects::nonNull)
+                .map(name -> {
+                    try {
+                        return getSession().getWorkspace().getNodeTypeManager().getNodeType(name.toString());
+                    } catch (RepositoryException e) {
+                        return new NodeType[0];

Review Comment:
   I more inclined to return an empty array because it is a mock implementation and it is safer to not return null and cause errors, but it is your call :)  



-- 
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@sling.apache.org

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


[GitHub] [sling-org-apache-sling-testing-jcr-mock] sonarcloud[bot] commented on pull request #10: Add implementation for addMixin method

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #10:
URL: https://github.com/apache/sling-org-apache-sling-testing-jcr-mock/pull/10#issuecomment-1439206890

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&resolved=false&types=CODE_SMELL)
   
   [![91.3%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '91.3%')](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&metric=new_coverage&view=list) [91.3% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-testing-jcr-mock&pullRequest=10&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.

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

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


[GitHub] [sling-org-apache-sling-testing-jcr-mock] catalinadumitruu commented on a diff in pull request #10: Add implementation for addMixin method

Posted by "catalinadumitruu (via GitHub)" <gi...@apache.org>.
catalinadumitruu commented on code in PR #10:
URL: https://github.com/apache/sling-org-apache-sling-testing-jcr-mock/pull/10#discussion_r1112683875


##########
src/main/java/org/apache/sling/testing/mock/jcr/MockNodeType.java:
##########
@@ -54,6 +54,12 @@ public boolean hasOrderableChildNodes() {
         // support only well-known built-in node type
         return StringUtils.equals(getName(), JcrConstants.NT_UNSTRUCTURED);
     }
+    
+        @Override
+    public boolean isMixin() {
+        // if it has at least 1 supertype -> isMixin
+       return (getDeclaredSupertypes().length > 0);

Review Comment:
   reverted changes
   



-- 
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@sling.apache.org

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