Persistent producer registration impl

[julien1]

I am starting to implementation of the producer registration in the core module based on hibernate.

I don't understand the logic in RegistrationManagerImpl#createConsumer(String name) and RegistrationManagerImpl #addConsumerToGroupNamed(String consumerName, String groupName, ...)

In createConsumer:
1/ a consumer is created
2/ its name is used to to retrieve a group

then the created consumer is never used anymore.

I see that the method is calling public Consumer addConsumerToGroupNamed(String consumerName, String groupName, boolean createGroupIfNeeded, boolean createConsumerIfNeeded) with

addConsumerToGroupNamed(groupName, groupName, true, false);

I have a few comments on that :

1/ perhaps it should be consumer.getName() as first argument
2/ is the method addConsumerToGroupNamed able to retrieve the same consumer from only its name and not its id, or do we have an effective dual creation of a consumer ?

String identity = policy.getConsumerIdFrom(consumerName, Collections.EMPTY_MAP));

I would have rather simply seen :

addConsumerToGroupNamed(consumer, groupName, true, false);

I would go even further by adding that constraint in the persistence manager and instead of having the sequence :

1/ create consumer
2/ associate it with a consumer group

change

Consumer createConsumer(String id, String name) throws RegistrationException;

to

Consumer createConsumer(ConsumerGroup consumerGroup, String id, String name) throws RegistrationException;

to enforce by construction the fact that a consumer is attached to a group.

[julien1]

Moved to correct forum ... oops

[julien1]

I have a question about QName entries persisted as properties of registration entities.

It looks like a qname is made of namespaceURI, localPart and prefix.

As far as I remember, the prefix usually is more or less found in a registry of namespaceURI and prefix. Shall we persist all that information or could we just persist the namespaceURI and localPart and the prefix could be resolved at runtime by the manager ?

So what I mean is that I want to be sure that we don't store redundant information in the persistent store, if the qname can be reconstructed from what I said above.

[julien1]

Actually after looking at the QName javadoc, it looks like only the namespaceURI and the localPart are relevant. The prefix is not used in the implementation of equals and hashCode and it is used to retain only lexical information in XML documents.

I am going to persist only the namespaceURI and localPart for now.

[julien1]

Is it safe to assume that in the registration properties we only map Strings or should we able to persist other kind of types, in that case I think we could only provide a mapping of simple types efficiently.

[claprun]

then the created consumer is never used anymore.

The method just creates the Consumer in the DB, the actual object is not used by the code since everything needs to go through the Manager to avoid unsafe modifications of Registration-related objects. Only safe mutators are exposed. I will detail the design in another post.

addConsumerToGroupNamed(groupName, groupName, true, false);

I have a few comments on that :
1/ perhaps it should be consumer.getName() as first argument

Indeed. This has been fixed in svn along with the test case to exhibit the problem (the group name was shadowing the consumer name in the test, thus preventing the detection of the bug).

2/ is the method addConsumerToGroupNamed able to retrieve the same consumer from only its name and not its id, or do we have an effective dual creation of a consumer ?

There's no dual creation of the consumer (at least, not an intented one! ^_^). You changed the name identity to id in your last commit but the identity name was intentional. It doesn't reflect the Consumer identity in the database, rather the consumer identity in the real world since we cannot trust its name. Hence the need for:

String identity = policy.getConsumerIdFrom(consumerName, Collections.EMPTY_MAP));

This will be detailed in the post on the Registration design, along with issues with the spec.

I would go even further by adding that constraint in the persistence manager and instead of having the sequence :
1/ create consumer
2/ associate it with a consumer group

change

Consumer createConsumer(String id, String name) throws RegistrationException;

to

Consumer createConsumer(ConsumerGroup consumerGroup, String id, String name) throws RegistrationException;

to enforce by construction the fact that a consumer is attached to a group.

No. ConsumerGroup is an artificial concept used only for GUI managers. In fact, I still haven't seen a use case that mandates its use. I don't think the Registration engine should depend on it and in particular, I think that Consumers should exist without a ConsumerGroup. I'm fine with the concept if you think there's a need but I see that need as limited and not central to the Registration concept, more as an external concept.

[claprun]

Actually after looking at the QName javadoc, it looks like only the namespaceURI and the localPart are relevant. The prefix is not used in the implementation of equals and hashCode and it is used to retain only lexical information in XML documents.

I am going to persist only the namespaceURI and localPart for now.

Yes. The prefix is just a shorthand for the namespaceURI.

[julien1]

Thanks for the clarification.

[julien1]

No. ConsumerGroup is an artificial concept used only for GUI managers. In fact, I still haven't seen a use case that mandates its use. I don't think the Registration engine should depend on it and in particular, I think that Consumers should exist without a ConsumerGroup. I'm fine with the concept if you think there's a need but I see that need as limited and not central to the Registration concept, more as an external concept.

I agree that the wsrp specs only specifies what is in its scope. Nevertheless that does not prevent us to structure our data in a convenient way that makes it efficient to use and administer.

I mean we don't have to provide an implementation of "just the WSRP" concepts, we need to provide an implementation around a robust model that should be the most compatible possible with the WSRP specification in order to avoid implementation mismatches.

In the same vein I don't see why we need the JSR-168 component model, after all the WSRP spec does not mandates any API :-)

I understand that your concern is not to have too many concepts in the WSRP module and it is my concern too. We'll find out a solution to decouple those concepts.

[claprun]

Is it safe to assume that in the registration properties we only map Strings or should we able to persist other kind of types, in that case I think we could only provide a mapping of simple types efficiently.

Registration properties can be arbitrary objects that are (supposedly) defined in XML Schema. According the spec:

Producers can assume that all Consumers support the basic types defined by (XML Schema Part 2)

The problem with properties is more on the Consumer side since Consumers registering with our Producer will only send information that WE require. So if we only require Strings, then we will only get Strings. After that, it's matter of deciding if we want to support more complex type (as part of our Producer).

On the other side, our Consumer implementation needs to be able to handle whatever the Producer might throw its way, so it's a little bit more tricky.

However, as far as I am aware, we can expect that most properties would be Strings and the rest would be types defined in the WSRP WSDL (thus both Consumer and Producer know that the type can be handled). I haven't run into more complex properties in my admitely very limited testing.

[claprun]

I mean we don't have to provide an implementation of "just the WSRP" concepts, we need to provide an implementation around a robust model that should be the most compatible possible with the WSRP specification in order to avoid implementation mismatches.

My problem is not with adding stuff above the spec. I'm fine with that. I'm just not convinced that we need the ConsumerGroup concept at all, i.e. I haven't seen a use case where things would be really difficult without it.

[julien1]

However, as far as I am aware, we can expect that most properties would be Strings and the rest would be types defined in the WSRP WSDL (thus both Consumer and Producer know that the type can be handled). I haven't run into more complex properties in my admitely very limited testing.

For now then it is wise to assume that we only require strings.

[julien1]

I have implemented the persistence manager for registration. I am using the test for the persistence manager only so far that I have abstracted in a simple way.

My next task will be to integrate the persistence manager implementation with the portlet clone entities and the appropriate tests.

Actually the PersistentStateStore that manages the portlet state implements the RegistrationPersistenceManager interface.

Most of the integration relies on the fact that a portlet state is related to a registration entity.

Fortunately I made room for that in 2.4 and the portlet state already carries a registration id which is a Long and has the value NULL for now. So that should save us migration efforts in wsrp.