Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TRUNK-3974 fix ConceptNumerix(concept) inncorrectly copies collecitions #294

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
43 changes: 36 additions & 7 deletions api/src/main/java/org/openmrs/ConceptNumeric.java
Expand Up @@ -62,25 +62,54 @@ public ConceptNumeric(Integer conceptId) {
* <br/>
* Note: This cannot copy over numeric specific values
*
*
* @param c
* @should make deep copie of collections
* @should change reference to the parent object for objects in answers collection
* @should change reference to the parent object for objects in concpetSets collection
* @should change reference to the parent object for objects in names collection
* @should change reference to the parent object for objects in descriptions collection
* @should change reference to the parent object for objects in conceptMappings collection
*/
public ConceptNumeric(Concept c) {
this.setAnswers(c.getAnswers(true));

this.setUuid(c.getUuid());
this.getAnswers().addAll(c.getAnswers());
for (ConceptAnswer cAnswer : this.getAnswers())
cAnswer.setConcept(this);

this.setChangedBy(c.getChangedBy());
this.setConceptClass(c.getConceptClass());
this.setConceptId(c.getConceptId());
this.setConceptSets(c.getConceptSets());

this.getConceptSets().addAll(c.getConceptSets());
for (ConceptSet cSet : this.getConceptSets())
cSet.setConcept(this);

this.setCreator(c.getCreator());
this.setDatatype(c.getDatatype());
this.setDateChanged(c.getDateChanged());
this.setDateCreated(c.getDateCreated());
this.setSet(c.isSet());
this.setNames(c.getNames());
this.setDescriptions(c.getDescriptions());
this.setConceptMappings(c.getConceptMappings());
this.setSet(c.getSet());

this.getNames().addAll(c.getNames());
for (ConceptName cName : this.getNames())
cName.setConcept(this);

this.getDescriptions().addAll(c.getDescriptions());
for (ConceptDescription cDesciption : this.getDescriptions())
cDesciption.setConcept(this);

this.getConceptMappings().addAll(c.getConceptMappings());
for (ConceptMap cMap : this.getConceptMappings())
cMap.setConcept(this);

this.setRetired(c.isRetired());
this.setRetiredBy(c.getRetiredBy());
this.setRetireReason(c.getRetireReason());
this.setVersion(c.getVersion());
this.setUuid(c.getUuid());
this.setVersion(c.getVersion());
this.setDateCreated(c.getDateCreated());

this.hiAbsolute = null;
this.hiCritical = null;
Expand Down
90 changes: 90 additions & 0 deletions api/src/test/java/org/openmrs/ConceptNumericTest.java
Expand Up @@ -36,4 +36,94 @@ public void equals_shouldNotReturnTrueIfObjIsConcept() throws Exception {
Assert.assertNotSame(c, cn);
Assert.assertNotSame(cn, c);
}

/**
* Tests for TRUNK-3974
*
*
* @throws Exception
*/

@Test
@Verifies(value = "should make deep copie of collections", method = "ConceptNumeric(Concept)")
public void equals_shouldNotBeTheSameReference() throws Exception {
Concept c = new Concept(123);
ConceptNumeric cn = new ConceptNumeric(c);

Assert.assertNotSame(c.getAnswers(), cn.getAnswers());
Assert.assertNotSame(c.getConceptSets(), cn.getConceptSets());
Assert.assertNotSame(cn.getConceptSets(), c.getConceptSets());
Assert.assertNotSame(c.getNames(), cn.getNames());
Assert.assertNotSame(c.getConceptMappings(), cn.getConceptMappings());
Assert.assertNotSame(c.getDescriptions(), cn.getDescriptions());
}

@Test
@Verifies(value = "should change reference to the parent object for objects in answers collection", method = "ConceptNumeric(Concept)")
public void check_parentOfAnswersShouldBeChangeToReferenceOfConceptAnswer() throws Exception {
Concept c = new Concept(123);
c.addAnswer(new ConceptAnswer(1));
c.addAnswer(new ConceptAnswer(2));
ConceptNumeric cn = new ConceptNumeric(c);

for (ConceptAnswer cAnswer : cn.getAnswers()) {
Assert.assertSame(cn, cAnswer.getConcept());
Assert.assertNotSame(c, cAnswer.getConcept());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is pointless to add the second assertion, it can never fail if the first passed because c and cn are already different objects, makes sense?

}
}

@Test
@Verifies(value = "should change reference to the parent object for objects in concpetSets collection", method = "ConceptNumeric(Concept)")
public void check_parentOfConceptSetShouldBeChangeToReferenceOfConceptNumeric() throws Exception {
Concept c = new Concept(123);
c.addSetMember(new Concept(1));
c.addSetMember(new Concept(2));
ConceptNumeric cn = new ConceptNumeric(c);

for (ConceptSet cSet : cn.getConceptSets()) {
Assert.assertSame(cn, cSet.getConcept());
Assert.assertNotSame(c, cSet.getConcept());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, no need for the second assertion

}
}

@Test
@Verifies(value = "should change reference to the parent object for objects in names collection", method = "ConceptNumeric(Concept)")
public void check_parentOfConceptNameShouldBeChangeToReferenceOfConceptNumeric() throws Exception {
Concept c = new Concept(123);
c.addName(new ConceptName(1));
c.addName(new ConceptName(2));
ConceptNumeric cn = new ConceptNumeric(c);

for (ConceptName cName : cn.getNames()) {
Assert.assertSame(cn, cName.getConcept());
Assert.assertNotSame(c, cName.getConcept());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, no need for the second assertion

}
}

@Test
@Verifies(value = "should change reference to the parent object for objects in descriptions collection", method = "ConceptNumeric(Concept)")
public void check_parentOfConceptDescriptionShouldBeChangeToReferenceOfConceptNumeric() throws Exception {
Concept c = new Concept(123);
c.addDescription(new ConceptDescription(1));
c.addDescription(new ConceptDescription(2));
ConceptNumeric cn = new ConceptNumeric(c);

for (ConceptDescription cDesc : cn.getDescriptions()) {
Assert.assertSame(cn, cDesc.getConcept());
Assert.assertNotSame(c, cDesc.getConcept());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, no need for the second assertion

}
}

@Test
@Verifies(value = "should change reference to the parent object for objects in conceptMappings collection", method = "ConceptNumeric(Concept)")
public void check_parentOfConceptMapShouldBeChangeToReferenceOfConceptNumeric() throws Exception {
Concept c = new Concept(123);
c.getConceptMappings().add(new ConceptMap(1));
c.getConceptMappings().add(new ConceptMap(2));
ConceptNumeric cn = new ConceptNumeric(c);
for (ConceptMap cMap : cn.getConceptMappings()) {
Assert.assertSame(cn, cMap.getConcept());
Assert.assertNotSame(c, cMap.getConcept());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, no need for the second assertion

}
}
}