Hi Dan,
> The problem I see is that there's not a way to override the ordering of
the models beyond changing the name of the Model class or priority of the
bundle
That's true, but you can still override the selection logic by implementing
your own ImplementationPicker to return the right adaptable.
> I could easily see a problem whereby a user is not aware of another model
which is registered against the same configuration and therefore would have
difficulty determining a) what is the issue and b) how to get their class
to work.
Well, there is a warning in the log, but I agree this could be missed. We
could emit a warning at the time of adaptation (e.g. in the
ResourceTypeImplementationPicker) if there are multiple matches. But that
might get really noisy.
Regards,
Justin
On Mon, Mar 20, 2017 at 11:14 AM Daniel Klco <daniel.k...@gmail.com> wrote:
> Hey Justin,
>
> I don't see why it would be an overly common use case. The problem I see is
> that there's not a way to override the ordering of the models beyond
> changing the name of the Model class or priority of the bundle. Both of
> which could have other effects. I could easily see a problem whereby a user
> is not aware of another model which is registered against the same
> configuration and therefore would have difficulty determining a) what is
> the issue and b) how to get their class to work.
>
> Would the solution be to add two items:
>
> - an order attribute for the annotation
> - the collection method which returns all of the models applicable based
> on the resource type
>
> Of course, the other option is just to put a big fat warning that only one
> model class should *ever* be registered for a particular resource type.
>
> Thanks,
> Dan
>
> On Mon, Mar 20, 2017 at 10:26 AM, Justin Edelson <jus...@justinedelson.com
> >
> wrote:
>
> > (changed the subject line for clarity)
> >
> > Hi Dan,
> > IIRC, if two adapter implementations match exactly, the first
> > alphabetically is returned. But is returning both of these really a use
> > case? Based on your initial explanation, I would have thought that the
> > interface would be different, i.e. you have a "main" model class per
> > resource type and then another representing the data layer (which is a
> > cross-cutting concern and so would have multiple implementation for
> > different resource types).
> >
> > I think if we were to go down this path, it means a replacement for the
> > ImplementationPicker SPI. Which isn't the end of the world (we can always
> > provide a compatibility layer), but is something which should be done
> > cautiously.
> >
> > Regards,
> > Justin
> >
> > On Sun, Mar 19, 2017 at 10:45 PM Daniel Klco <dk...@apache.org> wrote:
> >
> > > That's a pretty accurate summary and yes, this would make sense as a
> > > Lambda.
> > >
> > > I do wonder how the current implementation would handle if one were to
> > > register two sling models against the same resource type and interface?
> > > Would the returned model be predictable? For example if I had:
> > >
> > > public interface DoesStuff {
> > > int random();
> > > }
> > >
> > > @Model(adaptable=Resource.class, resourceType="test/rt")
> > > public class Type3 implements DoesStuff{
> > > public int random(){
> > > return 3;
> > > }
> > > }
> > >
> > > @Model(adaptable=Resource.class, resourceType="test/rt")
> > > public class Type7 implements DoesStuff{
> > > public int random(){
> > > return 7;
> > > }
> > > }
> > >
> > > If I adapted a resource of type test/rt to the DoesStuff interface,
> would
> > > it return 3 or 7? Thus the idea of returning a collection so that any
> > > registered Sling Models would be processed.
> > >
> > > Regards,
> > > Dan
> > >
> > > On Thu, Mar 16, 2017 at 6:30 PM, Justin Edelson <
> > jus...@justinedelson.com>
> > > wrote:
> > >
> > > > Hi Dan,
> > > > I don't think I'm understanding how that API would work.
> > > >
> > > > You have a single Resource there and a single Class (which I assume
> is
> > > > actually an interface for your use case). Are you saying that for one
> > > > Resource, there would be multiple Model classes implementing that
> same
> > > > interface mapped to the same resourceType?
> > > >
> > > > I have seen code which does something like this (which I think is
> > closer
> > > to
> > > > what your first paragraph is describing):
> > > >
> > > > Resource -> collect children to make a list -> adapt each item in the
> > > list
> > > > to some interface -> remove the null values -> return the list
> > > >
> > > > This is pretty easy to do with Lambdas. Maybe we should have a
> separate
> > > > module (which can require Java 8) to provide some
> > Functions/Predicates...
> > > >
> > > > Justin
> > > >
> > > > On Thu, Mar 16, 2017 at 4:57 PM Daniel Klco <dk...@apache.org>
> wrote:
> > > >
> > > > > I do think though that there's a ton of value in the concept of
> being
> > > > able
> > > > > to tie SlingModels to a resource type. I agree that this API is
> > clunky,
> > > > but
> > > > > the idea of limiting based on resourceType in Sling Models was
> really
> > > > > missing. I am using this concept for a library I have in progress
> > right
> > > > now
> > > > > so that developers can "register" sling Models with a particular
> > > > interface
> > > > > against particular resourceTypes. My library then traverses down a
> > > > resource
> > > > > tree, adapting the objects and generating a representation of the
> > tree
> > > > for
> > > > > a DataLayer representation. This works with the current
> functionality
> > > but
> > > > > could be cleaner.
> > > > >
> > > > > What I would really like to be able to do is something like this:
> > > > >
> > > > > Collection<T> models =
> modelFactory.getModelsByResourceType(Resource
> > > > > resource, Class<?> T);
> > > > >
> > > > > I would expect the method to use both the Class and the Resource
> type
> > > to
> > > > > generate the collection of models which match the requested class
> and
> > > are
> > > > > applicable for the resourceType. Does that make more sense?
> > > > >
> > > > > Thanks,
> > > > > Dan
> > > > >
> > > > > On Thu, Mar 16, 2017 at 3:34 PM, Dirk Rudolph (JIRA) <
> > j...@apache.org>
> > > > > wrote:
> > > > >
> > > > > >
> > > > > > [ https://issues.apache.org/jira/browse/SLING-6655?page=
> > > > > > com.atlassian.jira.plugin.system.issuetabpanels:comment-
> > > > > > tabpanel&focusedCommentId=15928701#comment-15928701 ]
> > > > > >
> > > > > > Dirk Rudolph commented on SLING-6655:
> > > > > > -------------------------------------
> > > > > >
> > > > > > The difference is that its not obvious to understand what the
> > purpose
> > > > of
> > > > > a
> > > > > > certain piece of the implementation is. Anyway, would you be so
> > kind
> > > > and
> > > > > > point us to the right place where those methods are used for
> > > > Handlebars?
> > > > > I
> > > > > > guess there is a JSR-223 compliant implementation somehow
> exposing
> > > > > > adaptTo() or the ModelFactory to the scripts themself. IMHO that
> > one
> > > is
> > > > > > responsible for finding the right adapterType instead of
> enforcing
> > > 1:1
> > > > > > resourceType to adapterType.
> > > > > >
> > > > > > > Remove untyped getModelFrom* methods from ModelFactory
> > > > > > > ------------------------------------------------------
> > > > > > >
> > > > > > > Key: SLING-6655
> > > > > > > URL: https://issues.apache.org/
> > > > jira/browse/SLING-6655
> > > > > > > Project: Sling
> > > > > > > Issue Type: Wish
> > > > > > > Affects Versions: Sling Models Impl 1.3.0
> > > > > > > Reporter: Dirk Rudolph
> > > > > > >
> > > > > > > As far as I understood the the whole story behind Sling Models
> > API
> > > > > > is/was that it can be used to adapt ordinary objects to typed
> > > objects.
> > > > > With
> > > > > > adding {{Object getModelFromResource(...)}} and
> > > > > > {{getModelFromRequest(...)}} this paradigm has been broken. Just
> > > > looking
> > > > > at
> > > > > > the API, what is the purpose of that methods? I agree that it
> makes
> > > > much
> > > > > > sense to have a binding between the resourceType of {{Resource}}
> > and
> > > > > maybe
> > > > > > even {{SlingHttpServletRequest}} and a model, but this resulting
> > into
> > > > an
> > > > > > ordinary object from API perspective makes it kind of pointless.
> > > > > > > I propose:
> > > > > > > * mark them as deprecated as already done in SLING-6652
> > > > > > > * let them throw {{UnsupportedOperationException}} to prevent
> > them
> > > > > > being used
> > > > > > > * remove them in the next major API release
> > > > > > > * move the logic of "finding the nearest implementation by
> > > > > resourceType*
> > > > > > to {{ResourceTypeBasedResourcePicker}}
> > > > > > > and for the exporter:
> > > > > > > * if a {{@Model}} has an {{@Exporter}} implicitly registered it
> > > with
> > > > > the
> > > > > > {{implType}} as {{adapterType}}, if not already done
> > > > > > > * use the {{implType}} in the {{ExporterServlet}} to adapt from
> > > > request
> > > > > > or {{Resource}} to the model object
> > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > This message was sent by Atlassian JIRA
> > > > > > (v6.3.15#6346)
> > > > > >
> > > > >
> > > >
> > >
> >
>
