Add a priority concept to components

Hi everyone,

so that proposal is about adding a new concept of Priority in the Component descriptor interface. This priority would have different usages:

  1. if 2 components have same hint and role, the one with most priority would override the other one (this would solve: [XCOMMONS-1814] Component with higher priority is not taking precedence when deployed as an extension - XWiki.org JIRA). This would need to be backward compatible with the priorities defined in components.txt
  2. when getting list or map of components through the API, those list and maps would be automatically ordered by priority: this would allow different UC, like some transformation happening before others, or some listener being called before others

For defining the priority in Java components there would be different mechanism:

  1. for backward compatibility reason, it should still be possible to define it through components.txt (which would have the priority over other mechanisms)
  2. it should be possible to also define the priority when defining a new component as a Java class, so I propose to add the priority field in the Component annotation
  3. I don’t know much the definition of components through xobject, but we should also find a way to define the priority there (probably through a new property)

For backward compatibility reason, all components which do not define their priority would have a same priority value (probably defined to 1000 as it’s the actual default value in components.txt if I’m correct).

Now I also propose that contrarily to what we have now, we do define a specific ordering of the components when they all share the same priority, to stop relying on randomness: so I propose that we use lexicographic sorting based on the fully qualified name of the components for that.

You don’t mention what your proposal means for ComponentDescriptor, but that’s what everything is based on.

For me, this does not really make sense.

There should be two completely different values since it’s two completely different use cases and needs and I can easily think of use cases where you need to override a specific component and at the same time order it among others components which share the same role and there is very little chance that you can use the same value for both (just think of a use case where you override a component to change its order).

If you really want to talk about both subjects in the same proposal, I would suggest the following changes:

  • ComponentDescriptor should have two different getters for those concepts (“override priority” and “role priority”)
  • As you proposed, having the “role priority” as a field of the @Component annotation sounds good
  • Keeping components.txt as the way to configure the “override priority” for Java components is probably enough, I’m fine having the possibility to also configure it at @Component level as long as it’s a different field
  • For wiki based components, all it takes is adding two properties for those concepts in whatever xclass which is used to define them

Indeed, I was more focused on the ordering UC and I mixed with the other ones because I found it when searching for jira tickets, and was using the same “priority” vocabulary. But yes, I agree they should be considered as two different things.

I’m fine with override priority but I think I would use ordering priority for the second one.

I think I would not put it in the annotation: AFAIU it’s not a requirement for fixing [XCOMMONS-1814] Component with higher priority is not taking precedence when deployed as an extension - XWiki.org JIRA.

+1 with the other changes

Actually, after thinking about it a bit more, I would suggest concept naming a little more consistent with what we currently have in the components API:

  • “role type priority” (what is used to order components sharing the same role type)
  • “role hint priority” (what is used to decide which of the components sharing the same role hint should be selected)

So that in ComponentDescriptor/ComponentRole you end up with:

  • getRoleType()
  • getRoleTypePriority()
  • getRoleHint()
  • getRoleHintPriority()

Hi,

We already have a concept of priority for components (through components.txt), as registration time (Component Module (XWiki.org)). It seems this proposal is about something else:

  • Regarding point 1), I consider that a bug. Right now we only look at the component priority during the initial scan and registration of components, and not during dynamic registration of components. Right now when we register a new component dynamically, it always replaces the existing component role/hint. We could decide to look at the priority (thus save it since right now it’s not part of the CD).

  • Regarding point 2), this is a different topic and it’s not related to priority for the same role/hint. It’s a priority only for roles.

Some general thoughts:

You didn’t mention if this proposal has any impact on CM performance. It’s very important that it doesn’t as CM is a critical piece of code and is used a lot, and we need to keep it performant. For example if you do the sorting when registering a new component then that’s going to impact perfs a lot.

Small note that our original idea was to make the simplest possible CM with the least amount of features, so that we could later dump our CM impl and use an existing impl (like CDI, Guice, OSGi, etc). So the more features we add, the harder it is to find an existing component system that has these features.

Now, in practice, it’s been over 15 years that XWiki has its own CM impl and I’m doubting that we would we able to switch to another impl. It would be interesting though to see if there are CM impl nowadays that support dynamic registration of components (that used to not be supported at the time by all existing CMs except OSGi which was too complex).

Sounds good globally (but only if general performances are not impacted, we’ll need a perf benchmark to verify this).

Thanks!

So as Thomas noticed in Add a priority concept to components - #2 by tmortagne I mixed two different things in the same proposal on the first post, because of the sharing of “priority” vocabulary. But indeed there’s two different things:

The idea would be to avoid the performance impact as much as possible. I didn’t mention it, because for me it was obvious that we need to be careful about it.
I actually already started to request opinion on this particular point in the draft PR: https://github.com/xwiki/xwiki-commons/pull/265#discussion_r959567118

So Thomas actually found back an old proposal about using a @Priority annotation, which has been implemented and which is used for the mandatory documents apparently. So I now propose to reuse the same annotation here.

yes obviously but for me it must not have any performance degradation for the main paths (getInstance()). That’s very important since the CM is used a lot.

In that thread we didn’t fully agree about using that annotation vs using dedicated XWiki priority annotations. I have the same comments I had back then.

Yes so I’m reading the thread and it looks like the problem was about (again) the possible usage of various kind of priorities.
Now the thing is the javax.annotation.Priority annotation is used for ordering the MandatoryDocumentInitializer components after retrieving them through ComponentManager#getComponentList.
My proposal here would be only to extend that behaviour to all other components. So basically any component could have a @Priority annotation, and that would be used for ordering the components in the CM.

If a specific kind of component needs a priority for another UC than “getting an ordering list of components from the CM”, then it would need to use another way to express it.