REPO-1357: HotFix: MNT-16889: With encryption enabled alf_content_url_encryption not set using ContentService getWriter(null,,false)

- Fixed creation of the content URL so that the key is not lost.
   - Added a test.


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/BRANCHES/DEV/5.2.N/root@131243 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

source/java/org/alfresco/repo/domain/contentdata/AbstractContentDataDAOImpl.java

@@ -1,28 +1,28 @@
-/*
+/*
- * #%L
+ * #%L
- * Alfresco Repository
+ * Alfresco Repository
- * %%
+ * %%
- * Copyright (C) 2005 - 2016 Alfresco Software Limited
+ * Copyright (C) 2005 - 2016 Alfresco Software Limited
- * %%
+ * %%
- * This file is part of the Alfresco software. 
+ * This file is part of the Alfresco software. 
- * If the software was purchased under a paid Alfresco license, the terms of 
+ * If the software was purchased under a paid Alfresco license, the terms of 
- * the paid license agreement will prevail.  Otherwise, the software is 
+ * the paid license agreement will prevail.  Otherwise, the software is 
- * provided under the following open source license terms:
+ * provided under the following open source license terms:
- * 
+ * 
- * Alfresco is free software: you can redistribute it and/or modify
+ * 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
+ * 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
+ * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
+ * (at your option) any later version.
- * 
+ * 
- * Alfresco is distributed in the hope that it will be useful,
+ * Alfresco is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Lesser General Public License for more details.
+ * GNU Lesser General Public License for more details.
- * 
+ * 
- * You should have received a copy of the GNU Lesser General Public License
+ * You should have received a copy of the GNU Lesser General Public License
- * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
+ * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
- * #L%
+ * #L%
- */
+ */
 package org.alfresco.repo.domain.contentdata;
 
 import java.io.Serializable;
@@ -197,8 +197,9 @@ public abstract class AbstractContentDataDAOImpl implements ContentDataDAO
     /**
      * Internally update a URL or create a new one if it does not exist
      */
-    private void updateContentUrl(ContentUrlEntity contentUrl)
+    private boolean updateContentUrl(ContentUrlEntity contentUrl)
     {
+        int result = 0;
         if (contentUrl == null)
         {
             throw new IllegalArgumentException("Cannot look up ContentData by null ID.");
@@ -206,13 +207,14 @@ public abstract class AbstractContentDataDAOImpl implements ContentDataDAO
         Pair<Long, ContentUrlEntity> pair = contentUrlCache.getByValue(contentUrl);
         if(pair != null)
         {
-            contentUrlCache.updateValue(pair.getFirst(), contentUrl);
+            result = contentUrlCache.updateValue(pair.getFirst(), contentUrl);
         }
         else
         {
             pair = contentUrlCache.getOrCreateByValue(contentUrl);
-            contentUrlCache.updateValue(pair.getFirst(), contentUrl);
+            result = contentUrlCache.updateValue(pair.getFirst(), contentUrl);
         }
+        return result == 1 ? true : false;
     }
 
     @Override
@@ -558,24 +560,13 @@ public abstract class AbstractContentDataDAOImpl implements ContentDataDAO
     @Override
     public boolean updateContentUrlKey(String contentUrl, ContentUrlKeyEntity contentUrlKey)
     {
-        boolean success = true;
-
         ContentUrlEntity existing = getContentUrl(contentUrl);
-        if(existing != null)
+        if (existing == null)
         {
-            ContentUrlEntity entity = ContentUrlEntity.setContentUrlKey(existing, contentUrlKey);
+            existing = getOrCreateContentUrl(contentUrl, contentUrlKey.getUnencryptedFileSize());
-            updateContentUrl(entity);
         }
-        else
+        ContentUrlEntity entity = ContentUrlEntity.setContentUrlKey(existing, contentUrlKey);
-        {
+        return updateContentUrl(entity);
-            if (logger.isDebugEnabled())
-            {
-                logger.debug("No content url, not updating symmetric key");
-            }
-            success = false;
-        }
-
-        return success;
     }
 
     @Override
@@ -613,6 +604,19 @@ public abstract class AbstractContentDataDAOImpl implements ContentDataDAO
         return contentUrlEntity;
     }
 
+    @Override
+    public ContentUrlEntity getOrCreateContentUrl(String contentUrl, long size)
+    {
+        ContentUrlEntity contentUrlEntity = new ContentUrlEntity();
+        contentUrlEntity.setContentUrl(contentUrl);
+        contentUrlEntity.setSize(size);
+        Pair<Long, ContentUrlEntity> pair = contentUrlCache.getOrCreateByValue(contentUrlEntity);
+        Long newContentUrlId = pair.getFirst();
+        contentUrlEntity.setId(newContentUrlId);
+        // Done
+        return contentUrlEntity;
+    }
+
     /**
      * @param contentUrl    the content URL to create or search for
      */

source/java/org/alfresco/repo/domain/contentdata/ContentDataDAO.java

@@ -1,28 +1,28 @@
-/*
+/*
- * #%L
+ * #%L
- * Alfresco Repository
+ * Alfresco Repository
- * %%
+ * %%
- * Copyright (C) 2005 - 2016 Alfresco Software Limited
+ * Copyright (C) 2005 - 2016 Alfresco Software Limited
- * %%
+ * %%
- * This file is part of the Alfresco software. 
+ * This file is part of the Alfresco software. 
- * If the software was purchased under a paid Alfresco license, the terms of 
+ * If the software was purchased under a paid Alfresco license, the terms of 
- * the paid license agreement will prevail.  Otherwise, the software is 
+ * the paid license agreement will prevail.  Otherwise, the software is 
- * provided under the following open source license terms:
+ * provided under the following open source license terms:
- * 
+ * 
- * Alfresco is free software: you can redistribute it and/or modify
+ * 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
+ * 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
+ * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
+ * (at your option) any later version.
- * 
+ * 
- * Alfresco is distributed in the hope that it will be useful,
+ * Alfresco is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Lesser General Public License for more details.
+ * GNU Lesser General Public License for more details.
- * 
+ * 
- * You should have received a copy of the GNU Lesser General Public License
+ * You should have received a copy of the GNU Lesser General Public License
- * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
+ * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
- * #L%
+ * #L%
- */
+ */
 package org.alfresco.repo.domain.contentdata;
 
 import java.util.Date;
@@ -167,6 +167,13 @@ public interface ContentDataDAO
      */
     ContentUrlEntity getOrCreateContentUrl(String contentUrl);
 
+    /**
+     * Get a content URL or create one if it does not exist
+     *
+     * @since 5.1
+     */
+    ContentUrlEntity getOrCreateContentUrl(String contentUrl, long size);
+
     /**
      * Updates the content key for the given content url
      * 

source/test-java/org/alfresco/repo/domain/contentdata/ContentDataDAOTest.java

@@ -35,7 +35,7 @@ import org.alfresco.repo.content.ContentContext;
 import org.alfresco.repo.content.ContentStore;
 import org.alfresco.repo.content.MimetypeMap;
 import org.alfresco.repo.content.filestore.FileContentStore;
-import org.alfresco.repo.domain.contentdata.ContentDataDAO.ContentUrlHandler;
+import org.alfresco.repo.domain.contentdata.ContentDataDAO.ContentUrlHandler;
 import org.alfresco.repo.transaction.RetryingTransactionHelper;
 import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback;
 import org.alfresco.service.ServiceRegistry;
@@ -48,7 +48,7 @@ import org.alfresco.util.Pair;
 import org.alfresco.util.TempFileProvider;
 import org.junit.experimental.categories.Category;
 import org.springframework.context.ConfigurableApplicationContext;
-import org.springframework.dao.DataIntegrityViolationException;
+import org.springframework.dao.DataIntegrityViolationException;
 
 /**
  * @see ContentDataDAO
@@ -90,32 +90,32 @@ public class ContentDataDAOTest extends TestCase
         return txnHelper.doInTransaction(callback, false, false);
     }
     
-    private Pair<Long, ContentData> update(final Long id, final ContentData contentData)
+    private Pair<Long, ContentData> update(final Long id, final ContentData contentData)
-    {
+    {
-        RetryingTransactionCallback<Pair<Long, ContentData>> callback = new RetryingTransactionCallback<Pair<Long, ContentData>>()
+        RetryingTransactionCallback<Pair<Long, ContentData>> callback = new RetryingTransactionCallback<Pair<Long, ContentData>>()
-        {
+        {
-            public Pair<Long, ContentData> execute() throws Throwable
+            public Pair<Long, ContentData> execute() throws Throwable
-            {
+            {
-                contentDataDAO.updateContentData(id, contentData);
+                contentDataDAO.updateContentData(id, contentData);
-                return new Pair<Long, ContentData>(id, contentData);
+                return new Pair<Long, ContentData>(id, contentData);
-            }
+            }
-        };
+        };
-        return txnHelper.doInTransaction(callback, false, false);
+        return txnHelper.doInTransaction(callback, false, false);
-    }
+    }
-    
+    
-    private void delete(final Long id)
+    private void delete(final Long id)
-    {
+    {
-        RetryingTransactionCallback<Void> callback = new RetryingTransactionCallback<Void>()
+        RetryingTransactionCallback<Void> callback = new RetryingTransactionCallback<Void>()
-        {
+        {
-            public Void execute() throws Throwable
+            public Void execute() throws Throwable
-            {
+            {
-                contentDataDAO.deleteContentData(id);
+                contentDataDAO.deleteContentData(id);
-                return null;
+                return null;
-            }
+            }
-        };
+        };
-        txnHelper.doInTransaction(callback, false, false);
+        txnHelper.doInTransaction(callback, false, false);
-    }
+    }
-    
+    
     /**
      * Retrieves and checks the ContentData for equality
      */
@@ -149,15 +149,15 @@ public class ContentDataDAOTest extends TestCase
     
     public void testGetWithInvalidId()
     {
-        try
+        try
-        {
+        {
-            contentDataDAO.getContentData(-1L);
+            contentDataDAO.getContentData(-1L);
-            fail("Invalid ContentData IDs must generate DataIntegrityViolationException.");
+            fail("Invalid ContentData IDs must generate DataIntegrityViolationException.");
-        }
+        }
-        catch (DataIntegrityViolationException e)
+        catch (DataIntegrityViolationException e)
-        {
+        {
-            // Expected
+            // Expected
-        }
+        }
     }
     
     /**
@@ -203,27 +203,27 @@ public class ContentDataDAOTest extends TestCase
         getAndCheck(resultPairLower.getFirst(), contentDataLower);
     }
     
-    public void testUpdate() throws Exception
+    public void testUpdate() throws Exception
-    {
+    {
-        ContentData contentData = getContentData();
+        ContentData contentData = getContentData();
-        Pair<Long, ContentData> resultPair = create(contentData);
+        Pair<Long, ContentData> resultPair = create(contentData);
-        Long id = resultPair.getFirst();
+        Long id = resultPair.getFirst();
-        // Update
+        // Update
-        contentData = ContentData.setMimetype(contentData, MimetypeMap.MIMETYPE_HTML);
+        contentData = ContentData.setMimetype(contentData, MimetypeMap.MIMETYPE_HTML);
-        contentData = ContentData.setEncoding(contentData, "UTF-16");
+        contentData = ContentData.setEncoding(contentData, "UTF-16");
-        // Don't update the content itself
+        // Don't update the content itself
-        update(id, contentData);
+        update(id, contentData);
-        // Check
+        // Check
-        getAndCheck(id, contentData);
+        getAndCheck(id, contentData);
-    }
+    }
-    
+    
     public void testDelete() throws Exception
     {
         ContentData contentData = getContentData();
         
         Pair<Long, ContentData> resultPair = create(contentData);
         getAndCheck(resultPair.getFirst(), contentData);
-        delete(resultPair.getFirst());
+        delete(resultPair.getFirst());
         try
         {
             getAndCheck(resultPair.getFirst(), contentData);
@@ -245,81 +245,91 @@ public class ContentDataDAOTest extends TestCase
         contentUrlEntity = contentDataDAO.getContentUrl(contentUrlEntity.getContentUrl());
         assertNotNull(contentUrlEntity);
         assertNotNull(contentDataDAO.getContentUrl(contentUrlEntity.getId()));
+
+        // test with size
+        long size = 100l;
+        String url = "store://" + GUID.generate();
+        contentUrlEntity = contentDataDAO.getOrCreateContentUrl(url, size);
+        contentUrlEntity = contentDataDAO.getContentUrl(contentUrlEntity.getContentUrl());
+        assertNotNull(contentUrlEntity);
+        assertNotNull(contentDataDAO.getContentUrl(contentUrlEntity.getId()));
+        assertEquals("The size does not match.", size, contentUrlEntity.getSize());
+        assertEquals("The content URL does not match.", url, contentUrlEntity.getContentUrl());
+    }
+    
+    /**
+     * Check that orphaned content can be re-instated.
+     */
+    public void testReinstate_ALF3867()
+    {
+        ContentData contentData = getContentData();
+        Pair<Long, ContentData> resultPair = create(contentData);
+        getAndCheck(resultPair.getFirst(), contentData);
+        delete(resultPair.getFirst());
+        // Now create a ContentData with the same URL
+        create(contentData);
+    }
+    
+    public void testContentUrl_FetchingOrphansNoLimit() throws Exception
+    {
+        ContentData contentData = getContentData();
+        Pair<Long, ContentData> resultPair = create(contentData);
+        getAndCheck(resultPair.getFirst(), contentData);
+        delete(resultPair.getFirst());
+        // The content URL is orphaned
+        final String contentUrlOrphaned = contentData.getContentUrl();
+        final boolean[] found = new boolean[] {false}; 
+        
+        // Iterate over all orphaned content URLs and ensure that we hit the one we just orphaned
+        ContentUrlHandler handler = new ContentUrlHandler()
+        {
+            public void handle(Long id, String contentUrl, Long orphanTime)
+            {
+                // Check
+                if (id == null || contentUrl == null || orphanTime == null)
+                {
+                    fail("Invalid orphan data returned to handler: " + id + "-" + contentUrl + "-" + orphanTime);
+                }
+                // Did we get the one we wanted?
+                if (contentUrl.equals(contentUrlOrphaned))
+                {
+                    found[0] = true;
+                }
+            }
+        };
+        contentDataDAO.getContentUrlsOrphaned(handler, Long.MAX_VALUE, Integer.MAX_VALUE);
+        assertTrue("Newly-orphaned content URL not found", found[0]);
+    }
+    
+    public void testContentUrl_FetchingOrphansWithLimit() throws Exception
+    {
+        // Orphan some content
+        for (int i = 0; i < 5; i++)
+        {
+            ContentData contentData = getContentData();
+            Pair<Long, ContentData> resultPair = create(contentData);
+            getAndCheck(resultPair.getFirst(), contentData);
+            delete(resultPair.getFirst());
+        }
+        final int[] count = new int[] {0}; 
+        
+        // Iterate over all orphaned content URLs and ensure that we hit the one we just orphaned
+        ContentUrlHandler handler = new ContentUrlHandler()
+        {
+            public void handle(Long id, String contentUrl, Long orphanTime)
+            {
+                // Check
+                if (id == null || contentUrl == null || orphanTime == null)
+                {
+                    fail("Invalid orphan data returned to handler: " + id + "-" + contentUrl + "-" + orphanTime);
+                }
+                count[0]++;
+            }
+        };
+        contentDataDAO.getContentUrlsOrphaned(handler, Long.MAX_VALUE, 5);
+        assertEquals("Expected exactly 5 results callbacks", 5, count[0]);
     }
     
-    /**
-     * Check that orphaned content can be re-instated.
-     */
-    public void testReinstate_ALF3867()
-    {
-        ContentData contentData = getContentData();
-        Pair<Long, ContentData> resultPair = create(contentData);
-        getAndCheck(resultPair.getFirst(), contentData);
-        delete(resultPair.getFirst());
-        // Now create a ContentData with the same URL
-        create(contentData);
-    }
-    
-    public void testContentUrl_FetchingOrphansNoLimit() throws Exception
-    {
-        ContentData contentData = getContentData();
-        Pair<Long, ContentData> resultPair = create(contentData);
-        getAndCheck(resultPair.getFirst(), contentData);
-        delete(resultPair.getFirst());
-        // The content URL is orphaned
-        final String contentUrlOrphaned = contentData.getContentUrl();
-        final boolean[] found = new boolean[] {false}; 
-        
-        // Iterate over all orphaned content URLs and ensure that we hit the one we just orphaned
-        ContentUrlHandler handler = new ContentUrlHandler()
-        {
-            public void handle(Long id, String contentUrl, Long orphanTime)
-            {
-                // Check
-                if (id == null || contentUrl == null || orphanTime == null)
-                {
-                    fail("Invalid orphan data returned to handler: " + id + "-" + contentUrl + "-" + orphanTime);
-                }
-                // Did we get the one we wanted?
-                if (contentUrl.equals(contentUrlOrphaned))
-                {
-                    found[0] = true;
-                }
-            }
-        };
-        contentDataDAO.getContentUrlsOrphaned(handler, Long.MAX_VALUE, Integer.MAX_VALUE);
-        assertTrue("Newly-orphaned content URL not found", found[0]);
-    }
-    
-    public void testContentUrl_FetchingOrphansWithLimit() throws Exception
-    {
-        // Orphan some content
-        for (int i = 0; i < 5; i++)
-        {
-            ContentData contentData = getContentData();
-            Pair<Long, ContentData> resultPair = create(contentData);
-            getAndCheck(resultPair.getFirst(), contentData);
-            delete(resultPair.getFirst());
-        }
-        final int[] count = new int[] {0}; 
-        
-        // Iterate over all orphaned content URLs and ensure that we hit the one we just orphaned
-        ContentUrlHandler handler = new ContentUrlHandler()
-        {
-            public void handle(Long id, String contentUrl, Long orphanTime)
-            {
-                // Check
-                if (id == null || contentUrl == null || orphanTime == null)
-                {
-                    fail("Invalid orphan data returned to handler: " + id + "-" + contentUrl + "-" + orphanTime);
-                }
-                count[0]++;
-            }
-        };
-        contentDataDAO.getContentUrlsOrphaned(handler, Long.MAX_VALUE, 5);
-        assertEquals("Expected exactly 5 results callbacks", 5, count[0]);
-    }
-    
     private static final String[] MIMETYPES = new String[]
                                                          {
                                                             MimetypeMap.MIMETYPE_ACP,