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.
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.
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
>