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 2021/03/30 15:03:18 UTC
[GitHub] [sling-whiteboard] badvision opened a new pull request #73: SlingModelPersist type handling cleanup
badvision opened a new pull request #73:
URL: https://github.com/apache/sling-whiteboard/pull/73
This cleans up how primary type and resource type are managed. I removed the child content type feature because it was too confusing to use and even more confusing in the code implementation. Now if you want a jcr:content child, add a content property to your object and use a @Named("jcr:content") annotation. This means that the java object graph should be more or less identical to the jcr node graph.
--
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
[GitHub] [sling-whiteboard] sonarcloud[bot] commented on pull request #73: SlingModelPersist type handling cleanup
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #73:
URL: https://github.com/apache/sling-whiteboard/pull/73#issuecomment-811667623
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-whiteboard&pullRequest=73&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C.png' alt='C' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-whiteboard&pullRequest=73&resolved=false&types=BUG) [2 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-whiteboard&pullRequest=73&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-whiteboard&pullRequest=73&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-whiteboard&pullRequest=73&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-whiteboard&pullRequest=73&resolved=false&types=VULNERABILITY)
[<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/security_hotspots?id=apache_sling-whiteboard&pullRequest=73&resolved=false&types=SECURITY_HOTSPOT) [<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/security_hotspots?id=apache_sling-whiteboard&pullRequest=73&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-whiteboard&pullRequest=73&resolved=false&types=SECURITY_HOTSPOT)
[<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-whiteboard&pullRequest=73&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-whiteboard&pullRequest=73&resolved=false&types=CODE_SMELL) [4 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-whiteboard&pullRequest=73&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-whiteboard&pullRequest=73&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-whiteboard&pullRequest=73&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-whiteboard&pullRequest=73&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-whiteboard&pullRequest=73&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
[GitHub] [sling-whiteboard] badvision commented on a change in pull request #73: SlingModelPersist type handling cleanup
Posted by GitBox <gi...@apache.org>.
badvision commented on a change in pull request #73:
URL: https://github.com/apache/sling-whiteboard/pull/73#discussion_r605242725
##########
File path: SlingModelPersist/src/main/java/org/apache/sling/models/persistor/ModelPersistor.java
##########
@@ -18,10 +18,10 @@
*/
package org.apache.sling.models.persistor;
+import com.drew.lang.annotations.NotNull;
Review comment:
Hmm good point, I think that was an oversight as I was having difficulty building in this one case -- happy to revert them back.
##########
File path: SlingModelPersist/src/main/java/org/apache/sling/models/persistor/impl/ResourceTypeKey.java
##########
@@ -50,22 +52,34 @@ public static ResourceTypeKey fromObject(Object obj) {
return NODE_TYPE_FOR_CLASS.get(obj.getClass());
}
+ String nodeType = getNodeType(obj);
Model modelAnnotation = obj.getClass().getAnnotation(Model.class);
- String primaryType = (String) getAnnotatedValue(obj, ResourceType.class);
+ String resourceType = (String) getAnnotatedValue(obj, ResourceType.class);
+
// Use the model annotation for resource type as needed
- if (primaryType == null
+ if (resourceType == null
&& modelAnnotation != null
&& modelAnnotation.resourceType() != null
- && modelAnnotation.resourceType().length == 1) {
- primaryType = modelAnnotation.resourceType()[0];
+ && modelAnnotation.resourceType().length >= 1) {
+ resourceType = modelAnnotation.resourceType()[0];
}
- String childType = (String) getAnnotatedValue(obj, ChildType.class);
- if (primaryType != null) {
- ResourceTypeKey key = new ResourceTypeKey(primaryType, childType);
+
+ if (nodeType != null || resourceType != null) {
+ ResourceTypeKey key = new ResourceTypeKey(nodeType, resourceType);
NODE_TYPE_FOR_CLASS.put(obj.getClass(), key);
return key;
} else {
return ResourceTypeKey.NT_UNSTRUCTURED;
}
}
+
+ private static String getNodeType(Object obj) throws IllegalArgumentException, IllegalAccessException {
+ for (Field f : obj.getClass().getDeclaredFields()) {
+ Named namedAnnotation = f.getAnnotation(Named.class);
+ if (namedAnnotation != null && namedAnnotation.value().equals("jcr:primaryType")) {
Review comment:
agree, I try to avoid hard-coding too. Thanks for catching that.
--
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
[GitHub] [sling-whiteboard] klcodanr commented on a change in pull request #73: SlingModelPersist type handling cleanup
Posted by GitBox <gi...@apache.org>.
klcodanr commented on a change in pull request #73:
URL: https://github.com/apache/sling-whiteboard/pull/73#discussion_r605163744
##########
File path: SlingModelPersist/src/main/java/org/apache/sling/models/persistor/ModelPersistor.java
##########
@@ -18,10 +18,10 @@
*/
package org.apache.sling.models.persistor;
+import com.drew.lang.annotations.NotNull;
Review comment:
@badvision any particular reason for changing annotation types? A number of other projects use the Jetbrains annotations, so it'd be better to be consistent
##########
File path: SlingModelPersist/src/main/java/org/apache/sling/models/persistor/impl/ModelPersistorImpl.java
##########
@@ -18,6 +18,7 @@
*/
package org.apache.sling.models.persistor.impl;
+import com.drew.lang.annotations.NotNull;
Review comment:
Same comment re:annotation
##########
File path: SlingModelPersist/src/main/java/org/apache/sling/models/persistor/impl/ResourceTypeKey.java
##########
@@ -50,22 +52,34 @@ public static ResourceTypeKey fromObject(Object obj) {
return NODE_TYPE_FOR_CLASS.get(obj.getClass());
}
+ String nodeType = getNodeType(obj);
Model modelAnnotation = obj.getClass().getAnnotation(Model.class);
- String primaryType = (String) getAnnotatedValue(obj, ResourceType.class);
+ String resourceType = (String) getAnnotatedValue(obj, ResourceType.class);
+
// Use the model annotation for resource type as needed
- if (primaryType == null
+ if (resourceType == null
&& modelAnnotation != null
&& modelAnnotation.resourceType() != null
- && modelAnnotation.resourceType().length == 1) {
- primaryType = modelAnnotation.resourceType()[0];
+ && modelAnnotation.resourceType().length >= 1) {
+ resourceType = modelAnnotation.resourceType()[0];
}
- String childType = (String) getAnnotatedValue(obj, ChildType.class);
- if (primaryType != null) {
- ResourceTypeKey key = new ResourceTypeKey(primaryType, childType);
+
+ if (nodeType != null || resourceType != null) {
+ ResourceTypeKey key = new ResourceTypeKey(nodeType, resourceType);
NODE_TYPE_FOR_CLASS.put(obj.getClass(), key);
return key;
} else {
return ResourceTypeKey.NT_UNSTRUCTURED;
}
}
+
+ private static String getNodeType(Object obj) throws IllegalArgumentException, IllegalAccessException {
+ for (Field f : obj.getClass().getDeclaredFields()) {
+ Named namedAnnotation = f.getAnnotation(Named.class);
+ if (namedAnnotation != null && namedAnnotation.value().equals("jcr:primaryType")) {
Review comment:
Probably should use: http://jackrabbit.apache.org/api/2.20/org/apache/jackrabbit/JcrConstants.html#JCR_PRIMARYTYPE
--
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