ALF-9613: caching content store. Various improvements and bug fixes. Including:

ALF-10097: disk-persistent cache settings in ehcache
ALF-10098: clean up  process should remove empty parent directories from content cache disk directory
ALF-10126: timeToIdle ehcache property was not affecting cache cleaner job
ALF-10127: externally deleted cached content files were not re-cached until after the items expired from ehcache

git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@30171 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

source/java/org/alfresco/repo/content/caching/CachingContentStore.java

@@ -51,6 +51,7 @@ public class CachingContentStore implements ContentStore
     private ContentStore backingStore;
     private ContentCache cache;
     private boolean cacheOnInbound;
+    private int maxCacheTries = 2;
     
     static
     {
@@ -175,30 +176,23 @@ public class CachingContentStore implements ContentStore
         return cacheAndRead(contentUrl);
     }    
     
+    
     private ContentReader cacheAndRead(String url)
     {
         WriteLock writeLock = readWriteLock(url).writeLock();
         writeLock.lock();
         try
         {
-            if (!cache.contains(url))
+            for (int i = 0; i < maxCacheTries; i++)
             {
-                if (cache.put(url, backingStore.getReader(url)))
+                ContentReader reader = attemptCacheAndRead(url);
+                if (reader != null)
                 {
-                    return cache.getReader(url);
-                }
-                else
-                {
-                    return backingStore.getReader(url);
+                    return reader;
                 }
             }
-            else
-            {
-                return cache.getReader(url);
-            }
-        }
-        catch(CacheMissException e)
-        {
+            // Have tried multiple times to cache the item and read it back from the cache
+            // but there is a recurring problem - give up and return the item from the backing store.
             return backingStore.getReader(url);
         }
         finally
@@ -207,6 +201,31 @@ public class CachingContentStore implements ContentStore
         }
     }
     
+    private ContentReader attemptCacheAndRead(String url)
+    {
+        ContentReader reader = null;
+        try
+        {
+            if (!cache.contains(url))
+            {
+                if (cache.put(url, backingStore.getReader(url)))
+                {
+                    reader = cache.getReader(url);
+                }
+            }
+            else
+            {
+                reader = cache.getReader(url);
+            }
+        }
+        catch(CacheMissException e)
+        {
+            cache.remove(url);
+        }
+        
+        return reader;
+    }
+    
     /*
      * @see org.alfresco.repo.content.ContentStore#getWriter(org.alfresco.repo.content.ContentContext)
      */
@@ -279,10 +298,40 @@ public class CachingContentStore implements ContentStore
     @Override
     public boolean delete(String contentUrl)
     {
-        if (cache.contains(contentUrl))
-            cache.remove(contentUrl);
+        ReentrantReadWriteLock readWriteLock = readWriteLock(contentUrl);
+        ReadLock readLock = readWriteLock.readLock();
+        readLock.lock();
+        try
+        {
+            if (!cache.contains(contentUrl))
+            {
+                // The item isn't in the cache, so simply delete from the backing store
+                return backingStore.delete(contentUrl);
+            }
+        }
+        finally
+        {
+            readLock.unlock();
+        }
         
-        return backingStore.delete(contentUrl);
+        WriteLock writeLock = readWriteLock.writeLock();
+        writeLock.lock();
+        try
+        {
+            // Double check the content still exists in the cache
+            if (cache.contains(contentUrl))
+            {
+                // The item is in the cache, so remove.
+                cache.remove(contentUrl);
+                
+            }
+            // Whether the item was in the cache or not, it must still be deleted from the backing store.
+            return backingStore.delete(contentUrl);
+        }
+        finally
+        {
+            writeLock.unlock();
+        }
     }
 
     /**

source/java/org/alfresco/repo/content/caching/CachingContentStoreTest.java

@@ -23,7 +23,9 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyString;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
@@ -45,6 +47,7 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
+import org.mockito.Mockito;
 import org.mockito.runners.MockitoJUnitRunner;
 
 /**
@@ -85,16 +88,40 @@ public class CachingContentStoreTest
     
     
     @Test
-    public void getReaderForItemMissingFromCache()
+    public void getReaderForItemMissingFromCacheWillGiveUpAfterRetrying()
     {
         ContentReader sourceContent = mock(ContentReader.class);
         when(cache.getReader("url")).thenThrow(new CacheMissException("url"));
         when(backingStore.getReader("url")).thenReturn(sourceContent);
         when(cache.put("url", sourceContent)).thenReturn(true);
         
-        cachingStore.getReader("url");
+        ContentReader returnedReader = cachingStore.getReader("url");
+        
+        // Upon failure, item is removed from cache
+        verify(cache, Mockito.atLeastOnce()).remove("url");
+        
+        // The content comes direct from the backing store
+        assertSame(returnedReader, sourceContent);
     }
     
+    
+    @Test
+    public void getReaderForItemMissingFromCacheWillRetryAndCanSucceed()
+    {
+        ContentReader sourceContent = mock(ContentReader.class);
+        ContentReader cachedContent = mock(ContentReader.class);
+        when(cache.getReader("url")).
+            thenThrow(new CacheMissException("url")).
+            thenReturn(cachedContent);
+        when(backingStore.getReader("url")).thenReturn(sourceContent);
+        when(cache.put("url", sourceContent)).thenReturn(true);
+        
+        ContentReader returnedReader = cachingStore.getReader("url");
+        
+        assertSame(returnedReader, cachedContent);
+    }
+    
+    
     @Test
     public void getReaderForItemMissingFromCacheButNoContentToCache()
     {
@@ -144,7 +171,35 @@ public class CachingContentStoreTest
         
         verify(backingStore).getWriter(ctx);
     }
-
+    
+    
+    @Test(expected=RuntimeException.class)
+    // Check that exceptions raised by the backing store's putContent(ContentReader)
+    // aren't swallowed and can therefore cause the transaction to fail.
+    public void exceptionRaisedWhenCopyingTempToBackingStoreIsPropogatedCorrectly() 
+        throws ContentIOException, IOException
+    {
+        cachingStore = new CachingContentStore(backingStore, cache, true);
+        ContentContext ctx = ContentContext.NULL_CONTEXT;
+        ContentWriter bsWriter = mock(ContentWriter.class);
+        when(backingStore.getWriter(ctx)).thenReturn(bsWriter);
+        when(bsWriter.getContentUrl()).thenReturn("url");
+        ContentWriter cacheWriter = mock(ContentWriter.class);
+        when(cache.getWriter("url")).thenReturn(cacheWriter);
+        ContentReader readerFromCacheWriter = mock(ContentReader.class);
+        when(cacheWriter.getReader()).thenReturn(readerFromCacheWriter);
+        
+        doThrow(new RuntimeException()).when(bsWriter).putContent(any(ContentReader.class));
+        
+        cachingStore.getWriter(ctx);
+        
+        // Get the stream listener and trigger it
+        ArgumentCaptor<ContentStreamListener> arg = ArgumentCaptor.forClass(ContentStreamListener.class);
+        verify(cacheWriter).addListener(arg.capture());
+        // Simulate a stream close
+        arg.getValue().contentStreamClosed();
+    }
+    
     
     @Test
     public void encodingAttrsCopiedToBackingStoreWriter()

source/java/org/alfresco/repo/content/caching/ContentCacheImpl.java

@@ -107,6 +107,12 @@ public class ContentCacheImpl implements ContentCache
         if (memoryStore.contains(url))
         {
             String path = memoryStore.get(url);
+            
+            // Getting the path for a URL from the memoryStore will reset the timeToIdle for
+            // that URL. It is important to perform a reverse lookup as well to ensure that the
+            // cache file path to URL mapping is also kept in the cache.
+            memoryStore.get(Key.forCacheFile(path));
+            
             File cacheFile = new File(path);
             if (cacheFile.exists())
             {

source/java/org/alfresco/repo/content/caching/ContentCacheImplTest.java

@@ -0,0 +1,228 @@
+/*
+ * Copyright (C) 2005-2011 Alfresco Software Limited.
+ *
+ * This file is part of Alfresco
+ *
+ * Alfresco is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Alfresco is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.alfresco.repo.content.caching;
+
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.io.File;
+
+import org.alfresco.repo.cache.SimpleCache;
+import org.alfresco.repo.content.filestore.FileContentReader;
+import org.alfresco.repo.content.filestore.FileContentWriter;
+import org.alfresco.service.cmr.repository.ContentReader;
+import org.alfresco.util.TempFileProvider;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.runners.MockitoJUnitRunner;
+
+/**
+ * Tests for the ContentCacheImpl class.
+ * 
+ * @author Matt Ward
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class ContentCacheImplTest
+{
+    private ContentCacheImpl contentCache;
+    private @Mock SimpleCache<Key, String> lookupTable;
+    
+
+    @Before
+    public void setUp() throws Exception
+    {
+        contentCache = new ContentCacheImpl();
+        contentCache.setMemoryStore(lookupTable);
+        contentCache.setCacheRoot(TempFileProvider.getTempDir());
+    }
+    
+    
+    @Test
+    public void canGetReaderForItemInCacheHavingLiveFile()
+    {
+        final String url = "store://content/url.bin";
+        Mockito.when(lookupTable.contains(Key.forUrl(url))).thenReturn(true);
+        final String path = tempfile().getAbsolutePath();
+        Mockito.when(lookupTable.get(Key.forUrl(url))).thenReturn(path);
+        
+        FileContentReader reader = (FileContentReader) contentCache.getReader(url);
+        
+        assertEquals("Reader should have correct URL", url, reader.getContentUrl());
+        assertEquals("Reader should be for correct cached content file", path, reader.getFile().getAbsolutePath());
+        // Important the get(path) was called, so that the timeToIdle is reset
+        // for the 'reverse lookup' as well as the URL to path mapping.
+        Mockito.verify(lookupTable).get(Key.forCacheFile(path));
+    }
+
+    
+    @Test(expected=CacheMissException.class)
+    public void getReaderForItemInCacheButMissingContentFile()
+    {
+        final String url = "store://content/url.bin";
+        Mockito.when(lookupTable.contains(Key.forUrl(url))).thenReturn(true);
+        final String path = "/no/content/file/at/this/path.bin";
+        Mockito.when(lookupTable.get(Key.forUrl(url))).thenReturn(path);
+          
+        try
+        {
+            contentCache.getReader(url);
+        }
+        finally
+        {
+            // Important the get(path) was called, so that the timeToIdle is reset
+            // for the 'reverse lookup' as well as the URL to path mapping.
+            Mockito.verify(lookupTable).get(Key.forCacheFile(path));
+        }
+    }
+    
+    
+    private File tempfile()
+    {
+        File file = TempFileProvider.createTempFile("cached-content", ".bin");
+        file.deleteOnExit();
+        return file;
+    }
+
+
+    @Test(expected=CacheMissException.class)
+    public void getReaderWhenItemNotInCache()
+    {
+        final String url = "store://content/url.bin";
+        Mockito.when(lookupTable.contains(Key.forUrl(url))).thenReturn(false);
+        
+        contentCache.getReader(url);
+    }
+    
+    
+    @Test
+    public void contains()
+    {
+        final String url = "store://content/url.bin";
+        
+        Mockito.when(lookupTable.contains(Key.forUrl(url))).thenReturn(true);
+        assertTrue(contentCache.contains(Key.forUrl(url)));
+        assertTrue(contentCache.contains(url));
+        
+        Mockito.when(lookupTable.contains(Key.forUrl(url))).thenReturn(false);
+        assertFalse(contentCache.contains(Key.forUrl(url)));
+        assertFalse(contentCache.contains(url));
+    }
+    
+    
+    @Test
+    public void putIntoLookup()
+    {
+        final Key key = Key.forUrl("store://some/url");
+        final String value = "/some/path";
+        
+        contentCache.putIntoLookup(key, value);
+        
+        Mockito.verify(lookupTable).put(key, value);
+    }
+    
+    
+    @Test
+    public void getCacheFilePath()
+    {
+        final String url = "store://some/url.bin";
+        final String expectedPath = "/some/cache/file/path";
+        Mockito.when(lookupTable.get(Key.forUrl(url))).thenReturn(expectedPath);
+        
+        String path = contentCache.getCacheFilePath(url);
+        
+        assertEquals("Paths must match", expectedPath, path);
+    }
+    
+    
+    @Test
+    public void getContentUrl()
+    {
+        final File cacheFile = new File("/some/path");
+        final String expectedUrl = "store://some/url";
+        Mockito.when(lookupTable.get(Key.forCacheFile(cacheFile))).thenReturn(expectedUrl);
+        
+        String url = contentCache.getContentUrl(cacheFile);
+        
+        assertEquals("Content URLs should match", expectedUrl, url);
+    }
+    
+    
+    @Test
+    public void putForZeroLengthFile()
+    {
+        ContentReader contentReader = Mockito.mock(ContentReader.class);
+        Mockito.when(contentReader.getSize()).thenReturn(0L);
+        
+        boolean putResult = contentCache.put("", contentReader);
+        
+        assertFalse("Zero length files should not be cached", putResult);
+    }
+    
+    
+    @Test
+    public void putForNonEmptyFile()
+    {
+        ContentReader contentReader = Mockito.mock(ContentReader.class);
+        Mockito.when(contentReader.getSize()).thenReturn(999000L);
+        final String url = "store://some/url.bin";
+        boolean putResult = contentCache.put(url, contentReader);
+        
+        assertTrue("Non-empty files should be cached", putResult);
+        ArgumentCaptor<File> cacheFileArg = ArgumentCaptor.forClass(File.class);
+        Mockito.verify(contentReader).getContent(cacheFileArg.capture());
+        // Check cached item is recorded properly in ehcache
+        Mockito.verify(lookupTable).put(Key.forUrl(url), cacheFileArg.getValue().getAbsolutePath());
+        Mockito.verify(lookupTable).put(Key.forCacheFile(cacheFileArg.getValue().getAbsolutePath()), url);
+    }
+    
+    
+    @Test
+    public void remove()
+    {
+        final String url = "store://some/url.bin";
+        final String path = "/some/path";
+        Mockito.when(lookupTable.get(Key.forUrl(url))).thenReturn(path);
+        
+        contentCache.remove(url);
+        
+        Mockito.verify(lookupTable).remove(Key.forUrl(url));
+        Mockito.verify(lookupTable).remove(Key.forCacheFile(path));
+    }
+    
+    
+    @Test
+    public void getWriter()
+    {
+        final String url = "store://some/url.bin";
+        
+        FileContentWriter writer = (FileContentWriter) contentCache.getWriter(url);
+        writer.putContent("Some test content for " + getClass().getName());
+        
+        assertEquals(url, writer.getContentUrl());
+        // Check cached item is recorded properly in ehcache
+        Mockito.verify(lookupTable).put(Key.forUrl(url), writer.getFile().getAbsolutePath());
+        Mockito.verify(lookupTable).put(Key.forCacheFile(writer.getFile().getAbsolutePath()), url);
+    }
+}

source/java/org/alfresco/repo/content/caching/cleanup/CachedContentCleaner.java

@@ -23,6 +23,7 @@ import java.io.File;
 import org.alfresco.repo.content.caching.CacheFileProps;
 import org.alfresco.repo.content.caching.ContentCacheImpl;
 import org.alfresco.repo.content.caching.FileHandler;
+import org.alfresco.util.Deleter;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.springframework.beans.factory.annotation.Required;
@@ -130,8 +131,11 @@ public class CachedContentCleaner implements FileHandler
         CacheFileProps props = new CacheFileProps(cacheFile);
         props.delete();
         cacheFile.delete();
+        Deleter.deleteEmptyParents(cacheFile, cache.getCacheRoot());
     }
 
+    
+
     @Required
     public void setCache(ContentCacheImpl cache)
     {

source/java/org/alfresco/repo/content/caching/cleanup/CachedContentCleanupJobTest.java

@@ -24,6 +24,7 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 import java.io.File;
+import java.io.FileNotFoundException;
 import java.io.PrintWriter;
 
 import org.alfresco.repo.content.caching.CacheFileProps;
@@ -93,6 +94,21 @@ public class CachedContentCleanupJobTest
         }
     }
 
+    @Test
+    public void emptyParentDirectoriesAreDeleted() throws FileNotFoundException
+    {
+        cleaner.setMaxDeleteWatchCount(0);
+        File file = new File(cacheRoot, "243235984/a/b/c/d.bin");
+        file.getParentFile().mkdirs();
+        PrintWriter writer = new PrintWriter(file);
+        writer.println("Content for emptyParentDirectoriesAreDeleted");
+        writer.close();
+        assertTrue("Directory should exist", new File(cacheRoot, "243235984/a/b/c").exists());
+        
+        cleaner.handle(file);
+        
+        assertFalse("Directory should have been deleted", new File(cacheRoot, "243235984").exists());
+    }
     
     @Test
     public void markedFilesHaveDeletionDeferredUntilCorrectPassOfCleaner()

source/java/org/alfresco/repo/content/caching/test/SlowContentStoreTest.java

@@ -59,7 +59,7 @@ public class SlowContentStoreTest
         TimedStoreReader storeReader = new TimedStoreReader();
         storeReader.execute();
         assertTrue("First read should take a while", storeReader.timeTakenMillis() > 1000);
-        logger.info(String.format("First read took %ds", storeReader.timeTakenMillis()));
+        logger.debug(String.format("First read took %ds", storeReader.timeTakenMillis()));
         // The content came from the slow backing store...
         assertEquals("This is the content for my slow ReadableByteChannel", storeReader.content);
         
@@ -69,7 +69,7 @@ public class SlowContentStoreTest
             storeReader = new TimedStoreReader();
             storeReader.execute();
             assertTrue("Subsequent reads should be fast", storeReader.timeTakenMillis() < 100);
-            logger.info(String.format("Cache read took %ds", storeReader.timeTakenMillis()));
+            logger.debug(String.format("Cache read took %ds", storeReader.timeTakenMillis()));
             // The content came from the slow backing store, but was cached...
             assertEquals("This is the content for my slow ReadableByteChannel", storeReader.content);
         }
@@ -89,7 +89,7 @@ public class SlowContentStoreTest
         TimedStoreReader storeReader = new TimedStoreReader();
         storeReader.execute();
         assertTrue("First read should be fast", storeReader.timeTakenMillis() < 100);
-        logger.info(String.format("First read took %ds", storeReader.timeTakenMillis()));
+        logger.debug(String.format("First read took %ds", storeReader.timeTakenMillis()));
         assertEquals("Content written from " + getClass().getSimpleName(), storeReader.content);
         
         // Subsequent reads will also hit the cache
@@ -98,7 +98,7 @@ public class SlowContentStoreTest
             storeReader = new TimedStoreReader();
             storeReader.execute();
             assertTrue("Subsequent reads should be fast", storeReader.timeTakenMillis() < 100);
-            logger.info(String.format("Cache read took %ds", storeReader.timeTakenMillis()));
+            logger.debug(String.format("Cache read took %ds", storeReader.timeTakenMillis()));
             // The original cached content, still cached...
             assertEquals("Content written from " + getClass().getSimpleName(), storeReader.content);
         }
@@ -113,7 +113,7 @@ public class SlowContentStoreTest
         protected void doExecute()
         {
             content = cachingStore.getReader("any-url").getContentString();
-            logger.info("Read content: " + content);
+            logger.debug("Read content: " + content);
         }   
     }