Merged BRANCHES/DEV/THOR1 to HEAD:

30458: ALF-10100: need to be able to apply %age or abs disk space usage constraints on ${dir.cachedcontent}
   30573: ALF-9613: Add min age of files checking to cached content cleaner
   30594: ALF-10100: added more sensible default in sample config for quota size (4GB)
   30695: ALF-10391, ALF-10392: Added MBeans and improved logging for monitoring purposes.
   30850: THOR-202: CachingContentStore quota manager should reject large files
   30901: Added warn-level logging about failure to cache content item
   30951: THOR-217 - when the quota is met or exceeded, then next time the cleaner runs it must use some strategy to make some space.





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

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

@@ -19,14 +19,20 @@
 package org.alfresco.repo.content.caching.cleanup;
 
 import java.io.File;
+import java.util.Date;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import org.alfresco.repo.content.caching.CacheFileProps;
 import org.alfresco.repo.content.caching.ContentCacheImpl;
 import org.alfresco.repo.content.caching.FileHandler;
+import org.alfresco.repo.content.caching.quota.UsageTracker;
 import org.alfresco.util.Deleter;
+import org.apache.commons.io.FileUtils;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.springframework.beans.factory.annotation.Required;
+import org.springframework.context.ApplicationEventPublisher;
+import org.springframework.context.ApplicationEventPublisherAware;
 
 /**
  * Cleans up redundant cache files from the cached content file store. Once references to cache files are
@@ -34,17 +40,113 @@ import org.springframework.beans.factory.annotation.Required;
  * 
  * @author Matt Ward
  */
-public class CachedContentCleaner implements FileHandler
+public class CachedContentCleaner implements FileHandler, ApplicationEventPublisherAware
 {
     private static final Log log = LogFactory.getLog(CachedContentCleaner.class);
     private ContentCacheImpl cache;   // impl specific functionality required
+    private long minFileAgeMillis = 0;
     private Integer maxDeleteWatchCount = 1;
-    
-    public void execute()
-    {   
-        cache.processFiles(this);
+    private ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
+    private boolean running;
+    private UsageTracker usageTracker;
+    private long newDiskUsage;
+    private long numFilesSeen;
+    private long numFilesDeleted;
+    private long sizeFilesDeleted;
+    private long numFilesMarked;
+    private Date timeStarted;
+    private Date timeFinished;
+    private ApplicationEventPublisher eventPublisher;
+    private long targetReductionBytes;
+   
+    /**
+     * This method should be called after the cleaner has been fully constructed
+     * to notify interested parties that the cleaner exists.
+     */
+    public void init()
+    {
+        eventPublisher.publishEvent(new CachedContentCleanerCreatedEvent(this));
     }
     
+    public void execute()
+    {
+        execute("none specified");
+    }
+    
+    public void executeAggressive(String reason, long targetReductionBytes)
+    {
+        this.targetReductionBytes = targetReductionBytes;
+        execute(reason);
+        this.targetReductionBytes = 0;
+    }
+    
+    public void execute(String reason)
+    {
+        lock.readLock().lock();
+        try
+        {
+            if (running)
+            {
+                // Do nothing - we only want one cleaner running at a time.
+                return;
+            }
+        }
+        finally
+        {
+            lock.readLock().unlock();
+        }
+        lock.writeLock().lock();
+        try
+        {
+            if (!running)
+            {
+                if (log.isInfoEnabled())
+                {
+                    log.info("Starting cleaner, reason: " + reason);
+                }
+                running = true;
+                resetStats();
+                timeStarted = new Date();
+                cache.processFiles(this);
+                timeFinished = new Date(); 
+                
+                if (usageTracker != null)
+                {
+                    usageTracker.setCurrentUsageBytes(newDiskUsage);
+                }
+                
+                running = false;
+                if (log.isInfoEnabled())
+                {
+                    log.info("Finished, duration: " + getDurationSeconds() + "s, seen: " + numFilesSeen +
+                                ", marked: " + numFilesMarked +
+                                ", deleted: " + numFilesDeleted +
+                                " (" + String.format("%.2f", getSizeFilesDeletedMB()) + "MB, " +
+                                sizeFilesDeleted + " bytes)" +
+                                ", target: " + targetReductionBytes + " bytes");
+                }
+            }
+        }
+        finally
+        {
+            lock.writeLock().unlock();
+        }
+    }
+    
+    
+    /**
+     * 
+     */
+    private void resetStats()
+    {
+        newDiskUsage = 0;
+        numFilesSeen = 0;
+        numFilesDeleted = 0;
+        sizeFilesDeleted = 0;
+        numFilesMarked = 0;
+    }
+
+
     @Override
     public void handle(File cachedContentFile)
     {
@@ -52,37 +154,84 @@ public class CachedContentCleaner implements FileHandler
         {
             log.debug("handle file: " + cachedContentFile);
         }
+        numFilesSeen++;
+        CacheFileProps props = null;
+        boolean deleted = false;
         
-        CacheFileProps props = null; // don't load unless required        
-        String url = cache.getContentUrl(cachedContentFile);
-        if (url == null)
+        if (targetReductionBytes > 0 && sizeFilesDeleted < targetReductionBytes)
         {
-            // Not in the cache, check the properties file
-            props = new CacheFileProps(cachedContentFile);
-            props.load();
-            url = props.getContentUrl();
-        }   
+            // Aggressive clean mode, delete file straight away.
+            deleted = deleteFilesNow(cachedContentFile);
+        }
+        else
+        {
+            if (oldEnoughForCleanup(cachedContentFile))
+            {
+                if (log.isDebugEnabled())
+                {
+                    log.debug("File is older than " + minFileAgeMillis + 
+                                "ms - considering for cleanup: " + cachedContentFile);
+                }
+                props = new CacheFileProps(cachedContentFile);        
+                String url = cache.getContentUrl(cachedContentFile);
+                if (url == null)
+                {
+                    // Not in the cache, check the properties file 
+                    props.load();
+                    url = props.getContentUrl();
+                }   
+                
+                if (url == null || !cache.contains(url))
+                {
+                    // If the url is null, it might still be in the cache, but we were unable to determine it
+                    // from the reverse lookup or the properties file. Delete the file as it is most likely orphaned.
+                    // If for some reason it is still in the cache, cache.getReader(url) must re-cache it.
+                    deleted = markOrDelete(cachedContentFile, props);
+                }
+            }
+            else
+            {
+                if (log.isDebugEnabled())
+                {
+                    log.debug("File too young for cleanup - ignoring " + cachedContentFile);
+                }
+            }
+        }
         
-        if (url != null && !cache.contains(url))
+        if (!deleted)
         {
             if (props == null)
             {
                 props = new CacheFileProps(cachedContentFile);
-                props.load();
             }
-            markOrDelete(cachedContentFile, props);
+            long size = cachedContentFile.length() + props.fileSize();
+            newDiskUsage += size;
         }
-        else if (url == null)
-        {
-            // It might still be in the cache, but we were unable to determine it from the reverse lookup
-            // or the properties file. Delete the file as it is most likely orphaned. If for some reason it is
-            // still in the cache, cache.getReader(url) must re-cache it.
-            markOrDelete(cachedContentFile, props);
-        }    
     }
 
     
 
+    /**
+     * Is the file old enough to be considered for cleanup/deletion? The file must be older than minFileAgeMillis
+     * to be considered for deletion - the state of the cache and the file's associated properties file will not
+     * be examined unless the file is old enough.
+     *  
+     * @return true if the file is older than minFileAgeMillis, false otherwise.
+     */
+    private boolean oldEnoughForCleanup(File file)
+    {
+        if (minFileAgeMillis == 0)
+        {
+            return true;
+        }
+        else
+        {
+            long now = System.currentTimeMillis();
+            return (file.lastModified() < (now - minFileAgeMillis));
+        }
+    }
+
+
     /**
      * Marks a file for deletion by a future run of the CachedContentCleaner. Each time a file is observed
      * by the cleaner as being ready for deletion, the deleteWatchCount is incremented until it reaches
@@ -99,8 +248,9 @@ public class CachedContentCleaner implements FileHandler
      * 
      * @param file
      * @param props
+     * @return true if the content file was deleted, false otherwise.
      */
-    private void markOrDelete(File file, CacheFileProps props)
+    private boolean markOrDelete(File file, CacheFileProps props)
     {
         Integer deleteWatchCount = props.getDeleteWatchCount();
 
@@ -108,16 +258,30 @@ public class CachedContentCleaner implements FileHandler
         if (deleteWatchCount < 0)
             deleteWatchCount = 0;
         
+        boolean deleted = false;
+        
         if (deleteWatchCount < maxDeleteWatchCount)
         {
             deleteWatchCount++;
+            
+            if (log.isDebugEnabled())
+            {
+                log.debug("Marking file for deletion, deleteWatchCount=" + deleteWatchCount + ", file: "+ file);
+            }
             props.setDeleteWatchCount(deleteWatchCount);
             props.store();
+            numFilesMarked++;
         }
         else
         {
-            deleteFilesNow(file);
+            if (log.isDebugEnabled())
+            {
+                log.debug("Deleting cache file " + file);
+            }
+            deleted = deleteFilesNow(file);
         }
+        
+        return deleted;
     }
 
     /**
@@ -125,13 +289,22 @@ public class CachedContentCleaner implements FileHandler
      * original content URL and deletion marker information.
      *  
      * @param cacheFile Location of cached content file.
+     * @return true if the content file was deleted, false otherwise.
      */
-    private void deleteFilesNow(File cacheFile)
+    private boolean deleteFilesNow(File cacheFile)
     {
         CacheFileProps props = new CacheFileProps(cacheFile);
         props.delete();
-        cacheFile.delete();
-        Deleter.deleteEmptyParents(cacheFile, cache.getCacheRoot());
+        long fileSize = cacheFile.length();
+        boolean deleted = cacheFile.delete();
+        if (deleted)
+        {
+            numFilesDeleted++;
+            sizeFilesDeleted += fileSize;
+            Deleter.deleteEmptyParents(cacheFile, cache.getCacheRoot());
+        }
+        
+        return deleted;
     }
 
     
@@ -142,6 +315,24 @@ public class CachedContentCleaner implements FileHandler
         this.cache = cache;
     }
 
+    
+    /**
+     * Sets the minimum age of a cache file before it will be considered for deletion.
+     * @see #oldEnoughForCleanup(File)
+     * @param minFileAgeMillis
+     */
+    public void setMinFileAgeMillis(long minFileAgeMillis)
+    {
+        this.minFileAgeMillis = minFileAgeMillis;
+    }
+
+
+    /**
+     * Sets the maxDeleteWatchCount value.
+     * 
+     * @see #markOrDelete(File, CacheFileProps)
+     * @param maxDeleteWatchCount
+     */
     public void setMaxDeleteWatchCount(Integer maxDeleteWatchCount)
     {
         if (maxDeleteWatchCount < 0)
@@ -150,4 +341,86 @@ public class CachedContentCleaner implements FileHandler
         }
         this.maxDeleteWatchCount = maxDeleteWatchCount;
     }
+
+
+    /**
+     * @param usageTracker the usageTracker to set
+     */
+    public void setUsageTracker(UsageTracker usageTracker)
+    {
+        this.usageTracker = usageTracker;
+    }
+
+    public boolean isRunning()
+    {
+        lock.readLock().lock();
+        try
+        {
+            return running;
+        }
+        finally
+        {
+            lock.readLock().unlock();
+        }
+    }
+    
+    public long getNumFilesSeen()
+    {
+        return this.numFilesSeen;
+    }
+    
+    public long getNumFilesDeleted()
+    {
+        return this.numFilesDeleted;
+    }
+
+    public long getSizeFilesDeleted()
+    {
+        return this.sizeFilesDeleted;
+    }
+        
+    public double getSizeFilesDeletedMB()
+    {
+        return (double) getSizeFilesDeleted() / FileUtils.ONE_MB;
+    }
+
+    public long getNumFilesMarked()
+    {
+        return numFilesMarked;
+    }
+    
+    public Date getTimeStarted()
+    {
+        return this.timeStarted;
+    }
+
+    public Date getTimeFinished()
+    {
+        return this.timeFinished;
+    }
+    
+    public long getDurationSeconds()
+    {
+        return getDurationMillis() / 1000;
+    }
+    
+    public long getDurationMillis()
+    {
+        return timeFinished.getTime() - timeStarted.getTime();
+    }
+
+    @Override
+    public void setApplicationEventPublisher(ApplicationEventPublisher eventPublisher)
+    {
+        this.eventPublisher = eventPublisher;
+    }
+
+    /**
+     * Returns the cacheRoot that this cleaner is responsible for.
+     * @return File
+     */
+    public File getCacheRoot()
+    {
+        return cache.getCacheRoot();
+    }
 }

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

@@ -0,0 +1,44 @@
+/*
+ * 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.cleanup;
+
+import org.alfresco.repo.content.caching.CachingContentStoreEvent;
+
+/**
+ * Event fired when CachedContentCleaner instances are created.
+ * 
+ * @author Matt Ward
+ */
+public class CachedContentCleanerCreatedEvent extends CachingContentStoreEvent
+{
+    private static final long serialVersionUID = 1L;
+    
+    /**
+     * @param source
+     */
+    public CachedContentCleanerCreatedEvent(CachedContentCleaner cleaner)
+    {
+        super(cleaner);
+    }
+
+    public CachedContentCleaner getCleaner()
+    {
+        return (CachedContentCleaner) source;
+    }
+}

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

@@ -37,7 +37,7 @@ public class CachedContentCleanupJob implements Job
     {
         JobDataMap jobData = context.getJobDetail().getJobDataMap();
         CachedContentCleaner cachedContentCleaner = cachedContentCleaner(jobData);
-        cachedContentCleaner.execute();
+        cachedContentCleaner.execute("scheduled");
     }
 
     

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

@@ -25,6 +25,7 @@ import static org.junit.Assert.assertTrue;
 
 import java.io.File;
 import java.io.FileNotFoundException;
+import java.io.IOException;
 import java.io.PrintWriter;
 
 import org.alfresco.repo.content.caching.CacheFileProps;
@@ -33,7 +34,11 @@ import org.alfresco.repo.content.caching.ContentCacheImpl;
 import org.alfresco.repo.content.caching.Key;
 import org.alfresco.service.cmr.repository.ContentReader;
 import org.alfresco.util.ApplicationContextHelper;
+import org.alfresco.util.GUID;
+import org.apache.commons.io.FileUtils;
 import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Ignore;
 import org.junit.Test;
 import org.springframework.context.ApplicationContext;
 
@@ -45,25 +50,35 @@ import org.springframework.context.ApplicationContext;
 public class CachedContentCleanupJobTest
 {
     private enum UrlSource { PROPS_FILE, REVERSE_CACHE_LOOKUP, NOT_PRESENT };
-    private ApplicationContext ctx;
+    private static ApplicationContext ctx;
     private CachingContentStore cachingStore;
     private ContentCacheImpl cache;
     private File cacheRoot;
     private CachedContentCleaner cleaner;
     
-    @Before
-    public void setUp()
+    
+    @BeforeClass
+    public static void beforeClass()
     {
         String conf = "classpath:cachingstore/test-context.xml";
         String cleanerConf = "classpath:cachingstore/test-cleaner-context.xml";
         ctx = ApplicationContextHelper.getApplicationContext(new String[] { conf, cleanerConf });
-        
+    }
+    
+    
+    @Before
+    public void setUp() throws IOException
+    {
         cachingStore = (CachingContentStore) ctx.getBean("cachingContentStore");
-        
         cache = (ContentCacheImpl) ctx.getBean("contentCache");
         cacheRoot = cache.getCacheRoot();
-        
         cleaner = (CachedContentCleaner) ctx.getBean("cachedContentCleaner");
+        cleaner.setMinFileAgeMillis(0);
+        cleaner.setMaxDeleteWatchCount(0);
+
+        // Clear the cache from disk and memory
+        cache.removeAll();
+        FileUtils.cleanDirectory(cacheRoot);
     }
 
     
@@ -72,7 +87,8 @@ public class CachedContentCleanupJobTest
     {
         cleaner.setMaxDeleteWatchCount(0);
         int numFiles = 300; // Must be a multiple of number of UrlSource types being tested
-        File[] files = new File[300];
+        long totalSize = 0; // what is the total size of the sample files?
+        File[] files = new File[numFiles];
         for (int i = 0; i < numFiles; i++)
         {
             // Testing with a number of files. The cached file cleaner will be able to determine the 'original'
@@ -80,8 +96,9 @@ public class CachedContentCleanupJobTest
             // a 'reverse lookup' in the cache (i.e. cache.contains(Key.forCacheFile(...))), or there will be no
             // URL determinable for the file.
             UrlSource urlSource = UrlSource.values()[i % UrlSource.values().length];
-            File cacheFile = createCacheFile(urlSource, i);
+            File cacheFile = createCacheFile(urlSource, i, false);
             files[i] = cacheFile;
+            totalSize += cacheFile.length();
         }
 
         // Run cleaner
@@ -92,8 +109,167 @@ public class CachedContentCleanupJobTest
         {
             assertFalse("File should have been deleted: " + file, file.exists());
         }
+        
+        assertEquals("Incorrect number of deleted files", numFiles, cleaner.getNumFilesDeleted());
+        assertEquals("Incorrect total size of files deleted", totalSize, cleaner.getSizeFilesDeleted());
+    }
+    
+    
+    @Test
+    public void filesNewerThanMinFileAgeMillisAreNotDeleted() throws InterruptedException
+    {
+        final long minFileAge = 1000;
+        cleaner.setMinFileAgeMillis(minFileAge);
+        cleaner.setMaxDeleteWatchCount(0);
+        int numFiles = 10;
+        
+        File[] oldFiles = new File[numFiles];
+        for (int i = 0; i < numFiles; i++)
+        {
+            oldFiles[i] = createCacheFile(UrlSource.REVERSE_CACHE_LOOKUP, i, false);
+        }
+        
+        // Sleep to make sure 'old' files really are older than minFileAgeMillis
+        Thread.sleep(minFileAge);
+        
+        File[] newFiles = new File[numFiles];
+        long newFilesTotalSize = 0;
+        for (int i = 0; i < numFiles; i++)
+        {
+            newFiles[i] = createCacheFile(UrlSource.REVERSE_CACHE_LOOKUP, i, false);
+            newFilesTotalSize += newFiles[i].length();
+        }
+
+
+        // The cleaner must finish before any of the newFiles are older than minFileAge. If the files are too
+        // old the test will fail and it will be necessary to rethink how to test this.
+        cleaner.execute();
+        
+        // check all 'old' files deleted
+        for (File file : oldFiles)
+        {
+            assertFalse("File should have been deleted: " + file, file.exists());
+        }
+        // check all 'new' files still present
+        for (File file : newFiles)
+        {
+            assertTrue("File should not have been deleted: " + file, file.exists());
+        }
+        
+        assertEquals("Incorrect number of deleted files", newFiles.length, cleaner.getNumFilesDeleted());
+        assertEquals("Incorrect total size of files deleted", newFilesTotalSize, cleaner.getSizeFilesDeleted());
     }
 
+    @Test
+    public void aggressiveCleanReclaimsTargetSpace() throws InterruptedException
+    {
+        int numFiles = 30;
+        File[] files = new File[numFiles];
+        for (int i = 0; i < numFiles; i++)
+        {
+            // Make sure it's in the cache - all the files will be in the cache, so the
+            // cleaner won't clean any up once it has finished aggressively reclaiming space.
+            files[i] = createCacheFile(UrlSource.REVERSE_CACHE_LOOKUP, i, true);
+        }
+
+        // How much space to reclaim - seven files worth (all files are same size)
+        long fileSize = files[0].length();
+        long sevenFilesSize = 7 * fileSize;
+        
+        // We'll get it to clean seven files worth aggressively and then it will continue non-aggressively.
+        // It will delete the older files aggressively (i.e. the ones prior to the two second sleep) and
+        // then will examine the new files for potential deletion.
+        // Since some of the newer files are not in the cache, it will delete those.
+        cleaner.executeAggressive("aggressiveCleanReclaimsTargetSpace()", sevenFilesSize);
+        
+        int numDeleted = 0;
+        
+        for (File f : files)
+        {
+            if (!f.exists())
+            {
+                numDeleted++;
+            }
+        }
+        // How many were definitely deleted?
+        assertEquals("Wrong number of files deleted", 7 , numDeleted);
+        
+        // The cleaner should have recorded the correct number of deletions
+        assertEquals("Incorrect number of deleted files", 7, cleaner.getNumFilesDeleted());
+        assertEquals("Incorrect total size of files deleted", sevenFilesSize, cleaner.getSizeFilesDeleted());
+    }
+    
+    @Ignore()
+    @Test
+    public void standardCleanAfterAggressiveFinished() throws InterruptedException
+    {
+        int numFiles = 30;
+        int newerFilesIndex = 14;
+        File[] files = new File[numFiles];
+        
+        for (int i = 0; i < numFiles; i++)
+        {
+            if (i == newerFilesIndex)
+            {
+                // Files after this sleep will definitely be in 'newer' directories.
+                Thread.sleep(2000);
+            }
+                        
+            if (i >= 21 && i <= 24)
+            {
+                // 21 to 24 will be deleted after the aggressive deletions (once the cleaner has returned
+                // to normal cleaning), because they are not in the cache.
+                files[i] = createCacheFile(UrlSource.NOT_PRESENT, i, false);
+            }
+            else
+            {
+                // All other files will be in the cache
+                files[i] = createCacheFile(UrlSource.REVERSE_CACHE_LOOKUP, i, true);
+            }
+        }
+        
+        // How much space to reclaim - seven files worth (all files are same size)
+        long fileSize = files[0].length();
+        long sevenFilesSize = 7 * fileSize;
+        
+        // We'll get it to clean seven files worth aggressively and then it will continue non-aggressively.
+        // It will delete the older files aggressively (i.e. the ones prior to the two second sleep) and
+        // then will examine the new files for potential deletion.
+        // Since some of the newer files are not in the cache, it will delete those.
+        cleaner.executeAggressive("standardCleanAfterAggressiveFinished()", sevenFilesSize);
+        
+        for (int i = 0; i < numFiles; i++)
+        {
+            File f = files[i];
+            String newerOrOlder = ((i >= newerFilesIndex) ? "newer" : "older");
+            System.out.println("files[" + i + "] = " + newerOrOlder + " file, exists=" + f.exists());
+        }
+        
+        int numOlderFilesDeleted = 0;
+        for (int i = 0; i < newerFilesIndex; i++)
+        {
+            if (!files[i].exists())
+            {
+                numOlderFilesDeleted++;
+            }
+        }
+        assertEquals("Wrong number of older files deleted", 7, numOlderFilesDeleted);
+        
+        int numNewerFilesDeleted = 0;
+        for (int i = newerFilesIndex; i < numFiles; i++)
+        {
+            if (!files[i].exists())
+            {
+                numNewerFilesDeleted++;
+            }
+        }
+        assertEquals("Wrong number of newer files deleted", 4, numNewerFilesDeleted);
+        
+        // The cleaner should have recorded the correct number of deletions
+        assertEquals("Incorrect number of deleted files", 11, cleaner.getNumFilesDeleted());
+        assertEquals("Incorrect total size of files deleted", (11*fileSize), cleaner.getSizeFilesDeleted());
+    }
+    
     @Test
     public void emptyParentDirectoriesAreDeleted() throws FileNotFoundException
     {
@@ -106,7 +282,7 @@ public class CachedContentCleanupJobTest
         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());
     }
     
@@ -116,14 +292,14 @@ public class CachedContentCleanupJobTest
         // A non-advisable setting but useful for testing, maxDeleteWatchCount of zero
         // which should result in immediate deletion upon discovery of content no longer in the cache.
         cleaner.setMaxDeleteWatchCount(0);
-        File file = createCacheFile(UrlSource.NOT_PRESENT, 0);
+        File file = createCacheFile(UrlSource.NOT_PRESENT, 0, false);
         
         cleaner.handle(file);
         checkFilesDeleted(file);
         
         // Anticipated to be the most common setting: maxDeleteWatchCount of 1.
         cleaner.setMaxDeleteWatchCount(1);
-        file = createCacheFile(UrlSource.NOT_PRESENT, 0);
+        file = createCacheFile(UrlSource.NOT_PRESENT, 0, false);
         
         cleaner.handle(file);
         checkWatchCountForCacheFile(file, 1);
@@ -133,7 +309,7 @@ public class CachedContentCleanupJobTest
         
         // Check that some other arbitrary figure for maxDeleteWatchCount works correctly.
         cleaner.setMaxDeleteWatchCount(3);
-        file = createCacheFile(UrlSource.NOT_PRESENT, 0);
+        file = createCacheFile(UrlSource.NOT_PRESENT, 0, false);
         
         cleaner.handle(file);
         checkWatchCountForCacheFile(file, 1);
@@ -173,10 +349,11 @@ public class CachedContentCleanupJobTest
         
         // The SlowContentStore will always give out content when asked,
         // so asking for any content will cause something to be cached.
+        String url = makeContentUrl();
         int numFiles = 50;
         for (int i = 0; i < numFiles; i++)
         {
-            ContentReader reader = cachingStore.getReader(String.format("store://caching/store/url-%03d.bin", i));
+            ContentReader reader = cachingStore.getReader(url);
             reader.getContentString();
         }
         
@@ -184,18 +361,23 @@ public class CachedContentCleanupJobTest
         
         for (int i = 0; i < numFiles; i++)
         {
-            File cacheFile = new File(cache.getCacheFilePath(String.format("store://caching/store/url-%03d.bin", i)));
+            File cacheFile = new File(cache.getCacheFilePath(url));
             assertTrue("File should exist", cacheFile.exists());
         }
     }
     
     
-    private File createCacheFile(UrlSource urlSource, int fileNum)
+    private File createCacheFile(UrlSource urlSource, int fileNum, boolean putInCache)
     {
         File file = new File(cacheRoot, ContentCacheImpl.createNewCacheFilePath());
         file.getParentFile().mkdirs();
         writeSampleContent(file);
-        String contentUrl = String.format("protocol://some/made/up/url-%03d.bin", fileNum);
+        String contentUrl = makeContentUrl();
+        
+        if (putInCache)
+        {
+            cache.putIntoLookup(Key.forUrl(contentUrl), file.getAbsolutePath());
+        }
         
         switch(urlSource)
         {
@@ -217,12 +399,19 @@ public class CachedContentCleanupJobTest
     }
 
 
+    private String makeContentUrl()
+    {
+        return "protocol://some/made/up/url/" + GUID.generate();
+    }
+
+
     private void writeSampleContent(File file)
     {
         try
         {
             PrintWriter writer = new PrintWriter(file);
             writer.println("Content for sample file in " + getClass().getName());
+            writer.close();
         }
         catch (Throwable e)
         {