Author: chris.laprun(a)jboss.com Date: 2007-05-03 22:43:12 -0400 (Thu, 03 May 2007) New Revision: 7189 Added: trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/RefreshResult.java Modified: trunk/wsrp/src/main/org/jboss/portal/test/wsrp/other/RegistrationInfoTestCase.java trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/RegistrationInfo.java Log: - Simplified refreshRegistrationRequirementsFor and fixed behavior with respect to returned properties in result. - Added RefreshResult class. - Updated tests. 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-04 01:04:31 UTC (rev 7188) +++ trunk/wsrp/src/main/org/jboss/portal/test/wsrp/other/RegistrationInfoTestCase.java 2007-05-04 02:43:12 UTC (rev 7189) @@ -32,7 +32,6 @@ 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> @@ -59,7 +58,7 @@ assertNull(info.getRegistrationState()); } - public void testSetGetRegistrationProperty() + /*public void testSetGetRegistrationProperty() { String key = "foo"; info.setRegistrationPropertyValue(key, "bar"); @@ -72,7 +71,7 @@ assertNotNull(properties.get(key)); assertEquals(properties.get(key), info.getRegistrationProperty(key)); assertEquals("bar", info.getRegistrationProperty(key).getValue()); - } + }*/ public void testRefreshNoRegistration() { @@ -83,7 +82,7 @@ ServiceDescription sd = createServiceDescription(false, 0); RegistrationInfo.RegistrationRefreshResult result = info.refreshRegistrationRequirementsFor(sd, producerId, true, false); assertNotNull(result); - assertTrue(result.isValid()); + assertFalse(result.hasIssues()); assertFalse(info.isRegistrationRequired().booleanValue()); assertTrue(info.isRegistrationDeterminedNotRequired()); assertFalse(info.isRegistrationDeterminedRequired()); @@ -91,7 +90,7 @@ result = info.refreshRegistrationRequirementsFor(sd, producerId, false, false); assertNotNull(result); - assertTrue(result.isValid()); + assertFalse(result.hasIssues()); assertFalse(info.isRegistrationRequired().booleanValue()); assertTrue(info.isRegistrationDeterminedNotRequired()); assertFalse(info.isRegistrationDeterminedRequired()); @@ -107,7 +106,7 @@ RegistrationInfo.RegistrationRefreshResult result = info.refreshRegistrationRequirementsFor( createServiceDescription(true, 0), producerId, true, false); assertNotNull(result); - assertTrue(result.isValid()); + assertFalse(result.hasIssues()); assertTrue(info.isRegistrationRequired().booleanValue()); assertTrue(info.isRegistrationDeterminedRequired()); assertFalse(info.isRegistrationDeterminedNotRequired()); @@ -122,10 +121,12 @@ RegistrationInfo.RegistrationRefreshResult result = info.refreshRegistrationRequirementsFor( createServiceDescription(true, 0), producerId, false, false); assertNotNull(result); - assertFalse(result.isValid()); + assertTrue(result.hasIssues()); String status = result.getStatus(); assertNotNull(status); assertTrue(status.indexOf("foo") != -1); + assertEquals(1, info.getRegistrationProperties().size()); + assertEquals(1, result.getRegistrationProperties().size()); Map regProps = result.getRegistrationProperties(); assertNotNull(regProps); @@ -144,10 +145,12 @@ RegistrationInfo.RegistrationRefreshResult result = info.refreshRegistrationRequirementsFor(sd, producerId, false, false); assertNotNull(result); - assertFalse(result.isValid()); + assertTrue(result.hasIssues()); String status = result.getStatus(); assertNotNull(status); assertTrue(status.indexOf("prop0") != -1 && status.indexOf("prop1") != -1); + assertEquals(0, info.getRegistrationProperties().size()); + assertEquals(2, result.getRegistrationProperties().size()); Map regProps = result.getRegistrationProperties(); assertNotNull(regProps); @@ -165,7 +168,7 @@ RegistrationInfo.RegistrationRefreshResult result = info.refreshRegistrationRequirementsFor(createServiceDescription(true, 2), producerId, true, false); assertNotNull(result); - assertFalse(result.isValid()); + assertTrue(result.hasIssues()); RegistrationProperty prop = info.getRegistrationProperty("prop0"); assertNotNull(prop); @@ -179,6 +182,7 @@ assertTrue(prop.isInvalid()); assertEquals(RegistrationProperty.MISSING_VALUE_STATUS, prop.getStatus()); assertEquals(2, info.getRegistrationProperties().size()); + assertEquals(2, result.getRegistrationProperties().size()); assertNull(info.getRegistrationProperty("foo")); } @@ -188,28 +192,28 @@ RegistrationInfo.RegistrationRefreshResult result = info.refreshRegistrationRequirementsFor( createServiceDescription(true, 0), producerId, false, false); assertNotNull(result); - assertTrue(result.isValid()); + assertFalse(result.hasIssues()); assertFalse(info.isRegistrationValid()); // Modifying a property renders the info dirty and hence should be refreshed info.setRegistrationPropertyValue("foo", "bar"); result = info.refreshRegistrationRequirementsFor(createServiceDescription(true, 0), producerId, false, false); - assertFalse(result.isValid()); + assertTrue(result.hasIssues()); assertFalse(info.isRegistrationValid()); info.removeRegistrationProperty("foo"); result = info.refreshRegistrationRequirementsFor(createServiceDescription(true, 0), producerId, false, false); - assertTrue(result.isValid()); + assertFalse(result.hasIssues()); assertFalse(info.isRegistrationValid()); // producer has changed but we're not forcing refresh so registration should still be invalid result = info.refreshRegistrationRequirementsFor(createServiceDescription(false, 0), producerId, false, false); - assertTrue(result.isValid()); + assertFalse(result.hasIssues()); assertFalse(info.isRegistrationValid()); // force refresh result = info.refreshRegistrationRequirementsFor(createServiceDescription(false, 0), producerId, false, true); - assertTrue(result.isValid()); + assertFalse(result.hasIssues()); assertTrue(info.isRegistrationValid()); } Added: trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/RefreshResult.java =================================================================== --- trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/RefreshResult.java (rev 0) +++ trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/RefreshResult.java 2007-05-04 02:43:12 UTC (rev 7189) @@ -0,0 +1,69 @@ +/****************************************************************************** + * JBoss, a division of Red Hat * + * Copyright 2007, Red Hat Middleware, LLC, and individual * + * contributors as indicated by the @authors tag. See the * + * copyright.txt in the distribution for a full listing of * + * individual contributors. * + * * + * This is free software; you can redistribute it and/or modify it * + * under the terms of the GNU Lesser General Public License as * + * published by the Free Software Foundation; either version 2.1 of * + * the License, or (at your option) any later version. * + * * + * This software is distributed in the hope that it will be useful, * + * but WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * + * Lesser General Public License for more details. * + * * + * You should have received a copy of the GNU Lesser General Public * + * License along with this software; if not, write to the Free * + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA * + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. * + ******************************************************************************/ + +package org.jboss.portal.wsrp.consumer; + +/** + * @author <a href="mailto:chris.laprun@jboss.com">Chris Laprun</a> + * @version $Revision$ + * @since 2.6 + */ +public class RefreshResult +{ + private StringBuffer status; + private boolean hasIssues; + + public boolean hasIssues() + { + return hasIssues; + } + + public void setHasIssues(boolean hasIssues) + { + this.hasIssues = hasIssues; + } + + public String getStatus() + { + if (status == null) + { + return null; + } + + return status.toString(); + } + + StringBuffer appendToStatus(String message) + { + if (status == null) + { + status = new StringBuffer(message); + } + else + { + status.append(" ").append(message); + } + + return status; + } +} Property changes on: trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/RefreshResult.java ___________________________________________________________________ Name: svn:keywords + Author Date Id Revision Name: svn:eol-style + native 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-04 01:04:31 UTC (rev 7188) +++ trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/RegistrationInfo.java 2007-05-04 02:43:12 UTC (rev 7189) @@ -275,7 +275,7 @@ */ boolean initialize(ServiceDescription serviceDescription, String producerId) { - return refreshRegistrationRequirementsFor(serviceDescription, producerId, false, false).isValid(); + return !refreshRegistrationRequirementsFor(serviceDescription, producerId, false, false).hasIssues(); } @@ -294,7 +294,14 @@ if (forceRefresh || isRefreshNeeded()) { + persistentRegistrationProperties = getOrCreateRegistrationPropertiesMap(true); + RegistrationRefreshResult result = new RegistrationRefreshResult(); + // if we're not merging, we need to copy the properties so that we can collect validation results. + if (!mergeWithLocalInfo) + { + result.setRegistrationProperties(new HashMap(persistentRegistrationProperties)); + } dirty = false; if (serviceDescription.isRequiresRegistration()) @@ -308,91 +315,52 @@ ModelDescription regPropDescs = serviceDescription.getRegistrationPropertyDescription(); if (regPropDescs != null) { - result.setValid(true); + result.setHasIssues(false); PropertyDescription[] propertyDescriptions = regPropDescs.getPropertyDescriptions(); if (propertyDescriptions != null && propertyDescriptions.length > 0) { Map descriptionsMap = getRegistrationPropertyDescriptionsFromWSRP(propertyDescriptions); - if (mergeWithLocalInfo) - { - persistentRegistrationProperties = getOrCreateRegistrationPropertiesMap(true); + // check that we don't have unexpected registration properties and if so, mark them as invalid or remove them + Set expectedNames = descriptionsMap.keySet(); + checkForExtraProperties(producerId, result, expectedNames, persistentRegistrationProperties, !mergeWithLocalInfo); - // Remove extra properties - Set unexpected = new HashSet(persistentRegistrationProperties.keySet()); - unexpected.removeAll(descriptionsMap.keySet()); - if (!unexpected.isEmpty()) + // Merge existing properties + for (Iterator props = descriptionsMap.values().iterator(); props.hasNext();) + { + RegistrationProperty prop = (RegistrationProperty)props.next(); + String name = prop.getName(); + RegistrationProperty existing = getRegistrationProperty(name); + if (existing != null) { - for (Iterator invalidProps = unexpected.iterator(); invalidProps.hasNext();) - { - persistentRegistrationProperties.remove(invalidProps.next()); - } + // take the opportunity to add the property description... ^_^ + existing.setDescription(prop.getDescription()); } - - // Merge existing properties - for (Iterator props = descriptionsMap.values().iterator(); props.hasNext();) + else { - RegistrationProperty prop = (RegistrationProperty)props.next(); - String name = prop.getName(); - RegistrationProperty existing = getRegistrationProperty(name); - if (existing != null) + if (mergeWithLocalInfo) { - existing.setDescription(prop.getDescription()); - } - else - { persistentRegistrationProperties.put(name, prop); } - } - - resetRegistration(); - result.setValid(false); - } - else - { - persistentRegistrationProperties = getOrCreateRegistrationPropertiesMap(true); - - // check that we don't have unexpected registration properties and if so, mark them as invalid - Set expectedNames = descriptionsMap.keySet(); - checkForExtraProperties(producerId, result, expectedNames, persistentRegistrationProperties); - - // now check for missing properties, add the missing ones with a null value - StringBuffer missingProps = new StringBuffer(); - for (Iterator descriptionNames = expectedNames.iterator(); descriptionNames.hasNext();) - { - String name = (String)descriptionNames.next(); - RegistrationProperty prop = (RegistrationProperty)persistentRegistrationProperties.get(name); - RegistrationProperty producerProp = (RegistrationProperty)descriptionsMap.get(name); - if (prop == null) - { - producerProp.setStatus(RegistrationProperty.MISSING_STATUS); - persistentRegistrationProperties.put(name, producerProp); - missingProps.append("Missing value for property '").append(name).append("'\n"); - } else { - // take the opportunity to add the property description... ^_^ - prop.setDescription(producerProp.getDescription()); + prop.setStatus(RegistrationProperty.MISSING_STATUS); + result.getRegistrationProperties().put(name, prop); } - } - if (missingProps.length() > 0) - { - log.debug(missingProps); - result.appendToStatus(missingProps.toString()); - resetRegistration(); - result.setValid(false); + result.appendToStatus("Missing value for property '").append(name).append("'\n"); + result.setHasIssues(true); } } } else { - handleNoRequiredRegistrationProperties(producerId, result); + handleNoRequiredRegistrationProperties(producerId, result, !mergeWithLocalInfo); } } else { - handleNoRequiredRegistrationProperties(producerId, result); + handleNoRequiredRegistrationProperties(producerId, result, !mergeWithLocalInfo); } } else @@ -401,11 +369,16 @@ log.debug(msg); result.appendToStatus(msg); requiresRegistration = Boolean.FALSE; - result.setValid(true); + result.setHasIssues(false); } - result.setRegistrationProperties(persistentRegistrationProperties); - String msg = "Registration configuration is " + (result.isValid() ? "" : "NOT") + " valid"; + // if we're merging, the resulting properties are the saved properties + if (mergeWithLocalInfo) + { + result.setRegistrationProperties(persistentRegistrationProperties); + } + + String msg = "Registration configuration is " + (result.hasIssues() ? "NOT" : "") + " valid"; result.appendToStatus(msg); log.debug(msg); return result; @@ -413,13 +386,13 @@ else { RegistrationRefreshResult result = new RegistrationRefreshResult(); - result.valid = true; - result.registrationProperties = persistentRegistrationProperties; + result.setHasIssues(false); + result.setRegistrationProperties(persistentRegistrationProperties); return result; } } - private void handleNoRequiredRegistrationProperties(String producerId, RegistrationRefreshResult result) + private void handleNoRequiredRegistrationProperties(String producerId, RegistrationRefreshResult result, boolean keepExtra) { log.info("The producer didn't require any specific registration properties."); Map properties = getRegistrationProperties(); @@ -428,13 +401,13 @@ 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); + checkForExtraProperties(producerId, result, Collections.EMPTY_SET, properties, keepExtra); } else { log.info("Using default registration data for producer '" + producerId + "'"); registrationData = WSRPTypeFactory.createDefaultRegistrationData(); - result.setValid(true); + result.setHasIssues(false); } } @@ -445,7 +418,7 @@ * @param properties * @return a set of the property names expected by the producer */ - private void checkForExtraProperties(String producerId, RegistrationRefreshResult result, Set expectedNames, Map properties) + private void checkForExtraProperties(String producerId, RegistrationRefreshResult result, Set expectedNames, Map properties, boolean keepExtra) { Set unexpected = new HashSet(properties.keySet()); unexpected.removeAll(expectedNames); @@ -457,14 +430,26 @@ { String name = (String)invalidProps.next(); message.append("\t- ").append(name).append("\n"); - RegistrationProperty prop = (RegistrationProperty)properties.get(name); - prop.setInvalid(true); - prop.setStatus(RegistrationProperty.INEXISTENT_STATUS); + if (keepExtra) + { + // mark the prop as invalid + RegistrationProperty prop = (RegistrationProperty)properties.get(name); + prop.setInvalid(true); + prop.setStatus(RegistrationProperty.INEXISTENT_STATUS); + + // do the same in the result + prop = (RegistrationProperty)result.getRegistrationProperties().get(name); + prop.setInvalid(true); + prop.setStatus(RegistrationProperty.INEXISTENT_STATUS); + } + else + { + properties.remove(name); + } } log.info(message); result.appendToStatus(message.toString()); - resetRegistration(); - result.setValid(false); + result.setHasIssues(true); } } @@ -538,22 +523,10 @@ return dirty || requiresRegistration == null; } - public class RegistrationRefreshResult + public class RegistrationRefreshResult extends RefreshResult { private Map registrationProperties; - private StringBuffer status; - private boolean valid; - public String getStatus() - { - if (status == null) - { - return null; - } - - return status.toString(); - } - public Map getRegistrationProperties() { return registrationProperties; @@ -563,28 +536,6 @@ { this.registrationProperties = registrationProperties; } - - public boolean isValid() - { - return valid; - } - - public void setValid(boolean valid) - { - this.valid = valid; - } - - void appendToStatus(String message) - { - if (status == null) - { - status = new StringBuffer(message); - } - else - { - status.append(" ").append(message); - } - } } private String modifyIfNeeded(String oldValue, String newValue)