From 72df8a826e9fece4178f69b7bd65b44d4356d033 Mon Sep 17 00:00:00 2001
From: felmer <felmer>
Date: Mon, 10 Dec 2012 14:03:32 +0000
Subject: [PATCH] BIS-292: Bug reproduce, system tests written, bug fixed.

SVN: 27899
---
 .../business/bo/AbstractBusinessObject.java   |   6 +-
 .../server/dataaccess/IAttachmentDAO.java     |   4 +-
 .../server/dataaccess/db/AttachmentDAO.java   |  12 +-
 .../RegistrationTest.java                     | 103 ++++++++++++++++++
 4 files changed, 116 insertions(+), 9 deletions(-)

diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/AbstractBusinessObject.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/AbstractBusinessObject.java
index 972db8428de..22177203364 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/AbstractBusinessObject.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/AbstractBusinessObject.java
@@ -627,20 +627,20 @@ abstract class AbstractBusinessObject implements IDAOFactory
         {
             return;
         }
+        AttachmentHolderPE actualAttachmentHolder = attachmentHolder;
         final IAttachmentDAO dao = getAttachmentDAO();
         for (final AttachmentPE attachment : attachments)
         {
             try
             {
-                dao.createAttachment(attachment, attachmentHolder);
+                actualAttachmentHolder = dao.createAttachment(attachment, actualAttachmentHolder);
             } catch (final DataAccessException e)
             {
                 final String fileName = attachment.getFileName();
                 throwException(
                         e,
                         String.format("Filename '%s' for %s '%s'", fileName,
-                                attachmentHolder.getHolderName(),
-                                attachmentHolder.getIdentifier()));
+                                attachmentHolder.getHolderName(), attachmentHolder.getIdentifier()));
             }
         }
         attachments.clear();
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 d6dbbc0b023..2d04539ffa8 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
@@ -44,8 +44,10 @@ public interface IAttachmentDAO extends IGenericDAO<AttachmentPE>
      * 
      * @param attachment The property to register.
      * @param owner Owner of the attachment. Should be a persistent object.
+     * @return the actual owner object to whom the attachment is attached. This either
+     *         <code>owner</code> itself or a copy of it.
      */
-    public void createAttachment(AttachmentPE attachment, AttachmentHolderPE owner)
+    public AttachmentHolderPE createAttachment(AttachmentPE attachment, AttachmentHolderPE owner)
             throws DataAccessException;
 
     /**
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 ac76d919591..313bd361867 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
@@ -20,6 +20,7 @@ import java.util.List;
 
 import org.apache.commons.lang.StringUtils;
 import org.apache.log4j.Logger;
+import org.hibernate.Session;
 import org.hibernate.SessionFactory;
 import org.springframework.dao.DataAccessException;
 import org.springframework.orm.hibernate3.HibernateTemplate;
@@ -75,9 +76,8 @@ final class AttachmentDAO extends AbstractGenericEntityDAO<AttachmentPE> impleme
     //
 
     @Override
-    public final void createAttachment(final AttachmentPE attachment,
-            final AttachmentHolderPE ownerParam)
-            throws DataAccessException
+    public final AttachmentHolderPE createAttachment(final AttachmentPE attachment,
+            final AttachmentHolderPE ownerParam) throws DataAccessException
     {
         assert attachment != null : "Unspecified attachment";
         assert attachment.getAttachmentContent() != null : "Unspecified attachment content.";
@@ -89,9 +89,10 @@ final class AttachmentDAO extends AbstractGenericEntityDAO<AttachmentPE> impleme
         fillAttachmentData(attachment, previousAttachmentVersionOrNull);
 
         final HibernateTemplate template = getHibernateTemplate();
-        if (getSession().contains(owner) == false)
+        Session session = getSession();
+        if (session.contains(owner) == false)
         {
-            owner = (AttachmentHolderPE) getSession().merge(owner);
+            owner = (AttachmentHolderPE) session.merge(owner);
         }
         owner.addAttachment(attachment);
         validatePE(attachment);
@@ -102,6 +103,7 @@ final class AttachmentDAO extends AbstractGenericEntityDAO<AttachmentPE> impleme
         {
             operationLog.info(String.format("ADD: file attachment '%s'.", attachment));
         }
+        return owner;
     }
 
     private void fillAttachmentData(AttachmentPE attachment,
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/perform_entity_operations/RegistrationTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/perform_entity_operations/RegistrationTest.java
index c842531fc9d..d4485af9a26 100644
--- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/perform_entity_operations/RegistrationTest.java
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/perform_entity_operations/RegistrationTest.java
@@ -35,6 +35,8 @@ import ch.systemsx.cisd.openbis.generic.shared.api.v1.dto.id.metaproject.Metapro
 import ch.systemsx.cisd.openbis.generic.shared.api.v1.dto.id.sample.SampleIdentifierId;
 import ch.systemsx.cisd.openbis.generic.shared.basic.IIdentifierHolder;
 import ch.systemsx.cisd.openbis.generic.shared.basic.TechId;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Attachment;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.AttachmentWithContent;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.EntityProperty;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Experiment;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.ExternalData;
@@ -42,12 +44,17 @@ import ch.systemsx.cisd.openbis.generic.shared.basic.dto.IEntityProperty;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.ListSampleCriteria;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Material;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Metaproject;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewAttachment;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewExperiment;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewMetaproject;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewSample;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.PropertyType;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Sample;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.SampleType;
 import ch.systemsx.cisd.openbis.generic.shared.dto.builders.AtomicEntityOperationDetailsBuilder;
+import ch.systemsx.cisd.openbis.generic.shared.dto.builders.SampleUpdatesDTOBuilder;
 import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.ExperimentIdentifierFactory;
+import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SampleIdentifierFactory;
 import ch.systemsx.cisd.openbis.systemtest.SystemTestCase;
 
 /**
@@ -55,6 +62,102 @@ import ch.systemsx.cisd.openbis.systemtest.SystemTestCase;
  */
 public class RegistrationTest extends SystemTestCase
 {
+    @Test
+    public void testCreateSampleWithTwoAttachments()
+    {
+        AtomicEntityOperationDetailsBuilder builder = new AtomicEntityOperationDetailsBuilder();
+        NewSample newSample = new NewSample();
+        SampleType sampleType = new SampleType();
+        sampleType.setCode("CELL_PLATE");
+        newSample.setSampleType(sampleType);
+        newSample.setIdentifier("/TEST-SPACE/S_2_ATT");
+        NewAttachment attachment1 = new NewAttachment("a/b/1", "Title 1", "Attachment 1");
+        attachment1.setContent("hello attachment one".getBytes());
+        NewAttachment attachment2 = new NewAttachment("a/b/2", "Title 2", "Attachment 2");
+        attachment2.setContent("hello attachment two".getBytes());
+        List<NewAttachment> attachments = Arrays.asList(attachment1, attachment2);
+        newSample.setAttachments(attachments);
+        builder.sample(newSample);
+
+        etlService.performEntityOperations(systemSessionToken, builder.getDetails());
+
+        Sample sample =
+                etlService.tryGetSampleWithExperiment(systemSessionToken,
+                        SampleIdentifierFactory.parse(newSample));
+        AttachmentWithContent a1 =
+                genericServer.getSampleFileAttachment(systemSessionToken, new TechId(sample), "1",
+                        null);
+        assertEquals("hello attachment one", new String(a1.getContent()));
+    }
+
+    @Test
+    public void testCreateSampleWithOneAttachment()
+    {
+        AtomicEntityOperationDetailsBuilder builder = new AtomicEntityOperationDetailsBuilder();
+        NewSample newSample = new NewSample();
+        SampleType sampleType = new SampleType();
+        sampleType.setCode("CELL_PLATE");
+        newSample.setSampleType(sampleType);
+        newSample.setIdentifier("/TEST-SPACE/S_2_ATT");
+        NewAttachment attachment1 = new NewAttachment("a/b/1", "Title 1", "Attachment 1");
+        attachment1.setContent("hello attachment one".getBytes());
+        List<NewAttachment> attachments = Arrays.asList(attachment1);
+        newSample.setAttachments(attachments);
+        builder.sample(newSample);
+
+        etlService.performEntityOperations(systemSessionToken, builder.getDetails());
+
+        Sample sample =
+                etlService.tryGetSampleWithExperiment(systemSessionToken,
+                        SampleIdentifierFactory.parse(newSample));
+        List<Attachment> loadedAttachments =
+                commonServer.listSampleAttachments(systemSessionToken, new TechId(sample));
+        assertEquals("Title 1", loadedAttachments.get(0).getTitle());
+        assertEquals("1", loadedAttachments.get(0).getFileName());
+        assertEquals(1, loadedAttachments.size());
+        AttachmentWithContent a1 =
+                genericServer.getSampleFileAttachment(systemSessionToken, new TechId(sample), "1",
+                        null);
+        assertEquals("hello attachment one", new String(a1.getContent()));
+    }
+
+    @Test
+    public void testAddOneAttachmentToAnExistingSample()
+    {
+        AtomicEntityOperationDetailsBuilder builder = new AtomicEntityOperationDetailsBuilder();
+        SampleUpdatesDTOBuilder updateBuilder = new SampleUpdatesDTOBuilder(1);
+        NewAttachment attachment1 = new NewAttachment("a/b/1", "Title 1", "Attachment 1");
+        attachment1.setContent("hello attachment one".getBytes());
+        updateBuilder.attachment(attachment1);
+        builder.sampleUpdate(updateBuilder.get());
+
+        etlService.performEntityOperations(systemSessionToken, builder.getDetails());
+
+        AttachmentWithContent a1 =
+                genericServer.getSampleFileAttachment(systemSessionToken, new TechId(1), "1", null);
+        assertEquals("hello attachment one", new String(a1.getContent()));
+    }
+
+    @Test
+    public void testAddTwoAttachmentsToAnExistingSample()
+    {
+        AtomicEntityOperationDetailsBuilder builder = new AtomicEntityOperationDetailsBuilder();
+        SampleUpdatesDTOBuilder updateBuilder = new SampleUpdatesDTOBuilder(1);
+        NewAttachment attachment1 = new NewAttachment("a/b/1", "Title 1", "Attachment 1");
+        attachment1.setContent("hello attachment one".getBytes());
+        NewAttachment attachment2 = new NewAttachment("a/b/2", "Title 2", "Attachment 2");
+        attachment2.setContent("hello attachment two".getBytes());
+        updateBuilder.attachment(attachment1);
+        updateBuilder.attachment(attachment2);
+        builder.sampleUpdate(updateBuilder.get());
+
+        etlService.performEntityOperations(systemSessionToken, builder.getDetails());
+
+        AttachmentWithContent a1 =
+                genericServer.getSampleFileAttachment(systemSessionToken, new TechId(1), "1", null);
+        assertEquals("hello attachment one", new String(a1.getContent()));
+    }
+
     @Test
     public void testRegistrationOfMetaprojectLinkedToExperimentSampleDataSetAndMaterial()
     {
-- 
GitLab