Author: chris.laprun(a)jboss.com Date: 2010-11-23 09:22:55 -0500 (Tue, 23 Nov 2010) New Revision: 5222 Modified: components/wsrp/trunk/consumer/src/main/java/org/gatein/wsrp/consumer/ProducerInfo.java components/wsrp/trunk/consumer/src/test/java/org/gatein/wsrp/consumer/ProducerInfoTestCase.java components/wsrp/trunk/consumer/src/test/java/org/gatein/wsrp/protocol/v1/MarkupTestCase.java components/wsrp/trunk/consumer/src/test/java/org/gatein/wsrp/protocol/v2/MarkupTestCase.java Log: - GTNWSRP-157: Now properly recomputes the next cache expiration time when the cache expiration duration is changed. - Properly deal with negative cache duration value. - Added and updated tests. Modified: components/wsrp/trunk/consumer/src/main/java/org/gatein/wsrp/consumer/ProducerInfo.java =================================================================== --- components/wsrp/trunk/consumer/src/main/java/org/gatein/wsrp/consumer/ProducerInfo.java 2010-11-23 13:51:30 UTC (rev 5221) +++ components/wsrp/trunk/consumer/src/main/java/org/gatein/wsrp/consumer/ProducerInfo.java 2010-11-23 14:22:55 UTC (rev 5222) @@ -789,11 +789,7 @@ private void resetCacheTimerIfNeeded() { - if (useCache()) - { - // reset expiration time - expirationTimeMillis = System.currentTimeMillis() + (persistentExpirationCacheSeconds * 1000); - } + expirationTimeMillis = System.currentTimeMillis() + (getSafeExpirationCacheSeconds() * 1000); } /** @@ -818,9 +814,37 @@ public void setExpirationCacheSeconds(Integer expirationCacheSeconds) { + // record the previous cache expiration duration + Integer previousMS = getSafeExpirationCacheSeconds() * 1000; + + // 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; + } + else + { + expirationTimeMillis = System.currentTimeMillis(); + } + } + /** + * Returns the cache expiration duration in seconds as a positive value or zero so that it's safe to use in cache + * expiration time computations. + * + * @return + */ + private int getSafeExpirationCacheSeconds() + { + return useCache() ? persistentExpirationCacheSeconds : 0; + } + private ServiceDescription getUnmanagedServiceDescription(boolean asUnregistered) throws PortletInvokerException, OperationFailed, InvalidRegistration, ModifyRegistrationRequired { //todo: might need to implement customization of default service description Modified: components/wsrp/trunk/consumer/src/test/java/org/gatein/wsrp/consumer/ProducerInfoTestCase.java =================================================================== --- components/wsrp/trunk/consumer/src/test/java/org/gatein/wsrp/consumer/ProducerInfoTestCase.java 2010-11-23 13:51:30 UTC (rev 5221) +++ components/wsrp/trunk/consumer/src/test/java/org/gatein/wsrp/consumer/ProducerInfoTestCase.java 2010-11-23 14:22:55 UTC (rev 5222) @@ -126,7 +126,7 @@ } } - public void testRefreshAndCache() throws Exception + public void testSetNullCache() throws PortletInvokerException { ServiceDescriptionBehavior behavior = new ServiceDescriptionBehavior(); serviceFactory.getRegistry().setServiceDescriptionBehavior(behavior); @@ -146,17 +146,86 @@ assertFalse(info.isRefreshNeeded(false)); assertTrue(info.isRegistrationChecked()); assertEquals(2, behavior.getCallCount()); + } + public void testSetNegativeCache() throws PortletInvokerException + { + ServiceDescriptionBehavior behavior = new ServiceDescriptionBehavior(); + serviceFactory.getRegistry().setServiceDescriptionBehavior(behavior); + + // we now have a default value for cache + assertEquals(ProducerInfo.DEFAULT_CACHE_VALUE, info.getExpirationCacheSeconds()); + + // check behavior when cache has been set to a negative value + info.setExpirationCacheSeconds(-100); + assertEquals(new Integer(-100), info.getExpirationCacheSeconds()); + assertTrue(info.isRefreshNeeded(false)); + assertFalse(info.isRegistrationChecked()); + assertTrue(info.refresh(false)); + assertFalse(info.isRefreshNeeded(false)); + assertTrue(info.isRegistrationChecked()); + assertTrue(info.refresh(false)); + assertFalse(info.isRefreshNeeded(false)); + assertTrue(info.isRegistrationChecked()); + assertEquals(2, behavior.getCallCount()); + } + + public void testSetZeroCache() throws PortletInvokerException + { + ServiceDescriptionBehavior behavior = new ServiceDescriptionBehavior(); + serviceFactory.getRegistry().setServiceDescriptionBehavior(behavior); + + // we now have a default value for cache + assertEquals(ProducerInfo.DEFAULT_CACHE_VALUE, info.getExpirationCacheSeconds()); + + // check behavior when cache has been set to zero + info.setExpirationCacheSeconds(0); + assertEquals(new Integer(0), info.getExpirationCacheSeconds()); + assertTrue(info.isRefreshNeeded(false)); + assertFalse(info.isRegistrationChecked()); + assertTrue(info.refresh(false)); + assertFalse(info.isRefreshNeeded(false)); + assertTrue(info.isRegistrationChecked()); + assertTrue(info.refresh(false)); + assertFalse(info.isRefreshNeeded(false)); + assertTrue(info.isRegistrationChecked()); + assertEquals(2, behavior.getCallCount()); + } + + public void testCacheTransitions() throws Exception + { + ServiceDescriptionBehavior behavior = new ServiceDescriptionBehavior(); + serviceFactory.getRegistry().setServiceDescriptionBehavior(behavior); + + // we now have a default value for cache + assertEquals(ProducerInfo.DEFAULT_CACHE_VALUE, info.getExpirationCacheSeconds()); + + // check behavior when no cache has been set + info.setExpirationCacheSeconds(null); + assertNull(info.getExpirationCacheSeconds()); + assertTrue(info.isRefreshNeeded(false)); + assertFalse(info.isRegistrationChecked()); + assertTrue(info.refresh(false)); + assertFalse(info.isRefreshNeeded(false)); + assertTrue(info.isRegistrationChecked()); + assertTrue(info.refresh(false)); + assertFalse(info.isRefreshNeeded(false)); + assertTrue(info.isRegistrationChecked()); + assertEquals(2, behavior.getCallCount()); + + // wait a little so that computations that are only precise to the ms can actually mean something + Thread.sleep(10); + + // set cache and check that we don't refresh as much info.setExpirationCacheSeconds(1); assertEquals(new Integer(1), info.getExpirationCacheSeconds()); - assertTrue(info.refresh(false)); - assertFalse(info.refresh(false)); + assertFalse("we refreshed less than a second ago so we don't need to refresh again", info.refresh(false)); assertFalse(info.isRefreshNeeded(false)); assertTrue(info.isRegistrationChecked()); - assertEquals(3, behavior.getCallCount()); + assertEquals(2, behavior.getCallCount()); // wait for cache expiration - Thread.sleep(1500); + Thread.sleep(1100); assertFalse("refresh is not needed if cache is not considered", info.isRefreshNeeded(false)); assertTrue("refresh is needed if cache is considered since it has expired", info.isRefreshNeeded(true)); assertTrue(info.refresh(false)); @@ -166,7 +235,20 @@ assertTrue(info.refresh(true)); assertFalse(info.isRefreshNeeded(false)); assertTrue(info.isRegistrationChecked()); - assertEquals(5, behavior.getCallCount()); + assertEquals(4, behavior.getCallCount()); + + // wait a little so that computations that are only precise to the ms can actually mean something + Thread.sleep(10); + + // now ask to not use the cache anymore and check that we do refresh + info.setExpirationCacheSeconds(0); + assertEquals(new Integer(0), info.getExpirationCacheSeconds()); + assertTrue(info.refresh(false)); + assertTrue(info.refresh(false)); + assertFalse("since we've been refreshed at least once before, refreshing the endpoint and registration has been done so refresh is not needed", info.isRefreshNeeded(false)); + assertTrue(info.isRefreshNeeded(true)); + assertTrue(info.isRegistrationChecked()); + assertEquals(6, behavior.getCallCount()); } public void testGetPortlet() throws Exception Modified: components/wsrp/trunk/consumer/src/test/java/org/gatein/wsrp/protocol/v1/MarkupTestCase.java =================================================================== --- components/wsrp/trunk/consumer/src/test/java/org/gatein/wsrp/protocol/v1/MarkupTestCase.java 2010-11-23 13:51:30 UTC (rev 5221) +++ components/wsrp/trunk/consumer/src/test/java/org/gatein/wsrp/protocol/v1/MarkupTestCase.java 2010-11-23 14:22:55 UTC (rev 5222) @@ -163,6 +163,10 @@ String handle = InitCookieNotRequiredMarkupBehavior.INIT_COOKIE_NOT_REQUIRED_HANDLE; InitCookieMarkupBehavior behavior = (InitCookieMarkupBehavior)producer.getBehaviorRegistry().getMarkupBehaviorFor(handle); + // this test requires that the consumer refreshes which is not the case with the setUp, so force refresh + consumer.getProducerInfo().setExpirationCacheSeconds(0); + ExtendedAssert.assertTrue(consumer.getProducerInfo().isRefreshNeeded(true)); + ProducerSessionInformation sessionInfo = commonInitCookieTest(handle, behavior, V1CookieProtocol.NONE.value()); ExtendedAssert.assertNotNull(sessionInfo); Modified: components/wsrp/trunk/consumer/src/test/java/org/gatein/wsrp/protocol/v2/MarkupTestCase.java =================================================================== --- components/wsrp/trunk/consumer/src/test/java/org/gatein/wsrp/protocol/v2/MarkupTestCase.java 2010-11-23 13:51:30 UTC (rev 5221) +++ components/wsrp/trunk/consumer/src/test/java/org/gatein/wsrp/protocol/v2/MarkupTestCase.java 2010-11-23 14:22:55 UTC (rev 5222) @@ -168,6 +168,10 @@ String handle = InitCookieNotRequiredMarkupBehavior.INIT_COOKIE_NOT_REQUIRED_HANDLE; InitCookieMarkupBehavior behavior = (InitCookieMarkupBehavior)producer.getBehaviorRegistry().getMarkupBehaviorFor(handle); + // this test requires that the consumer refreshes which is not the case with the setUp, so force refresh + consumer.getProducerInfo().setExpirationCacheSeconds(0); + ExtendedAssert.assertTrue(consumer.getProducerInfo().isRefreshNeeded(true)); + ProducerSessionInformation sessionInfo = commonInitCookieTest(handle, behavior, CookieProtocol.NONE.value()); ExtendedAssert.assertNotNull(sessionInfo);