Author: chris.laprun(a)jboss.com Date: 2007-05-08 01:38:51 -0400 (Tue, 08 May 2007) New Revision: 7221 Modified: trunk/wsrp/src/main/org/jboss/portal/test/wsrp/other/RegistrationInfoTestCase.java trunk/wsrp/src/main/org/jboss/portal/wsrp/admin/ui/ConsumerBean.java trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/ProducerInfo.java trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/RegistrationInfo.java trunk/wsrp/src/resources/portal-wsrp-sar/conf/consumer/hibernate/domain.hbm.xml trunk/wsrp/src/resources/portal-wsrp-war/WEB-INF/jsf/consumers/editConsumer.xhtml Log: - Fixed incorrect mapping of registration properties which wouldn't be properly updated when the parent RegistrationInfo was modified. - JBPORTAL-715: more work on modifyRegistration (detection of when calling modifyRegistration is not properly done yet). - Added RegistrationInfo.isConsistentWithProducerExpectations and improved computing of whether a Registration is valid or not (replaced incorrect state). - ServiceDescription is now computed on each request in ProducerInfo since it wasn't worth maintaining a state for it. - Added more test cases. Modified: trunk/wsrp/src/main/org/jboss/portal/test/wsrp/other/RegistrationInfoTestCase.java =================================================================== --- trunk/wsrp/src/main/org/jboss/portal/test/wsrp/other/RegistrationInfoTestCase.java 2007-05-07 14:44:07 UTC (rev 7220) +++ trunk/wsrp/src/main/org/jboss/portal/test/wsrp/other/RegistrationInfoTestCase.java 2007-05-08 05:38:51 UTC (rev 7221) @@ -26,12 +26,15 @@ import junit.framework.TestCase; import org.jboss.portal.wsrp.WSRPConstants; import org.jboss.portal.wsrp.WSRPTypeFactory; +import org.jboss.portal.wsrp.consumer.RefreshResult; import org.jboss.portal.wsrp.consumer.RegistrationInfo; import org.jboss.portal.wsrp.consumer.RegistrationProperty; import org.jboss.portal.wsrp.core.PropertyDescription; +import org.jboss.portal.wsrp.core.RegistrationContext; import org.jboss.portal.wsrp.core.ServiceDescription; import java.util.Map; +import java.util.Set; /** * @author <a href="mailto:chris.laprun@jboss.com">Chris Laprun</a> @@ -56,12 +59,40 @@ assertNotNull(info.getRegistrationData()); assertNull(info.getRegistrationHandle()); assertNull(info.getRegistrationState()); + + // before refresh, refresh is needed... + assertTrue(info.isRefreshNeeded()); + // we don't know if the the local info is consistent with the producer expectations... + assertNull(info.isConsistentWithProducerExpectations()); + // we don't know if the registration is required... + assertNull(info.isRegistrationRequired()); + // and we don't know if the registration is valid + assertNull(info.isRegistrationValid()); + + try + { + info.isRegistrationDeterminedNotRequired(); + fail("refresh hasn't been called, isRegistrationDeterminedNotRequired should fail"); + } + catch (IllegalStateException expected) + { + } + + try + { + info.isRegistrationDeterminedRequired(); + fail("refresh hasn't been called, isRegistrationDeterminedRequired should fail"); + } + catch (IllegalStateException expected) + { + } } - /*public void testSetGetRegistrationProperty() + public void testSetGetRegistrationProperty() { String key = "foo"; info.setRegistrationPropertyValue(key, "bar"); + assertNull(info.isConsistentWithProducerExpectations()); Map properties = info.getRegistrationProperties(); assertFalse(properties.isEmpty()); Set names = info.getRegistrationPropertyNames(); @@ -71,37 +102,36 @@ assertNotNull(properties.get(key)); assertEquals(properties.get(key), info.getRegistrationProperty(key)); assertEquals("bar", info.getRegistrationProperty(key).getValue()); - }*/ + } public void testRefreshNoRegistration() { - // before refresh registration status is undetermined - assertNull(info.isRegistrationRequired()); - assertFalse(info.isRegistrationValid()); - + // no registration expected ServiceDescription sd = createServiceDescription(false, 0); - RegistrationInfo.RegistrationRefreshResult result = info.refresh(sd, producerId, true, false); + RefreshResult result = info.refresh(sd, producerId, true, false); assertNotNull(result); assertFalse(result.hasIssues()); + assertTrue(info.isConsistentWithProducerExpectations().booleanValue()); assertFalse(info.isRegistrationRequired().booleanValue()); assertTrue(info.isRegistrationDeterminedNotRequired()); assertFalse(info.isRegistrationDeterminedRequired()); - assertTrue(info.isRegistrationValid()); + assertTrue(info.isRegistrationValid().booleanValue()); result = info.refresh(sd, producerId, false, false); assertNotNull(result); assertFalse(result.hasIssues()); + assertTrue(info.isConsistentWithProducerExpectations().booleanValue()); assertFalse(info.isRegistrationRequired().booleanValue()); assertTrue(info.isRegistrationDeterminedNotRequired()); assertFalse(info.isRegistrationDeterminedRequired()); - assertTrue(info.isRegistrationValid()); + assertTrue(info.isRegistrationValid().booleanValue()); } public void testRefreshRegistrationDefaultRegistrationNoLocalInfo() { // before refresh registration status is undetermined assertNull(info.isRegistrationRequired()); - assertFalse(info.isRegistrationValid()); + assertNull(info.isRegistrationValid()); RegistrationInfo.RegistrationRefreshResult result = info.refresh( createServiceDescription(true, 0), producerId, true, false); @@ -110,7 +140,7 @@ assertTrue(info.isRegistrationRequired().booleanValue()); assertTrue(info.isRegistrationDeterminedRequired()); assertFalse(info.isRegistrationDeterminedNotRequired()); - assertFalse(info.isRegistrationValid()); + assertFalse(info.isRegistrationValid().booleanValue()); } public void testRefreshRegistrationDefaultRegistrationExtraLocalInfo() @@ -189,34 +219,53 @@ public void testForceRefreshRegistration() { - RegistrationInfo.RegistrationRefreshResult result = info.refresh( - createServiceDescription(true, 0), producerId, false, false); + // + RefreshResult result = info.refresh(createServiceDescription(true, 0), producerId, false, false); assertNotNull(result); assertFalse(result.hasIssues()); - assertFalse(info.isRegistrationValid()); + assertFalse(info.isRegistrationValid().booleanValue()); // Modifying a property renders the info dirty and hence should be refreshed info.setRegistrationPropertyValue("foo", "bar"); result = info.refresh(createServiceDescription(true, 0), producerId, false, false); assertTrue(result.hasIssues()); - assertFalse(info.isRegistrationValid()); + assertFalse(info.isRegistrationValid().booleanValue()); info.removeRegistrationProperty("foo"); result = info.refresh(createServiceDescription(true, 0), producerId, false, false); assertFalse(result.hasIssues()); - assertFalse(info.isRegistrationValid()); + assertFalse(info.isRegistrationValid().booleanValue()); // producer has changed but we're not forcing refresh so registration should still be invalid result = info.refresh(createServiceDescription(false, 0), producerId, false, false); assertFalse(result.hasIssues()); - assertFalse(info.isRegistrationValid()); + assertFalse(info.isRegistrationValid().booleanValue()); - // force refresh + // force refresh, registration should now be valid result = info.refresh(createServiceDescription(false, 0), producerId, false, true); assertFalse(result.hasIssues()); - assertTrue(info.isRegistrationValid()); + assertTrue(info.isRegistrationValid().booleanValue()); } + public void testSetRegistrationContext() + { + assertNull(info.isConsistentWithProducerExpectations()); + assertNull(info.getRegistrationHandle()); + + String registrationHandle = "registrationHandle"; + info.setRegistrationContext(WSRPTypeFactory.createRegistrationContext(registrationHandle)); + RegistrationContext registrationContext = info.getRegistrationContext(); + assertNotNull(registrationContext); + assertEquals(registrationHandle, registrationContext.getRegistrationHandle()); + assertNull(registrationContext.getRegistrationState()); + assertTrue(info.isConsistentWithProducerExpectations().booleanValue()); + assertTrue(info.isRegistrationValid().booleanValue()); + assertFalse(info.isRefreshNeeded()); + assertTrue(info.isRegistrationRequired().booleanValue()); + assertTrue(info.isRegistrationDeterminedRequired()); + assertFalse(info.isRegistrationDeterminedNotRequired()); + } + private ServiceDescription createServiceDescription(boolean requiresRegistration, int numberOfProperties) { ServiceDescription sd = WSRPTypeFactory.createServiceDescription(requiresRegistration); Modified: trunk/wsrp/src/main/org/jboss/portal/wsrp/admin/ui/ConsumerBean.java =================================================================== --- trunk/wsrp/src/main/org/jboss/portal/wsrp/admin/ui/ConsumerBean.java 2007-05-07 14:44:07 UTC (rev 7220) +++ trunk/wsrp/src/main/org/jboss/portal/wsrp/admin/ui/ConsumerBean.java 2007-05-08 05:38:51 UTC (rev 7221) @@ -181,7 +181,11 @@ public boolean isRegistrationValid() { - return getProducerInfo().getRegistrationInfo().isRegistrationValid(); + if (isRegistrationChecked()) + { + return getProducerInfo().getRegistrationInfo().isRegistrationValid().booleanValue(); + } + throw new IllegalStateException("Need to check the registration before determining if it's valid!"); } public ProducerInfo getProducerInfo() Modified: trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/ProducerInfo.java =================================================================== --- trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/ProducerInfo.java 2007-05-07 14:44:07 UTC (rev 7220) +++ trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/ProducerInfo.java 2007-05-08 05:38:51 UTC (rev 7221) @@ -99,12 +99,6 @@ /** Time at which the cache expires */ private long expirationTimeMillis; - /** The service description request sent to producer before registration happens. */ - private final GetServiceDescription unregisteredServiceDescriptionRequest; - - /** The service description sent to producer. */ - private GetServiceDescription serviceDescriptionRequest; - private ConsumerRegistry registry; private static final String REGISTER_MEANING = "Should clients ask for a new service description?"; private static final String REFRESH_MEANING = "Did just refresh?"; @@ -121,7 +115,6 @@ public ProducerInfo() { - unregisteredServiceDescriptionRequest = initUnregisteredServiceDescriptionRequest(); persistentEndpointInfo = new EndpointConfigurationInfo(this); } @@ -167,7 +160,13 @@ public boolean isRegistered() { - return persistentRegistrationInfo != null && persistentRegistrationInfo.isRegistrationValid(); + if (persistentRegistrationInfo != null) + { + Boolean valid = persistentRegistrationInfo.isRegistrationValid(); + return valid != null && valid.booleanValue(); + } + + return false; } public boolean isRegistrationRequired() @@ -277,17 +276,27 @@ if (serviceDescription.isRequiresRegistration()) { // attempt to register and determine if the current service description can be used to extract POPs - RefreshResult registrationResult = register(serviceDescription, forceRefresh); - if (registrationResult.specificCode()) + RefreshResult registrationResult = internalRefreshRegistration(serviceDescription, true); + String status = registrationResult.getStatus(); + if (status != null) { - // registration occurred, so we should ask for a new service description - serviceDescription = getServiceDescription(false); + result.appendToStatus(status); } - // extract the POPs - extractOfferedPortlets(serviceDescription); + if (!registrationResult.hasIssues()) + { + registrationResult = register(serviceDescription, forceRefresh); + if (registrationResult.specificCode()) + { + // registration occurred, so we should ask for a new service description + serviceDescription = getServiceDescription(false); + } - String status = registrationResult.getStatus(); + // extract the POPs + extractOfferedPortlets(serviceDescription); + } + + status = registrationResult.getStatus(); if (status != null) { result.appendToStatus(status); @@ -301,7 +310,6 @@ { log.debug("Registration not required"); persistentRegistrationInfo = new RegistrationInfo(this, false); - setServiceDescriptionRequest(getUnregisteredServiceDescriptionRequest()); extractOfferedPortlets(serviceDescription); return result; } @@ -503,18 +511,8 @@ private ServiceDescription getServiceDescription(boolean asUnregistered) throws PortletInvokerException { - GetServiceDescription request; - if (asUnregistered) - { - request = getUnregisteredServiceDescriptionRequest(); - } - else - { - request = getServiceDescriptionRequest(); - request.setRegistrationContext(getRegistrationContext()); - } + GetServiceDescription request = getServiceDescriptionRequest(asUnregistered); - ServiceDescription serviceDescription; try { @@ -522,13 +520,11 @@ if (serviceDescription != null) { - // from now on (that is until getServiceDescription fails) use this successful request - setServiceDescriptionRequest(request); return serviceDescription; } else { - // fix-me: do something better + // todo: do something better throw new NullPointerException("null service description: deal with it!"); } } @@ -545,38 +541,21 @@ } } - private void setServiceDescriptionRequest(GetServiceDescription request) + private GetServiceDescription getServiceDescriptionRequest(boolean asUnregistred) { - serviceDescriptionRequest = request; - } - - private GetServiceDescription getServiceDescriptionRequest() - { - return (serviceDescriptionRequest != null) ? serviceDescriptionRequest : getUnregisteredServiceDescriptionRequest(); - } - - /** - * Retrieves the service description request this consumer uses when not registered. - * - * @return the service description request this consumer uses when not registered. - */ - private GetServiceDescription getUnregisteredServiceDescriptionRequest() - { - return unregisteredServiceDescriptionRequest; - } - - /** - * Initializes the service description request this consumer uses when not registered. TODO: Implement - * customization. - * - * @return the service description request this consumer uses when not registered. - */ - private GetServiceDescription initUnregisteredServiceDescriptionRequest() - { + //todo: might need to implement customization of default service description GetServiceDescription gsd = WSRPTypeFactory.createGetServiceDescription(); gsd.setDesiredLocales(WSRPConstants.DEFAULT_LOCALES); - return gsd; + if (asUnregistred) + { + return gsd; + } + else + { + gsd.setRegistrationContext(getRegistrationContext()); + return gsd; + } } public RegistrationContext getRegistrationContext() @@ -654,7 +633,7 @@ persistentRegistrationInfo = new RegistrationInfo(this); } - if (forceRefresh || !isRegistered()) + if (!isRegistered()) { if (serviceDescription == null) @@ -738,9 +717,14 @@ { try { - ModifyRegistration modifyRegistration = WSRPTypeFactory.createModifyRegistration(getRegistrationContext(), + RegistrationContext registrationContext = getRegistrationContext(); + ModifyRegistration modifyRegistration = WSRPTypeFactory.createModifyRegistration(registrationContext, persistentRegistrationInfo.getRegistrationData()); RegistrationState state = persistentEndpointInfo.getRegistrationService().modifyRegistration(modifyRegistration); + + // force refresh of internal RegistrationInfo state + persistentRegistrationInfo.setRegistrationValidInternalState(); + if (state != null) { persistentRegistrationInfo.setRegistrationState(state.getRegistrationState()); @@ -760,15 +744,20 @@ public RefreshResult refreshRegistrationInfo(boolean mergeWithLocalInfo) throws PortletInvokerException { + return internalRefreshRegistration(getServiceDescription(true), mergeWithLocalInfo); + } + + private RefreshResult internalRefreshRegistration(ServiceDescription serviceDescription, boolean mergeWithLocalInfo) throws PortletInvokerException + { if (persistentRegistrationInfo == null) { persistentRegistrationInfo = new RegistrationInfo(this); } - RegistrationInfo.RegistrationRefreshResult result; + RefreshResult result; try { - result = persistentRegistrationInfo.refresh(getServiceDescription(true), persistentId, mergeWithLocalInfo, true); + result = persistentRegistrationInfo.refresh(serviceDescription, persistentId, mergeWithLocalInfo, true); } finally { Modified: trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/RegistrationInfo.java =================================================================== --- trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/RegistrationInfo.java 2007-05-07 14:44:07 UTC (rev 7220) +++ trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/RegistrationInfo.java 2007-05-08 05:38:51 UTC (rev 7221) @@ -62,7 +62,7 @@ private Map persistentRegistrationProperties; private transient Boolean requiresRegistration; - private transient Boolean isRegistrationValid; + private transient Boolean consistentWithProducerExpectations; private transient RegistrationData registrationData; private transient boolean dirty; @@ -84,7 +84,6 @@ persistentConsumerName = WSRPConstants.DEFAULT_CONSUMER_NAME; } - public Long getKey() { return key; @@ -115,6 +114,28 @@ this.persistentRegistrationState = registrationState; } + public boolean isRefreshNeeded() + { + return dirty || requiresRegistration == null; + } + + public Boolean isRegistrationValid() + { + if (consistentWithProducerExpectations == null || requiresRegistration == null) + { + return null; + } + + boolean hasRegisteredIfNeeded = + (persistentRegistrationHandle != null && isRegistrationDeterminedRequired()) || isRegistrationDeterminedNotRequired(); + return Boolean.valueOf(consistentWithProducerExpectations.booleanValue() && hasRegisteredIfNeeded); + } + + public Boolean isConsistentWithProducerExpectations() + { + return consistentWithProducerExpectations; + } + /** * Determines whether the associated Producer requires registration. * @@ -133,10 +154,16 @@ * * @return <code>true</code> if and only if the associated Producer has been queried and mandates registration, * <code>false</code> otherwise. + * @throws IllegalStateException if {@link #refresh} has not yet been called */ public boolean isRegistrationDeterminedRequired() { - return requiresRegistration != null && requiresRegistration.booleanValue(); + if (requiresRegistration == null) + { + throw new IllegalStateException("Registration status not yet known: call refresh first!"); + } + + return requiresRegistration.booleanValue(); } /** @@ -145,10 +172,16 @@ * * @return <code>true</code> if and only if the associated Producer has been queried and does NOT mandate * registration, <code>false</code> otherwise. + * @throws IllegalStateException if {@link #refresh} has not yet been called */ public boolean isRegistrationDeterminedNotRequired() { - return requiresRegistration != null && !requiresRegistration.booleanValue(); + if (requiresRegistration == null) + { + throw new IllegalStateException("Registration status not yet known: call refresh first!"); + } + + return !requiresRegistration.booleanValue(); } public boolean hasLocalInfo() @@ -273,18 +306,6 @@ /** * @param serviceDescription * @param producerId - * @return <code>true</code> if the associated producer is ready to be interacted with as far as registration is - * concerned, <code>false</code> otherwise. - */ - boolean initialize(ServiceDescription serviceDescription, String producerId) - { - return !refresh(serviceDescription, producerId, false, false).hasIssues(); - } - - - /** - * @param serviceDescription - * @param producerId * @param mergeWithLocalInfo * @param forceRefresh * @return @@ -385,7 +406,7 @@ } // if issues have been detected, mark the registration as invalid (but do not reset the data) - isRegistrationValid = Boolean.valueOf(!result.hasIssues()); + consistentWithProducerExpectations = Boolean.valueOf(!result.hasIssues()); String msg = "Registration configuration is " + (result.hasIssues() ? "NOT" : "") + " valid"; result.appendToStatus(msg); @@ -497,13 +518,9 @@ persistentRegistrationHandle = null; persistentRegistrationState = null; requiresRegistration = null; + consistentWithProducerExpectations = null; } - public boolean isRegistrationValid() - { - return isRegistrationValid != null && isRegistrationValid.booleanValue(); - } - public void setRegistrationContext(RegistrationContext registrationContext) { ParameterValidation.throwIllegalArgExceptionIfNull(registrationContext, "RegistrationContext"); @@ -511,8 +528,16 @@ ParameterValidation.throwIllegalArgExceptionIfNullOrEmpty(handle, "registration handle", "RegistrationContext"); persistentRegistrationHandle = handle; persistentRegistrationState = registrationContext.getRegistrationState(); + setRegistrationValidInternalState(); } + void setRegistrationValidInternalState() + { + consistentWithProducerExpectations = Boolean.TRUE; // since we have a registration context, we're consistent with the Producer + requiresRegistration = Boolean.TRUE; // we know we require registration + dirty = false; // our state is clean :) + } + public RegistrationContext getRegistrationContext() { if (persistentRegistrationHandle != null) @@ -527,11 +552,6 @@ } } - public boolean isRefreshNeeded() - { - return dirty || requiresRegistration == null || isRegistrationValid == null; - } - public class RegistrationRefreshResult extends RefreshResult { public RegistrationRefreshResult() @@ -568,6 +588,7 @@ { oldValue = newValue; dirty = true; + consistentWithProducerExpectations = null; } return oldValue; Modified: trunk/wsrp/src/resources/portal-wsrp-sar/conf/consumer/hibernate/domain.hbm.xml =================================================================== --- trunk/wsrp/src/resources/portal-wsrp-sar/conf/consumer/hibernate/domain.hbm.xml 2007-05-07 14:44:07 UTC (rev 7220) +++ trunk/wsrp/src/resources/portal-wsrp-sar/conf/consumer/hibernate/domain.hbm.xml 2007-05-08 05:38:51 UTC (rev 7221) @@ -79,7 +79,7 @@ <map name="persistentRegistrationProperties" cascade="all,delete-orphan" lazy="false" access="field"> <cache usage=&quot;@portal.hibernate.cache.usage(a)&quot;/&gt; <key column="REG_PK" not-null="true"/> - <map-key type="string" column="name"/> + <map-key type="string" formula="name"/> <one-to-many class="org.jboss.portal.wsrp.consumer.RegistrationProperty"/> </map> </class> @@ -91,7 +91,7 @@ <param name="sequence">wsrpconsumer_seq</param> </generator> </id> - <property name="name" type="java.lang.String" not-null="true" unique="false" insert="false" update="false"/> + <property name="name" type="java.lang.String" not-null="true"/> <property name="value" type="java.lang.String"/> <property name="invalid" type="boolean"/> <property name="lang" type="java.lang.String" not-null="true"/> Modified: trunk/wsrp/src/resources/portal-wsrp-war/WEB-INF/jsf/consumers/editConsumer.xhtml =================================================================== --- trunk/wsrp/src/resources/portal-wsrp-war/WEB-INF/jsf/consumers/editConsumer.xhtml 2007-05-07 14:44:07 UTC (rev 7220) +++ trunk/wsrp/src/resources/portal-wsrp-war/WEB-INF/jsf/consumers/editConsumer.xhtml 2007-05-08 05:38:51 UTC (rev 7221) @@ -73,14 +73,14 @@ </c:choose> </td> </tr> -<c:if test="#{!consumer.refreshNeeded or consumer.localInfoPresent}"> +<c:if test="#{consumer.localInfoPresent or !consumer.refreshNeeded}"> <tr> <th>Registration information</th> <td> <c:choose> <c:when test="#{consumer.localInfoPresent}"> <c:choose> - <c:when test="#{!empty consumer.producerInfo.registrationInfo.registrationPropertyNames}"> + <c:when test="#{!empty consumer.producerInfo.registrationInfo.registrationProperties}"> <table border='1' cellspacing='0' class='registration-prop-table'> <tr> <th>Name</th> @@ -168,7 +168,7 @@ </h:commandLink> <h:commandLink action="#{consumersMgr.registerConsumer}" styleClass="portlet-form-button" - rendered="#{consumer.active and consumer.registrationRequired}" + rendered="#{!consumer.refreshNeeded and consumer.active and consumer.registrationRequired}" value="#{consumer.registered ? 'Deregister' : 'Register'}"> <f:param name="id" value="#{consumer.id}"/> <f:param name="register" value="#{!consumer.registered}"/>