On 10.6.16 9:26 , Carsten Ziegeler wrote:
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.
FWIW: in Oak we have a similar situation with Tree instances. Retrieving
a non existing child from a Tree instance returns a Tree instance whose
.exists() method returns false. Calling .getParent() on that non
existing Tree instance returns the original parent Tree instance from
which it was acquired.
Michael
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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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
