Author: chris.laprun(a)jboss.com Date: 2007-11-30 02:09:29 -0500 (Fri, 30 Nov 2007) New Revision: 9213 Modified: branches/JBoss_Portal_Branch_2_6/core-wsrp/src/main/org/jboss/portal/wsrp/admin/ui/ConsumerBean.java branches/JBoss_Portal_Branch_2_6/core-wsrp/src/main/org/jboss/portal/wsrp/admin/ui/ConsumerManagerBean.java branches/JBoss_Portal_Branch_2_6/core-wsrp/src/resources/portal-wsrp-admin-war/WEB-INF/jsf/consumers/editConsumer.xhtml branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/test/wsrp/consumer/ProducerInfoTestCase.java branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/test/wsrp/consumer/RegistrationInfoTestCase.java branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/consumer/ProducerInfo.java branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/consumer/RefreshResult.java branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/consumer/RegistrationInfo.java branches/JBoss_Portal_Branch_2_6/wsrp/src/resources/portal-wsrp-sar/conf/hibernate/consumer/domain.hbm.xml Log: - JBPORTAL-1825: + Added support to display both current and expected registration information: modifications are made to the expected version and only persisted if successful. Expected registration information is put in the session when the consumer refreshes and issues are found. + Better handling of modifyRegistration status. - JBPORTAL-1827: Properly handle empty registration prop description when registered and no error is sent by producer. This was incorrectly interpreted as a need to modify the registration. - Better ProducerInfo.refresh behavior. - RefreshResult improvements: + Now remembers the service description that was used so that it can be reused by calling code if needed. + Simplified handling of appendToStatus. - RegistrationInfo improvements: + Copy constructor. + Can now directly call refresh more easily. + Better handling of empty registration property descriptions for JBPORTAL-1827. + Use direct access to persistentConsumerName and persistentRegistrationHandle by Hibernate to avoid marking a newly restored RegistrationInfo as modified. - Some code moved to Java 5. Modified: branches/JBoss_Portal_Branch_2_6/core-wsrp/src/main/org/jboss/portal/wsrp/admin/ui/ConsumerBean.java =================================================================== --- branches/JBoss_Portal_Branch_2_6/core-wsrp/src/main/org/jboss/portal/wsrp/admin/ui/ConsumerBean.java 2007-11-30 01:31:05 UTC (rev 9212) +++ branches/JBoss_Portal_Branch_2_6/core-wsrp/src/main/org/jboss/portal/wsrp/admin/ui/ConsumerBean.java 2007-11-30 07:09:29 UTC (rev 9213) @@ -27,13 +27,13 @@ import org.jboss.portal.wsrp.consumer.ConsumerRegistry; import org.jboss.portal.wsrp.consumer.EndpointConfigurationInfo; import org.jboss.portal.wsrp.consumer.ProducerInfo; -import org.jboss.portal.wsrp.consumer.RefreshResult; import org.jboss.portal.wsrp.consumer.RegistrationInfo; import javax.faces.context.FacesContext; import javax.faces.event.ValueChangeEvent; import java.net.MalformedURLException; import java.net.URL; +import java.util.Map; /** * @author <a href="mailto:chris.laprun@jboss.com">Chris Laprun</a> @@ -47,7 +47,7 @@ private Boolean useWSDL = null; private ConsumerManagerBean manager; private boolean modified; - private boolean registrationModified; + private boolean registrationLocallyModified; private String serviceDescription; private String markup; @@ -55,6 +55,8 @@ private String registration; private String wsdl; + private transient RegistrationInfo expectedRegistrationInfo; + public ConsumerBean() { useWSDL = Boolean.TRUE; // use WSDL by default @@ -72,7 +74,7 @@ public boolean isModified() { - return modified || registrationModified; + return modified || getProducerInfo().isModifyRegistrationRequired() || isRegistrationLocallyModified(); } public boolean isUseWSDL() @@ -233,9 +235,14 @@ public boolean isRegistrationModified() { - return registrationModified; + return getProducerInfo().isModifyRegistrationRequired(); } + public boolean isRegistrationLocallyModified() + { + return registrationLocallyModified; + } + public boolean isRegistrationChecked() { return getProducerInfo().isRegistrationChecked(); @@ -260,6 +267,17 @@ return getProducerInfo().hasLocalRegistrationInfo(); } + public RegistrationInfo getExpectedRegistrationInfo() + { + if (expectedRegistrationInfo == null) + { + Map sessionMap = FacesContext.getCurrentInstance().getExternalContext().getSessionMap(); + expectedRegistrationInfo = (RegistrationInfo)sessionMap.get(ConsumerManagerBean.EXPECTED_REG_INFO_KEY); + } + + return expectedRegistrationInfo; + } + // Actions public String update() @@ -302,7 +320,7 @@ { registry.updateProducerInfo(prodInfo); modified = false; - registrationModified = false; + registrationLocallyModified = false; } public String refreshConsumer() @@ -318,15 +336,7 @@ } } - RefreshResult result = manager.refresh(consumer); - if (result.hasIssues()) - { - registrationModified = true; // if we have issues, it might indicate a need to modify the registration - } - else - { - registrationModified = false; - } + manager.refresh(consumer); return ConsumerManagerBean.CONFIGURE_CONSUMER; } @@ -339,28 +349,57 @@ { if (consumer != null) { - if (registrationModified) + ProducerInfo info = getProducerInfo(); + if (isModified()) { + // get updated registration info + RegistrationInfo newReg = getExpectedRegistrationInfo(); + + // check that we have the proper state: need to check before saving modifications as this will reset the modified status + if (newReg == null && !isRegistrationLocallyModified()) + { + IllegalStateException e = + new IllegalStateException("Registration not locally modified: there should be expected registration from producer!"); + log.debug(e); + throw e; + } + + // make sure we save any modified registration properties + saveToRegistry(info); + + // save old info in case something goes wrong + RegistrationInfo oldReg = getProducerInfo().getRegistrationInfo(); + + // if we want to change an existing registration property (for example, to upgrade service) then there are + // no expected information, we're just using the modified local version + if (newReg == null) + { + newReg = oldReg; + } + try { - ProducerInfo info = getProducerInfo(); - saveToRegistry(info); // make sure we save any modified registration properties - // todo: this should be done better cf regPropListener - getProducerInfo().getRegistrationInfo().setModified(true); // mark as modified to force refresh of RegistrationData + newReg.setModified(true); // mark as modified to force refresh of RegistrationData + // attempt to modify the registration using new registration info + info.setRegistrationInfo(newReg); info.modifyRegistration(); - getProducerInfo().getRegistrationInfo().setModified(false); + newReg.setModified(false); + registrationLocallyModified = false; + beanContext.createInfoMessage("Successfully modified Registration!"); - registrationModified = false; - - refreshConsumer(); } catch (Exception e) { + // restore old info + info.setRegistrationInfo(oldReg); + beanContext.createErrorMessageFrom(e); return null; } + + refreshConsumer(); return null; } else @@ -423,7 +462,7 @@ { // todo: should use modifyIfNeeded but it might be tricky as the name of the property that has been changed // cannot be easily retrieved (we cannot use it as an id directly), could use title but that would be hackish - registrationModified = true; + registrationLocallyModified = true; // bypass the rest of the life cycle and re-display page FacesContext.getCurrentInstance().renderResponse(); Modified: branches/JBoss_Portal_Branch_2_6/core-wsrp/src/main/org/jboss/portal/wsrp/admin/ui/ConsumerManagerBean.java =================================================================== --- branches/JBoss_Portal_Branch_2_6/core-wsrp/src/main/org/jboss/portal/wsrp/admin/ui/ConsumerManagerBean.java 2007-11-30 01:31:05 UTC (rev 9212) +++ branches/JBoss_Portal_Branch_2_6/core-wsrp/src/main/org/jboss/portal/wsrp/admin/ui/ConsumerManagerBean.java 2007-11-30 07:09:29 UTC (rev 9213) @@ -27,6 +27,7 @@ import org.jboss.portal.wsrp.WSRPConsumer; import org.jboss.portal.wsrp.consumer.ConsumerRegistry; import org.jboss.portal.wsrp.consumer.RefreshResult; +import org.jboss.portal.wsrp.consumer.RegistrationInfo; import javax.faces.context.FacesContext; import javax.faces.event.ActionEvent; @@ -47,6 +48,7 @@ static final String CONFIGURE_CONSUMER = "configureConsumer"; static final String INDEX = "index"; + static final String EXPECTED_REG_INFO_KEY = "expectedRegistrationInfo"; public ConsumerRegistry getRegistry() { @@ -226,8 +228,15 @@ try { RefreshResult result = consumer.refresh(true); + + if (result.hasIssues()) { + // create the expected registration info and make it available + RegistrationInfo expected = new RegistrationInfo(consumer.getProducerInfo().getRegistrationInfo()); + expected.refresh(result.getServiceDescription(), consumer.getProducerId(), true, true, true); + setExpectedRegistrationInfo(expected); + beanContext.createErrorMessage(result.getStatus()); // refresh had issues, we should deactivate this consumer @@ -259,11 +268,18 @@ RefreshResult refresh(WSRPConsumer consumer) { RefreshResult result = internalRefresh(consumer); + selectedId = consumer.getProducerId(); setConsumerIdInSession(false); return result; } + private void setExpectedRegistrationInfo(RegistrationInfo expected) + { + Map sessionMap = FacesContext.getCurrentInstance().getExternalContext().getSessionMap(); + sessionMap.put(EXPECTED_REG_INFO_KEY, expected); + } + public String listConsumers() { setConsumerIdInSession(true); @@ -287,8 +303,10 @@ { Map sessionMap = FacesContext.getCurrentInstance().getExternalContext().getSessionMap(); String consumerBeanName = "consumer"; // must match ConsumerBean name in faces-config.xml - sessionMap.remove(consumerBeanName); // force recreation of ConsumerBean + // force recreation of ConsumerBean otherwise switching to the consumer view might not show the proper consumer + sessionMap.remove(consumerBeanName); + if (!remove) { sessionMap.put(CONSUMER_ID, selectedId); Modified: branches/JBoss_Portal_Branch_2_6/core-wsrp/src/resources/portal-wsrp-admin-war/WEB-INF/jsf/consumers/editConsumer.xhtml =================================================================== --- branches/JBoss_Portal_Branch_2_6/core-wsrp/src/resources/portal-wsrp-admin-war/WEB-INF/jsf/consumers/editConsumer.xhtml 2007-11-30 01:31:05 UTC (rev 9212) +++ branches/JBoss_Portal_Branch_2_6/core-wsrp/src/resources/portal-wsrp-admin-war/WEB-INF/jsf/consumers/editConsumer.xhtml 2007-11-30 07:09:29 UTC (rev 9213) @@ -79,41 +79,79 @@ <td> <c:choose> <c:when test="#{consumer.localInfoPresent}"> - <c:choose> - <c:when test="#{!empty consumer.producerInfo.registrationInfo.registrationProperties}"> - <table class="registration-prop-table #{consumer.active ? 'active' : 'inactive'}"> - <tr> - <th class="nameColumn">Name</th> - <th class="descColumn">Description</th> - <th>Value</th> - </tr> - <c:forEach items="#{consumer.producerInfo.registrationInfo.registrationProperties}" var="prop"> - <tr title="#{prop.description.label.value}"> - <td>#{prop.name}</td> - <td>#{prop.description.label.value}</td> - <td> - <h:inputText value="#{prop.value}" size="50" onchange="this.form.submit()" - immediate="true" valueChangeListener="#{consumer.regPropListener}"/> - <h:outputText styleClass="portlet-msg-error" value="#{prop.status}" - rendered="#{prop.determinedInvalid}"/> - </td> - </tr> - </c:forEach> - <c:if test="#{consumer.registrationModified}"> + <h3 class="portlet-area-header">Current registration information:</h3> + <h:panelGroup styleClass="portlet-area-body"> + <c:choose> + <c:when test="#{!empty consumer.producerInfo.registrationInfo.registrationProperties}"> + <table class="registration-prop-table #{consumer.active ? 'active' : 'inactive'}"> <tr> - <td colspan="3"> - <h:commandLink action="#{consumer.modifyRegistration}" value="Modify registration" - title="Modify the registration held with this Producer" - styleClass="portlet-form-button portlet-section-buttonrow"/> - </td> + <th class="nameColumn">Name</th> + <th class="descColumn">Description</th> + <th>Value</th> </tr> - </c:if> - </table> - </c:when> - <c:otherwise> - Registration is indicated as required without registration properties. - </c:otherwise> - </c:choose> + <c:forEach items="#{consumer.producerInfo.registrationInfo.registrationProperties}" + var="prop"> + <tr title="#{prop.description.label.value}"> + <td>#{prop.name}</td> + <td>#{prop.description.label.value}</td> + <td> + <h:inputText value="#{prop.value}" size="50" onblur="this.form.submit()" + immediate="true" valueChangeListener="#{consumer.regPropListener}" + disabled="#{consumer.registrationModified}"/> + <h:outputText styleClass="portlet-msg-error" value="#{prop.status}" + rendered="#{prop.determinedInvalid}"/> + </td> + </tr> + </c:forEach> + </table> + </c:when> + <c:otherwise> + Registration is indicated as required without registration properties. + </c:otherwise> + </c:choose> + <h:commandLink action="#{consumer.modifyRegistration}" value="Modify registration" + rendered="#{consumer.registrationLocallyModified}" + title="Modify the registration held with this Producer" + styleClass="portlet-form-button portlet-section-buttonrow"/> + <br style="clear:both;"/> + </h:panelGroup> + + <br/> + + <c:if test="#{consumer.registrationModified}"> + <h3 class="portlet-area-header">Expected registration information:</h3> + <h:panelGroup styleClass="portlet-area-body"> + <c:choose> + <c:when test="#{!empty consumer.expectedRegistrationInfo.registrationProperties}"> + <table class="registration-prop-table #{consumer.active ? 'active' : 'inactive'}"> + <tr> + <th class="nameColumn">Name</th> + <th class="descColumn">Description</th> + <th>Value</th> + </tr> + <c:forEach items="#{consumer.expectedRegistrationInfo.registrationProperties}" var="prop"> + <tr title="#{prop.description.label.value}"> + <td>#{prop.name}</td> + <td>#{prop.description.label.value}</td> + <td> + <h:inputText value="#{prop.value}" size="50"/> + <h:outputText styleClass="portlet-msg-error" value="#{prop.status}" + rendered="#{prop.determinedInvalid}"/> + </td> + </tr> + </c:forEach> + </table> + </c:when> + <c:otherwise> + Registration is indicated as required without registration properties. + </c:otherwise> + </c:choose> + <h:commandLink action="#{consumer.modifyRegistration}" value="Modify registration" + title="Modify the registration held with this Producer" + styleClass="portlet-form-button portlet-section-buttonrow"/> + <br style="clear:both;"/> + </h:panelGroup> + </c:if> </c:when> <c:when test="#{consumer.registrationChecked and !consumer.registrationRequired}"> Producer doesn't require registration. Modified: branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/test/wsrp/consumer/ProducerInfoTestCase.java =================================================================== --- branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/test/wsrp/consumer/ProducerInfoTestCase.java 2007-11-30 01:31:05 UTC (rev 9212) +++ branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/test/wsrp/consumer/ProducerInfoTestCase.java 2007-11-30 07:09:29 UTC (rev 9213) @@ -91,7 +91,7 @@ assertTrue(info.isRegistrationChecked()); assertEquals(2, behavior.getCallCount()); - info.setExpirationCacheSeconds(new Integer(1)); + info.setExpirationCacheSeconds(1); assertEquals(new Integer(1), info.getExpirationCacheSeconds()); assertTrue(info.refresh(false)); assertFalse(info.refresh(false)); @@ -270,6 +270,7 @@ regInfo.setRegistrationPropertyValue(TestRegistrationBehavior.PROP_NAME, "invalid"); assertTrue(info.isRefreshNeeded(false)); assertTrue(info.isRegistered()); + assertTrue(info.isModifyRegistrationRequired()); RegistrationProperty prop = regInfo.getRegistrationProperty(TestRegistrationBehavior.PROP_NAME); assertNull(prop.isInvalid()); @@ -285,16 +286,18 @@ regInfo.setRegistrationPropertyValue(TestRegistrationBehavior.PROP_NAME, TestRegistrationBehavior.MODIFIED_VALUE); assertTrue(info.isRefreshNeeded(false)); assertNull(prop.isInvalid()); + assertTrue(info.isModifyRegistrationRequired()); info.modifyRegistration(); assertTrue(info.isRefreshNeeded(true)); // cache should have been invalidated assertFalse(info.isRefreshNeeded(false)); // but the rest of the information is valid so no refresh needed there assertTrue(info.refresh(false)); // however, if we refresh the producer info, it should have refreshed + assertFalse(info.isModifyRegistrationRequired()); Boolean invalid = prop.isInvalid(); assertNotNull(invalid); - assertFalse(invalid.booleanValue()); + assertFalse(invalid); } private static class TestPortletManagementBehavior extends PortletManagementBehavior Modified: branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/test/wsrp/consumer/RegistrationInfoTestCase.java =================================================================== --- branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/test/wsrp/consumer/RegistrationInfoTestCase.java 2007-11-30 01:31:05 UTC (rev 9212) +++ branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/test/wsrp/consumer/RegistrationInfoTestCase.java 2007-11-30 07:09:29 UTC (rev 9213) @@ -70,6 +70,8 @@ // and we don't know if the registration is valid assertNull(info.isRegistrationValid()); + assertFalse(info.isModified()); + try { info.isRegistrationDeterminedNotRequired(); @@ -93,7 +95,11 @@ { String key = "foo"; info.setRegistrationPropertyValue(key, "bar"); + + // check status assertNull(info.isConsistentWithProducerExpectations()); + assertTrue(info.isModified()); + Map properties = info.getRegistrationProperties(); assertFalse(properties.isEmpty()); Set names = info.getRegistrationPropertyNames(); @@ -108,10 +114,12 @@ public void testRegistrationPropertiesAndRefresh() { info.setRegistrationPropertyValue("prop0", "value0"); - RefreshResult result = info.refresh(createServiceDescription(true, 1), producerId, true, false); + RefreshResult result = info.refresh(createServiceDescription(true, 1), producerId, true, false, false); RegistrationProperty prop = info.getRegistrationProperty("prop0"); assertNull(prop.isInvalid()); assertFalse(result.hasIssues()); + assertFalse(info.isModified()); + assertTrue(info.isConsistentWithProducerExpectations()); // specifiy that the prop is valid to simulate a successful registration (integration test, should have something // testing that in ProducerInfoTestCase) @@ -128,7 +136,7 @@ { // no registration expected ServiceDescription sd = createServiceDescription(false, 0); - RefreshResult result = info.refresh(sd, producerId, true, false); + RefreshResult result = info.refresh(sd, producerId, true, false, false); assertNotNull(result); assertFalse(result.hasIssues()); assertTrue(info.isConsistentWithProducerExpectations().booleanValue()); @@ -137,7 +145,7 @@ assertFalse(info.isRegistrationDeterminedRequired()); assertTrue(info.isRegistrationValid().booleanValue()); - result = info.refresh(sd, producerId, false, false); + result = info.refresh(sd, producerId, false, false, false); assertNotNull(result); assertFalse(result.hasIssues()); assertTrue(info.isConsistentWithProducerExpectations().booleanValue()); @@ -154,7 +162,7 @@ assertNull(info.isRegistrationValid()); RegistrationInfo.RegistrationRefreshResult result = info.refresh( - createServiceDescription(true, 0), producerId, true, false); + createServiceDescription(true, 0), producerId, true, false, false); assertNotNull(result); assertFalse(result.hasIssues()); assertTrue(info.isRegistrationRequired().booleanValue()); @@ -169,7 +177,7 @@ info.setRegistrationPropertyValue("foo", "bar"); RegistrationInfo.RegistrationRefreshResult result = info.refresh( - createServiceDescription(true, 0), producerId, false, false); + createServiceDescription(true, 0), producerId, false, false, false); assertNotNull(result); assertTrue(result.hasIssues()); String status = result.getStatus(); @@ -193,7 +201,7 @@ // producer requests 2 registration properties ServiceDescription sd = createServiceDescription(true, 2); - RegistrationInfo.RegistrationRefreshResult result = info.refresh(sd, producerId, false, false); + RegistrationInfo.RegistrationRefreshResult result = info.refresh(sd, producerId, false, false, false); assertNotNull(result); assertTrue(result.hasIssues()); String status = result.getStatus(); @@ -216,7 +224,7 @@ info.setRegistrationPropertyValue("foo", "bar"); RegistrationInfo.RegistrationRefreshResult result = info.refresh(createServiceDescription(true, 2), - producerId, true, false); + producerId, true, false, false); assertNotNull(result); assertTrue(result.hasIssues()); @@ -240,29 +248,29 @@ public void testForceRefreshRegistration() { // - RefreshResult result = info.refresh(createServiceDescription(true, 0), producerId, false, false); + RefreshResult result = info.refresh(createServiceDescription(true, 0), producerId, false, false, false); assertNotNull(result); assertFalse(result.hasIssues()); 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); + result = info.refresh(createServiceDescription(true, 0), producerId, false, false, false); assertTrue(result.hasIssues()); assertFalse(info.isRegistrationValid().booleanValue()); info.removeRegistrationProperty("foo"); - result = info.refresh(createServiceDescription(true, 0), producerId, false, false); + result = info.refresh(createServiceDescription(true, 0), producerId, false, false, false); assertFalse(result.hasIssues()); 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); + result = info.refresh(createServiceDescription(false, 0), producerId, false, false, false); assertFalse(result.hasIssues()); assertFalse(info.isRegistrationValid().booleanValue()); // force refresh, registration should now be valid - result = info.refresh(createServiceDescription(false, 0), producerId, false, true); + result = info.refresh(createServiceDescription(false, 0), producerId, false, true, false); assertFalse(result.hasIssues()); assertTrue(info.isRegistrationValid().booleanValue()); } @@ -310,6 +318,30 @@ assertEquals("value1", properties[0].getStringValue()); } + public void testRefreshWhileRegisteredAndProducerNotSendingPropertyDescriptions() + { + info.setRegistrationPropertyValue("prop0", "value0"); + info.refresh(createServiceDescription(true, 1), producerId, true, true, false); + + // simulate successful registration + info.setRegistrationContext(WSRPTypeFactory.createRegistrationContext("handle")); + + assertTrue(info.isRegistrationRequired()); + assertTrue(info.isRegistrationValid()); + + ServiceDescription description = createServiceDescription(true, 0); + info.refresh(description, producerId, true, true, false); + assertTrue(info.isRegistrationValid()); + RegistrationProperty prop = info.getRegistrationProperty("prop0"); + assertNotNull(prop); + assertFalse(prop.isInvalid()); + + // check that forcing check of extra properties work + info.refresh(description, producerId, true, true, true); + assertFalse(info.isRegistrationValid()); + assertNull(info.getRegistrationProperty("prop0")); + } + private ServiceDescription createServiceDescription(boolean requiresRegistration, int numberOfProperties) { return ServiceDescriptionBehavior.createServiceDescription(requiresRegistration, numberOfProperties); Modified: branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/consumer/ProducerInfo.java =================================================================== --- branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/consumer/ProducerInfo.java 2007-11-30 01:31:05 UTC (rev 9212) +++ branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/consumer/ProducerInfo.java 2007-11-30 07:09:29 UTC (rev 9213) @@ -41,6 +41,7 @@ import org.jboss.portal.wsrp.core.InvalidHandleFault; import org.jboss.portal.wsrp.core.InvalidRegistrationFault; import org.jboss.portal.wsrp.core.ModifyRegistration; +import org.jboss.portal.wsrp.core.OperationFailedFault; import org.jboss.portal.wsrp.core.PortletDescription; import org.jboss.portal.wsrp.core.PortletDescriptionResponse; import org.jboss.portal.wsrp.core.PortletPropertyDescriptionResponse; @@ -103,6 +104,8 @@ /** Time at which the cache expires */ private long expirationTimeMillis; + private boolean isModifyRegistrationRequired; + 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?"; @@ -158,7 +161,7 @@ return persistentRegistrationInfo; } - void setRegistrationInfo(RegistrationInfo registrationInfo) + public void setRegistrationInfo(RegistrationInfo registrationInfo) { this.persistentRegistrationInfo = registrationInfo; } @@ -223,6 +226,11 @@ registry.updateProducerInfo(this); } + public boolean isModifyRegistrationRequired() + { + return isModifyRegistrationRequired || (persistentRegistrationInfo != null && persistentRegistrationInfo.isModified()); + } + CookieProtocol getRequiresInitCookie() { return requiresInitCookie; @@ -288,25 +296,56 @@ // save changes to endpoint registry.updateProducerInfo(this); - // if we don't yet have registration information, get an unregistered service description - serviceDescription = getServiceDescription(persistentRegistrationInfo == null); + boolean registeredSDSucceeded = true; + try + { + // if we don't yet have registration information, get an unregistered service description + serviceDescription = getUnmanagedServiceDescription(persistentRegistrationInfo == null); + result.setServiceDescription(serviceDescription); + } + catch (OperationFailedFault operationFailedFault) + { + if (hasLocalRegistrationInfo()) + { + log.debug("OperationFailedFault occurred, might indicate a need to modify registration"); + registeredSDSucceeded = false; + // attempt to get unregistered service description + serviceDescription = getServiceDescription(true); + result.setServiceDescription(serviceDescription); + // validate the registration information + RefreshResult registrationResult = internalRefreshRegistration(serviceDescription, false, true, true); + if (registrationResult.hasIssues()) + { + isModifyRegistrationRequired = true; + setActiveAndSave(false); + } + + result.appendToStatus(registrationResult.getStatus()); + result.setHasIssues(registrationResult.hasIssues()); + return result; + } + else + { + serviceDescription = rethrowAsInvokerUnvailable(operationFailedFault); + } + } + // do we need to call initCookie or not? requiresInitCookie = serviceDescription.getRequiresInitCookie(); // do we need to register? if (serviceDescription.isRequiresRegistration()) { + // refresh and force check for extra props if the registered SD failed + RefreshResult registrationResult = internalRefreshRegistration(serviceDescription, true, forceRefresh, !registeredSDSucceeded); + registry.updateProducerInfo(this); + + result.appendToStatus(registrationResult.getStatus()); + // attempt to register and determine if the current service description can be used to extract POPs - RefreshResult registrationResult = internalRefreshRegistration(serviceDescription, true, forceRefresh); - String status = registrationResult.getStatus(); - if (status != null) + if (registeredSDSucceeded && !registrationResult.hasIssues()) { - result.appendToStatus(status); - } - - if (!registrationResult.hasIssues()) - { registrationResult = register(serviceDescription, false); if (registrationResult.specificCode()) { @@ -318,11 +357,7 @@ extractOfferedPortlets(serviceDescription); } - status = registrationResult.getStatus(); - if (status != null) - { - result.appendToStatus(status); - } + result.appendToStatus(registrationResult.getStatus()); result.setHasIssues(registrationResult.hasIssues()); result.appendToStatus("Producer information successfully refreshed"); @@ -573,7 +608,7 @@ this.persistentExpirationCacheSeconds = expirationCacheSeconds; } - private ServiceDescription getServiceDescription(boolean asUnregistered) throws PortletInvokerException + private ServiceDescription getUnmanagedServiceDescription(boolean asUnregistered) throws PortletInvokerException, OperationFailedFault { GetServiceDescription request = getServiceDescriptionRequest(asUnregistered); @@ -596,20 +631,41 @@ { log.debug("Caught Exception in getServiceDescription:\n", e); + // de-activate + setActiveAndSave(false); + if (e instanceof InvalidRegistrationFault) { resetRegistration(); } + else if (e instanceof OperationFailedFault) + { + throw (OperationFailedFault)e; // rethrow to deal at higher level as meaning can vary depending on context + } - // de-activate - setActiveAndSave(false); + return rethrowAsInvokerUnvailable(e); + } + } - Throwable cause = e.getCause(); - throw new InvokerUnavailableException("Problem getting service description for producer " - + persistentId, cause == null ? e : cause); + ServiceDescription getServiceDescription(boolean asUnregistered) throws PortletInvokerException + { + try + { + return getUnmanagedServiceDescription(asUnregistered); } + catch (OperationFailedFault operationFailedFault) + { + return rethrowAsInvokerUnvailable(operationFailedFault); + } } + private ServiceDescription rethrowAsInvokerUnvailable(Exception e) throws InvokerUnavailableException + { + Throwable cause = e.getCause(); + throw new InvokerUnavailableException("Problem getting service description for producer " + + persistentId + ", please see the logs for more information.", cause == null ? e : cause); + } + private GetServiceDescription getServiceDescriptionRequest(boolean asUnregistred) throws PortletInvokerException { //todo: might need to implement customization of default service description @@ -721,7 +777,7 @@ if (serviceDescription.isRequiresRegistration()) { // check if the configured registration information is correct and if we can get the service description - RefreshResult result = persistentRegistrationInfo.refresh(serviceDescription, persistentId, true, forceRefresh); + RefreshResult result = persistentRegistrationInfo.refresh(serviceDescription, persistentId, true, forceRefresh, false); if (!result.hasIssues()) { try @@ -739,7 +795,7 @@ persistentRegistrationInfo.setRegistrationContext(registrationContext); String msg = "Consumer with id '" + persistentId + "' successfully registered with handle: '" + registrationContext.getRegistrationHandle() + "'"; - log.info(msg); + log.debug(msg); result.appendToStatus("\n").append(msg); return new RefreshResult(result, true, REGISTER_MEANING); } @@ -756,7 +812,7 @@ } else { - log.info(result.getStatus()); + log.debug(result.getStatus()); setActiveAndSave(false); throw new PortletInvokerException("Consumer is not ready to be registered with producer because of missing or invalid registration information."); } @@ -798,7 +854,7 @@ public void modifyRegistration() throws PortletInvokerException { - if (isRegistered()) + if (persistentRegistrationInfo != null && persistentRegistrationInfo.getRegistrationHandle() != null) { persistentEndpointInfo.refresh(); @@ -812,6 +868,9 @@ // force refresh of internal RegistrationInfo state persistentRegistrationInfo.setRegistrationValidInternalState(); + // registration is not modified anymore :) + isModifyRegistrationRequired = false; + if (state != null) { persistentRegistrationInfo.setRegistrationState(state.getRegistrationState()); @@ -845,29 +904,17 @@ } } - public RefreshResult refreshRegistrationInfo(boolean mergeWithLocalInfo) throws PortletInvokerException + private RefreshResult internalRefreshRegistration(ServiceDescription serviceDescription, boolean mergeWithLocalInfo, boolean forceRefresh, boolean forceCheckOfExtraProps) throws PortletInvokerException { - return internalRefreshRegistration(getServiceDescription(true), mergeWithLocalInfo, true); - } - - private RefreshResult internalRefreshRegistration(ServiceDescription serviceDescription, boolean mergeWithLocalInfo, boolean forceRefresh) throws PortletInvokerException - { if (persistentRegistrationInfo == null) { persistentRegistrationInfo = new RegistrationInfo(this); } - RefreshResult result; - try - { - result = persistentRegistrationInfo.refresh(serviceDescription, persistentId, mergeWithLocalInfo, forceRefresh); - } - finally - { - registry.updateProducerInfo(this); - } + RefreshResult result = + persistentRegistrationInfo.refresh(serviceDescription, persistentId, mergeWithLocalInfo, forceRefresh, forceCheckOfExtraProps); - log.info("Refreshed registration information for consumer with id '" + persistentId + "'"); + log.debug("Refreshed registration information for consumer with id '" + persistentId + "'"); return result; } Modified: branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/consumer/RefreshResult.java =================================================================== --- branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/consumer/RefreshResult.java 2007-11-30 01:31:05 UTC (rev 9212) +++ branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/consumer/RefreshResult.java 2007-11-30 07:09:29 UTC (rev 9213) @@ -23,6 +23,8 @@ package org.jboss.portal.wsrp.consumer; +import org.jboss.portal.wsrp.core.ServiceDescription; + /** * @author <a href="mailto:chris.laprun@jboss.com">Chris Laprun</a> * @version $Revision$ @@ -30,10 +32,11 @@ */ public class RefreshResult { - private StringBuffer status; + private StringBuffer status = new StringBuffer(); private boolean hasIssues; private boolean specificCode; private final String meaning; + private ServiceDescription serviceDescription; public RefreshResult(boolean specificCode, String specificCodeMeaning) { @@ -44,11 +47,7 @@ public RefreshResult(RefreshResult previous, boolean specificCode, String specificCodeMeaning) { this(specificCode, specificCodeMeaning); - String status = previous.getStatus(); - if (status != null) - { - appendToStatus(status); - } + appendToStatus(previous.getStatus()); setHasIssues(previous.hasIssues); } @@ -64,25 +63,15 @@ public String getStatus() { - if (status == null) - { - return null; - } - return status.toString(); } StringBuffer appendToStatus(String message) { - if (status == null) + if (message != null && message.length() > 1) { - status = new StringBuffer(message); - } - else - { status.append(".\n").append(message); } - return status; } @@ -100,4 +89,14 @@ { return meaning; } + + public void setServiceDescription(ServiceDescription serviceDescription) + { + this.serviceDescription = serviceDescription; + } + + public ServiceDescription getServiceDescription() + { + return serviceDescription; + } } Modified: branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/consumer/RegistrationInfo.java =================================================================== --- branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/consumer/RegistrationInfo.java 2007-11-30 01:31:05 UTC (rev 9212) +++ branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/consumer/RegistrationInfo.java 2007-11-30 07:09:29 UTC (rev 9213) @@ -25,6 +25,7 @@ import org.jboss.logging.Logger; import org.jboss.portal.common.util.ParameterValidation; +import org.jboss.portal.portlet.PortletInvokerException; import org.jboss.portal.wsrp.WSRPConstants; import org.jboss.portal.wsrp.WSRPTypeFactory; import org.jboss.portal.wsrp.WSRPUtils; @@ -65,18 +66,20 @@ private transient Boolean consistentWithProducerExpectations; private transient RegistrationData registrationData; private transient boolean dirty; + private transient ProducerInfo parent; public RegistrationInfo(ProducerInfo producerInfo) { this(); ParameterValidation.throwIllegalArgExceptionIfNull(producerInfo, "ProducerInfo"); producerInfo.setRegistrationInfo(this); + parent = producerInfo; } public RegistrationInfo(ProducerInfo producerInfo, boolean requiresRegistration) { this(producerInfo); - this.requiresRegistration = Boolean.valueOf(requiresRegistration); + this.requiresRegistration = requiresRegistration; } public RegistrationInfo() @@ -84,6 +87,32 @@ persistentConsumerName = WSRPConstants.DEFAULT_CONSUMER_NAME; } + public RegistrationInfo(RegistrationInfo other) + { + ParameterValidation.throwIllegalArgExceptionIfNull(other, "RegistrationInfo to clone from"); + this.persistentConsumerName = other.persistentConsumerName; + this.persistentRegistrationHandle = other.persistentRegistrationHandle; + + if (other.persistentRegistrationState != null) + { + this.persistentRegistrationState = new byte[other.persistentRegistrationState.length]; + System.arraycopy(other.persistentRegistrationState, 0, this.persistentRegistrationState, 0, other.persistentRegistrationState.length); + } + + if (other.persistentRegistrationProperties != null) + { + this.persistentRegistrationProperties = new HashMap(other.persistentRegistrationProperties.size()); + for (Object o : other.persistentRegistrationProperties.values()) + { + RegistrationProperty otherProp = (RegistrationProperty)o; + String name = otherProp.getName(); + RegistrationProperty prop = new RegistrationProperty(name, otherProp.getValue(), otherProp.getLang()); + prop.setStatus(otherProp.getStatus()); + this.persistentRegistrationProperties.put(name, prop); + } + } + } + public Long getKey() { return key; @@ -114,9 +143,19 @@ this.persistentRegistrationState = registrationState; } + public ProducerInfo getParent() + { + return parent; + } + + public void setParent(ProducerInfo parent) + { + this.parent = parent; + } + public boolean isRefreshNeeded() { - boolean result = isModified() || requiresRegistration == null; + boolean result = requiresRegistration == null || isModified(); if (result) { log.debug("Refresh needed"); @@ -130,10 +169,12 @@ { return null; } + return consistentWithProducerExpectations && hasRegisteredIfNeeded(); + } - boolean hasRegisteredIfNeeded = - (persistentRegistrationHandle != null && isRegistrationDeterminedRequired()) || isRegistrationDeterminedNotRequired(); - return Boolean.valueOf(consistentWithProducerExpectations.booleanValue() && hasRegisteredIfNeeded); + private boolean hasRegisteredIfNeeded() + { + return (persistentRegistrationHandle != null && isRegistrationDeterminedRequired()) || isRegistrationDeterminedNotRequired(); } public Boolean isConsistentWithProducerExpectations() @@ -313,15 +354,36 @@ * @param producerId * @param mergeWithLocalInfo * @param forceRefresh + * @param forceCheckOfExtraProps * @return */ public RegistrationRefreshResult refresh(ServiceDescription serviceDescription, String producerId, - boolean mergeWithLocalInfo, boolean forceRefresh) + boolean mergeWithLocalInfo, boolean forceRefresh, boolean forceCheckOfExtraProps) { log.debug("RegistrationInfo refresh requested"); if (forceRefresh || isRefreshNeeded()) { + if (serviceDescription == null && parent != null) + { + try + { + serviceDescription = parent.getServiceDescription(true); + } + catch (PortletInvokerException e) + { + log.debug(e); + serviceDescription = null; + } + } + + if (serviceDescription == null) + { + String msg = "Couldn't get a service description to refresh from!"; + log.debug(msg); + throw new IllegalArgumentException(msg); + } + persistentRegistrationProperties = getOrCreateRegistrationPropertiesMap(true); RegistrationRefreshResult result = new RegistrationRefreshResult(); @@ -387,12 +449,12 @@ } else { - handleNoRequiredRegistrationProperties(producerId, result, !mergeWithLocalInfo); + handleNoRequiredRegistrationProperties(producerId, result, !mergeWithLocalInfo, forceCheckOfExtraProps); } } else { - handleNoRequiredRegistrationProperties(producerId, result, !mergeWithLocalInfo); + handleNoRequiredRegistrationProperties(producerId, result, !mergeWithLocalInfo, forceCheckOfExtraProps); } } else @@ -427,20 +489,28 @@ } } - private void handleNoRequiredRegistrationProperties(String producerId, RegistrationRefreshResult result, boolean keepExtra) + private void handleNoRequiredRegistrationProperties(String producerId, RegistrationRefreshResult result, boolean keepExtra, boolean forceCheckOfExtraProps) { - log.info("The producer didn't require any specific registration properties"); + log.debug("The producer didn't require any specific registration properties"); Map properties = getOrCreateRegistrationPropertiesMap(false); if (properties != null && !properties.isEmpty()) { String msg = "Registration data is available for producer '" + producerId + "' when none is expected by the producer"; - log.info(msg); - checkForExtraProperties(producerId, result, Collections.EMPTY_SET, properties, keepExtra); + if (forceCheckOfExtraProps || !hasRegisteredIfNeeded()) + { + log.debug(msg); + checkForExtraProperties(producerId, result, Collections.EMPTY_SET, properties, keepExtra); + } + else + { + log.debug("Consumer is registered: producer most likely did not resend property descriptions"); + result.setHasIssues(false); + } } else { - log.info("Using default registration data for producer '" + producerId + "'"); + log.debug("Using default registration data for producer '" + producerId + "'"); registrationData = WSRPTypeFactory.createDefaultRegistrationData(); result.setHasIssues(false); } @@ -461,10 +531,10 @@ { StringBuffer message = new StringBuffer("The registration for producer '" + producerId + "' provided values for unexpected registration properties: "); - for (Iterator invalidProps = unexpected.iterator(); invalidProps.hasNext();) + for (Object anUnexpected : unexpected) { - String name = (String)invalidProps.next(); - message.append("\t- ").append(name).append("\n"); + String name = (String)anUnexpected; + message.append("\t- ").append(name); if (keepExtra) { // mark the prop as invalid @@ -477,10 +547,12 @@ } else { + message.append(" (was removed)"); properties.remove(name); } + message.append("\n"); } - log.info(message); + log.debug(message); result.appendToStatus(message.toString()); result.setHasIssues(true); } Modified: branches/JBoss_Portal_Branch_2_6/wsrp/src/resources/portal-wsrp-sar/conf/hibernate/consumer/domain.hbm.xml =================================================================== --- branches/JBoss_Portal_Branch_2_6/wsrp/src/resources/portal-wsrp-sar/conf/hibernate/consumer/domain.hbm.xml 2007-11-30 01:31:05 UTC (rev 9212) +++ branches/JBoss_Portal_Branch_2_6/wsrp/src/resources/portal-wsrp-sar/conf/hibernate/consumer/domain.hbm.xml 2007-11-30 07:09:29 UTC (rev 9213) @@ -25,7 +25,7 @@ <!DOCTYPE hibernate-mapping PUBLIC "-//Hibernate/Hibernate Mapping DTD 3.0//EN" "http://hibernate.sourceforge.net/hibernate-mapping-3.0.dtd"> - <hibernate-mapping> +<hibernate-mapping> <class name="org.jboss.portal.wsrp.consumer.ProducerInfo" table="JBP_PRODUCER_INFO"> <cache usage=&quot;@portal.hibernate.cache.usage(a)&quot;/&gt; <id name="key" column="PK" access="field" type="java.lang.Long"> @@ -74,8 +74,9 @@ <param name="sequence">wsrpconsumer_seq</param> </generator> </id> - <property name="consumerName" column="CONSUMER_NAME" type="java.lang.String" not-null="true"/> - <property name="registrationHandle" column="HANDLE" type="java.lang.String"/> + <property name="persistentConsumerName" column="CONSUMER_NAME" type="java.lang.String" not-null="true" + access="field"/> + <property name="persistentRegistrationHandle" column="HANDLE" type="java.lang.String" access="field"/> <property name="registrationState" column="STATE" type="binary" length="50000000"/> <map name="persistentRegistrationProperties" cascade="all,delete-orphan" lazy="false" access="field"> <cache usage=&quot;@portal.hibernate.cache.usage(a)&quot;/&gt;