On Apr 6, 2012, at 11:43 AM, Anca Luca wrote:

> I On 04/06/2012 09:35 AM, Vincent Massol wrote:
>> Hi Sergiu,
>> 
>> Note that below I'm going to play the devil's advocate since I think it's 
>> important we really think hard before changing anything and verify if our 
>> current implementation is not enough.
>> 
>> See below.
>> 
>> On Apr 5, 2012, at 8:44 PM, Sergiu Dumitriu wrote:
>> 
>>> On 04/05/2012 12:51 PM, Anca Luca wrote:
>>>> On 04/05/2012 06:42 PM, Vincent Massol wrote:
>>>>> Hi Sergiu,
>>>>> 
>>>>> On Apr 5, 2012, at 5:55 PM, Sergiu Dumitriu wrote:
>>>>> 
>>>>>> Hi devs,
>>>>>> 
>>>>>> Currently, requesting a component instance without a hint will look
>>>>>> for the implementation that uses the "default" hint, which makes it
>>>>>> difficult to change the implementation in an XWiki instance. Sure, it
>>>>>> is easy as long as all the implementations use the "default" hint,
>>>>>> but choosing the default between alternative implementations that
>>>>>> should all still be usable by themselves is not possible.
>>>>>> 
>>>>>> Also, "default" is not really a good hint, since it describes the
>>>>>> state of the implementation, not the technology, the aspect that
>>>>>> makes it different from the others. It would be better to name each
>>>>>> implementation with a proper hint.
>>>>>> 
>>>>>> I propose to define a mapping that can specify which hint is the
>>>>>> default for a component. In a text file,
>>>>>> META-INF/component-defaults.txt, we'll keep
>>>>>> componentinterface=defaulthint mappings. For example:
>>>>>> 
>>>>>> com.xpn.xwiki.store.XWikiStoreInterface=hibernate
>>>>>> com.xpn.xwiki.store.migration.DataMigrationManager=hibernate
>>>>>> 
>>>>>> And then, when we lookup the current storage implementation, we don't
>>>>>> need to check what is the configured hint in xwiki.cfg (or
>>>>>> xwiki.properties), we can just request the default implementation.
>>>>>> 
>>>>>> If there's no mapping for a component, we'll continue to use the
>>>>>> "default" hint.
>>>>>> 
>>>>>> I'm not sure where exactly to keep such files. We bundle a
>>>>>> components.txt file in each jar containing component implementations.
>>>>>> We could do the same for the components we consider the platform
>>>>>> defaults, and allow overrides in the
>>>>>> WEB-INF/classes/META-INF/component-defaults.txt file. Still, this
>>>>>> means that whenever platform defaults change, we need to keep another
>>>>>> special section in the release notes, to let users know about these
>>>>>> changes, so that they can manually revert to the old default if they
>>>>>> need to.
>>>>>> 
>>>>>> In the future we could change existing components to give proper
>>>>>> hints instead of "default", where such a change is applicable.
>>>>>> 
>>>>>> Another idea is to not use "default" at all, and instead go for a
>>>>>> generic "xwiki", "xe", "xwiki-platform" or something like that
>>>>>> whenever there's just one implementation for a component and we can't
>>>>>> find another hint to describe it.
>>>>>> 
>>>>>> WDYT?
>>>>> This is not really how it's been designed ATM. Whenever you wish to
>>>>> use a different implementation of a component you use a component
>>>>> implementation with the same role and same hint. You then make it
>>>>> available in your classpath. (Of course you can also do this at
>>>>> runtime simply by registering a new implementation over the old one).
>>>>> 
>>>>> To decide which implementation is used you use a priority order, as
>>>>> described on:
>>>>> http://extensions.xwiki.org/xwiki/bin/view/Extension/Component+Module#HOverrides
>>>>> 
>>>>> 
>>>>> I'd be curious to know your exact use case and understand why the
>>>>> current mechanism doesn't work for it.
>>> "...choosing the default between alternative implementations that should 
>>> all **still be usable by themselves** is not possible"
>>> 
>>> The overrides mechanism allows to change which component will be returned 
>>> for the "default" hint, but all the others will be invisible.
>>> 
>>>> One usecase I see is that you have multiple implementations and you want
>>>> to change the default one for a specific running instance of xwiki.
>>>> 
>>>> Overwrite mechanism only allows you to say which impl should be used
>>>> from the _components with the same hint_. However, you cannot change the
>>>> hint of a component at configuration time, so if you have a standard
>>>> distr of xwiki and you want to use ldap authentication, let's say (if
>>>> only auth was impl with components), unless you do some java to add the
>>>> default hint to the ldap implementation and then to specify that this
>>>> one has priority over all the default ones, I don't see how you can
>>>> re-wire the default.
>>> Exactly. For most of the "services" the XWiki platform currently has 
>>> (storage, cache), we don't have a "default" implementation, but we rely on 
>>> a kind of factory to lookup the configured default. That is an actual 
>>> factory class in the case of the cache service, but just more code in the 
>>> old XWiki.java class for the storage initialization.
>> Yep that's how it works.
>> 
>>> A standard way of selecting the default means that we'll need less 
>>> factories, and less code is always a good thing.
>> Yes but it has more limitation to what we have since with a proper Factory 
>> you can imagine all kind of logic to decide which implementation to use 
>> (hour of the day, whether the user is a premium user or not, etc).
>> 
>> BTW we do support Providers and the goal of the Provider is to be a factory 
>> for a given Role. So from now on, there should be no need to implement a 
>> Factory proper. Implementing a Provider is the new best practice for this.
>> 
>>> Now, suppose one of the older components that had only one implementation, 
>>> "default", gets alternative implementations, and we want to be able to 
>>> allow more than one to be active in a wiki, and let the administrator 
>>> decide which one should be considered the default. How can we approach 
>>> this? The only way is indeed to have multiple hints, but last time I 
>>> checked this resulted in more than one instance, even for @Singleton 
>>> implementations.
>> Correct, ATM we support only one hint per implementation.
> 
> ? I think Marius needed that for some wysiwyg stuff and we actually support 
> multiple hints per implementation. It's just gonna give you a new instance 
> for each hint, which in my view is a bug, not a missing feature.

We don't support multiple hints per implementation. What we do is if we find 
multiple hints in the @Component annotation then we register several 
components. But you always have one component per role/hint.

Using several hints in @Component is bad practice ATM. It means the code is 
serving different Roles and it's a better design to separate the concerns.

Thanks
-Vincent

>>> Another approach is to deprecate the direct dependency declaration and 
>>> instead introduce a factory that is responsible for selecting the default, 
>>> but this breaks backwards compatibility.
>> Not completely true. Imagine you have (Role, "default") as your current 
>> component and you wish to be able to choose various implementation from now 
>> on. What you could do is this:
>> 
>> * Write a new component, a Factory, that overrides the current component, 
>> i.e. by using the same (Role, "default"). This factory will then delegate to 
>> whatever other implementation it wishes.
> 
> That's only possible if the public API of the CM allows you to grab overriden 
> components (non-default defaults :) )
> 
>> 
>> From the user of (Role, "default")'s POV he won't see a single change.
>> 
>>> The "I don't care, just give me the default" strategy works as long as the 
>>> component implementation is self-contained and doesn't involve data 
>>> communication. Let's take an example, PDF export.
>>> 
>>> Currently the PDF export is only possible via FOP. If we were to convert 
>>> the current interface + implementation class into a proper component, we'd 
>>> name it "default", since it's the only one and who needs a factory for only 
>>> one implementation.
>> Yes but we could also be thorough and instead implement a 
>> Provider<PDFExporter>  instead.
>> 
>>> People use it and they're happy because it just works. But we might soon 
>>> add support for export via an office server. Suppose we want either FOP or 
>>> Office to be usable as the default PDF export implementation. And suppose 
>>> we'll want to keep both types of export active, so that we can use either 
>>> one as the implementation for the default export of documents, the FOP 
>>> implementation for exporting some kinds of documents like scientific 
>>> articles, and the office connector for generating PDFs for presentations. 
>>> Using the overrides mechanism we can only have one active at the same time, 
>>> unless we introduce yet another factory which we can bypass using manual 
>>> component lookup.
>> Based on your use case you'll need a UI to ask the user what he wants to 
>> export, i.e. either a "scientific article" or a "presentation" and from his 
>> choice you'll pick the right implementation. You could also try to guess 
>> that dynamically by looking at what is being exported but that'll require a 
>> Factory to hold that logic.
>> 
>> I know what you mean though: for some reason you don't want that to be 
>> dynamic and you wish it to be static.
>> 
>> There's a problem with dynamicity though. Imagine an extension that wants to 
>> replace an implementation. With your proposal the best practice would be to 
>> introduce a new Hint since the "default" is chosen statically at 
>> configuration time. So that wouldn't work. After you install extensions 
>> you'd need to stop the wiki, change the binding to the new implementation 
>> from the extension and restart it. Of course you'd need to read the 
>> documentation of the extension to know you have to use it in replacement. 
>> Basically it would mean that extensions cannot override behaviors. They 
>> would just be able to add new components (like new Macros) but not modify 
>> behaviors at runtime.
> 
> But this only means we need to be able to change configuration without 
> stopping the wiki and make it reload configs (might prove useful for other 
> things as well), it doesn't mean we cannot do it "static".
> 
>> 
>>> If at some point we decide that the office implementation works better, we 
>>> might consider changing the default implementation for the pdf component. 
>>> This means that we'll change the hint of the FOP implementation from 
>>> "default" to "fop", change the hint of the Office implementation from 
>>> "office" to "default", and our new version of XE works great if people read 
>>> the installation guide and properly configure the office connector.
>> In practice we would have 2 choices with our current component impl.:
>> 
>> Choice 1:
>> * Add 2 new implementations with hint1 and hint2 (hint1 being what was 
>> "default" before)
>> * Keep the implementation with "default" hint but deprecate it and move it 
>> to legacy. Refactor it so that it delegates yo hint1
>> * Add a Provider to decide which impl to use based on whatever conditions we 
>> want
>> 
>> Choice 2:
>> * Move "default" impl to "hint1"
>> * Add "hint2"
>> * Change "default" implementation to be a Composite that delegates to hint1 
>> or hint2
>> 
>>> But what about those that want to upgrade, but are happy with the older FOP 
>>> implementation and don't want to add support for the office connector?
>> Yes, in this case this is transparent for them (and it's a good thing in 
>> most caes!). This means they get autoupgrade to something better.
> 
> No, it means that for some reason we decided to change the underlying 
> technology (e.g. change of licence), it doesn't mean that everybody is happy 
> with the change.
>> 
>>> They'll have to use patched versions of the two component implementations 
>>> where their hints are reverted back to the old values. Easy? No. And even 
>>> though "default" means "I don't care what the implementation does"
>> It doesn't really mean this. We use "default" only when there's a single 
>> implementation. When there are more than 1 each one has a hint that is a 
>> qualifier to the implementation since there's no reason one is more default 
>> than the other.
>> 
>>> , here it does matter a lot which implementation you're using. They have 
>>> different requirements, and it's important to know if the implementation 
>>> will require an office instance or not.
>>> 
>>> Saying that this won't happen since we're all really careful when designing 
>>> our components is wishful thinking. We've refactored other more critical 
>>> pieces of code than providing alternative implementations for a component, 
>>> so we will find ourselves in situations where we'll have to switch from 
>>> only one "default" implementation to several.
>> I agree that we've refactored. And it has worked so far right? ;)
>> 
>>> Designing our component manager to make it easy to transition is the right 
>>> thing to do.
>>> 
>>> Still, my major problem is not about overrides, but about the semantics of 
>>> "default" (or lack of it). This says nothing about the actual mechanisms 
>>> behind the implementation, it just reflects the state of that particular 
>>> component implementation: it's the default at the moment. Not caring what 
>>> the implementation actually does works only when there is indeed just one 
>>> possible implementation that is straight-forward. But in most cases, we do 
>>> rely on another library that does the work for us, and libraries die, 
>>> better alternatives come along, and changing that internal aspect of the 
>>> implementation will sometimes be backwards incompatible, or have a 
>>> different behavior. Sure, it does the same job, but it does it so 
>>> differently that some will prefer to use the other approach. We have to let 
>>> users decide which is their "default", and having multiple implementations 
>>> with the "default" hint but different priorities is not very intuitive. Why 
>>> not make everything default and remove hints completely if we don't really 
>>> put any meaning into the hint?
> 
> This also means that we should only have components where there is a chance 
> that another implementation might exist, because to the limit you can 
> separate the interface from the implementation for any little piece of code 
> that you write.
> 
> I sort of feel you for this default thing, but at the same time, it's also a 
> matter of education of the developer, which needs to make sure that they put 
> a technology hint to the component, besides the default hint. The pb is that, 
> as long as there is only one implementation, regardless of the technology 
> it's based on, you also need to put the default hint since otherwise you'll 
> have to hardcode the reference to the technology everywhere if you wanna use 
> that service. so in this case default would mean 'this is the one that should 
> be used because there's no other, you dumb CM that is not capable of seeing 
> that'.
> 
> This brings back some memories, but I don't know from what, about a system 
> that was giving the available implementation regardless of its name. For 
> example, we could make the CM return the only implementation, if only one 
> exists, when asking for a component, regardless of its hint, so we don't have 
> to put default everywhere. But then we need a strategy for the case when 
> there are more.
> 
>>> 
>>> And "default" adds another assumption: XWiki Enterprise is the ultimate 
>>> target. Our defaults are the only ones that matter. As an example, all the 
>>> *Configuration components have just one "default" implementation, which 
>>> relies on xwiki.properties, XWiki.XWikiPreferences etc. Doesn't that tie 
>>> the platform to the XWiki Enterprise wiki?
>> This not true in xwiki commons and rendering because I've made sure that we 
>> could use them outside of the XWiki Platform. They have default 
>> implementation that don't use XWiki Configuration module.
>> 
>>> It's not a direct dependency visible at compilation time, it's worse, and 
>>> invisible assumption about the final runtime. It's certainly not the 
>>> default for other types of users that want to embed xwiki-commons or 
>>> xwiki-platform components in a different type of end product. To me this 
>>> isn't the default configuration, this is the default configuration used by 
>>> XWiki Enterprise, thus my proposal of using something else as the generic 
>>> component hint instead of "default".
>> Ok thanks for the explanations. I understand better now what you mean.
>> 
>> Actually what you suggest could already be implemented using a best practice 
>> of always using Providers when you want a Component injected. In this manner 
>> by default you'll get the Generic Provider but anyone could implement a 
>> specific Provider implementation for it that would choose between various 
>> implementation based on whatever (a value in a META-INF/role-bindings.txt 
>> file, data from DB, etc).
>> 
>> <side note>Only issue with having Providers everywhere is that you get late 
>> verification of your system coherence since dependencies will be resolved 
>> only when they're used. OTOH this is a necessity in a fully-dynamic system 
>> ;)</side note>
>> 
>> Also, the notion of default doesn't always have a meaning. There are lots of 
>> cases when there are NO default. For example Macros or Transformations or…
>> 
>> Let's continue the discussion it's interesting :)
>> 
>> I'd like to review a bit the other Component system out there again to see 
>> what they do for this. It's important that they have support for this since 
>> we want to be able to switch to them one day. The 3 that I would review are:
>> * Guice
>> * CDI
>> * OSGi
> 
> yes, we should learn from others. Maybe even use one?
> 
> Thanks,
> Anca
> 
>> 
>> Thanks
>> -Vincent
>> 
>> _______________________________________________
>> devs mailing list
>> [email protected]
>> http://lists.xwiki.org/mailman/listinfo/devs
> 
> 
> _______________________________________________
> devs mailing list
> [email protected]
> http://lists.xwiki.org/mailman/listinfo/devs

_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs

Reply via email to