I've had a look at
https://community.jboss.org/wiki/PicketLink3Subsystem and also had
a bit of a play with it. It's starting to look really good. I've just got a few
suggestions:
Suppress logging
----------------
At the moment there's a lot of logging at info level produced by the subsystem, this
is mostly Hibernate. It would be great if we could somehow manage to suppress this logging
output, might be problematic though as Hibernate logs this stuff at INFO level when it
really should be DEBUG. There's also a few WARN's we might want to look into
fixing.
JNDI names in standalone.xml
----------------------------
I think it makes sense to use the same format for JNDI names as the datasource element,
since folks will already be used to that. So I suggest we change it slightly to look like
this:
<jpa-store data-source=”java:jboss/datasources/ExampleDS" ...>
<identity-management jndi-name="java:picketlink/ExampleIDM" ...>
* Full jndi name (including java:) and use jndi-name instead of jndi-url
Manifest.mf
-----------
We need to make sure it works when including org.picketlink, org.picketlink.idm, etc in
manifest.mf as well as jboss-deployment-structure.xml. The documentation should also
reflect this. One thing I also thought of is that for the future it may be nice to have
something that detects PicketLink usage in a deployment and automatically adds
dependencies as required. For example if deployment uses @IdentityManager, @Identity, etc.
annotations.
JNDI
----
@Resource doesn't require CDI, so it should be possible to do the following without
CDI (and without org.picketlink.core):
@Resource(lookup = "java:/picketlink/DevIdentityManager")
private IdentityManagerFactory identityManagerFactory;
I was wondering if we wanted to have the IdentityManager available in JNDI as well?
CDI
---
I was thinking about a nice way to do the CDI support of injecting a 'default'
IdentityManager. I propose adding the attribute 'default' to the
'identity-management' element (<identity-management default="true"
...>). We should throw a warning if a user has specified multiple, then we just pick
one (first one?).
This does mean that if a 'identity-management' has the 'default' attribute
set on it all deployments will by default have that IdentityManager injected into it. We
also need a way for users to override this on a per-deployment basis. Can we easily detect
if a deployment contains configuration for a IdentityManager itself?
Further we need to have a way for a user to specify which IdentityManager to inject. I
think this should be done based on the 'alias' attribute and not the
'jndi-name', as we should leave jndi completely out of the picture for CDI
(resource-ref in web.xml/ejb.xml should be used for JNDI lookup, InitialContext#lookup and
@Resource, I find it confusing to use this for CDI). I propose that we use the
ServiceRegistry to retrieve the IdentityManagerFactory service based on the alias
specified by a @Alias qualifer:
@Inject
@Alias(“development”)
private IdentityManager identityManager;
Obviously users should also be able to add their own qualifiers, I think this should
work:
@Inject @Alias(“development”)
@Produces @Development
private IdentityManager identityManager;
One alternative to the above is to change 'alias' to 'name' then we could
use the standard @Named annotation instead of @Alias.
Custom Entity Classes
---------------------
Personally I don't like the idea of custom entity classes (and persistence.xml) being
deployed as JavaEE deployments (i.e. standalone/deployments). This is also problematic for
sub-systems that wants to use the IDM if they need to use custom entity classes
(there's a good chance we'll need this for EventJuggler). I also think this will
be problematic if multiple deployments uses the same IdentityManager.
One idea I had was that we could create a module that contains the custom Entity classes,
then specify that on the 'jpa-store' element:
<jpa-store data-source=”java:jboss/datasources/ExampleDS"
custom-entity-module='org.company.acme.pl' />
The module 'org.company.acme.pl' would contain a single jar with the Entity
classes. When 'custom-entity-module' is used we include that module instead of
'org.picketlink.idm.schema' module when creating the EMF + we should be able to
detect the correct classes using the @IDMEntity.