On Mon, Sep 18, 2017 at 2:58 PM, Ian Boston <i...@tfd.co.uk> wrote: > Hi, > > On 18 September 2017 at 13:51, Carsten Ziegeler <cziege...@apache.org> > wrote: >> >> Julian Sedding wrote >> > Hi Ian >> > >> > Would it make sense to remove the AdapterFactory implementation and >> > instead pass along the URIProvider service(s) using the >> > o.a.s.jcr.resource.internal.HelperData object instead? According to >> > its javadoc, it is "used to pass several services/data to the >> > resource". >> > >> > I assume the AdapterFactory is used because you don't have access to >> > the OSGi service registry in JcrNodeResource where you need the >> > URIProvider. >> > >> >> Sounds like a good change to me, right a AdapterFactory shouldn't be >> needed. > > > The patch is quite extensive and it would need care to ensure that the list > that is passed is fully thread safe. So far this approach has only been used > with an AtomicReference holding a single value.
I don't see the problem, Java has pretty decent support for concurrent collections. Another option would be to pass a BundleContext to HelperData and retrieve required services on-the-fly. IMHO that would improve encapsulation and clean up both JcrProviderState and JcrProviderStateFactory. > > If a larger patch is wanted, I can track down everywhere the the > JcrProviderState is created, as this holds the HelperData for the session, > although the createProviderState method is already deprecated, which is the > only way to create JcrProviderState afaict. See the JcrProviderStateFactory. I doubt that the patch would be larger (in terms of LOC). It would likely touch more existing classes. HelperData's constructor is called precisely once. So is JcrProviderState's constructor. The non-private method createProviderState is called once (and yes, it's the only way to create a JcrProviderState). The method is not deprecated (it suppresses the deprecation warning of loginAdministrative). > > Adapting to the URIConverter from the ResourceResolver seems a lot cleaner, > assuming we believe the AdapterFactory pattern is a good pattern. IMHO publicly registering an AdapterFactory for private use is a violation of the principle of least surprise and should thus be avoided. Or in other words, I think it makes the code unnecessarily hard to understand. Regards Julian > > Best Regards > Ian > >> >> >> Carsten >> >> >> >> -- >> Carsten Ziegeler >> Adobe Research Switzerland >> cziege...@apache.org > >
