From 615a61236f3bb4b65cbaed26adb2a31d40dad0eb Mon Sep 17 00:00:00 2001
From: juanf <juanf@ethz.ch>
Date: Fri, 10 Feb 2023 12:41:27 +0100
Subject: [PATCH] SSDM-13207 : Fixing Tests
---
.../dataaccess/db/SampleRelationshipDAO.java | 14 +++++
.../openbis/generic/shared/dto/SamplePE.java | 61 ++++++++++---------
.../cisd/openbis/generic/SamplePENoDAO.java | 46 ++++++++++++++
.../generic/server/ETLServiceTest.java | 8 ++-
.../bo/SampleGenericBusinessRulesTest.java | 18 ++++--
.../server/dataaccess/db/SampleDAOTest.java | 1 +
6 files changed, 113 insertions(+), 35 deletions(-)
create mode 100644 server-application-server/sourceTest/java/ch/systemsx/cisd/openbis/generic/SamplePENoDAO.java
diff --git a/server-application-server/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/SampleRelationshipDAO.java b/server-application-server/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/SampleRelationshipDAO.java
index 875eee2dac6..b6732c6796f 100644
--- a/server-application-server/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/SampleRelationshipDAO.java
+++ b/server-application-server/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/SampleRelationshipDAO.java
@@ -5,6 +5,7 @@ import ch.systemsx.cisd.openbis.generic.shared.basic.BasicConstant;
import ch.systemsx.cisd.openbis.generic.shared.dto.ISampleRelationshipDAO;
import ch.systemsx.cisd.openbis.generic.shared.dto.RelationshipTypePE;
import ch.systemsx.cisd.openbis.generic.shared.dto.SampleRelationshipPE;
+import ch.systemsx.cisd.openbis.generic.shared.dto.SequenceNames;
import org.hibernate.SessionFactory;
import org.hibernate.criterion.DetachedCriteria;
import org.hibernate.criterion.Restrictions;
@@ -48,6 +49,10 @@ public class SampleRelationshipDAO extends AbstractGenericEntityDAO<SampleRelati
return cast.get(0);
}
+// private long getNextId() {
+// return getNextSequenceId(SequenceNames.SAMPLE_RELATIONSHIPS_SEQUENCE);
+// }
+
//
// DAO Methods
//
@@ -58,6 +63,15 @@ public class SampleRelationshipDAO extends AbstractGenericEntityDAO<SampleRelati
for (SampleRelationshipPE sampleRelationship : sampleRelationships)
{
sampleRelationship.setRelationship(relationshipType);
+// This alternative implementations attaches the object to the session without flushing it to the database
+// // Set id so PE object can be attached to session, if the id is null an Exception is thrown.
+// if (sampleRelationship.getId() == null) {
+// sampleRelationship.setId(getNextId());
+// }
+// // Attach object to session if is not already, attaching an already attach object results in an Exception.
+// if (getHibernateTemplate().contains(sampleRelationship) == false) {
+// getHibernateTemplate().update(sampleRelationship);
+// }
getHibernateTemplate().persist(sampleRelationship);
}
}
diff --git a/server-application-server/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/SamplePE.java b/server-application-server/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/SamplePE.java
index 38c2d1a2822..a30ff671d2c 100644
--- a/server-application-server/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/SamplePE.java
+++ b/server-application-server/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/SamplePE.java
@@ -22,7 +22,6 @@ import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
import java.util.HashSet;
-import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -44,6 +43,12 @@ import javax.persistence.Version;
import javax.validation.constraints.NotNull;
import javax.validation.constraints.Pattern;
+import ch.systemsx.cisd.openbis.generic.server.CommonServiceProvider;
+import ch.systemsx.cisd.openbis.generic.server.dataaccess.ISampleDAO;
+import ch.systemsx.cisd.openbis.generic.server.dataaccess.db.SampleRelationshipDAO;
+import ch.systemsx.cisd.openbis.generic.shared.basic.BasicConstant;
+import ch.systemsx.cisd.openbis.generic.shared.basic.IIdHolder;
+import ch.systemsx.cisd.openbis.generic.shared.basic.IIdentityHolder;
import org.apache.commons.lang3.builder.EqualsBuilder;
import org.apache.commons.lang3.builder.HashCodeBuilder;
import org.apache.commons.lang3.builder.ToStringBuilder;
@@ -62,9 +67,6 @@ import ch.rinn.restrictions.Private;
import ch.systemsx.cisd.common.collection.UnmodifiableSetDecorator;
import ch.systemsx.cisd.common.reflection.ModifiedShortPrefixToStringStyle;
import ch.systemsx.cisd.openbis.generic.shared.IServer;
-import ch.systemsx.cisd.openbis.generic.shared.basic.BasicConstant;
-import ch.systemsx.cisd.openbis.generic.shared.basic.IIdHolder;
-import ch.systemsx.cisd.openbis.generic.shared.basic.IIdentityHolder;
import ch.systemsx.cisd.openbis.generic.shared.basic.dto.AttachmentHolderKind;
import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.IdentifierHelper;
import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SampleIdentifier;
@@ -74,7 +76,7 @@ import ch.systemsx.cisd.openbis.generic.shared.util.HibernateUtils;
/**
* <i>Persistent Entity</i> object of an entity 'sample'.
- *
+ *
* @author Christian Ribeaud
*/
@@ -129,32 +131,29 @@ public class SamplePE extends AttachmentHolderPE implements IIdAndCodeHolder, Co
private String permId;
- private Set<SampleRelationshipPE> parentRelationships = new LinkedHashSet<SampleRelationshipPE>();
+ protected Set<SampleRelationshipPE> parentRelationships;
- private Set<SampleRelationshipPE> childRelationships = new LinkedHashSet<SampleRelationshipPE>();
+ protected Set<SampleRelationshipPE> childRelationships;
private Set<MetaprojectAssignmentPE> metaprojectAssignments =
new HashSet<MetaprojectAssignmentPE>();
- @OptimisticLock(excluded = true)
- @OneToMany(fetch = FetchType.LAZY, mappedBy = "parentSample")
- @Fetch(FetchMode.SUBSELECT)
- private Set<SampleRelationshipPE> getSampleChildRelationships()
+ @Transient
+ protected Set<SampleRelationshipPE> getSampleChildRelationships()
{
+ if (childRelationships == null) {
+ if (id == null) {
+ childRelationships = new HashSet<>();
+ } else {
+ childRelationships = new HashSet<>(CommonServiceProvider.getDAOFactory().getSampleRelationshipDAO().listSampleChildren(List.of(id)));
+ }
+ }
return childRelationships;
}
- // Required by Hibernate.
- @SuppressWarnings("unused")
- private void setSampleChildRelationships(final Set<SampleRelationshipPE> childRelationships)
- {
- this.childRelationships = childRelationships;
- }
-
@Transient
public Set<SampleRelationshipPE> getChildRelationships()
{
-
return new UnmodifiableSetDecorator<SampleRelationshipPE>(getSampleChildRelationships());
}
@@ -171,23 +170,22 @@ public class SamplePE extends AttachmentHolderPE implements IIdAndCodeHolder, Co
{
relationship.setParentSample(this);
getSampleChildRelationships().add(relationship);
+ CommonServiceProvider.getDAOFactory().getSampleRelationshipDAO().persist(List.of(relationship));
}
- @OptimisticLock(excluded = true)
- @OneToMany(fetch = FetchType.LAZY, cascade = CascadeType.ALL, mappedBy = "childSample", orphanRemoval = true)
- @Fetch(FetchMode.SUBSELECT)
- private Set<SampleRelationshipPE> getSampleParentRelationships()
+ @Transient
+ protected Set<SampleRelationshipPE> getSampleParentRelationships()
{
+ if (parentRelationships == null) {
+ if (id == null) {
+ parentRelationships = new HashSet<>();
+ } else {
+ parentRelationships = new HashSet<>(CommonServiceProvider.getDAOFactory().getSampleRelationshipDAO().listSampleParents(List.of(id)));
+ }
+ }
return parentRelationships;
}
- // Required by Hibernate.
- @SuppressWarnings("unused")
- private void setSampleParentRelationships(final Set<SampleRelationshipPE> parentRelationships)
- {
- this.parentRelationships = parentRelationships;
- }
-
@Transient
public Set<SampleRelationshipPE> getParentRelationships()
{
@@ -205,6 +203,7 @@ public class SamplePE extends AttachmentHolderPE implements IIdAndCodeHolder, Co
public void setParentRelationships(final Set<SampleRelationshipPE> parentRelationships)
{
+ CommonServiceProvider.getDAOFactory().getSampleRelationshipDAO().delete(getSampleParentRelationships());
getSampleParentRelationships().clear();
for (final SampleRelationshipPE sampleRelationship : parentRelationships)
{
@@ -221,6 +220,7 @@ public class SamplePE extends AttachmentHolderPE implements IIdAndCodeHolder, Co
{
relationship.setChildSample(this);
getSampleParentRelationships().add(relationship);
+ CommonServiceProvider.getDAOFactory().getSampleRelationshipDAO().persist(List.of(relationship));
}
public void removeParentRelationship(final SampleRelationshipPE relationship)
@@ -229,6 +229,7 @@ public class SamplePE extends AttachmentHolderPE implements IIdAndCodeHolder, Co
relationship.getParentSample().getSampleChildRelationships().remove(relationship);
relationship.setChildSample(null);
relationship.setParentSample(null);
+ CommonServiceProvider.getDAOFactory().getSampleRelationshipDAO().delete(List.of(relationship));
}
/**
diff --git a/server-application-server/sourceTest/java/ch/systemsx/cisd/openbis/generic/SamplePENoDAO.java b/server-application-server/sourceTest/java/ch/systemsx/cisd/openbis/generic/SamplePENoDAO.java
new file mode 100644
index 00000000000..ec6b3effe31
--- /dev/null
+++ b/server-application-server/sourceTest/java/ch/systemsx/cisd/openbis/generic/SamplePENoDAO.java
@@ -0,0 +1,46 @@
+package ch.systemsx.cisd.openbis.generic;
+
+import ch.systemsx.cisd.openbis.generic.shared.dto.SamplePE;
+import ch.systemsx.cisd.openbis.generic.shared.dto.SampleRelationshipPE;
+
+import java.util.HashSet;
+import java.util.Set;
+
+/*
+ * Many tests have never used the DB, they lack a lot of necessary constraints.
+ *
+ * Under a real scenario those tests will actually throw errors.
+ *
+ * This abstraction allows them to avoid using the introduced DAO behaving as naively as before introducing it.
+ */
+public class SamplePENoDAO extends SamplePE {
+ @Override
+ protected Set<SampleRelationshipPE> getSampleChildRelationships() {
+ if (childRelationships == null) {
+ childRelationships = new HashSet<>();
+ }
+ return childRelationships;
+ }
+
+ @Override
+ protected Set<SampleRelationshipPE> getSampleParentRelationships() {
+ if(parentRelationships == null) {
+ parentRelationships = new HashSet<>();
+ }
+ return parentRelationships;
+ }
+
+ @Override
+ public void addChildRelationship(final SampleRelationshipPE relationship)
+ {
+ relationship.setParentSample(this);
+ getSampleChildRelationships().add(relationship);
+ }
+
+ @Override
+ public void addParentRelationship(final SampleRelationshipPE relationship)
+ {
+ relationship.setChildSample(this);
+ getSampleParentRelationships().add(relationship);
+ }
+}
diff --git a/server-application-server/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/ETLServiceTest.java b/server-application-server/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/ETLServiceTest.java
index 6069c5a1740..f2084190b2f 100644
--- a/server-application-server/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/ETLServiceTest.java
+++ b/server-application-server/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/ETLServiceTest.java
@@ -31,6 +31,7 @@ import java.util.Map;
import java.util.Properties;
import java.util.Set;
+import ch.systemsx.cisd.openbis.generic.SamplePENoDAO;
import org.hamcrest.BaseMatcher;
import org.hamcrest.Description;
import org.jmock.Expectations;
@@ -717,9 +718,14 @@ public class ETLServiceTest extends AbstractServerTestCase
context.assertIsSatisfied();
}
+ /*
+ * These tests have never used the DB, they lack a lot of necessary constraints.
+ *
+ * This abstraction allows them to avoid using the introduced DAO behaving as naively as before introducing it.
+ */
private SamplePE createSample(String code)
{
- SamplePE sample = new SamplePE();
+ SamplePE sample = new SamplePENoDAO();
sample.setCode(code);
return sample;
}
diff --git a/server-application-server/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/SampleGenericBusinessRulesTest.java b/server-application-server/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/SampleGenericBusinessRulesTest.java
index 9c62ed202ea..d7b04c81922 100644
--- a/server-application-server/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/SampleGenericBusinessRulesTest.java
+++ b/server-application-server/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/SampleGenericBusinessRulesTest.java
@@ -16,6 +16,7 @@
package ch.systemsx.cisd.openbis.generic.server.business.bo;
+import ch.systemsx.cisd.openbis.generic.SamplePENoDAO;
import org.testng.AssertJUnit;
import org.testng.annotations.Test;
@@ -28,7 +29,7 @@ import ch.systemsx.cisd.openbis.generic.shared.dto.SpacePE;
/**
* Test cases for corresponding {@link SampleGenericBusinessRules} class.
- *
+ *
* @author Izabela Adamczyk
*/
public final class SampleGenericBusinessRulesTest extends AssertJUnit
@@ -45,6 +46,15 @@ public final class SampleGenericBusinessRulesTest extends AssertJUnit
private static final String SAMPLE_3 = "sample-3";
+ /*
+ * These tests have never used the DB, they lack a lot of necessary constraints.
+ *
+ * This abstraction allows them to avoid using the introduced DAO behaving as naively as before introducing it.
+ */
+ private static SamplePE getSamplePE() {
+ return new SamplePENoDAO();
+ }
+
private static SpacePE createGroup(String code)
{
SpacePE g = new SpacePE();
@@ -54,7 +64,7 @@ public final class SampleGenericBusinessRulesTest extends AssertJUnit
private static SamplePE createGroupSample(SpacePE g, String code)
{
- SamplePE s = new SamplePE();
+ SamplePE s = getSamplePE();
s.setCode(code);
s.setSpace(g);
return s;
@@ -62,7 +72,7 @@ public final class SampleGenericBusinessRulesTest extends AssertJUnit
private static SamplePE createSharedSample(String code)
{
- SamplePE s = new SamplePE();
+ SamplePE s = getSamplePE();
s.setCode(code);
return s;
}
@@ -155,7 +165,7 @@ public final class SampleGenericBusinessRulesTest extends AssertJUnit
}
private void setBidirectionalRelationships(SamplePE child, SamplePE generatorOrNull,
- SamplePE containerOrNull)
+ SamplePE containerOrNull)
{
child.setContainer(containerOrNull);
if (generatorOrNull != null)
diff --git a/server-application-server/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/SampleDAOTest.java b/server-application-server/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/SampleDAOTest.java
index 0bbcc23653e..bb18527e2c8 100644
--- a/server-application-server/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/SampleDAOTest.java
+++ b/server-application-server/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/SampleDAOTest.java
@@ -662,6 +662,7 @@ public final class SampleDAOTest extends AbstractDAOTest
sample.setPermId(daoFactory.getPermIdDAO().createPermId());
sample.setSampleType(type);
sample.setSpace(sampleOwner.tryGetSpace());
+ sample.setModificationDate(new Date());
if (generatorOrNull != null)
{
sample.addParentRelationship(new SampleRelationshipPE(generatorOrNull, sample,
--
GitLab