Author: chris.laprun(a)jboss.com Date: 2006-12-29 02:05:37 -0500 (Fri, 29 Dec 2006) New Revision: 5940 Modified: trunk/common/src/main/org/jboss/portal/common/util/Tools.java trunk/wsrp/src/main/org/jboss/portal/test/wsrp/v1/consumer/PortletManagementTestCase.java trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/WSRPConsumerImpl.java trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/portlet/WSRPPortlet.java trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/portlet/info/WSRPPortletInfo.java Log: - Consumer now is able to discriminate between POPs and CCPs. Still need to handle implicit cloning! - getPortletMap now also returns clones. - Improved fast failure of several methods to avoid calls to WSRP when not needed. - Added possibility of cloning WSRPPortlets. - Cleaner management of cache. - Modified incorrect assumption in PortletManagementTestCase. Needs more tests! Modified: trunk/common/src/main/org/jboss/portal/common/util/Tools.java =================================================================== --- trunk/common/src/main/org/jboss/portal/common/util/Tools.java 2006-12-28 22:58:58 UTC (rev 5939) +++ trunk/common/src/main/org/jboss/portal/common/util/Tools.java 2006-12-29 07:05:37 UTC (rev 5940) @@ -26,6 +26,8 @@ import org.apache.log4j.Logger; import org.jboss.portal.common.logging.Log4JWriter; +import java.io.BufferedInputStream; +import java.io.BufferedOutputStream; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -38,12 +40,10 @@ import java.io.Serializable; import java.io.StringWriter; import java.io.Writer; -import java.io.BufferedInputStream; -import java.io.BufferedOutputStream; +import java.lang.reflect.Array; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.lang.reflect.Modifier; -import java.lang.reflect.Array; import java.math.BigInteger; import java.net.URL; import java.net.URLClassLoader; @@ -380,6 +380,7 @@ return new Enumeration() { boolean hasMore = false; + public boolean hasMoreElements() { return hasMore; @@ -391,7 +392,8 @@ { hasMore = false; } - else{ + else + { throw new NoSuchElementException(); } return o; @@ -446,7 +448,7 @@ public static List toList(Object[] objects) { - List list = new ArrayList(); + List list = new ArrayList(objects.length); for (int i = 0; i < objects.length; i++) { Object object = objects[i]; @@ -520,8 +522,8 @@ * Returns an iterator over the array elements within the specified range. * * @param objects the array containing the objects to iterate on - * @param from the inclusive start index - * @param to the exclusive stop index + * @param from the inclusive start index + * @param to the exclusive stop index * @return the iterator * @throws IllegalArgumentException if the object array is null or the specified range is not valid */ Modified: trunk/wsrp/src/main/org/jboss/portal/test/wsrp/v1/consumer/PortletManagementTestCase.java =================================================================== --- trunk/wsrp/src/main/org/jboss/portal/test/wsrp/v1/consumer/PortletManagementTestCase.java 2006-12-28 22:58:58 UTC (rev 5939) +++ trunk/wsrp/src/main/org/jboss/portal/test/wsrp/v1/consumer/PortletManagementTestCase.java 2006-12-29 07:05:37 UTC (rev 5940) @@ -27,7 +27,6 @@ import org.jboss.portal.common.value.StringValue; import org.jboss.portal.portlet.Portlet; import org.jboss.portal.portlet.PortletContext; -import org.jboss.portal.portlet.PortletInvokerException; import org.jboss.portal.portlet.info.MetaInfo; import org.jboss.portal.portlet.state.PropertyChange; import org.jboss.portal.portlet.state.PropertyMap; @@ -108,9 +107,9 @@ try { consumer.setProperties(original, null); - ExtendedAssert.fail("Shouldn't be possible to set properties on non-cloned portlet"); + ExtendedAssert.fail("Shouldn't be possible to set properties with null changes"); } - catch (PortletInvokerException expected) + catch (IllegalArgumentException expected) { //expected } Modified: trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/WSRPConsumerImpl.java =================================================================== --- trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/WSRPConsumerImpl.java 2006-12-28 22:58:58 UTC (rev 5939) +++ trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/WSRPConsumerImpl.java 2006-12-29 07:05:37 UTC (rev 5940) @@ -119,9 +119,12 @@ /** The service factory. */ private ServiceFactory serviceFactory; - /** The cached portlets. */ - private Map cachedPortletMap; + /** The Producer-Offered Portlets (handle -> WSRPPortlet) */ + private Map popsMap; + /** Consumer-Configured Portlets (handle -> WSRPPortlet) */ + private Map ccpsMap = new HashMap(); + /** Portlet groups. */ private Map portletGroups; @@ -191,44 +194,40 @@ { ParameterValidation.throwIllegalArgExceptionIfNullOrEmpty(portletId, "portlet id", "PortletContext"); - Portlet portlet; + // first try to see if we want a cloned portlet + Portlet portlet = (Portlet)ccpsMap.get(portletId); - // if cache is still valid, use information from cached service description - if (useCache() && !isCacheExpired()) + // the portlet is not cloned, so try POP ones... + if (portlet == null) { - log.debug("getPortlet from cached service description"); - Map portlets; - try + // if cache is still valid, use information from cached service description + if (useCache() && !isCacheExpired()) { - portlets = getPortletMap(); - } - catch (ServiceDescriptionUnavailableException e) - { - throw new InvokerUnavailableException(e.getMessage(), e.getCause()); - } + log.debug("getPortlet from cached service description"); - portlet = (Portlet)portlets.get(portletId); - } - else // otherwise, retrieve just the information for the appropriate portlet - { - log.debug("getPortlet via getPortletDescription"); - initializeProducerInteractionAndRegisterIfPossible(); - GetPortletDescription gpd = WSRPTypeFactory.createGetPortletDescription(registrationContext, portletId); - gpd.setUserContext(null); // todo: deal with user context!! - try - { - PortletDescriptionResponse response = getPortletManagementService().getPortletDescription(gpd); - ParameterValidation.throwIllegalArgExceptionIfNull(response, "PortletDescriptionResponse"); - portlet = createWSRPPortletFromPortletDescription(response.getPortletDescription()); + portlet = (Portlet)popsMap.get(portletId); } - catch (InvalidHandleFault invalidHandleFault) + else // otherwise, retrieve just the information for the appropriate portlet { - throw new NoSuchPortletException(invalidHandleFault, portletId); + log.debug("getPortlet via getPortletDescription"); + initializeProducerInteractionAndRegisterIfPossible(); + GetPortletDescription gpd = WSRPTypeFactory.createGetPortletDescription(registrationContext, portletId); + gpd.setUserContext(null); // todo: deal with user context!! + try + { + PortletDescriptionResponse response = getPortletManagementService().getPortletDescription(gpd); + ParameterValidation.throwIllegalArgExceptionIfNull(response, "PortletDescriptionResponse"); + portlet = createWSRPPortletFromPortletDescription(response.getPortletDescription()); + } + catch (InvalidHandleFault invalidHandleFault) + { + throw new NoSuchPortletException(invalidHandleFault, portletId); + } + catch (Exception e) + { + throw new InvokerUnavailableException("Couldn't access remote producer '" + producerId + "'", e); + } } - catch (Exception e) - { - throw new InvokerUnavailableException("Couldn't access remote producer '" + producerId + "'", e); - } } if (portlet == null) @@ -266,6 +265,12 @@ { ParameterValidation.throwIllegalArgExceptionIfNull(portletContext, "PortletContext"); + WSRPPortlet original = getWSRPPortlet(portletContext.getId()); + if (original == null) + { + throw new PortletInvokerException("No portlet '" + portletContext.getId() + "' to clone!"); + } + initializeProducerInteractionAndRegisterIfPossible(); // todo: deal with user context @@ -273,7 +278,13 @@ WSRPUtils.convertToWSRPPortletContext(portletContext), null); try { - return WSRPUtils.convertToPortalPortletContext(getPortletManagementService().clonePortlet(clonePortlet)); + PortletContext newPortletContext = WSRPUtils.convertToPortalPortletContext(getPortletManagementService().clonePortlet(clonePortlet)); + if (newPortletContext != null) + { + ccpsMap.put(newPortletContext.getId(), WSRPPortlet.createClone(newPortletContext, original.getWSRPInfo())); + } + + return newPortletContext; } catch (Exception e) { @@ -281,14 +292,33 @@ } } - public List destroyClones(List portletIds) throws IllegalArgumentException, PortletInvokerException, UnsupportedOperationException + public List destroyClones(List portletContexts) throws IllegalArgumentException, PortletInvokerException, UnsupportedOperationException { - ParameterValidation.throwIllegalArgExceptionIfNull(portletIds, "Portlet identifiers"); + ParameterValidation.throwIllegalArgExceptionIfNull(portletContexts, "Portlet identifiers"); + int numberOfClones = portletContexts.size(); + if (numberOfClones == 0) + { + return Collections.EMPTY_LIST; + } + + List handles = new ArrayList(numberOfClones); + List result = new ArrayList(numberOfClones); + for (Iterator contexts = portletContexts.iterator(); contexts.hasNext();) + { + PortletContext context = (PortletContext)contexts.next(); + String id = context.getId(); + if (getPortlet(id) == null) + { + result.add(new DestroyCloneFailure(id, "Couldn't destroy clone '" + id + "' because there is no such clone!")); + } + handles.add(id); + } + initializeProducerInteractionAndRegisterIfPossible(); DestroyPortlets destroyPortlets = WSRPTypeFactory.createDestroyPortlets(registrationContext, - (String[])portletIds.toArray(new String[0])); + (String[])handles.toArray(new String[0])); try { @@ -296,12 +326,22 @@ DestroyFailed[] failures = response.getDestroyFailed(); if (failures != null) { - List result = new ArrayList(failures.length); + // list all the failures and successes + List successfullyDestroyed = new ArrayList(handles); for (int i = 0; i < failures.length; i++) { DestroyFailed failure = failures[i]; - result.add(new DestroyCloneFailure(failure.getPortletHandle(), failure.getReason())); + String handle = failure.getPortletHandle(); + result.add(new DestroyCloneFailure(handle, failure.getReason())); + successfullyDestroyed.remove(handle); } + + // remove from the CCP map the clones that were sucessfully destroyed + for (Iterator clonesToRemove = successfullyDestroyed.iterator(); clonesToRemove.hasNext();) + { + ccpsMap.remove(clonesToRemove.next()); + } + return result; } else @@ -373,6 +413,13 @@ ParameterValidation.throwIllegalArgExceptionIfNull(portletContext, "PortletContext"); ParameterValidation.throwIllegalArgExceptionIfNullOrEmpty(changes, "Property changes"); + WSRPPortlet portlet = getWSRPPortlet(portletContext.getId()); + if (portlet == null) + { + throw new PortletInvokerException("Cannot set properties on portlet '" + portletContext.getId() + + "' because there is no such portlet."); + } + initializeProducerInteractionAndRegisterIfPossible(); PropertyList propertyList = WSRPTypeFactory.createPropertyList(); @@ -405,7 +452,15 @@ try { - return WSRPUtils.convertToPortalPortletContext(getPortletManagementService().setPortletProperties(setPortletProperties)); + PortletContext newPortletContext = WSRPUtils.convertToPortalPortletContext(getPortletManagementService().setPortletProperties(setPortletProperties)); + portlet.setPortletContext(newPortletContext); + // if the portlet was cloned, add to the CCP map + if (!newPortletContext.getId().equals(portletContext.getId())) + { + ccpsMap.put(newPortletContext, portlet); + } + + return newPortletContext; } catch (Exception e) { @@ -434,14 +489,7 @@ public Map getPortletGroupMap() throws ServiceDescriptionUnavailableException, PortletInvokerException { - if (useCache()) - { - updateCachesIfNeeded(); - } - else - { - extractOfferedPortlets(getServiceDescriptionAndRegisterIfNeeded()); // this updates portletGroups - } + refreshPOPsIfNeeded(); return portletGroups; } @@ -471,16 +519,17 @@ private Map getPortletMap() throws ServiceDescriptionUnavailableException, PortletInvokerException { - if (useCache()) + refreshPOPsIfNeeded(); + + if (ccpsMap.isEmpty()) { - // Get most accurate value - updateCachesIfNeeded(); - - return cachedPortletMap; + return popsMap; } else { - return extractOfferedPortlets(getServiceDescriptionAndRegisterIfNeeded()); + Map all = new HashMap(popsMap); + all.putAll(ccpsMap); + return all; } } @@ -503,7 +552,7 @@ { int length = portletDescriptions.length; log.debug("Extracting " + length + " portlets."); - Map portlets = new LinkedHashMap(length); + popsMap = new LinkedHashMap(length); portletGroups = new HashMap(); for (int i = 0; i < length; i++) @@ -513,16 +562,16 @@ if (wsrpPortlet != null) { - portlets.put(wsrpPortlet.getContext().getId(), wsrpPortlet); + popsMap.put(wsrpPortlet.getContext().getId(), wsrpPortlet); } } - - return portlets; } else { - return Collections.EMPTY_MAP; + popsMap = Collections.EMPTY_MAP; } + + return popsMap; } /** @@ -552,7 +601,7 @@ } else { - wsrpPortlet = new WSRPPortlet(portletHandle, info); + wsrpPortlet = new WSRPPortlet(PortletContext.createPortletContext(portletHandle), info); // add the portlet to the appropriate group if needed String portletGroupId = portletDescription.getGroupID(); @@ -577,16 +626,22 @@ return expirationCacheSeconds != null && expirationCacheSeconds.intValue() > 0; } - private void updateCachesIfNeeded() - throws ServiceDescriptionUnavailableException, PortletInvokerException + private void refreshPOPsIfNeeded() throws ServiceDescriptionUnavailableException, PortletInvokerException { - if (isCacheExpired()) + if (useCache()) { - cachedPortletMap = extractOfferedPortlets(getServiceDescriptionAndRegisterIfNeeded()); + if (isCacheExpired()) + { + extractOfferedPortlets(getServiceDescriptionAndRegisterIfNeeded()); - // reset expiration time - expirationTimeMillis = System.currentTimeMillis() + (expirationCacheSeconds.intValue() * 1000); + // reset expiration time + expirationTimeMillis = System.currentTimeMillis() + (expirationCacheSeconds.intValue() * 1000); + } } + else + { + extractOfferedPortlets(getServiceDescriptionAndRegisterIfNeeded()); + } } /** @@ -595,7 +650,7 @@ */ private boolean isCacheExpired() { - return System.currentTimeMillis() > expirationTimeMillis || cachedPortletMap == null || portletGroups == null; + return System.currentTimeMillis() > expirationTimeMillis || popsMap == null || portletGroups == null; } public Integer getExpirationCacheSeconds() @@ -879,6 +934,7 @@ { // the producer doesn't need registration, let this consumer know for future reference registrationData = REGISTRATION_NOT_NEEDED; + extractOfferedPortlets(serviceDescription); } } Modified: trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/portlet/WSRPPortlet.java =================================================================== --- trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/portlet/WSRPPortlet.java 2006-12-28 22:58:58 UTC (rev 5939) +++ trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/portlet/WSRPPortlet.java 2006-12-29 07:05:37 UTC (rev 5940) @@ -22,6 +22,7 @@ ******************************************************************************/ package org.jboss.portal.wsrp.consumer.portlet; +import org.jboss.portal.common.util.ParameterValidation; import org.jboss.portal.portlet.Portlet; import org.jboss.portal.portlet.PortletContext; import org.jboss.portal.portlet.info.PortletInfo; @@ -36,30 +37,31 @@ public class WSRPPortlet implements Portlet { - private String id; + private PortletContext portletContext; private PortletInfo info; public WSRPPortlet() { - } - public WSRPPortlet(String id) + public WSRPPortlet(PortletContext context, PortletInfo info) { - this.id = id; + ParameterValidation.throwIllegalArgExceptionIfNull(context, "PortletContext"); + this.portletContext = context; + this.info = info; } - public WSRPPortlet(String id, PortletInfo info) + public static WSRPPortlet createClone(PortletContext newContext, WSRPPortletInfo originalInfo) { - this.id = id; - this.info = info; + ParameterValidation.throwIllegalArgExceptionIfNull(newContext, "PortletContext"); + return new WSRPPortlet(newContext, new WSRPPortletInfo(originalInfo)); } /** Portlet interface implemented methods */ public PortletContext getContext() { - return PortletContext.createPortletContext(id); + return portletContext; } public PortletInfo getInfo() @@ -72,13 +74,6 @@ return true; } - /** */ - - public void setId(String id) - { - this.id = id; - } - public void setInfo(PortletInfo info) { this.info = info; @@ -89,4 +84,10 @@ { return (WSRPPortletInfo)getInfo(); } + + public void setPortletContext(PortletContext context) + { + ParameterValidation.throwIllegalArgExceptionIfNull(context, "PortletContext"); + this.portletContext = context; + } } Modified: trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/portlet/info/WSRPPortletInfo.java =================================================================== --- trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/portlet/info/WSRPPortletInfo.java 2006-12-28 22:58:58 UTC (rev 5939) +++ trunk/wsrp/src/main/org/jboss/portal/wsrp/consumer/portlet/info/WSRPPortletInfo.java 2006-12-29 07:05:37 UTC (rev 5940) @@ -26,6 +26,7 @@ import org.jboss.portal.Mode; import org.jboss.portal.WindowState; import org.jboss.portal.common.util.LocaleInfo; +import org.jboss.portal.common.util.ParameterValidation; import org.jboss.portal.portlet.info.CacheInfo; import org.jboss.portal.portlet.info.CapabilitiesInfo; import org.jboss.portal.portlet.info.MetaInfo; @@ -67,10 +68,7 @@ public WSRPPortletInfo(final PortletDescription portletDescription) { - if (portletDescription == null) - { - throw new IllegalArgumentException("PortletDescription cannot be null!"); - } + ParameterValidation.throwIllegalArgExceptionIfNull(portletDescription, "PortletDescription"); createCapabilitiesInfo(portletDescription); @@ -79,6 +77,29 @@ createWSRPInfo(portletDescription); } + + public WSRPPortletInfo(WSRPPortletInfo other) + { + ParameterValidation.throwIllegalArgExceptionIfNull(other, "WSRPPortletInfo"); + + usesMethodGet = other.usesMethodGet; + defaultMarkupSecure = other.defaultMarkupSecure; + onlySecure = other.onlySecure; + hasUserSpecificState = other.hasUserSpecificState; + userContextStoredInSession = other.userContextStoredInSession; + templatesStoredInSession = other.templatesStoredInSession; + doesUrlTemplateProcessing = other.doesUrlTemplateProcessing; + groupId = other.groupId; // should be duplicate group id? + + WSRPCapabilitiesInfo otherCapabilities = (WSRPCapabilitiesInfo)other.getCapabilities(); + capabilities = new WSRPCapabilitiesInfo(new HashMap(otherCapabilities.mimeTypes), + new HashSet(otherCapabilities.modes), new HashSet(otherCapabilities.windowStates), + new HashSet(otherCapabilities.locales)); + + WSRPMetaInfo otherMeta = (WSRPMetaInfo)other.getMeta(); + metaInfo = new WSRPMetaInfo(new HashMap(otherMeta.metaInfos)); + } + public CapabilitiesInfo getCapabilities() { return capabilities; @@ -195,61 +216,7 @@ locales.addAll(mimeTypeInfo.locales); } - capabilities = new CapabilitiesInfo() - { - public Set getMimeTypes() - { - return mimeTypes.keySet(); - } - - public Set getAllModes() - { - return modes; - } - - public Set getModes(String mimeType) - { - MIMETypeInfo mimeTypeInfo = (MIMETypeInfo)mimeTypes.get(mimeType); - if (mimeTypeInfo == null) - { - return Collections.EMPTY_SET; - } - - return Collections.unmodifiableSet(mimeTypeInfo.modes); - } - - public Set getAllWindowStates() - { - return windowStates; - } - - public Set getWindowStates(String mimeType) - { - MIMETypeInfo mimeTypeInfo = (MIMETypeInfo)mimeTypes.get(mimeType); - if (mimeTypeInfo == null) - { - return Collections.EMPTY_SET; - } - - return Collections.unmodifiableSet(mimeTypeInfo.windowStates); - } - - public Set getAllLocales() - { - return locales; - } - - public Set getLocales(String mimeType) - { - MIMETypeInfo mimeTypeInfo = (MIMETypeInfo)mimeTypes.get(mimeType); - if (mimeTypeInfo == null) - { - return Collections.EMPTY_SET; - } - - return Collections.unmodifiableSet(mimeTypeInfo.locales); - } - }; + capabilities = new WSRPCapabilitiesInfo(mimeTypes, modes, windowStates, locales); } private void createMetaInfo(PortletDescription portletDescription) @@ -283,13 +250,7 @@ } } - metaInfo = new MetaInfo() - { - public org.jboss.portal.common.util.LocalizedString getMetaValue(String key) - { - return (org.jboss.portal.common.util.LocalizedString)metaInfos.get(key); - } - }; + metaInfo = new WSRPMetaInfo(metaInfos); } private org.jboss.portal.common.util.LocalizedString createPortalLocalizedStringFrom(LocalizedString wsrpLocalizedString) @@ -395,4 +356,88 @@ return mode.toString(); } } + + private class WSRPCapabilitiesInfo implements CapabilitiesInfo + { + private final Map mimeTypes; + private final Set modes; + private final Set windowStates; + private final Set locales; + + public WSRPCapabilitiesInfo(Map mimeTypes, Set modes, Set windowStates, Set locales) + { + this.mimeTypes = mimeTypes; + this.modes = modes; + this.windowStates = windowStates; + this.locales = locales; + } + + public Set getMimeTypes() + { + return mimeTypes.keySet(); + } + + public Set getAllModes() + { + return modes; + } + + public Set getModes(String mimeType) + { + MIMETypeInfo mimeTypeInfo = (MIMETypeInfo)mimeTypes.get(mimeType); + if (mimeTypeInfo == null) + { + return Collections.EMPTY_SET; + } + + return Collections.unmodifiableSet(mimeTypeInfo.modes); + } + + public Set getAllWindowStates() + { + return windowStates; + } + + public Set getWindowStates(String mimeType) + { + MIMETypeInfo mimeTypeInfo = (MIMETypeInfo)mimeTypes.get(mimeType); + if (mimeTypeInfo == null) + { + return Collections.EMPTY_SET; + } + + return Collections.unmodifiableSet(mimeTypeInfo.windowStates); + } + + public Set getAllLocales() + { + return locales; + } + + public Set getLocales(String mimeType) + { + MIMETypeInfo mimeTypeInfo = (MIMETypeInfo)mimeTypes.get(mimeType); + if (mimeTypeInfo == null) + { + return Collections.EMPTY_SET; + } + + return Collections.unmodifiableSet(mimeTypeInfo.locales); + } + } + + private static class WSRPMetaInfo implements MetaInfo + { + private final Map metaInfos; + + public WSRPMetaInfo(Map metaInfos) + { + this.metaInfos = metaInfos; + } + + public org.jboss.portal.common.util.LocalizedString getMetaValue(String key) + { + return (org.jboss.portal.common.util.LocalizedString)metaInfos.get(key); + } + } }