Hi,
On Thu, Mar 12, 2015 at 9:44 PM, Sergiu Dumitriu <sergiu(a)xwiki.org> wrote:
On 03/12/2015 07:24 AM, Eduard Moraru wrote:
Hi,
On Thu, Mar 12, 2015 at 12:03 PM, vincent(a)massol.net <vincent(a)massol.net
wrote:
> Hi Edy,
>
> On 12 Mar 2015 at 10:49:29, Eduard Moraru (enygma2002(a)gmail.com(mailto:
> enygma2002(a)gmail.com)) wrote:
>
>> Hi,
>>
>> As it's documentation [1] mentions, the usage of the @Priority
annotation
>> should be defined by the classes it is
used on:
>>
>> "The effect of using the Priority annotation in any particular instance
> is
>> defined by other specifications that define the use of a specific
class.
>> For example, the Interceptors
specification defines the use of
priorities
>> on interceptors to control the order in
which interceptors are called."
>>
>> Therefore, I suggest we use the @Priority annotation on components that
>> need it and that like to specify the order in which they are *used*
(i.e.
perform their main task).
so what you’re suggesting is that:
@Component
@Name(“content”)
@Priority(1000)
public class ContentMacro implement Macro
has a different meaning than:
@Component
@Named(“XWiki.WatchListJobClass")
@Priority(1000)
public class WatchListJobClassDocumentInitializer ...
because one if a Macro and the other one is a Document Initializer
right?
...and because they clearly express it, in their documentations, that
they
accept some annotations and they define how those
annotations will be
interpreted. Basically, the purpose of the javax.annotations package.
(BTW note that this wouldn’t work if in the
future we start supporting
several roles per component impl.)
So it means that people reading the code need to understand that even
though it’s the same annotation, it’ll have a different meaning.
Compare this to:
@Component
@Name(“content”)
@MacroPriority(1000)
I don`t find this better since it does not tell me what the macro does
with
that priority. @MacroExcutionPriority would have
been clear, if that is
what we pursue.
public class ContentMacro implement Macro
and
@Component
@Named(“XWiki.WatchListJobClass")
@DocumentInitializerPriority(1000)
public class WatchListJobClassDocumentInitializer ...
IMO the second one is more clear in its intent. WDYT?
Honestly, I am not a big fan of annotations, specially in Java, and I try
to keep them to a minimal as much as possible. It feels like a shortcut
that leads to a dead end. They are not code, but configuration and, as
such, modifying configuration should not require recompiling the code.
Back to our particular discussion, AFAIK, we are not doing multiple roles
per implementation. That, indeed, would probably not work with the javax
Priority annotation due to lack of specificity.
I do see the advantages of typed annotations, but also the need to be
aware
of more and more annotations, as they come, when
our usecase is pretty
simple and would be well satisfied by the javax Priority one. That is the
main reason why I looked for a more generic solution instead of just
making
a new annotation for the document initializer use
case. I find it
uselessly
polluting.
I`d love to hear more opinions on this :)
For your love, here's another opinion.
Always appreciated :)
1. I spent a while looking for javax.annotations.Priority, since I
didn't see it at
http://docs.oracle.com/javase/7/docs/api/javax/annotation/package-summary.h…
... Turns out it's a JEE class that's not in JSE. AFAIK, we're not
really using JEE yet, just JSE with a few JEE modules. Am I wrong? If
not, do we want to require JEE from now on, or is that annotation
available in a Maven Central library that doesn't bring in the whole JEE?
Just an individual library:
http://mvnrepository.com/artifact/javax.annotation/javax.annotation-api/1.2
2. I agree with you that this is configuration, not code, thus changing
it shouldn't require recompiling code. I'm -0.5 for the original
proposal because of this.
3. I'd like to resurect
http://markmail.org/thread/mrbmbn45cltfvh57 and
enhance/build on top of it. Since we're already using an external file
for listing components, and since we already have a kind of priority in
that file, why not use that file to also give components priorities. If
someone wants to change the priority of a component, all that's required
is to add a new file in WEB-INF/classes with the new priority of the
component.
Re 2) and 3): I also thought about externalizing all of this into a file,
but I do not think this is the right situation where this applies. For
instance, in JPA annotations, I would agree: That stuff belongs in a
configuration file, not in class annotations. However, for components, it's
not really something you get to configure that often. Also, if for JPA you
had control over the DB to change things so a config file would be handy to
just fix the config without a recompile, for components everything is
related to code. If you want to change something, i.e. a priority, you do
it because you changed some code that needs this config modification, so
these annotations are tightly related to the code (i.e. you have to build
something anyway) and I see no advantage in externalizing them in a config
file.
I am not completely against annotations, just against abusing them. 2)
seems to suggest that you would like us to remove them (all component
annotations, since you revived the discussion on 3) altogether.
Re 3), AFAIR (did not re-read it now, just scanned it quickly), I believe
that file only makes sense for the component manager and component related
priorities like default implementation priority, execution priority and
dispose priority. Anything else that is related to how a specific component
is used by another specific component (i.e. not the component manager
itself; e.g. How the macro manager is running macros or how the
transformation manager is running transformations or whatnot), IMO, does
not belong in that file.
Thanks,
Eduard
Before we discuss a syntax, anybody wants to veto
this?
Thanks,
Eduard
>
>> Priorities on other behaviors that are added to a component (for
example
>> through interfaces like Initializable or
Disposable, interfaces which
are
>> not components themselves) should provide
their own specialized
>> (behavior-driven) priority annotations (e.g. @DisposePriority,
>> @InitializationPriority, etc.).
>>
>> Note: If we want to explore the possibility of using our own generic
>> Priority annotation, we need to consider the fact that multiple
> annotations
>> on the same java class is only supported [1] starting with java 1.8.
> Until
>> then, the commonly used workaround [3] seems cumbersome to use.
>
> Yep, I’d really not like to use a generic annotation with the namespace
> being specified. I much much prefer typed annotations.
>
> Thanks
> -Vincent
>
>> Thanks,
>> Eduard
>>
>> ----------
>> [1]
http://docs.oracle.com/javaee/7/api/javax/annotation/Priority.html
>> [2]
http://bugs.java.com/bugdatabase/view_bug.do?bug_id=7151010
>> [3]
>>
>
http://stackoverflow.com/questions/1554112/multiple-annotations-of-the-same…
>>
>> On Thu, Mar 12, 2015 at 10:41 AM, vincent(a)massol.net
>> wrote:
>>
>>> Hi devs,
>>>
>>> As part of
http://jira.xwiki.org/browse/XWIKI-11905, Edy has started
>>> using the Java @Priority annotation.
>>>
>>> This seems very good and I personally didn’t know about this
annotation
>>> before (maybe it’s been introduced
not that long ago?). So for me it
> raises
>>> the question of: do we want to use this annotation more and how does
it
>>> compare with what we’ve done so far.
>>>
>>> I can think of a few places that could have used it:
>>>
>>> * Macros.get/setPriority(). It should be possible to add support for
>>> @Priority and modify MacroTransformation to use that annotation.
>>> * Transformations. We have a jira issue opened for adding support for
>>> Priority in Transformation’s executions (in TransformationManager).
>>> * @DisposePriority (used by ECM).
>>> * TranslationBundle.get/setPriority()
>>> * … and probably some other places…
>>>
>>> However, I think there’s a namespacing problem. For example imagine
> that
>>> we code a Macro and set @Priority on that Macro component. The ECM
> could
>>> interpret it as a dispose priority while the MacroTransformation could
>>> interpret it as an execution priority…
>>>
>>> Globally I think that use an annotation for expressing priority is
> great
>>> and much better than what we’ve done in the past with
get/setPriority()
>>> methods. It’s better because priority
is not a business concept and
> we’re
>>> polluting the business interface with it.
>>>
>>> Now, in order to fix the namespacing issue, I think that the best
> solution
>>> is that each module requiring some priority should introduce its own
>>> annotation and should NOT depend on the @Priority one from the JDK
> (i.e. we
>>> ban the usage of it).
>>>
>>> WDYT?
>>>
>>> Thanks
>>> -Vincent
>>>
>>>
>>> W
--
Sergiu Dumitriu
http://purl.org/net/sergiu
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs