Hi, On 18 September 2017 at 14:17, Carsten Ziegeler <cziege...@apache.org> wrote:
> Ian Boston 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. > > > > 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. > > > > Adapting to the URIConverter from the ResourceResolver seems a lot > cleaner, > > assuming we believe the AdapterFactory pattern is a good pattern. > > > I think the adapter factory pattern is a good pattern *if* you need to > add an > adaption to a resource outside of the implementation providing that > resource. > > If your code is in the same bundle, then adapter factory usually > complicates things a little bit as its more expensive to invoke it (and > there might be subtle service activation races). Now I guess both can be > neglected, however I think the cleaner way is to not use an adapter > factory when the resource and the adapter reside in the same bundle. > > I see your problem with the HelperData/State object though. > > Now, an AdapterFactory is not wrong, so we could first go with this and > improve over time and see how we can move this into the implementation > directly. > Ok, btw, this patch can't be merged until there is a suitable Oak release, which I assume would need to be a 1.5.x release, same as the current version in Sling. Best Regards Ian > > Regards > Carsten > > > > -- > Carsten Ziegeler > Adobe Research Switzerland > cziege...@apache.org >
