[ http://issues.apache.org/jira/browse/JCR-473?page=comments#action_12419419 ]
angela commented on JCR-473: ---------------------------- InternalValue: > we should find something nicer. for example, drop the to methods > that take the nsresolver: maybe i'm missing your point. from my point of view, the NamespaceResolver is a different story than Value-conversion. While the NamespaceResolver is always used to build a qualified value from a JCR Value (for Name and Path values), the value conversion takes place if the create methods specifies a targetType which may differ from the type of the passed String or Value. the conversion helper methods are offered by the ValueHelper, but currently the ValueHelper itself knows about which value objects to create and i would argue, that this should be done by the ValueFactory only. i guess, this was the point of the ValueFactory in jsr170, which was introduced quite a while after the initial version of the spec. moving the InternalValue.create to the ValueHelper looks strange to me: - the InternalValue is a core functionality (and named 'internal') - the ValueHelper is a utility class only and in the commons module. - moving the IV-create methods to ValueHelper would require the InternalValue to become a common one. - and last it would not solve my point, that the ValueHelper should not know about the Value implementations. So, not passing the ValueFactory throughout the code, could be solved by: - don't deprecate InternalValue.create(Value/String, int, NamespaceResolver) - remove the new methods in InternalValue that take the ValueFactory - in any case: directly use ValueFactoryImpl within InternalValue (such as in the patch) in order to retrieved a converted jcr value from the ValueHelper. Since currently ValueHelper creates Value objects itself (in addition to the ValueFactoryImpl and from the same value implementations, see above), this would not change the resulting InternalValue... i simply didn't like, that the InternalValue knows which ValueFactory to use. but maybe i'm wrong... would that be acceptable for you? > Some enhancements to jackrabbit commons > --------------------------------------- > > Key: JCR-473 > URL: http://issues.apache.org/jira/browse/JCR-473 > Project: Jackrabbit > Type: Improvement > Versions: 1.0 > Reporter: angela > Assignee: angela > Attachments: JCR-473_rev_417443.patch > > I would like to suggest a couple of enhancements to the commons module. > The patch was created against rev. 417443 and the tests did not reveal any > problems. > Summary of suggestion modifications: > QName > ------------------------------------------------------------------------------------------------------------------------- > - reduce QName to its core functionality and put conversion from and to JCR > name to > a separate class 'NameFormat' > - in order not to break existing code, all methods that deal with the > conversion in QName > are marked deprecated. > - add constant for the name of the root node. > Path > ------------------------------------------------------------------------------------------------------------------------- > - reduce Path to its core functionality and put conversion from and to JCR > path to > a separate class 'PathFormat' > - in order not to break existing code, all methods that deal with the > conversion in Path > are marked deprecated. > - introduce new constants for UNDEFINED_INDEX (0) and DEFAULT_INDEX (1), that > are currently hardcoded throughout the jackrabbit project. > - new method Path.getElement(int) [PathElement] > - make PathElement constants public (used by PathFormat) > Path.PathBuilder > ------------------------------------------------------------------------------------------------------------------------- > - additional constructor PathBuilder(Path) > Path.PathElement > ------------------------------------------------------------------------------------------------------------------------- > - add PathElement.getNormalizedIndex() that always asserts a 1-based index. > - change subclasses to be private (no usage within the jackrabbit, except > inside Path). > PathMap > ------------------------------------------------------------------------------------------------------------------------- > - move o.a.j.core.PathMap to o.a.j.util.PathMap in order to make it > available in the > commons module. > NamespaceResolver > ------------------------------------------------------------------------------------------------------------------------- > - add methods for resolution of paths: > > getQPath(String jcrPath) [Path] > > getJCRPath(Path qPath) [String] > NamespaceListener > ------------------------------------------------------------------------------------------------------------------------- > - add method 'namespaceRemove(String)' > ValueHelper > ------------------------------------------------------------------------------------------------------------------------- > currently JCR value objects are 'manually' created in the ValueHelper > despite the > fact, that JSR170 defines a ValueFactory interface. Consequently the > ValueHelper > present in the commons module can only be used by implementations that use > the same value implementations. > - add new helper methods that take a ValueFactory as argument. > - in order not to break existing code the original methods are marked > deprecated and > may be removed at a later time. > consequently: > - modify signature of InternalValue.create that include a value conversion > to take a > ValueFactory param and adjust all usages inside the core package. > ValueFactoryImpl > ------------------------------------------------------------------------------------------------------------------------- > - createValue(String, int ): used to call the conversion on ValueHelper. with > the > changes suggested to ValueHelper, the code must be changed in order to > created instances of the Value implementations within the factory. > - together with the modification to ValueHelper, stefan suggested to replace > the public > constructor with a static 'getInstance' method. All usages within > jackrabbit.core, were > modified accordingly. > Text > ------------------------------------------------------------------------------------------------------------------------- > - add getName(String, boolean) where the boolean flag indicates whether a > trailing slash > should be ignored. > - add getRelativeParent(String, int, boolean) where the boolean flag > indicates whether a > trailing slash should be ignored. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira
