[ https://issues.apache.org/jira/browse/SLING-2779?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13748499#comment-13748499 ]
Felix Meschberger commented on SLING-2779: ------------------------------------------ Some comments on the provided patch (dated Aug. 20th): * If the DefaultsValueMap composites two maps, I wonder why it is a ValueMap... Particularly since this implementation does not properly support type conversions and expectations as defined for ValueMap * The JavaDoc on the constructors are wrong and not very understandable * The get(String) method does not really convert types thus, the get(String, Class<?>) method might in fact throw a ClassCastException which is not intended. I would assume that get(String) would actually turn to get(String, Class<?>) with an appropriate type instead of the other way around. Likewise for get(String, T) * The ?: expressions are much complicated (and not enough parenthesized) to understand. Please change to explicit if-then-else. Otherwise errors may creep in which are hard to find * The keySet(), entrySet(), and values() methods don't work: They either refer to the wrong defaults entries (e.g. values()) or modify the defaults map (e.g. keySet() gets the keySet() from the defaults and just adds to that set. This may or may not work and modify the defaults map, actually ! * The checkKey() method seems duplicate with existing ValueMap implementations As a whole, this class looks rather fragile and I would not add this as is to our code base. > Support for default properties values of a resource > --------------------------------------------------- > > Key: SLING-2779 > URL: https://issues.apache.org/jira/browse/SLING-2779 > Project: Sling > Issue Type: New Feature > Components: API > Affects Versions: API 2.3.0 > Reporter: Gilles Knobloch > Attachments: SLING-2779.patch > > > I already noticed several times it would be useful to be able to specify a > default properties for a resource: > * if the resource itself contains the property, it will override the default > one. > * but if it doesn't, the default value is used. > This could be done either via: > * specifying a {{sling:defaults}} property on the resource, which contains > the path to the resource which properties will be used by default. > * providing a default map of properties > Attaching a patch for review. > For testing purpose, I put it under {{org.apache.sling.defaults}}, but I > imagine it could go to {{org.apache.sling.api.resource}}. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira