Skip to content

ATLAS-2933:Empty array attributes are returned as null instead of an empty list #29

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.

Sign up for GitHub

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

Closed
mahaha000 wants to merge 1 commit into from
Closed

ATLAS-2933:Empty array attributes are returned as null instead of an empty list #29

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasGraphUtilsV2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,12 +207,15 @@ public static <T extends AtlasElement> void setProperty(T element, String proper
Object existingValue = element.getProperty(propertyName, Object.class);

if (value == null || (value instanceof Collection && ((Collection)value).isEmpty())) {
Copy link
Copy Markdown
Contributor

@mneethiraj mneethiraj Mar 12, 2019

Choose a reason for hiding this comment

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

Consider the following alternate, for easier readability:

if (value == null) {
  if (existingValue != null) {
    if (LOG.isDebugEnabled()) {
      LOG.debug("Removing property {} from {}", propertyName, toString(element));
    }

    element.removeProperty(propertyName);
  }
} else {
  element.setProperty(propertyName, value);
}

if (existingValue != null) {
if (LOG.isDebugEnabled()) {
LOG.debug("Removing property {} from {}", propertyName, toString(element));
if (value == null) {
if (existingValue != null) {
if (LOG.isDebugEnabled()) {
LOG.debug("Removing property {} from {}", propertyName, toString(element));
}
element.removeProperty(propertyName);
}

element.removeProperty(propertyName);
} else {
element.setProperty(propertyName, value);
}
} else {
if (!value.equals(existingValue)) {
Expand Down
6 changes: 5 additions & 1 deletion ...sitory/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphRetriever.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,10 @@ private List<AtlasObjectId> mapVertexToArrayForSoftRef(AtlasVertex entityVertex,
List<AtlasObjectId> ret = null;
List softRefVal = entityVertex.getListProperty(attribute.getVertexPropertyName(), List.class);

if (CollectionUtils.isEmpty(softRefVal)) {
return softRefVal;
}

if (CollectionUtils.isNotEmpty(softRefVal)) {
Copy link
Copy Markdown
Contributor

@mneethiraj mneethiraj Mar 12, 2019

Choose a reason for hiding this comment

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

replace line #732 with: "else {", as the reverse condition is already tested in line #728 above.

ret = new ArrayList<>();

Expand Down Expand Up @@ -822,7 +826,7 @@ private List<Object> mapVertexToArray(AtlasVertex entityVertex, AtlasEntityExtIn
List<Object> arrayElements = getArrayElementsProperty(arrayElementType, entityVertex, attribute);

if (CollectionUtils.isEmpty(arrayElements)) {
return null;
return arrayElements;
}

if (LOG.isDebugEnabled()) {
Expand Down
42 changes: 41 additions & 1 deletion .../src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasComplexAttributesTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
import java.util.Iterator;
Expand All @@ -48,31 +49,37 @@
import static org.apache.atlas.TestUtilsV2.ENTITY_TYPE_MAP;
import static org.apache.atlas.TestUtilsV2.ENTITY_TYPE_WITH_COMPLEX_COLLECTION_ATTR;
import static org.apache.atlas.TestUtilsV2.ENTITY_TYPE_WITH_COMPLEX_COLLECTION_ATTR_DELETE;
import static org.apache.atlas.TestUtilsV2.ENTITY_TYPE_WITH_NESTED_COLLECTION_ATTR;
import static org.apache.atlas.TestUtilsV2.NAME;
import static org.apache.atlas.repository.graph.GraphHelper.getStatus;
import static org.apache.atlas.type.AtlasTypeUtil.getAtlasObjectId;
import static org.testng.Assert.assertNull;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertTrue;
import static org.testng.AssertJUnit.assertEquals;

@Guice(modules = TestModules.TestOnlyModule.class)
public class AtlasComplexAttributesTest extends AtlasEntityTestBase {
private AtlasEntityWithExtInfo complexCollectionAttrEntity;
private AtlasEntityWithExtInfo complexCollectionAttrEntityForDelete;
private AtlasEntityWithExtInfo mapAttributesEntity;
private AtlasEntityWithExtInfo nestedCollectionAttrEntity;

@BeforeClass
public void setUp() throws Exception {
super.setUp();

// create typeDefs
AtlasTypesDef[] testTypesDefs = new AtlasTypesDef[] { TestUtilsV2.defineTypeWithComplexCollectionAttributes(),
TestUtilsV2.defineTypeWithMapAttributes() };
TestUtilsV2.defineTypeWithMapAttributes(),
TestUtilsV2.defineTypeWithNestedCollectionAttributes()};
createTypesDef(testTypesDefs);

// create entity
complexCollectionAttrEntity = TestUtilsV2.createComplexCollectionAttrEntity();
complexCollectionAttrEntityForDelete = TestUtilsV2.createComplexCollectionAttrEntity();
mapAttributesEntity = TestUtilsV2.createMapAttrEntity();
nestedCollectionAttrEntity = TestUtilsV2.createNestedCollectionAttrEntity();
}

@Test
Expand Down Expand Up @@ -195,6 +202,39 @@ private void updateEntityMapAttributes(AtlasEntity attrEntity, Map<String, Strin
attrEntity.setAttribute("mapAttr5", map5);
}

@Test
public void testEmptyArrayAttribute() throws Exception {
init();
AtlasEntity nestedEntity = nestedCollectionAttrEntity.getEntity();

// set the attribute - arrayOfArrayOfStrings of the nestedEntity to
// an empty list. Expected behavior is that retrieved entity will
// have the value of this field as an empty list instead of null.
nestedEntity.setAttribute("arrayOfArrayOfStrings", Collections.emptyList());
AtlasEntitiesWithExtInfo nestedEntitiesInfo = new AtlasEntitiesWithExtInfo(nestedEntity);

EntityMutationResponse response = entityStore.createOrUpdate(new AtlasEntityStream(nestedEntitiesInfo), false);
AtlasEntityHeader updatedNestedEntity = response.getFirstCreatedEntityByTypeName(ENTITY_TYPE_WITH_NESTED_COLLECTION_ATTR);
validateEntity(nestedEntitiesInfo, getEntityFromStore(updatedNestedEntity));
}

@Test(dependsOnMethods = "testCreateComplexAttributeEntity")
public void testNullArrayAttribute() throws Exception {
init();

// set the attribute - listOfStructs of the complexEntity to null.
// Expected behavior is that the retrieved entity will
// not have the attribute of listOfStructs.
AtlasEntity complexEntity = getEntityFromStore(complexCollectionAttrEntity.getEntity().getGuid());
assertTrue(complexEntity.getAttributes().containsKey("listOfStructs"));

complexEntity.setAttribute("listOfStructs", null);
AtlasEntitiesWithExtInfo complexEntityInfo = new AtlasEntitiesWithExtInfo(complexEntity);
EntityMutationResponse responseUpdated = entityStore.createOrUpdate(new AtlasEntityStream(complexEntityInfo), false);
AtlasEntityHeader updatedComplexEntity = responseUpdated.getFirstUpdatedEntityByTypeName(ENTITY_TYPE_WITH_COMPLEX_COLLECTION_ATTR);
Copy link
Copy Markdown
Contributor

@mneethiraj mneethiraj Mar 14, 2019

Choose a reason for hiding this comment

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

updatedComplexEntity is a AtlasEntityHeader - which will not have all attributes of an entity, hence the asset at line #235 passes. Please get an entire entity and use that in assert:

  AtlasEntityHeader      updatedEntityHeader  = responseUpdated.getFirstUpdatedEntityByTypeName(ENTITY_TYPE_WITH_COMPLEX_COLLECTION_ATTR);
  AtlasEntity            updatedComplexEntity = getEntityFromStore(updatedEntityHeader);
  Object                 updatedAttrValue     = updatedComplexEntity.getAttribute("listOfStructs");

  assertNull(updatedAttrValue, "listOfStructs");

assertFalse(updatedComplexEntity.getAttributes().containsKey("listOfStructs"));
}

@Test(dependsOnMethods = "testCreateComplexAttributeEntity")
public void testStructArray() throws Exception {
init();
Expand Down