Java ize2

[polx]

This branch is the second proposal to port the groovy code to java.
Earlier PR and comments.

[mflorea]

Where / how is this event listener used?

[polx]

Ouch. That's a good. That got moved away for readability and to prevent double-instantiation but was not constructed when the manager starts. Now fixed in 70368b2 .

[mflorea]

Still, why isn't this a component?

[polx]

I've spent quite some time trying to make the various objects a component and, at the end, considered it a failure. Here is the reason:

• There is no practice (apparently) to make package-internal components. This is a requirement for almost all classes here: They make no sense if not configured with the rest and many methods need to be public even though their function is only internal (this follows from attempts but also from a discussion with Vincent Massol).
• When building, e.g. GoogleAppsXWikiObjects, you need to refer to GoogleAppsManager and conversely. Same for just about any component. That means you need providers... not very elegant and bringing a big uncertainty as to whether your object is actually built.
• Normal practice for components is to apply "the interface dance": Each component's function is described in an interface and a component is an implementation of this interface. This works but doubles the code which is really not useful for functions that internal.

I agree that having injection for some of the objects (in particular loggers) would be a lot better but requiring this for all seems to be too much and expose too much of the API.

[mflorea]

There is no practice (apparently) to make package-internal components.

There is a practice and I believe I mentioned it to you.. We do have internal components that don't implement any interface, where you can have package protected methods or public internal methods as you like. See https://github.com/xwiki/xwiki-platform/blob/master/xwiki-platform-core/xwiki-platform-extension/xwiki-platform-extension-handlers/xwiki-platform-extension-handler-xar/src/main/java/org/xwiki/extension/xar/internal/doc/InstalledExtensionDocumentTree.java for instance.

That means you need providers...

I don't see the problem with that.

Normal practice for components is to apply "the interface dance"

Again. You can have internal components that don't have an interface. The best practice requires an interface only for public components.

[mflorea]

Yes, a component class needs to be public and its default constructor needs to be public as well, but that doesn't make it public API.

So that makes the class part of the public API even if without public methods.

No, what makes it a public API is the package. Anything inside the *.internal.* package is internal API. Anything not inside the *.internal.* package is public API. Of course, you can use any public class, but using internal API means your code can break at any upgrade (we can add, remove, modify internal APIs as we wish without caring how they are used outside, because they are internal API). For public APIs we need to maintain backwards compatibility. So is that your only concern regarding using components: the fact that they "become public API"? Because they don't.

[polx]

So I can remove any annotation of Unstable in there too?

This issue is what I remember now to have been the only problem I met.
I'll try it again (it might be that I was dissisatisfied by using provider everywhere too but I can accept that).
I'll report when done.

[mflorea]

@Unstable annotation doesn't make sense inside the internal package, for internal APIs.

[polx]

I have now pushed a version with just components and no complex constructors anymore.
It would be cool to have the ability to have package-internal classes... but that's just a wish for the future.
Thanks for insisting. It does make the code more focussed (e.g. the manager does not need to grab everything).
paul

[polx]

Hello @mflorea,
I believe that a review for the latest "componentization" (commit d810949 ) has it complete now.
I would love a review.
paul

[polx]

Hello @mflorea and @acotiuga ,
I've now pushed a series of commit with the last one respecting formatting and checkstyle.
Except for more testing and a decision on whether to change the OAuth return URL, this should be good for review.
paul

[mflorea]

Still, why isn't this a component?