[gatein-commits] gatein SVN: r8570 - in components/wsrp/branches/2.1.x/consumer/src: main/java/org/gatein/wsrp/consumer/registry and 1 other directories.
do-not-reply at jboss.org
do-not-reply at jboss.org
Mon Mar 12 15:29:39 EDT 2012
Author: chris.laprun at jboss.com
Date: 2012-03-12 15:29:39 -0400 (Mon, 12 Mar 2012)
New Revision: 8570
Modified:
components/wsrp/branches/2.1.x/consumer/src/main/java/org/gatein/wsrp/consumer/ProducerInfo.java
components/wsrp/branches/2.1.x/consumer/src/main/java/org/gatein/wsrp/consumer/RegistrationInfo.java
components/wsrp/branches/2.1.x/consumer/src/main/java/org/gatein/wsrp/consumer/registry/AbstractConsumerRegistry.java
components/wsrp/branches/2.1.x/consumer/src/main/java/org/gatein/wsrp/consumer/registry/InMemoryConsumerRegistry.java
components/wsrp/branches/2.1.x/consumer/src/test/java/org/gatein/wsrp/consumer/ProducerInfoTestCase.java
Log:
- GTNWSRP-279: avoid updating ProducerInfo when it's not needed and added modification detection.
Modified: components/wsrp/branches/2.1.x/consumer/src/main/java/org/gatein/wsrp/consumer/ProducerInfo.java
===================================================================
--- components/wsrp/branches/2.1.x/consumer/src/main/java/org/gatein/wsrp/consumer/ProducerInfo.java 2012-03-12 19:29:29 UTC (rev 8569)
+++ components/wsrp/branches/2.1.x/consumer/src/main/java/org/gatein/wsrp/consumer/ProducerInfo.java 2012-03-12 19:29:39 UTC (rev 8570)
@@ -105,7 +105,6 @@
/** The activated status of the associated Consumer */
private boolean persistentActive;
- // GTNWSRP-239: information that's currently transient but should probably be persistent
/**
* GTNWSRP-239: whether or not this ProducerInfo requires ModifyRegistration to be called, currently persisted via
* mixin
@@ -175,7 +174,7 @@
{
return false;
}
- if (!persistentId.equals(that.persistentId))
+ if (!getId().equals(that.getId()))
{
return false;
}
@@ -187,7 +186,7 @@
public int hashCode()
{
int result = key != null ? key.hashCode() : 0;
- result = 31 * result + persistentId.hashCode();
+ result = 31 * result + getId().hashCode();
return result;
}
@@ -197,7 +196,7 @@
final StringBuilder sb = new StringBuilder();
sb.append("ProducerInfo");
sb.append("{key='").append(key).append('\'');
- sb.append(", id='").append(persistentId).append('\'');
+ sb.append(", id='").append(getId()).append('\'');
sb.append('}');
return sb.toString();
}
@@ -297,13 +296,46 @@
*/
public void setActive(boolean active)
{
+ setInternalActive(active);
+ }
+
+ private boolean setInternalActive(boolean active)
+ {
+ final boolean modified = modifyNowIfNeeded(persistentActive, active);
this.persistentActive = active;
+ return modified;
}
+ public String getId()
+ {
+ return persistentId;
+ }
+
+ public void setId(String id)
+ {
+ modifyNowIfNeeded(persistentId, id);
+ this.persistentId = id;
+ }
+
+ private boolean modifyNowIfNeeded(Object oldValue, Object newValue)
+ {
+ if (ParameterValidation.isOldAndNewDifferent(oldValue, newValue))
+ {
+ modifyNow();
+ return true;
+ }
+ else
+ {
+ return false;
+ }
+ }
+
public void setActiveAndSave(boolean active)
{
- setActive(active);
- registry.updateProducerInfo(this);
+ if(setInternalActive(active))
+ {
+ registry.updateProducerInfo(this);
+ }
}
public boolean isModifyRegistrationRequired()
@@ -315,6 +347,7 @@
public void setModifyRegistrationRequired(boolean modifyRegistrationRequired)
{
+ modifyNowIfNeeded(isModifyRegistrationRequired, modifyRegistrationRequired);
this.isModifyRegistrationRequired = modifyRegistrationRequired;
}
@@ -562,16 +595,16 @@
}
result.setRegistrationResult(registrationResult);
-
- return result;
}
else
{
log.debug("Registration not required");
persistentRegistrationInfo = new RegistrationInfo(this, false);
extractOfferedPortlets(serviceDescription);
- return result;
}
+
+ modifyNow();
+ return result;
}
private Map<String, ItemDescription> toMap(List<ItemDescription> itemDescriptions)
@@ -591,16 +624,6 @@
}
}
- public String getId()
- {
- return persistentId;
- }
-
- public void setId(String id)
- {
- this.persistentId = id;
- }
-
/**
* Extracts a map of offered Portlet objects from ServiceDescription
*
@@ -661,7 +684,7 @@
{
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);
+ "the list of offered portlets for producer " + getId());
}
else
{
@@ -737,7 +760,7 @@
}
catch (Exception e)
{
- log.debug("Couldn't get portlet via getPortletDescription for producer '" + persistentId
+ log.debug("Couldn't get portlet via getPortletDescription for producer '" + getId()
+ "'. Attempting to retrieve it from the service description as this producer might not support the PortletManagement interface.", e);
justRefreshed = refresh(true);
@@ -819,24 +842,26 @@
public void setExpirationCacheSeconds(Integer expirationCacheSeconds)
{
- // record the previous cache expiration duration
- Integer previousMS = getSafeExpirationCacheSeconds() * 1000;
+ if (modifyNowIfNeeded(persistentExpirationCacheSeconds, expirationCacheSeconds))
+ {
+ // record the previous cache expiration duration
+ Integer previousMS = getSafeExpirationCacheSeconds() * 1000;
- // assign the new value
- this.persistentExpirationCacheSeconds = expirationCacheSeconds;
+ // assign the new value
+ this.persistentExpirationCacheSeconds = expirationCacheSeconds;
- // recompute the expiration time based on previous value and new one
- long lastExpirationTimeChange = expirationTimeMillis - previousMS;
- int newMS = getSafeExpirationCacheSeconds() * 1000;
- if (lastExpirationTimeChange > 0)
- {
- expirationTimeMillis = lastExpirationTimeChange + newMS;
+ // recompute the expiration time based on previous value and new one
+ long lastExpirationTimeChange = expirationTimeMillis - previousMS;
+ int newMS = getSafeExpirationCacheSeconds() * 1000;
+ if (lastExpirationTimeChange > 0)
+ {
+ expirationTimeMillis = lastExpirationTimeChange + newMS;
+ }
+ else
+ {
+ expirationTimeMillis = System.currentTimeMillis();
+ }
}
- else
- {
- expirationTimeMillis = System.currentTimeMillis();
- }
-
}
/**
@@ -972,7 +997,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);
+ + getId() + ", please see the logs for more information. ", cause == null ? e : cause);
}
public RegistrationContext getRegistrationContext() throws PortletInvokerException
@@ -990,9 +1015,15 @@
persistentRegistrationInfo.resetRegistration();
invalidateCache();
+ modifyNow();
registry.updateProducerInfo(this);
}
+ void modifyNow()
+ {
+ setLastModified(System.currentTimeMillis());
+ }
+
// make package only after package reorg
public PortletPropertyDescriptionResponse getPropertyDescriptionsFor(String portletHandle)
@@ -1082,7 +1113,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, false);
+ RefreshResult result = persistentRegistrationInfo.refresh(serviceDescription, getId(), true, forceRefresh, false);
if (!result.hasIssues())
{
try
@@ -1110,7 +1141,7 @@
if (debug)
{
- String msg = "Consumer with id '" + persistentId + "' successfully registered with handle: '"
+ String msg = "Consumer with id '" + getId() + "' successfully registered with handle: '"
+ registrationContext.getRegistrationHandle() + "'";
log.debug(msg);
}
@@ -1124,7 +1155,7 @@
{
persistentRegistrationInfo.resetRegistration();
setActive(false);
- throw new PortletInvokerException("Couldn't register with producer '" + persistentId + "'", e);
+ throw new PortletInvokerException("Couldn't register with producer '" + getId() + "'", e);
}
}
else
@@ -1149,11 +1180,11 @@
{
RegistrationContext registrationContext = getRegistrationContext();
persistentEndpointInfo.getRegistrationService().deregister(registrationContext, UserAccess.getUserContext());
- log.info("Consumer with id '" + persistentId + "' deregistered.");
+ log.info("Consumer with id '" + getId() + "' deregistered.");
}
catch (Exception e)
{
- throw new PortletInvokerException("Couldn't deregister with producer '" + persistentId + "'", e);
+ throw new PortletInvokerException("Couldn't deregister with producer '" + getId() + "'", e);
}
finally
{
@@ -1162,7 +1193,7 @@
}
else
{
- throw new IllegalStateException("Cannot deregister producer '" + persistentId + "' as it's not registered");
+ throw new IllegalStateException("Cannot deregister producer '" + getId() + "' as it's not registered");
}
}
@@ -1210,20 +1241,20 @@
// update state
persistentRegistrationInfo.setRegistrationState(registrationState.value);
- log.info("Consumer with id '" + persistentId + "' sucessfully modified its registration.");
+ log.info("Consumer with id '" + getId() + "' sucessfully modified its registration.");
// reset cache to be able to see new offered portlets on the next refresh
invalidateCache();
}
catch (Exception e)
{
- throw new PortletInvokerException("Couldn't modify registration with producer '" + persistentId + "'", e);
+ throw new PortletInvokerException("Couldn't modify registration with producer '" + getId() + "'", e);
}
}
}
else
{
- throw new IllegalStateException("Cannot modify registration for producer '" + persistentId
+ throw new IllegalStateException("Cannot modify registration for producer '" + getId()
+ "' as it's not registered");
}
}
@@ -1239,9 +1270,9 @@
private RefreshResult internalRefreshRegistration(ServiceDescription serviceDescription, boolean mergeWithLocalInfo, boolean forceRefresh, boolean forceCheckOfExtraProps) throws PortletInvokerException
{
RefreshResult result =
- persistentRegistrationInfo.refresh(serviceDescription, persistentId, mergeWithLocalInfo, forceRefresh, forceCheckOfExtraProps);
+ persistentRegistrationInfo.refresh(serviceDescription, getId(), mergeWithLocalInfo, forceRefresh, forceCheckOfExtraProps);
- log.debug("Refreshed registration information for consumer with id '" + persistentId + "'");
+ log.debug("Refreshed registration information for consumer with id '" + getId() + "'");
return result;
}
@@ -1253,7 +1284,7 @@
|| persistentEndpointInfo.isRefreshNeeded();
if (result)
{
- log.debug("Refresh needed for producer '" + persistentId + "'");
+ log.debug("Refresh needed for producer '" + getId() + "'");
}
return result;
}
Modified: components/wsrp/branches/2.1.x/consumer/src/main/java/org/gatein/wsrp/consumer/RegistrationInfo.java
===================================================================
--- components/wsrp/branches/2.1.x/consumer/src/main/java/org/gatein/wsrp/consumer/RegistrationInfo.java 2012-03-12 19:29:29 UTC (rev 8569)
+++ components/wsrp/branches/2.1.x/consumer/src/main/java/org/gatein/wsrp/consumer/RegistrationInfo.java 2012-03-12 19:29:39 UTC (rev 8570)
@@ -745,6 +745,9 @@
}
regenerateRegistrationData = true;
+
+ // make sure that the parent is marked as modified so that changes can be properly saved
+ parent.modifyNow();
}
private void setModifyRegistrationNeeded(boolean modifyRegistrationNeeded)
Modified: components/wsrp/branches/2.1.x/consumer/src/main/java/org/gatein/wsrp/consumer/registry/AbstractConsumerRegistry.java
===================================================================
--- components/wsrp/branches/2.1.x/consumer/src/main/java/org/gatein/wsrp/consumer/registry/AbstractConsumerRegistry.java 2012-03-12 19:29:29 UTC (rev 8569)
+++ components/wsrp/branches/2.1.x/consumer/src/main/java/org/gatein/wsrp/consumer/registry/AbstractConsumerRegistry.java 2012-03-12 19:29:39 UTC (rev 8570)
@@ -65,7 +65,7 @@
private static final String CONSUMER_WITH_ID = "Consumer with id '";
private static final String RELEASE_SESSIONS_LISTENER = "release_sessions_listener_";
- private static final Logger log = LoggerFactory.getLogger(AbstractConsumerRegistry.class);
+ protected static final Logger log = LoggerFactory.getLogger(AbstractConsumerRegistry.class);
protected ConsumerCache consumerCache;
@@ -268,7 +268,7 @@
ProducerInfo producerInfo = loadProducerInfo(id);
if (producerInfo == null)
{
- return Long.MAX_VALUE;
+ return Long.MIN_VALUE;
}
else
{
@@ -280,25 +280,33 @@
{
ParameterValidation.throwIllegalArgExceptionIfNull(producerInfo, "ProducerInfo");
- String oldId = update(producerInfo);
-
- // if we updated and oldId is not null, we need to update the local information
- if (oldId != null)
+ // only save producer info if we have local modifications that postdate last persisted change
+ if (producerInfo.getLastModified() > getPersistedLastModifiedForProducerInfoWith(producerInfo.getId()))
{
- WSRPConsumer consumer = createConsumerFrom(producerInfo, true);
+ String oldId = update(producerInfo);
- // update the federating portlet invoker if needed
- if (federatingPortletInvoker.isResolved(oldId))
+ // if we updated and oldId is not null, we need to update the local information
+ if (oldId != null)
{
- federatingPortletInvoker.unregisterInvoker(oldId);
+ WSRPConsumer consumer = createConsumerFrom(producerInfo, true);
+
+ // update the federating portlet invoker if needed
+ if (federatingPortletInvoker.isResolved(oldId))
+ {
+ federatingPortletInvoker.unregisterInvoker(oldId);
+ }
+
+ // update cache
+ consumerCache.removeConsumer(oldId);
+ consumerCache.putConsumer(producerInfo.getId(), consumer);
}
- // update cache
- consumerCache.removeConsumer(oldId);
- consumerCache.putConsumer(producerInfo.getId(), consumer);
+ return oldId;
}
-
- return oldId;
+ else
+ {
+ return null;
+ }
}
public void start() throws Exception
Modified: components/wsrp/branches/2.1.x/consumer/src/main/java/org/gatein/wsrp/consumer/registry/InMemoryConsumerRegistry.java
===================================================================
--- components/wsrp/branches/2.1.x/consumer/src/main/java/org/gatein/wsrp/consumer/registry/InMemoryConsumerRegistry.java 2012-03-12 19:29:29 UTC (rev 8569)
+++ components/wsrp/branches/2.1.x/consumer/src/main/java/org/gatein/wsrp/consumer/registry/InMemoryConsumerRegistry.java 2012-03-12 19:29:39 UTC (rev 8570)
@@ -134,7 +134,8 @@
{
if (keysToIds.containsValue(id))
{
- return consumers.get(id).getProducerInfo();
+ final WSRPConsumer consumer = consumers.get(id);
+ return consumer != null ? consumer.getProducerInfo() : null;
}
else
{
Modified: components/wsrp/branches/2.1.x/consumer/src/test/java/org/gatein/wsrp/consumer/ProducerInfoTestCase.java
===================================================================
--- components/wsrp/branches/2.1.x/consumer/src/test/java/org/gatein/wsrp/consumer/ProducerInfoTestCase.java 2012-03-12 19:29:29 UTC (rev 8569)
+++ components/wsrp/branches/2.1.x/consumer/src/test/java/org/gatein/wsrp/consumer/ProducerInfoTestCase.java 2012-03-12 19:29:39 UTC (rev 8570)
@@ -81,6 +81,64 @@
info.setEndpointConfigurationInfo(eci);
}
+ public void testSettersWithoutModificationShouldNotChangeLastModified()
+ {
+ final long initial = info.getLastModified();
+
+ info.setActive(info.isActive());
+ assertEquals(initial, info.getLastModified());
+
+ info.setActiveAndSave(info.isActive());
+ assertEquals(initial, info.getLastModified());
+
+ info.setExpirationCacheSeconds(info.getExpirationCacheSeconds());
+ assertEquals(initial, info.getLastModified());
+
+ info.setId(info.getId());
+ assertEquals(initial, info.getLastModified());
+
+ info.setModifyRegistrationRequired(info.isModifyRegistrationRequired());
+ assertEquals(initial, info.getLastModified());
+ }
+
+ public void testSettersWithModificationShouldChangeLastModified() throws InterruptedException
+ {
+ long initial = info.getLastModified();
+ Thread.sleep(10); // to allow for System.currentTimeMillis() to catch up
+ info.setActive(!info.isActive());
+
+ initial = info.getLastModified();
+ Thread.sleep(10); // to allow for System.currentTimeMillis() to catch up
+ info.setActiveAndSave(!info.isActive());
+ assertTrue(initial != info.getLastModified());
+
+ initial = info.getLastModified();
+ Thread.sleep(10); // to allow for System.currentTimeMillis() to catch up
+ info.setExpirationCacheSeconds(info.getExpirationCacheSeconds() + 1);
+ assertTrue(initial != info.getLastModified());
+
+ initial = info.getLastModified();
+ Thread.sleep(10); // to allow for System.currentTimeMillis() to catch up
+ info.setId(info.getId() + "other");
+ assertTrue(initial != info.getLastModified());
+
+ initial = info.getLastModified();
+ Thread.sleep(10); // to allow for System.currentTimeMillis() to catch up
+ info.setModifyRegistrationRequired(!info.isModifyRegistrationRequired());
+ assertTrue(initial != info.getLastModified());
+ }
+
+ public void testSetKeyDoesNotChangeLastModified()
+ {
+ long initial = info.getLastModified();
+ info.setKey(info.getKey());
+ assertEquals(initial, info.getLastModified());
+
+ initial = info.getLastModified();
+ info.setKey(info.getKey() + "other");
+ assertEquals(initial, info.getLastModified());
+ }
+
public void testSetRegistrationInfo()
{
RegistrationInfo regInfo = new RegistrationInfo(info);
More information about the gatein-commits
mailing list