Agreed with Carsten. Reading the docs and considering how people use the API, all resources should have types, the potential for breaking changes in downstream code if Resource Types are allowed to be nullable is significant.
On Thu, Jan 9, 2020 at 8:37 AM Carsten Ziegeler <[email protected]> wrote: > Thanks for the pointer, Konrad: that page clearly states: > > "Each resource has a resource type which is used by the Servlet and > Script resolver to find the appropriate Servlet or Script to handle the > request for the Resource." > > The javadocs are not very precise, but they do not mention that it is > possible to return null. > > I would assume that all our resource providers implement this already > like this that always a type is returned. getResourceType is marked with > @NotNull as well. > > I personally think that every resource should have a type to identify > what this is about, it's like a mime type just specified differently. > > I also think that if we now start to allow null as a return value, there > will be code that simply blows up because it does not expect that the > method returns null. So this would be a breaking change. > > In summary, we have documented it in our docs and javadocs to always > return something, so we should keep it like that. > > Unless of course there is something fundamentally wrong with it which > justifies a breaking change > > Regards > Carsten > > On 09.01.2020 14:09, Konrad Windszus wrote: > > I tend to agree with the statement that the resource type should be > optional. Not every resource should be rendered and having no resource type > set on those resources feels natural to me. > > Enforcing a non-null resource type for every resource is more > complicated as each resource provider must be aware of it (even if you > rarely have to deal with it explicitly). For super types we explicitly have > the default "sling/servlet/default". I think we should make it the implicit > resource type as well (in case none is set, i.e. is null) > > In case there are issues in the code like NPEs for such resources, I > would rather fix those. > > > > In any case the documentation at > https://sling.apache.org/documentation/the-sling-engine/resources.html#resource-properties > should be clarified. > > > > Konrad > > > > > >> On 9. Jan 2020, at 14:00, Stefan Seifert <[email protected]> > wrote: > >> > >> i've created a ticket concerning the Sling API that the method > Resource.getResourceType should be marke das @Nullable, i always thought it > was a bug and that the resource type is optional [1]. > >> > >> carsten pointed out that the resource type is mandatory. the javadocs > are not precise on this, i've also found no clear statement in the sling > docs (I might not have found it). > >> > >> technically there is always a resource type when the JCR resource > provider is used (fallback to JCR primary type if sling:resourceType is not > set), but that's not implicitly the case for other resource providers or > synthetic resources. > >> > >> if we think a resource type should be never null, we should return a > default value for resources that do not have one, and update the > documentation. > >> > >> WDYT? > >> > >> stefan > >> > >> [1] https://issues.apache.org/jira/browse/SLING-8975 > >> > > > > -- > -- > Carsten Ziegeler > Adobe Research Switzerland > [email protected] >
