Hi Felix,
On Fri, Dec 20, 2013 at 5:32 AM, Felix Meschberger <[email protected]> wrote:
> Hi Justin
>
> Wow ! I really like this !
>
> Still, as always, I have some comments:
>
> * I share Dan's concerns with respect to the @Inject annotation: Isn't that
> too generic ? (Yes, I am the strongly typed guy, so I love to know things
> upfront in a fail early style)
I'd love to better understand this. What is a use case in which you
would need to explicitly state that an injected field should be
sourced from a particular Injector?
Even if there was, I'd prefer to do that as a separate annotation, i.e.
@Inject @Source("osgi-service")
private Something something;
Keep in mind that in my worldview, 80% of cases will be handled by
*just* @Model and @Inject.
>
> * I see support for interfaces is implemented as proxies which are loaded
> through the interface type's class loader. But the InvocationHandler is
> created for each invocation of adaptTo. Would it make sense to cache the
> generated proxy classes and just create new instances of them on each
> invocation ? (well this maybe already falls into the optimization category)
Perhaps, but this is definitely premature optimization :)
>
> * I assum the value to @Filter is supposed to be a valid OSGi Filter
> argument. Should thus the respective sample in the wiki be fixed by having
> the string value be surrounded by parentheses ? Also, the existence of the
> @Filter annotation would stipulate the respective @Incject annotation to only
> be considered by the service injector.
Filter example has been fixed. I'm not sure that there's a need to
restrict the @Filter annotation to one particular Injector.
>
> * I would think the api bundle and package should rather be called
> annotations, right ?
>
> * The annotations are currently RUNTIME scoped and the packaged classes are
> eagerly loaded on bundle start time (you might want to leverage the
> BundleTracker for easier tracking of bundles). This sounds like a performance
> problem. How about doing it like Declarative Services and tooling generate a
> descriptor which will be loaded on bundle startup and the respective classes
> loaded on-demand ? (OTOH this runtime analysis copes better with Java
> extensions)
And another Maven plugin and then an Ant task and an Eclipse plugin
and an IntelliJ plugin... I really think runtime is the right
approach.
The better solution IMHO would to implement the wildcard adapter
support we discussed when Dan introduced dynamic proxies. The
YamfAdapterFactory could figure out that this was enabled and skip the
whole bundle scanning bit. But since one of the design goals was to
work with existing Sling bundles, I didn't deal with this now.
I was also trying to use Scannotation to scan the class files instead
of doing class loading. Ran into trouble, but will come back to this.
>
> * YAMF model classes must be exported for them to be usable in code. Maybe
> that is just how this kind of thing works but we should be aware of that and
> that exporting these classes in fact defines API. This would fall into the
> Model class requirements category: Must be exported and is considered API,
> classes must have public default constructore, etc.
Actually, this isn't the case - YAMF model classes do not need to be
exported *to be used by YAMF*. They do need to be exported to be
referenced in JSPs or other bundles, but that's just normal stuff.
YAMF itself doesn't put any requirements on the visibility of
packages.
Justin
>
> Thanks
> Felix
>
> Am 19.12.2013 um 18:07 schrieb Justin Edelson <[email protected]>:
>
>> Hi,
>> I've published a page to the wiki about a concept I've been working on
>> to consolidate the various appproaches I have seen in the wild to
>> model object creation. I'm calling this YAMF for now, although ideally
>> we'll just call it Sling Models :)
>>
>> Without repeating the whole contents of the wiki page, at a high
>> level, this is a purely annotation driven approach supporting both
>> classes and interfaces. Your model class simply needs to declare from
>> which other classes it can be adapted:
>>
>> @Model(adaptables=Resource.class)
>>
>> And then annotate the fields (for classes) and methods (for
>> interfaces) which need injection:
>>
>> @Inject
>> private String propertyName;
>>
>> You can inject properties, OSGi services, request attributes, and
>> entries from SlingBindings.
>>
>> New injector types can be created through an SPI.
>>
>> Additional annotations are supported for special cases:
>>
>> @Optional - mark a field/method as optional.
>> @Filter - provide a filter (i.e. for OSGi services)
>> @Named - specify a name (other than the default field/method name) to
>> use for the inejction lookup.
>>
>> More detail can be found here:
>> https://cwiki.apache.org/confluence/display/SLING/YAMF+-+Yet+Another+Model+Factory
>>
>> The working code is up in my whiteboard:
>> https://svn.apache.org/repos/asf/sling/whiteboard/justin/yamf/
>>
>> Look forward to your feedback.
>>
>> Regards,
>> Justin
>
