[ https://jira.jboss.org/jira/browse/DNA-339?page=com.atlassian.jira.plugin... ] Randall Hauch commented on DNA-339: ----------------------------------- I like the approach of using templates (ala 283). Is there a way to change the NodeTypeSource-approach to use the new registration mechanism? Seems like that might clean up things quite a bit. I also do like where you are going with refactoring of findPropertyDefinition to use Predicates. However, I'm not convinced it's better. One goal I had with the old design was that the logic was very obvious and "in your face" (not masked by a lot of method calls), and I deemed this as more beneficial than the ugliness of the duplication. So while the new approach gets rid of the duplication, it doesn't really keep the "in your face" logic. Plus, the new approach doesn't reduce the number of lines (385 new vs 373 old). Besides, I'd prefer for this change to be pulled out into a different patch (or even a different issue). Also, considering the size of the change to support registration and unregistration, would it be better to not address thread-safety in the same patch? Also, although using a thread-safe container would be beneficial, we made need to use an explicit ReadWriteLock if the thread-safe container doesn't offer the required level of atomicity. Requiring users to oder the types to be registered is a perfectly valid assumption, I think. At least for now, since it'd likely keep the implementation much more simplistic. One thing we're going to have to address is that we can't update existing JcrNodeType objects. So, if a supertype is changed (re-registered), we really need to rebuild all of the node types that extend it. Unfortunately this is the only way to truly have immutable objects. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://jira.jboss.org/jira/browse/DNA-339?page=com.atlassian.jira.plugin... ] Randall Hauch updated DNA-339: ------------------------------ Fix Version/s: 0.4 Affects Version/s: 0.3 (was: 0.4) -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://jira.jboss.org/jira/browse/DNA-339?page=com.atlassian.jira.plugin... ] Brian Carothers updated DNA-339: -------------------------------- Attachment: DNA-new_type_registration.patch Version of the earlier patch that seeks to address the feedback provided and changes the basic approach from being *Template-based (a la JSR-283) to being based on NodeTypeSources like our current built-in types. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://jira.jboss.org/jira/browse/DNA-339?page=com.atlassian.jira.plugin... ] Brian Carothers updated DNA-339: -------------------------------- Attachment: DNA-339_no_key_class.patch Based on feedback, added patch to remove the JcrNodeDefinition.Key and JcrPropertyDefinition.Key classes. Also added in test cases for type registration. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://jira.jboss.org/jira/browse/DNA-339?page=com.atlassian.jira.plugin... ] Randall Hauch commented on DNA-339: ----------------------------------- Applying the "no key" patch, which removes the Key nested class from JcrNodeDefinition and JcrPropertyDefinition and instead uses the (existing) NodeDefinitionId and PropertyDefinitionId. The patch also adds test cases for type registration. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira