[Container] Models should allow access item resources

[ky940819]

Feature Request

Is your feature request related to a problem? Please describe. The current container models - i.e. those that implement Container interface / extend AbstractContainer class - do not allow access to the container item resources.

Rather, these containers return List<ListItem> from their Container.getItems() method. Since the ListItem interface provides only the resource name/path, we are forced into re-resolving the resource via the path to make use of it.

There are two reasons why this is not optimal. The first is that we already have the resource, using the path to re-resolve the resource is just extra (needless) work. The second reason is that the resource accessed by using the path may not be the resource that was used to construct the ResourceListItemImpl.

There is a common misconception that the following statement is true

Resource resource = // some resource
resource.getResourceResolver().getResource(resource.getPath()).equals(resource)

The above statement may be true in the JCR world, but it is not guaranteed to be true in sling. In the above example, resource may have been a wrapped or synthetic resource, while the result of resourceResolver().getResource(resource.getPath()) would, assuming no other ResourceProviders are in play, likely be a JCR backed resource that may or may not be equivalent.

An example of where this causes serious issues is when dealing with TemplatedResources, which create a synthetic tree that is the merger of the template resources and the page resources.

This issue can make it very difficult to use container components in template structure where it may have nested containers with a mixture of editable and non-editable children. This is certainly true for layout containers, but I can also foresee a situation where someone may want to have tabs, accordions, or carousels where some of the panels are part of the template structure and other panels are editable by the author in-page. (for transparency, i haven't actually checked that this doesn't work for tabs/accordions/carousels, but i would be surprised if it did, as it does not for the layout container)

Describe the solution you'd like There a couple ways this can be resolved.

The container components item lists are already backed by ResourceListItemImpl, which guarantees that the resource does exist and is known. An interface could be added:

public interface ResourceListItem extends ListItem {
    @NotNull
    public Resource getResource();
}

and then the Container.getItems() method could be changed to return ResourceListItem instead of ListItem. This isn't a lot of work, but may introduce API changes that are breaking. Probably not API changes that impact the HTL because ResourceListItems would still be ListItems; but projects using these methods in Java might have to update their APIs because List is not the same as List even if ResourceListItem isa ListItem.

Alternatively, the getChildren() method can be added to the Container interface and made public in AbstractContainerImpl.getChildren(). This would be backwards compatible, but isn't as clean if a consumer needs access to both the information in the ListItem and the resource within the same context.

Documentation N/A

---

side note: This is related to #999 which cannot be resolved in the "simple" container without access to the templated resource, and cannot be resolved in the "responsive" layout without the issue being resolved in wcm/foundation/components/responsivegrid.

Other containers can be resolved along the same lines once the API makes the nessisary information available.

[jckautzmann]

@ky940819 Thx for your valuable feedback. One easy way to achieve it would be to add a getResource() method to the ListItem model. We may return null by default to not break existing ListItem implementations. WDYT?

[ky940819]

That solution would certainly fix the issue. I do have a slight concern that the List item concept may be overloaded.

Sometimes it is used for lists of pages, links, etc, and other times for lists of child resources for a container component. I feel like those might be two different concepts, but yes from a maintaining backwards comparability adding ListItem.getResource would solve the problem.

[ky940819]

I would be happy to submit a PR for this, either adding a ResourceListItem interface, which i think would be the best way to distinguish between a list of items and a container of items, or just adding ListItem.getResource which would preserve backwards compatibility.

I assume that preserving backwards compatibility is highly important. if someone from the core team would comment on what solution would be most in-line with the project, I can do the PR.

[jckautzmann]

To preserve backwards compatibility, I would prefer that we add ListItem.getResource. I'll let others chime in.

[ky940819]

Hello, Still looking for some feedback on this one. It's an issue that impacts my projects so I would love to be able to contribute a fix sooner rather than later.

The above suggestion of ListItem.getResource is easy to add, but the requirement to handle potential null makes the presentation logic a little trickier.

Kyle

[vladbailescu]

I'm with @jckautzmann on this one

[rajeevyadav2]

I m with @jckautzmann as well.

[gabrielwalt]

This also relates to #1083.