Skip to content

Commit

Permalink
fix(search): Fix empty description (datahub-project#11514)
Browse files Browse the repository at this point in the history
  • Loading branch information
@david-leifker
david-leifker authored Oct 4, 2024
1 parent 9e0f68c commit fcd6365
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 23 deletions.
11 changes: 8 additions & 3 deletions ...-io/src/main/java/com/linkedin/metadata/search/transformer/SearchDocumentTransformer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,13 @@ public void setSearchableValue(
fieldName,
JsonNodeFactory.instance.booleanNode((Boolean) firstValue.orElse(false)));
} else {
searchDocument.set(
fieldName, JsonNodeFactory.instance.booleanNode(!fieldValues.isEmpty()));
final boolean hasValue;
if (DataSchema.Type.STRING.equals(valueType)) {
hasValue = firstValue.isPresent() && !String.valueOf(firstValue.get()).isEmpty();
} else {
hasValue = !fieldValues.isEmpty();
}
searchDocument.set(fieldName, JsonNodeFactory.instance.booleanNode(hasValue));
}
});

Expand Down Expand Up @@ -390,7 +395,7 @@ private Optional<JsonNode> getNodeForValue(
default:
String value = fieldValue.toString();
return value.isEmpty()
? Optional.empty()
? Optional.of(JsonNodeFactory.instance.nullNode())
: Optional.of(JsonNodeFactory.instance.textNode(value));
}
}
Expand Down
81 changes: 61 additions & 20 deletions ...src/test/java/com/linkedin/metadata/search/transformer/SearchDocumentTransformerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@
import com.fasterxml.jackson.databind.node.JsonNodeType;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.linkedin.common.urn.Urn;
import com.linkedin.common.urn.UrnUtils;
import com.linkedin.data.DataMapBuilder;
import com.linkedin.dataset.DatasetProperties;
import com.linkedin.dataset.EditableDatasetProperties;
import com.linkedin.entity.Aspect;
import com.linkedin.metadata.TestEntitySpecBuilder;
import com.linkedin.metadata.TestEntityUtil;
Expand All @@ -39,6 +42,17 @@

public class SearchDocumentTransformerTest {
private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
private static final EntityRegistry ENTITY_REGISTRY =
TestOperationContexts.defaultEntityRegistry();
private static final EntityRegistry TEST_ENTITY_REGISTRY;

static {
TEST_ENTITY_REGISTRY =
new ConfigEntityRegistry(
TestSearchFieldConfig.class
.getClassLoader()
.getResourceAsStream("test-entity-registry.yaml"));
}

static {
int maxSize =
Expand Down Expand Up @@ -214,7 +228,6 @@ public void testSetSearchableRefValue() throws URISyntaxException, RemoteInvocat
SearchDocumentTransformer searchDocumentTransformer =
new SearchDocumentTransformer(1000, 1000, 1000);

EntityRegistry entityRegistry = getTestEntityRegistry();
List<Object> urnList = List.of(Urn.createFromString("urn:li:refEntity:1"));

DataMapBuilder dataMapBuilder = new DataMapBuilder();
Expand All @@ -225,10 +238,10 @@ public void testSetSearchableRefValue() throws URISyntaxException, RemoteInvocat

ObjectNode searchDocument = JsonNodeFactory.instance.objectNode();
SearchableRefFieldSpec searchableRefFieldSpec =
entityRegistry.getEntitySpec("testRefEntity").getSearchableRefFieldSpecs().get(0);
TEST_ENTITY_REGISTRY.getEntitySpec("testRefEntity").getSearchableRefFieldSpecs().get(0);

// Mock Behaviour
Mockito.when(aspectRetriever.getEntityRegistry()).thenReturn(entityRegistry);
Mockito.when(aspectRetriever.getEntityRegistry()).thenReturn(TEST_ENTITY_REGISTRY);
Mockito.when(aspectRetriever.getLatestAspectObject(any(), anyString())).thenReturn(aspect);
OperationContext opContext =
TestOperationContexts.systemContextNoSearchAuthorization(
Expand Down Expand Up @@ -258,14 +271,13 @@ public void testSetSearchableRefValue_WithNonURNField() throws URISyntaxExceptio
SearchDocumentTransformer searchDocumentTransformer =
new SearchDocumentTransformer(1000, 1000, 1000);

EntityRegistry entityRegistry = getTestEntityRegistry();
OperationContext opContext =
TestOperationContexts.systemContextNoSearchAuthorization(entityRegistry);
TestOperationContexts.systemContextNoSearchAuthorization(TEST_ENTITY_REGISTRY);
List<Object> urnList = List.of(Urn.createFromString("urn:li:refEntity:1"));

ObjectNode searchDocument = JsonNodeFactory.instance.objectNode();
SearchableRefFieldSpec searchableRefFieldSpecText =
entityRegistry.getEntitySpec("testRefEntity").getSearchableRefFieldSpecs().get(1);
TEST_ENTITY_REGISTRY.getEntitySpec("testRefEntity").getSearchableRefFieldSpecs().get(1);
searchDocumentTransformer.setSearchableRefValue(
opContext, searchableRefFieldSpecText, urnList, searchDocument, false);
assertTrue(searchDocument.isEmpty());
Expand All @@ -278,10 +290,9 @@ public void testSetSearchableRefValue_RuntimeException()
SearchDocumentTransformer searchDocumentTransformer =
new SearchDocumentTransformer(1000, 1000, 1000);

EntityRegistry entityRegistry = getTestEntityRegistry();
List<Object> urnList = List.of(Urn.createFromString("urn:li:refEntity:1"));

Mockito.when(aspectRetriever.getEntityRegistry()).thenReturn(entityRegistry);
Mockito.when(aspectRetriever.getEntityRegistry()).thenReturn(TEST_ENTITY_REGISTRY);
Mockito.when(
aspectRetriever.getLatestAspectObject(
eq(Urn.createFromString("urn:li:refEntity:1")), anyString()))
Expand All @@ -296,7 +307,7 @@ public void testSetSearchableRefValue_RuntimeException()

ObjectNode searchDocument = JsonNodeFactory.instance.objectNode();
SearchableRefFieldSpec searchableRefFieldSpec =
entityRegistry.getEntitySpec("testRefEntity").getSearchableRefFieldSpecs().get(0);
TEST_ENTITY_REGISTRY.getEntitySpec("testRefEntity").getSearchableRefFieldSpecs().get(0);
searchDocumentTransformer.setSearchableRefValue(
opContext, searchableRefFieldSpec, urnList, searchDocument, false);
assertTrue(searchDocument.isEmpty());
Expand All @@ -309,15 +320,14 @@ public void testSetSearchableRefValue_RuntimeException_URNExist()
SearchDocumentTransformer searchDocumentTransformer =
new SearchDocumentTransformer(1000, 1000, 1000);

EntityRegistry entityRegistry = getTestEntityRegistry();
List<Object> urnList = List.of(Urn.createFromString("urn:li:refEntity:1"));
DataMapBuilder dataMapBuilder = new DataMapBuilder();
dataMapBuilder.addKVPair("fieldPath", "refEntityUrn");
dataMapBuilder.addKVPair("name", "refEntityUrnName");
dataMapBuilder.addKVPair("description", "refEntityUrn1 description details");

Aspect aspect = new Aspect(dataMapBuilder.convertToDataMap());
Mockito.when(aspectRetriever.getEntityRegistry()).thenReturn(entityRegistry);
Mockito.when(aspectRetriever.getEntityRegistry()).thenReturn(TEST_ENTITY_REGISTRY);
Mockito.when(
aspectRetriever.getLatestAspectObject(
eq(Urn.createFromString("urn:li:refEntity:1")), anyString()))
Expand All @@ -333,7 +343,7 @@ public void testSetSearchableRefValue_RuntimeException_URNExist()

ObjectNode searchDocument = JsonNodeFactory.instance.objectNode();
SearchableRefFieldSpec searchableRefFieldSpec =
entityRegistry.getEntitySpec("testRefEntity").getSearchableRefFieldSpecs().get(0);
TEST_ENTITY_REGISTRY.getEntitySpec("testRefEntity").getSearchableRefFieldSpecs().get(0);
searchDocumentTransformer.setSearchableRefValue(
opContext, searchableRefFieldSpec, urnList, searchDocument, false);
assertTrue(searchDocument.has("refEntityUrns"));
Expand All @@ -349,13 +359,12 @@ void testSetSearchableRefValue_WithInvalidURN()
SearchDocumentTransformer searchDocumentTransformer =
new SearchDocumentTransformer(1000, 1000, 1000);

EntityRegistry entityRegistry = getTestEntityRegistry();
List<Object> urnList = List.of(Urn.createFromString("urn:li:refEntity:1"));

Mockito.when(aspectRetriever.getEntityRegistry()).thenReturn(entityRegistry);
Mockito.when(aspectRetriever.getEntityRegistry()).thenReturn(TEST_ENTITY_REGISTRY);
Mockito.when(aspectRetriever.getLatestAspectObject(any(), anyString())).thenReturn(null);
SearchableRefFieldSpec searchableRefFieldSpec =
entityRegistry.getEntitySpec("testRefEntity").getSearchableRefFieldSpecs().get(0);
TEST_ENTITY_REGISTRY.getEntitySpec("testRefEntity").getSearchableRefFieldSpecs().get(0);
OperationContext opContext =
TestOperationContexts.systemContextNoSearchAuthorization(
RetrieverContext.builder()
Expand All @@ -371,10 +380,42 @@ void testSetSearchableRefValue_WithInvalidURN()
assertTrue(searchDocument.get("refEntityUrns").getNodeType().equals(JsonNodeType.NULL));
}

private EntityRegistry getTestEntityRegistry() {
return new ConfigEntityRegistry(
TestSearchFieldConfig.class
.getClassLoader()
.getResourceAsStream("test-entity-registry.yaml"));
@Test
public void testEmptyDescription() throws RemoteInvocationException, URISyntaxException {
String entityUrn = "urn:li:dataset:(urn:li:dataPlatform:hive,fct_users_created,PROD)";
SearchDocumentTransformer test = new SearchDocumentTransformer(1000, 1000, 1000);

// editedDescription - empty string
Optional<ObjectNode> transformed =
test.transformAspect(
mock(OperationContext.class),
UrnUtils.getUrn(entityUrn),
new EditableDatasetProperties().setDescription(""),
ENTITY_REGISTRY
.getEntitySpec(DATASET_ENTITY_NAME)
.getAspectSpec(EDITABLE_DATASET_PROPERTIES_ASPECT_NAME),
false);

assertTrue(transformed.isPresent());
assertEquals(transformed.get().get("urn").asText(), entityUrn);
assertTrue(transformed.get().has("editedDescription"));
assertTrue(transformed.get().get("editedDescription").isNull());

// description - empty string
transformed =
test.transformAspect(
mock(OperationContext.class),
UrnUtils.getUrn(entityUrn),
new DatasetProperties().setDescription(""),
ENTITY_REGISTRY
.getEntitySpec(DATASET_ENTITY_NAME)
.getAspectSpec(DATASET_PROPERTIES_ASPECT_NAME),
false);

assertTrue(transformed.isPresent());
assertEquals(transformed.get().get("urn").asText(), entityUrn);
assertTrue(transformed.get().has("description"));
assertTrue(transformed.get().get("description").isNull());
assertFalse(transformed.get().get("hasDescription").asBoolean());
}
}

0 comments on commit fcd6365

Please sign in to comment.