[portal-commits] JBoss Portal SVN: r11257 - branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/consumer.

Author: chris.laprun(a)jboss.com Date: 2008-07-02 04:18:20 -0400 (Wed, 02 Jul 2008) New Revision: 11257 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/RefreshResult.java branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/consumer/RegistrationInfo.java Log: - Improved logic in status handling: tests should now pass. - A RegistrationRefreshResult now only affects the total refresh result if it wasn't bypassed. - Cleaned-up code somewhat. 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 2008-07-02 07:18:28 UTC (rev 11256) +++ branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/consumer/ProducerInfo.java 2008-07-02 08:18:20 UTC (rev 11257) @@ -303,8 +303,8 @@ if (forceRefresh || isRefreshNeeded(true)) { log.debug("Refreshing info for producer '" + getId() + "'"); -// RefreshResult result = new RefreshResult(true, REFRESH_MEANING); - RefreshResult result = new RefreshResult(); + // Refresh occurs until proven otherwise! :) + RefreshResult result = new RefreshResult(RefreshResult.Status.SUCCESS); try { persistentEndpointInfo.refresh(); @@ -312,13 +312,12 @@ catch (InvokerUnavailableException e) { log.debug("Couldn't refresh endpoint information: " + e); -// result.appendToStatus("Couldn't refresh endpoint information: ").append(e.getLocalizedMessage()); + // todo: should we fail fast here? // throw new PortletInvokerException("Couldn't refresh endpoint information: " + e.getLocalizedMessage()); } // save changes to endpoint registry.updateProducerInfo(this); - boolean registeredSDSucceeded = true; try { // if we don't yet have registration information, get an unregistered service description @@ -330,7 +329,6 @@ 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); @@ -344,8 +342,6 @@ } result.setRegistrationResult(registrationResult); - /*result.appendToStatus(registrationResult.getStatus(), false); - result.setHasIssues(registrationResult.hasIssues());*/ return result; } else @@ -367,16 +363,15 @@ if (serviceDescription.isRequiresRegistration()) { // refresh and force check for extra props if the registered SD failed - RefreshResult registrationResult = internalRefreshRegistration(serviceDescription, true, forceRefresh, !registeredSDSucceeded); + // todo: deal with forcing check of extra registration properties properly (if needed) + RefreshResult registrationResult = internalRefreshRegistration(serviceDescription, true, forceRefresh, false); registry.updateProducerInfo(this); -// result.appendToStatus(registrationResult.getStatus()); - // attempt to register and determine if the current service description can be used to extract POPs - if (registeredSDSucceeded && !registrationResult.hasIssues()) + if (!registrationResult.hasIssues()) { registrationResult = register(serviceDescription, false); - if (RefreshResult.Status.SUCCESS.equals(registrationResult.getStatus())) + if (!registrationResult.hasIssues()) { // registration occurred, so we should ask for a new service description serviceDescription = getServiceDescription(false); @@ -386,10 +381,7 @@ extractOfferedPortlets(serviceDescription); } -// result.appendToStatus(registrationResult.getStatus()); -// result.setHasIssues(registrationResult.hasIssues()); result.setRegistrationResult(registrationResult); -// result.appendToStatus("Producer information successfully refreshed"); return result; } @@ -492,8 +484,8 @@ if (info.isUsesMethodGet()) { log.warn("Portlet '" + portletHandle - + "' uses the GET method in forms. Since we don't handle this, this portlet will be excluded from " + - "the list of offered portlets for producer " + persistentId); + + "' uses the GET method in forms. Since we don't handle this, this portlet will be excluded from " + + "the list of offered portlets for producer " + persistentId); } else { @@ -566,7 +558,7 @@ catch (Exception e) { log.debug("Couldn't get portlet via getPortletDescription for producer '" + persistentId - + "'. Attempting to retrieve it from the service description as this producer might not support the PortletManagement interface.", e); + + "'. Attempting to retrieve it from the service description as this producer might not support the PortletManagement interface.", e); justRefreshed = refresh(true); portlet = getPortletFromCaches(portletHandle, justRefreshed); @@ -637,7 +629,7 @@ private boolean isCacheExpired() { boolean result = !useCache() || System.currentTimeMillis() > expirationTimeMillis || popsMap == null - || portletGroups == null; + || portletGroups == null; if (result) { log.debug("Cache expired or not used"); @@ -710,7 +702,7 @@ { 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); + + persistentId + ", please see the logs for more information.", cause == null ? e : cause); } private GetServiceDescription getServiceDescriptionRequest(boolean asUnregistred) throws PortletInvokerException @@ -779,7 +771,7 @@ throw new RuntimeException("Couldn't reset registration", e); } throw new IllegalArgumentException("Couldn't get property descriptions for portlet '" + portletHandle - + "' because the provided registration is invalid!"); + + "' because the provided registration is invalid!"); } catch (Exception e) { @@ -831,7 +823,7 @@ { log.debug("Attempting registration"); RegistrationContext registrationContext = persistentEndpointInfo.getRegistrationService() - .register(persistentRegistrationInfo.getRegistrationData()); + .register(persistentRegistrationInfo.getRegistrationData()); if (registrationContext == null) { @@ -841,7 +833,7 @@ persistentRegistrationInfo.setRegistrationContext(registrationContext); String msg = "Consumer with id '" + persistentId + "' successfully registered with handle: '" - + registrationContext.getRegistrationHandle() + "'"; + + registrationContext.getRegistrationHandle() + "'"; log.debug(msg); // result.appendToStatus(msg); RefreshResult res = new RefreshResult(); @@ -912,7 +904,7 @@ { RegistrationContext registrationContext = getRegistrationContext(); ModifyRegistration modifyRegistration = WSRPTypeFactory.createModifyRegistration(registrationContext, - persistentRegistrationInfo.getRegistrationData()); + persistentRegistrationInfo.getRegistrationData()); RegistrationState state = persistentEndpointInfo.getRegistrationService().modifyRegistration(modifyRegistration); // force refresh of internal RegistrationInfo state @@ -942,7 +934,7 @@ else { throw new IllegalStateException("Cannot modify registration for producer '" + persistentId - + "' as it's not registered"); + + "' as it's not registered"); } } @@ -962,7 +954,7 @@ } RefreshResult result = - persistentRegistrationInfo.refresh(serviceDescription, persistentId, mergeWithLocalInfo, forceRefresh, forceCheckOfExtraProps); + persistentRegistrationInfo.refresh(serviceDescription, persistentId, mergeWithLocalInfo, forceRefresh, forceCheckOfExtraProps); log.debug("Refreshed registration information for consumer with id '" + persistentId + "'"); @@ -972,9 +964,9 @@ public boolean isRefreshNeeded(boolean considerCache) { boolean result = (considerCache && isCacheExpired()) - || persistentRegistrationInfo == null - || persistentRegistrationInfo.isRefreshNeeded() - || persistentEndpointInfo.isRefreshNeeded(); + || persistentRegistrationInfo == null + || persistentRegistrationInfo.isRefreshNeeded() + || persistentEndpointInfo.isRefreshNeeded(); if (result) { log.debug("Refresh needed for producer '" + persistentId + "'"); 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 2008-07-02 07:18:28 UTC (rev 11256) +++ branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/consumer/RefreshResult.java 2008-07-02 08:18:20 UTC (rev 11257) @@ -33,7 +33,10 @@ */ public class RefreshResult { - public enum Status { SUCCESS, FAILURE, UNAVAILABLE, BYPASSED, UNKNOWN } + public enum Status + { + SUCCESS, FAILURE, UNAVAILABLE, BYPASSED, UNKNOWN + } private ServiceDescription serviceDescription; private Status status; @@ -59,7 +62,13 @@ if (registrationResult != null) { this.registrationResult = registrationResult; - status = registrationResult.getStatus(); + + // result of registration only impacts the result of the total refresh if it wasn't bypassed + RefreshResult.Status regStatus = registrationResult.getStatus(); + if (!BYPASSED.equals(regStatus)) + { + status = regStatus; + } } } @@ -70,7 +79,7 @@ public boolean hasIssues() { - return !(SUCCESS.equals(status) || BYPASSED.equals(status)) ; + return !(SUCCESS.equals(status) || BYPASSED.equals(status)); } public Status getStatus() 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 2008-07-02 07:18:28 UTC (rev 11256) +++ branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/consumer/RegistrationInfo.java 2008-07-02 08:18:20 UTC (rev 11257) @@ -321,14 +321,14 @@ return persistentRegistrationProperties; } - public Map<String,RegistrationProperty> getRegistrationProperties() + public Map<String, RegistrationProperty> getRegistrationProperties() { return getRegistrationProperties(true); } - private Map<String,RegistrationProperty> getRegistrationProperties(boolean immutable) + private Map<String, RegistrationProperty> getRegistrationProperties(boolean immutable) { - Map<String,RegistrationProperty> properties = getOrCreateRegistrationPropertiesMap(false); + Map<String, RegistrationProperty> properties = getOrCreateRegistrationPropertiesMap(false); if (properties != null) { if (immutable) @@ -400,16 +400,14 @@ // if we're not merging, we need to copy the current properties so that we can collect validation results. if (!mergeWithLocalInfo) { - result.setRegistrationProperties(new HashMap<String,RegistrationProperty>(persistentRegistrationProperties)); + result.setRegistrationProperties(new HashMap<String, RegistrationProperty>(persistentRegistrationProperties)); } dirty = false; if (serviceDescription.isRequiresRegistration()) { requiresRegistration = Boolean.TRUE; - StringBuffer message = new StringBuffer("Producer '").append(producerId).append("' requires registration"); -// result.appendToStatus(message.toString()); - log.debug(message); + log.debug("Producer '" + producerId + "' requires registration"); // check if the configured registration properties match the producer expectations ModelDescription regPropDescs = serviceDescription.getRegistrationPropertyDescription(); @@ -419,7 +417,7 @@ PropertyDescription[] propertyDescriptions = regPropDescs.getPropertyDescriptions(); if (propertyDescriptions != null && propertyDescriptions.length > 0) { - Map<String,RegistrationProperty> descriptionsMap = getRegistrationPropertyDescriptionsFromWSRP(propertyDescriptions); + Map<String, RegistrationProperty> descriptionsMap = getRegistrationPropertyDescriptionsFromWSRP(propertyDescriptions); // check that we don't have unexpected registration properties and if so, mark them as invalid or remove them Set<String> expectedNames = descriptionsMap.keySet(); @@ -452,7 +450,6 @@ } log.debug("Missing value for property '" + name + "'"); -// result.appendToStatus("Missing value for property '" + name + "'"); result.setStatus(RefreshResult.Status.FAILURE); } } @@ -471,7 +468,6 @@ { String msg = "Producer '" + producerId + "' doesn't require registration"; log.debug(msg); -// result.appendToStatus(msg); requiresRegistration = Boolean.FALSE; result.setStatus(RefreshResult.Status.SUCCESS); } @@ -488,7 +484,6 @@ consistentWithProducerExpectations = !result.hasIssues(); String msg = "Registration configuration is " + (consistentWithProducerExpectations ? "" : "NOT ") + "valid"; -// result.appendToStatus(msg); log.debug(msg); return result; } @@ -566,7 +561,6 @@ } } log.debug(message); -// result.appendToStatus(message.toString()); result.setStatus(RefreshResult.Status.FAILURE); } } @@ -575,7 +569,7 @@ * @param descriptions * @return */ - private Map<String,RegistrationProperty> getRegistrationPropertyDescriptionsFromWSRP(PropertyDescription[] descriptions) + private Map<String, RegistrationProperty> getRegistrationPropertyDescriptionsFromWSRP(PropertyDescription[] descriptions) { if (descriptions != null) { @@ -673,10 +667,10 @@ return registrationProperties; } - public void setRegistrationProperties(Map<String,RegistrationProperty> registrationProperties) + public void setRegistrationProperties(Map<String, RegistrationProperty> registrationProperties) { this.registrationProperties = registrationProperties; - } + } } private String modifyIfNeeded(String oldValue, String newValue)