I think calling resolve() for traversal is totally unexpected. Now looking at this, I'm really wondering if it was a good idea to implement getParent on NonExistingResource. Let the client deal with it instead of adding complex special cases.
Carsten Konrad Windszus wrote > There is one related problem with mapping: > > Consider the case you have a resource resolver mapping from "/content/" to > "/". In the underlying repository you have a resource in > "/content/existingParent". > > Now it might happen that a NonExistingResource is resolved for path > "/existingParent/nonExistingResource". Since this is pointing towards a > non-existing resource the mapping is not active (i.e. the path will not > contain "/content"). So you end up with a NonExistingResource with path > "/existingParent/nonExistingResource". > Now you call getParent() on this resource. > > You would expect to end up with an existing resource in > "/content/existingParent" (because that path does exist in the repository). > Unfortunately due to the logic in AbstractResource.getParent() this just > calls ResourceResolver.getParent() which will not use > ResourceResolver.resolve(...) but rather ResourceResolver.getResource(...) > internally. So the mapping is not considered here. Therefore no resource is > found at "/existingParent", although it does exist at > "/content/existingParent". > > This is unexpected and wrong in my opinion. So the NonExistingResource should > not rely on AbstractResource.getParent() at all, but rather use its own > implementation relying on ResourceResolver.resolve(...). WDYT? > > Thanks, > Konrad > > >> On 02 Jun 2016, at 14:48, Konrad Windszus <konra...@gmx.de> wrote: >> >> Thanks for the input. >> I created https://issues.apache.org/jira/browse/SLING-5757 for tracking that >> and I am going to propose a patch in there. >> >> What about SyntheticResources which are not NonExistingResources? If we >> would follow the same approach as for NonExistingResources the question is, >> with which resource type the non-existing parent resource should be created? >> Or should a SyntheticResource (which is not a NonExistingResource) return a >> NonExistingResource for getParent() instead in that case? >> >> Konrad >> >> >>> On 02 Jun 2016, at 13:47, Daniel Klco <dk...@apache.org> wrote: >>> >>> I would imagine that the only thing this would change is to make a small >>> number of null checks irrelevant. >>> >>> +1 for making the behavior more consistent, however, the JavaDocs and >>> release notes should be explicit about this change. >>> >>> On Thu, Jun 2, 2016 at 4:45 AM, Georg Henzler <slin...@ghenzler.de> wrote: >>> >>>> Hi Konrad, >>>> >>>> +1 for making the behaviour of NonExistingResource more consistent - I >>>> personally can't think of any places this would break existing code. >>>> >>>> Regards >>>> Georg >>>> >>>> >>>> >>>> On 2016-06-01 15:09, Konrad Windszus wrote: >>>> >>>>> Hi Robert, >>>>> thanks for your input. >>>>> >>>>> >>>>>> I am not sure whether this would confuse existing clients though... >>>>>> >>>>> >>>>> I am also a bit worried about that but the only example I could think >>>>> of is a code trying to create the parent nodes or collecting the >>>>> non-existing ones by checking getParent() for null. >>>>> >>>>> This would be pretty bad style IMHO therefore I would deliberately be >>>>> willing to break that code. I wonder what do others think about >>>>> changing the semantics of getParent() for NonExistingResource and >>>>> probably also SyntheticResource. >>>>> Konrad >>>>> >>>> >>>> >> > > -- Carsten Ziegeler Adobe Research Switzerland cziege...@apache.org
