diff --git a/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/concurrent/AbstractCache.java b/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/concurrent/AbstractCache.java index f3b512f04f6..f6da42170a8 100644 --- a/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/concurrent/AbstractCache.java +++ b/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/concurrent/AbstractCache.java @@ -18,23 +18,23 @@ import org.eclipse.cdt.dsf.internal.DsfPlugin; import org.eclipse.core.runtime.IStatus; import org.eclipse.core.runtime.Status; -/** - * A base implementation of a general purpose cache. Sub classes must implement - * {@link #retrieve(DataRequestMonitor)} to fetch data from the data source. - * Sub-classes or clients are also responsible for calling {@link #disable()} - * and {@link #reset()} to manage the state of the cache in response to events +/** + * A base implementation of a general purpose cache. Sub classes must implement + * {@link #retrieve(DataRequestMonitor)} to fetch data from the data source. + * Sub-classes are also responsible for calling {@link #set(Object, IStatus)} + * and {@link #reset()} to manage the state of the cache in response to events * from the data source. *

- * This cache requires an executor to use. The executor is used to synchronize - * access to the cache state and data. + * This cache requires an executor to use. The executor is used to synchronize + * access to the cache state and data. *

+ * * @since 2.2 */ @ConfinedToDsfExecutor("fExecutor") public abstract class AbstractCache implements ICache { private static final IStatus INVALID_STATUS = new Status(IStatus.ERROR, DsfPlugin.PLUGIN_ID, IDsfStatusConstants.INVALID_STATE, "Cache invalid", null); //$NON-NLS-1$ - private static final IStatus DISABLED_STATUS = new Status(IStatus.ERROR, DsfPlugin.PLUGIN_ID, IDsfStatusConstants.INVALID_STATE, "Cache disabled", null); //$NON-NLS-1$ private class RequestCanceledListener implements RequestMonitor.ICanceledListener { public void requestCanceled(final RequestMonitor canceledRm) { @@ -69,13 +69,16 @@ public abstract class AbstractCache implements ICache { protected ImmediateInDsfExecutor getImmediateInDsfExecutor() { return fExecutor; } - - /** - * Sub-classes should override this method to retrieve the cache data - * from its source. - * - * @param rm Request monitor for completion of data retrieval. - */ + + /** + * Sub-classes should override this method to retrieve the cache data from + * its source. The implementation should call {@link #set(Object, IStatus)} + * to store the newly retrieved data when it arrives (or an error, if one + * occurred retrieving the data) + * + * @param rm + * Request monitor for completion of data retrieval. + */ abstract protected void retrieve(); @@ -98,11 +101,7 @@ public abstract class AbstractCache implements ICache { return fStatus; } - public void request(final DataRequestMonitor rm) { - wait(rm); - } - - public void wait(RequestMonitor rm) { + public void update(RequestMonitor rm) { assert fExecutor.getDsfExecutor().isInExecutorThread(); if (!fValid) { @@ -139,11 +138,6 @@ public abstract class AbstractCache implements ICache { retrieve(); } } else { - if (rm instanceof DataRequestMonitor) { - @SuppressWarnings("unchecked") - DataRequestMonitor drm = (DataRequestMonitor)rm; - drm.setData(fData); - } rm.setStatus(fStatus); rm.done(); } @@ -259,65 +253,59 @@ public abstract class AbstractCache implements ICache { return canceled; } - - /** - * Resets the cache with a data value null and an error - * status with code {@link IDsfStatusConstants#INVALID_STATE}. - * - * @see #reset(Object, IStatus) - */ + + /** + * Resets the cache, setting its data to null, and status to + * {@link #INVALID_STATUS}. Equivalent to reset(null, INVALID_STATUS) + * + * @see #reset(Object, IStatus) + */ protected void reset() { reset(null, INVALID_STATUS); } - - /** - * Resets the cache with given data and status. Resetting the cache - * forces the cache to be invalid and cancels any current pending requests - * from data source. - *

- * This method should be called when the data source has issued an event - * indicating that the source data has changed but data may still be - * retrieved. Clients may need to re-request data following cache reset. - *

- * @param data The data that should be returned to any clients currently - * waiting for cache data. - * @status The status that should be returned to any clients currently - * waiting for cache data. - */ + + /** + * Resets the cache, setting its data to [data], and status to [status]. + * Resetting the cache puts it in the invalid state and cancels any current + * pending requests to the data source. + * + *

+ * The cache should be reset when the data source has issued an event + * indicating that the source data has changed but data may still be + * retrieved. Clients may need to re-request data following a cache reset. + * + * @param data + * The data that should be returned to any client that calls + * {@link #getData()} despite the invalid state + * @status The status that should be returned to any client that calls + * {@link #getStatus()()} despite the invalid state + * @see #reset() + * @see #set(Object, IStatus) + */ protected void reset(V data, IStatus status) { doSet(data, status, false); - } - - /** - * Disables the cache from retrieving data from the source. If the cache - * is already valid the data and status is retained. If the cache is not - * valid, then data value null and an error status with code - * {@link IDsfStatusConstants#INVALID_STATE} are set. - * - * @see #set(Object, IStatus) - */ - protected void disable() { - if (!fValid) { - set(null, DISABLED_STATUS); - } } - /** - * Resets the cache then disables it. When a cache is disabled it means - * that it is valid and requests to the data source will not be sent. - *

- * This method should be called when the data source has issued an event - * indicating that the source data has changed and future requests for - * data will return the given data and status. Once the source data - * becomes available again, clients should call {@link #reset()}. - *

- * @param data The data that should be returned to any clients waiting for - * cache data and for clients requesting data until the cache is reset again. - * @status The status that should be returned to any clients waiting for - * cache data and for clients requesting data until the cache is reset again. - * - * @see #reset(Object, IStatus) - */ + /** + * Puts the cache into the valid state, given it new data and status. + * + * This method should be called when the subclass has received a response + * for updated data from the source. Note that such a response may be an + * error. That does not make the cache invalid. Invalid strictly means that + * the cache's data has either gone stale or that it's in the initial unset + * state. + * + * @param data + * The data that should be returned to any clients waiting for + * cache data and for clients requesting data, until the cache is + * invalidated via one of the reset methods. + * @status The status that should be returned to any clients waiting for + * cache data and for clients requesting status, until the cache is + * invalidated via one of the reset methods. + * + * @see #reset() + * @see #reset(Object, IStatus) + */ protected void set(V data, IStatus status) { doSet(data, status, true); } diff --git a/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/concurrent/ICache.java b/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/concurrent/ICache.java index a92d45103ad..c9b18f2f7a8 100644 --- a/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/concurrent/ICache.java +++ b/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/concurrent/ICache.java @@ -42,24 +42,17 @@ public interface ICache { */ public IStatus getStatus(); - /** - * Wait for the cache to become valid. If the cache is valid already, the - * request returns immediately, otherwise data will first be retrieved from the - * source. - * - * @param rm RequestMonitor that is called when cache becomes valid. - */ - public void wait(RequestMonitor rm); - - /** - * Request data from the cache. The cache is valid, it will complete the - * request immediately, otherwise data will first be retrieved from the - * source. - * - * @param rm RequestMonitor that is called when cache becomes valid. - */ - public void request(DataRequestMonitor rm); - + /** + * Asks the cache to update its value from the source. If the cache is + * already valid, the request is completed immediately, otherwise data will + * first be retrieved from the source. Typically, this method is called by a + * client after it discovers the cache is invalid via {@link #isValid()} + * + * @param rm + * RequestMonitor that is called when cache becomes valid. + */ + public void update(RequestMonitor rm); + /** * Returns true if the cache is currently valid. I.e. * whether the cache can return a value immediately without first diff --git a/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/concurrent/RequestCache.java b/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/concurrent/RequestCache.java index 1c52b3099eb..8b8d157eaca 100644 --- a/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/concurrent/RequestCache.java +++ b/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/concurrent/RequestCache.java @@ -97,15 +97,6 @@ public abstract class RequestCache extends AbstractCache { super.reset(data, status); } - @Override - protected void disable() { - if (fRm != null) { - fRm.cancel(); - fRm = null; - } - super.disable(); - } - @Override protected void set(V data, IStatus status) { if (fRm != null) { diff --git a/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/concurrent/Transaction.java b/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/concurrent/Transaction.java index 985bd89c4b2..b6c78264a1e 100644 --- a/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/concurrent/Transaction.java +++ b/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/concurrent/Transaction.java @@ -128,10 +128,11 @@ public abstract class Transaction { throw new CoreException(cache.getStatus()); } } else { - // Throw the invalid cache exception, but first schedule a - // re-attempt of the transaction logic, to occur when the - // stale/unset cache object has been updated - cache.wait(new RequestMonitor(ImmediateExecutor.getInstance(), fRm) { + // Throw the invalid cache exception, but first ask the cache to + // update itself from its source, and schedule a re-attempt of the + // transaction logic to occur when the stale/unset cache has been + // updated + cache.update(new RequestMonitor(ImmediateExecutor.getInstance(), fRm) { @Override protected void handleCompleted() { execute(); @@ -179,7 +180,7 @@ public abstract class Transaction { int count = 0; for (ICache cache : caches) { if (!cache.isValid()) { - cache.wait(countringRm); + cache.update(countringRm); count++; } } diff --git a/dsf/org.eclipse.cdt.tests.dsf/src/org/eclipse/cdt/tests/dsf/concurrent/CacheTests.java b/dsf/org.eclipse.cdt.tests.dsf/src/org/eclipse/cdt/tests/dsf/concurrent/CacheTests.java index b6455fd62dd..6923e5b1e3e 100644 --- a/dsf/org.eclipse.cdt.tests.dsf/src/org/eclipse/cdt/tests/dsf/concurrent/CacheTests.java +++ b/dsf/org.eclipse.cdt.tests.dsf/src/org/eclipse/cdt/tests/dsf/concurrent/CacheTests.java @@ -65,11 +65,6 @@ public class CacheTests { super.reset(data, status); } - @Override - public void disable() { - super.disable(); - } - @Override public void set(Integer data, IStatus status) { super.set(data, status); @@ -127,13 +122,6 @@ public class CacheTests { Assert.assertEquals(fTestCache.getStatus().getCode(), IDsfStatusConstants.INVALID_STATE); } - private void assertCacheDisabledWithoutData() { - Assert.assertTrue(fTestCache.isValid()); - Assert.assertEquals(null, fTestCache.getData()); - Assert.assertFalse(fTestCache.getStatus().isOK()); - Assert.assertEquals(fTestCache.getStatus().getCode(), IDsfStatusConstants.INVALID_STATE); - } - private void assertCacheWaiting() { Assert.assertFalse(fTestCache.isValid()); Assert.assertEquals(null, fTestCache.getData()); @@ -156,7 +144,7 @@ public class CacheTests { Query q = new Query() { @Override protected void execute(DataRequestMonitor rm) { - fTestCache.request(rm); + fTestCache.update(rm); } }; // Check initial state @@ -200,7 +188,7 @@ public class CacheTests { Query q = new Query() { @Override protected void execute(DataRequestMonitor rm) { - fTestCache.request(rm); + fTestCache.update(rm); } }; fExecutor.execute(q); @@ -230,7 +218,7 @@ public class CacheTests { Query q1 = new Query() { @Override protected void execute(DataRequestMonitor rm) { - fTestCache.request(rm); + fTestCache.update(rm); } }; fExecutor.execute(q1); @@ -239,7 +227,7 @@ public class CacheTests { Query q2 = new Query() { @Override protected void execute(DataRequestMonitor rm) { - fTestCache.request(rm); + fTestCache.update(rm); } }; fExecutor.execute(q2); @@ -272,7 +260,7 @@ public class CacheTests { Query q = new Query() { @Override protected void execute(DataRequestMonitor rm) { - fTestCache.request(rm); + fTestCache.update(rm); } }; fExecutor.execute(q); @@ -296,103 +284,6 @@ public class CacheTests { assertCacheValidWithData(1); } - @Test - public void disableBeforeRequestTest() throws InterruptedException, ExecutionException { - // Disable the cache with a given value - fExecutor.submit(new DsfRunnable() { - public void run() { - fTestCache.disable(); - } - }).get(); - - assertCacheDisabledWithoutData(); - - // Try to request data from cache - Query q = new Query() { - @Override - protected void execute(DataRequestMonitor rm) { - fTestCache.request(rm); - } - }; - fExecutor.execute(q); - - Thread.sleep(100); - - // Retrieval should never have been made. - Assert.assertEquals(null, fRetrieveRm); - - try { - Assert.assertEquals(null, q.get()); - } catch (ExecutionException e) { - // expected the exception - return; - } - Assert.fail("expected an exeption"); - } - - @Test - public void disableWhilePendingTest() throws InterruptedException, ExecutionException { - // Request data from cache - Query q = new Query() { - @Override - protected void execute(DataRequestMonitor rm) { - fTestCache.request(rm); - } - }; - fExecutor.execute(q); - - // Disable the cache with a given value - fExecutor.submit(new DsfRunnable() { - public void run() { - fTestCache.disable(); - } - }).get(); - - assertCacheDisabledWithoutData(); - - // Completed the retrieve RM - fExecutor.submit(new DsfRunnable() { - public void run() { - fRetrieveRm.setData(1); - fRetrieveRm.done(); - } - }).get(); - - // Validate that cache is still disabled without data. - assertCacheDisabledWithoutData(); - } - - @Test - public void disableWhileValidTest() throws InterruptedException, ExecutionException { - // Request data from cache - Query q = new Query() { - @Override - protected void execute(DataRequestMonitor rm) { - fTestCache.request(rm); - } - }; - fExecutor.execute(q); - - // Wait until the cache starts data retrieval. - waitForRetrieveRm(); - - // Complete the request - fRetrieveRm.setData(1); - fRetrieveRm.done(); - - q.get(); - - // Disable cache - fExecutor.submit(new DsfRunnable() { - public void run() { - fTestCache.disable(); - } - }).get(); - - // Check final state - assertCacheValidWithData(1); - } - @Test public void disableWithValueTest() throws InterruptedException, ExecutionException { // Disable the cache with a given value @@ -422,7 +313,7 @@ public class CacheTests { Query q = new Query() { @Override protected void execute(DataRequestMonitor rm) { - fTestCache.request(rm); + fTestCache.update(rm); } }; fExecutor.execute(q); @@ -446,7 +337,7 @@ public class CacheTests { Query q = new Query() { @Override protected void execute(DataRequestMonitor rm) { - fTestCache.request(rm); + fTestCache.update(rm); } }; fExecutor.execute(q); @@ -481,7 +372,7 @@ public class CacheTests { Query q = new Query() { @Override protected void execute(DataRequestMonitor rm) { - fTestCache.request(rm); + fTestCache.update(rm); } }; fExecutor.execute(q); @@ -516,7 +407,7 @@ public class CacheTests { Query q = new Query() { @Override protected void execute(DataRequestMonitor rm) { - fTestCache.request(new DataRequestMonitor(ImmediateExecutor.getInstance(), rm) { + fTestCache.update(new DataRequestMonitor(ImmediateExecutor.getInstance(), rm) { @Override public synchronized void addCancelListener(ICanceledListener listener) { // Do not add the cancel listener so that the cancel request is not @@ -558,7 +449,7 @@ public class CacheTests { Query q1 = new Query() { @Override protected void execute(DataRequestMonitor rm) { - fTestCache.request(rm); + fTestCache.update(rm); } }; fExecutor.execute(q1); @@ -567,7 +458,7 @@ public class CacheTests { Query q2 = new Query() { @Override protected void execute(DataRequestMonitor rm) { - fTestCache.request(rm); + fTestCache.update(rm); } }; fExecutor.execute(q2); @@ -613,7 +504,7 @@ public class CacheTests { Query q = new Query() { @Override protected void execute(DataRequestMonitor rm) { - fTestCache.request(rm); + fTestCache.update(rm); } }; fExecutor.execute(q); @@ -644,7 +535,7 @@ public class CacheTests { Query q = new Query() { @Override protected void execute(DataRequestMonitor rm) { - fTestCache.request(rm); + fTestCache.update(rm); } }; fExecutor.execute(q); @@ -679,7 +570,7 @@ public class CacheTests { Query q = new Query() { @Override protected void execute(DataRequestMonitor rm) { - fTestCache.request(rm); + fTestCache.update(rm); } }; fExecutor.execute(q);