From 026ccf29f2019a4baf15eda2dd021f33e2c7d908 Mon Sep 17 00:00:00 2001
From: buczekp <buczekp>
Date: Mon, 18 Jul 2011 07:57:09 +0000
Subject: [PATCH] [LMS-2366] fixed broken deletion of attachments

SVN: 22166
---
 .../generic/server/dataaccess/IAttachmentDAO.java     |  2 +-
 .../generic/server/dataaccess/db/AttachmentDAO.java   |  7 ++++---
 .../generic/shared/dto/AttachmentHolderPE.java        | 11 +++++++++--
 .../server/dataaccess/db/AttachmentDAOTest.java       |  3 +--
 4 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/IAttachmentDAO.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/IAttachmentDAO.java
index b357a7fc089..d6dbbc0b023 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/IAttachmentDAO.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/IAttachmentDAO.java
@@ -75,7 +75,7 @@ public interface IAttachmentDAO extends IGenericDAO<AttachmentPE>
      * Deletes all attachment versions with specified <var>fileName</var> and <var>owner</var>.
      * There will be no error if no such attachment exist.
      * <p>
-     * NOTE: Attachments are removed from DB - not from the owner object.
+     * NOTE: Attachments are removed from DB and from the owner object.
      * 
      * @return number of attachments deleted
      */
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/AttachmentDAO.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/AttachmentDAO.java
index 121eb92f39b..6971255d58e 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/AttachmentDAO.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/AttachmentDAO.java
@@ -44,8 +44,8 @@ final class AttachmentDAO extends AbstractGenericEntityDAO<AttachmentPE> impleme
 
     private final static String TABLE_NAME = ATTACHMENT_CLASS.getSimpleName();
 
-    private static final Logger operationLog =
-            LogFactory.getLogger(LogCategory.OPERATION, AttachmentDAO.class);
+    private static final Logger operationLog = LogFactory.getLogger(LogCategory.OPERATION,
+            AttachmentDAO.class);
 
     AttachmentDAO(final SessionFactory sessionFactory, final DatabaseInstancePE databaseInstance)
     {
@@ -191,8 +191,9 @@ final class AttachmentDAO extends AbstractGenericEntityDAO<AttachmentPE> impleme
         {
             if (fileName.equals(att.getFileName()))
             {
-                deletedRows++;
+                owner.removeAttachment(att);
                 hibernateTemplate.delete(att);
+                deletedRows++;
             }
         }
 
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/AttachmentHolderPE.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/AttachmentHolderPE.java
index 577c8a5b9a1..8c7aaa3bd87 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/AttachmentHolderPE.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/AttachmentHolderPE.java
@@ -48,8 +48,8 @@ public abstract class AttachmentHolderPE implements Serializable, IIdentifierHol
     //
     public static final char HIDDEN_EXPERIMENT_PROPERTY_PREFIX_CHARACTER = '$';
 
-    public static final String HIDDEN_EXPERIMENT_PROPERTY_PREFIX =
-            Character.toString(HIDDEN_EXPERIMENT_PROPERTY_PREFIX_CHARACTER);
+    public static final String HIDDEN_EXPERIMENT_PROPERTY_PREFIX = Character
+            .toString(HIDDEN_EXPERIMENT_PROPERTY_PREFIX_CHARACTER);
 
     public static final String HIDDEN_EXPERIMENT_PROPERTY_PREFIX2 =
             HIDDEN_EXPERIMENT_PROPERTY_PREFIX + HIDDEN_EXPERIMENT_PROPERTY_PREFIX;
@@ -127,6 +127,13 @@ public abstract class AttachmentHolderPE implements Serializable, IIdentifierHol
         getInternalAttachments().add(child);
     }
 
+    // Should be called ONLY before deletion of the attachment.
+    // Doesn't clear connection with parent - it is handled by cascade delete.
+    public void removeAttachment(final AttachmentPE child)
+    {
+        getInternalAttachments().remove(child);
+    }
+
     public final static boolean isHiddenFile(final String fileName)
     {
         return fileName.startsWith(HIDDEN_EXPERIMENT_PROPERTY_PREFIX)
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/AttachmentDAOTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/AttachmentDAOTest.java
index 27e5c35d99c..b0d6898a87a 100644
--- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/AttachmentDAOTest.java
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/AttachmentDAOTest.java
@@ -41,8 +41,7 @@ public final class AttachmentDAOTest extends AbstractDAOTest
 
     private static final String ATT_CONTENTS_TABLE = "attachment_contents";
 
-    @Test(groups = "broken")
-    // FIXME
+    @Test
     public final void testDeleteAttachment()
     {
         IAttachmentDAO attachmentDAO = daoFactory.getAttachmentDAO();
-- 
GitLab