Enhancement: Create ServerManager & ClientManager Interaces

[GoogleCodeExporter]

What steps will create the enhancement?
1a.) Create an interface called ServerManager with all of the current
ServerManager functions.
1b.) Rename the current net.openid.server.ServerManager class to be
something like net.openid.server.DefaultServerManager, (or
StaticServerManager, or similar). 
2a.) Create an interface called ClientManager with all of the current
ClientManager functions.
2b.) Rename the current net.openid.client.ClientManager class to be
something like net.openid.client.DefaultClientManager, (or
StaticClientManager, or similar). 

Rationale: This would allow anyone to write their own
ServerManager/ClientManager, yet conform to the existing contract.  I
envision creating EJB backed managers to use OpenId4Java on a J2EE
application server. Ideally, the ServerManager would be implemented as a
service, like a Stateless Sessionbean.  That way, it is directly accessible
from other services running in the container.  As it stands now,  however,
the current ServerManager has static ServerAssociationStore's, which don't
fit well with J2EE (According to the J2EE spec, static member variables are
not recommended unless they're final --- static members are not portable
across app servers, among other things).

ps - I have much of this code written, and would be open to contributing
back to the project.  I'm just waiting to integrate it into the new
interfaces.  My EJB code works on JBoss, ATM.  But, other app servers could
be supported (bea, oracle, etc), but would need their own implementations.

Original issue reported on code.google.com by sappenin on 5 Feb 2007 at 6:12

[GoogleCodeExporter]

The main issues seems the fact that both the ServerManager and the 
ConsumerManager
(not ClientManager) classes have static members. This should be fixed.

Once the static members are taken care of, can you use the two manager classes 
as
they are in your EJB implementation? If not, can you just use composition or
inheritance to create the right class?

I don't see why would you introduce interfaces. These are top level helper 
classes,
there is no contract here.

Original comment by mscurte...@pingidentity.com on 6 Feb 2007 at 1:17

[GoogleCodeExporter]

Ok, I wasn't aware that ServerManager and ConsumerManager were simply top-level
helper classes, in the sense that there is no contract for them (officially).  
Since
the samples show each "manager" being used by even higher level code, I wonder 
if it
would be worthwhile to standardize these Managers (?)

Regardless, I'm not sure that the ServerManger or ConsumerManager need to be 
"fixed"
with regard to static AssociationStores.  For a typical servlet implemention on 
a
single server, it might be a good solution.  However, for J2EE, they (perhaps) 
should
be re-written to use Entity bean persistence, or some other in-memory 
Association
mechanism that can be replicated across servers).

Anyway, inheritence would work for everything except the AssociationStores (I 
guess
these could be ignored, or overridden in my subclass).  In my opinion, these 
Manager
classes should be interfaced for clarity, and for the next guy that comes along 
who
wants to create an impl for BEA or Oracle or some other server.  

At the least, it may make sense to have an intermediate abstract class for each
Manager that has everything minus the AssociationStore.  That way, non-abstract
implemenations can implement the store however they want.  My $.02 -- what do 
you think?

Original comment by sappenin on 6 Feb 2007 at 4:39

[GoogleCodeExporter]

With revision 81 we removed all static members, in both the manager and sample 
classes.

Both association stores are defined as interfaces, and very simple 
implementations
are defined. For a production system you should definitely use your own
implementation (and they should be very easy to implement).

The sample classes just give an example of how you would use the managers, still
don't think an interface is needed.

I hope the other issues were addresses by removing the static fields.

Original comment by mscurte...@pingidentity.com on 6 Feb 2007 at 6:45

[GoogleCodeExporter]

Original comment by marius.s...@gmail.com on 6 Feb 2007 at 8:17

• Changed state: Started

[GoogleCodeExporter]

Thanks for the above changes (especially the Association stores).

For the in-memory Managers, however, if the Manager doesn't use static fields, 
is it
ok to just declare the Manager as a non-static member variable of a servlet 
(e.g.,
similar to what SampleConsumer does)?

Does a servlet get instantiated once at runtime?  Or is it init'd and destroyed
periodically?  If the latter is the case, then we might inadvertently lose
association data, since its not static.

If the servlet is created once at runtime, then this won't be an issue.

Original comment by sappenin on 6 Feb 2007 at 11:33

[GoogleCodeExporter]

Servlets get instantiated and initialized only once. But you still bring up a 
very
good point. Currently the two manager classes are not thread-safe, and this is a
major issue in a servlet environment.

I am closing this issue and we will open a new issue regarding thread-safety.

Keep them coming :-)

Original comment by marius.s...@gmail.com on 7 Feb 2007 at 2:05

• Changed state: Fixed