Re: [wildfly-dev] Capabilities and Requirements - Common Definitions

On 05/22/2015 12:55 PM, Darran Lofthouse wrote: > > > On 22/05/15 17:25, Brian Stansberry wrote: >> On 5/22/15 9:21 AM, Darran Lofthouse wrote: >>> >>> >>> On 22/05/15 13:57, Brian Stansberry wrote: >>>> Thanks for raising this; it's a good set of questions. >>>> >>>> I'll be AFK for a week starting late today, so I'll start with point #2 >>>> even though that's backwards. It's just the one where I have a stronger >>>> opinion. >>>> >>>> The controller module should be about basic API for creating resources, >>>> attributes, handlers. Having other stuff like the actual >>>> ModelControllerImpl in there isn't pure, but I don't mind much. But then >>>> over time I and others have put some stuff in there basically just >>>> because both server and host-controller use it. We need to move away >>>> from that kind of thing, and not add more. >>> >>> I can move it to a different module but most likely I think that would >>> be a new module, 'server' could have been a candidate but that depends >>> on too many artifacts that will also need to reference these. >>> >>>> As for your question 1, I don't feel strongly at all, just have some >>>> thoughts. To initially use a capability, people are going to have to >>>> look up data. We're not going to stick all of these in controller, so >>>> that means there's going to be a search, and these kinds of classes are >>>> going to be dispersed. Trying to maintain some fairly centralized >>>> document somewhere would actually make that initial search easier. >>> >>> At the moment with this PR all I am proposing is a central location for >>> capabilities that we know will be widely used, in this case the server >>> and subsystems etc. are going to have hard coded dependencies on Elytron >>> APIs - what is not hard coded is how the implementations of those APIs >>> are actually made available. >>> >> >> Who is the consumer of these? A requirer only needs the String names, >> none of the "public static final RuntimeCapability" instances. > > The consumers will be: - > - Various resources in core. > - Subsystems in general. > - Deployments. > > One possible problem I am seeing with only using a String on the > consumer side is that there is no type associated even though the > dependency injection for dynamic capabilities at least is going to > depend on a mutually agreed type. I don't think that really makes a huge difference in this case. There will be a fairly small number of consumers, and type safety in this case is not going to be critical. And really I don't anticipate that type safety would catch any bugs that wouldn't be caught in any event by the simpler string-based system at the same time. >> The different providers of the capability can use the RuntimeCapability >> constants, but I think those should be separated somehow. They are >> really none of the business of the requirers. > > The providers will be: - > - The Elytron Subsystem (Optional) > - Any other subsytem that wants to provide one or more of these > capabilities. > > For separation we could have different classes, different modules or > even some factory class to convert from the name to the > RuntimeCapability instance. Too complex - I think we need to think minimally here. Keep the mechanism as simple as possible.

> I think single new module with two classes may be the better compromise > for now otherwise we risk two new modules - each with a single class. > > The main point being that WildFly Core is the single common dependency > for everything For the moment. Not forever though.

>>> The Elytron project could not contain these definitions as that has no >>> dependencies on WildFly, the Elytron Subsystem can not contain this as >>> that is an optional module. >>> >>>> Using constants like in your PR is helpful for later maintenance. If >>>> some constant is deprecated, referrers will see that in their IDE. They >>>> can easily click to read javadoc and learn if the requirement's dynamic >>>> and the service type. >>>> >>>> The biggest concern I have is avoiding premature classloading links. >>>> This is important in the case of optional requirements. If a requirement >>>> is optional, it's critical that the declaration of the requirement in >>>> the code doesn't force loading of classes from the required capability's >>>> module. Otherwise that module is no longer optional and its harder to >>>> create a slim distribution. >>> >>> In this case the only classes being referenced are classes from the >>> Elytron module and that module is going to be mandatory within the >>> server. >>> >> >> Yep. This classloading thing is more of a general concern, not >> particularly relevant to this particular case. People tend to code >> things up by copying existing code though, so it's good to consider the >> general problem before establishing precedents. >> >>> Generally it sounds like 'controller' is a no-go so the format of any >>> constants in code will probably dictate where this lives. >>> >>> If we are only talking Strings I may be able to add it to the >>> 'wildfly-core-security-api' module although javadoc can not reference >>> any capability related classes so that may not be preferable - of I can >>> create a new module with a dependency on controller and everything else >>> that uses Elytron capabilities can then depend on this new artifact / >>> module. >>> >>>> On 5/22/15 5:31 AM, Darran Lofthouse wrote: >>>>> Following on from PR https://github.com/wildfly/wildfly-core/pull/757 >>>>> and my final comment in the PR we need to have a discussion on how we >>>>> coordinate capability and requirement definitions - especially where >>>>> multiple components need a common definition. >>>>> >>>>> The first option is to do it all by convention and have no shared >>>>> constants, the down side of this is we now need to document this and >>>>> keep the document maintained. A document would also make it hard in >>>>> the >>>>> future to flag certain capabilities as deprecated if preferred >>>>> alternatives are made available. >>>>> >>>>> The second option would be to just define the Strings somewhere and use >>>>> Javadoc to specify if the capability is dynamic and it's service type. >>>>> >>>>> The third option is defining the string and RuntimeCapability instances >>>>> in a central place so they can both be referenced as needed. >>>>> >>>>> Any other options? >>>>> >>>>> Where these live will be a second point to discuss but that is heavily >>>>> driven based on what we will share in the first place. >>>>> >>>>> Regards, >>>>> Darran Lofthouse. >>>>> _______________________________________________ >>>>> wildfly-dev mailing list >>>>> wildfly-dev(a)lists.jboss.org >>>>> https://lists.jboss.org/mailman/listinfo/wildfly-dev >>>>> >>>> >>>> >> >> > _______________________________________________ > wildfly-dev mailing list > wildfly-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/wildfly-dev >