Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import org.openmetadata.schema.api.configuration.rdf.RdfConfiguration;
import org.openmetadata.schema.type.EntityReference;
import org.openmetadata.schema.type.EntityRelationship;
import org.openmetadata.schema.type.Relationship;
import org.openmetadata.service.Entity;
import org.openmetadata.service.monitoring.RequestLatencyContext;
import org.openmetadata.service.util.AsyncService;

Expand Down Expand Up @@ -72,6 +74,15 @@ public static void addRelationship(EntityRelationship relationship) {
if (rdfRepository == null || !rdfRepository.isEnabled()) {
return;
}
if (isGlossaryTermRelatedTo(relationship)) {
// Glossary term ⇔ glossary term RELATED_TO is owned by the typed path
// (addGlossaryTermRelation), which writes the precise predicate —
// skos:exactMatch for synonym, skos:broader for broader, om:relatedTo
// for relatedTo, etc. The generic addRelationship would unconditionally
// write om:relatedTo on top of that, so every type change would leak a
// residual om:relatedTo triple that nothing later cleans up.
return;
}
submitAsync(
"addRelationship",
() -> {
Expand All @@ -90,6 +101,11 @@ public static void removeRelationship(EntityRelationship relationship) {
if (rdfRepository == null || !rdfRepository.isEnabled()) {
return;
}
if (isGlossaryTermRelatedTo(relationship)) {
// See addRelationship — the typed removal path
// (removeGlossaryTermRelation) owns these deletions.
return;
}
submitAsync(
"removeRelationship",
() -> {
Expand All @@ -104,6 +120,12 @@ public static void removeRelationship(EntityRelationship relationship) {
});
}

private static boolean isGlossaryTermRelatedTo(EntityRelationship relationship) {
return Entity.GLOSSARY_TERM.equals(relationship.getFromEntity())
&& Entity.GLOSSARY_TERM.equals(relationship.getToEntity())
&& relationship.getRelationshipType() == Relationship.RELATED_TO;
}

public static boolean isEnabled() {
return rdfRepository != null && rdfRepository.isEnabled();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1128,7 +1128,17 @@ private void addTypedProperty(
XSDDatatype datatype = getXSDDatatype(xsdType);

if (datatype != null && !value.isNull()) {
resource.addProperty(property, model.createTypedLiteral(value.asText(), datatype));
String literal = value.asText();
// Skip blank xsd:string triples. An empty literal carries no real
// information and downstream readers had to special-case it — most
// visibly skos:prefLabel="" winning over rdfs:label in the glossary
// term graph SPARQL. By not writing the triple at all, OPTIONAL
// patterns and COALESCE on the read side behave correctly with no
// extra logic.
if (XSDDatatype.XSDstring.equals(datatype) && literal.isBlank()) {
return;
}
resource.addProperty(property, model.createTypedLiteral(literal, datatype));
}
} else {
addSimpleProperty(resource, propertyId, value, model);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1065,6 +1065,85 @@ void testHelperMethodsResolveNamespacesContextsAndDatatypes() throws Exception {
}
}

@Nested
@DisplayName("addTypedProperty: blank xsd:string skip")
class AddTypedPropertyBlankString {

@Test
@DisplayName("Blank xsd:string value should not produce a literal triple")
void blankStringIsNotEmitted() throws Exception {
JsonNode blank = objectMapper.getNodeFactory().textNode("");
invokePrivate(
"addTypedProperty",
new Class[] {Resource.class, String.class, JsonNode.class, String.class, Model.class},
entityResource,
"skos:prefLabel",
blank,
"xsd:string",
model);

Property pref = model.createProperty(SKOS.getURI(), "prefLabel");
assertFalse(
model.contains(entityResource, pref),
"Blank xsd:string literals must not be emitted — they masked rdfs:label "
+ "on the read side and rendered as empty UI labels");
}

@Test
@DisplayName("Whitespace-only xsd:string value should not produce a literal triple")
void whitespaceOnlyStringIsNotEmitted() throws Exception {
JsonNode whitespace = objectMapper.getNodeFactory().textNode(" ");
invokePrivate(
"addTypedProperty",
new Class[] {Resource.class, String.class, JsonNode.class, String.class, Model.class},
entityResource,
"skos:prefLabel",
whitespace,
"xsd:string",
model);

Property pref = model.createProperty(SKOS.getURI(), "prefLabel");
assertFalse(model.contains(entityResource, pref));
}

@Test
@DisplayName("Non-blank xsd:string value should still be emitted")
void nonBlankStringIsEmitted() throws Exception {
JsonNode value = objectMapper.getNodeFactory().textNode("Pretty Name");
invokePrivate(
"addTypedProperty",
new Class[] {Resource.class, String.class, JsonNode.class, String.class, Model.class},
entityResource,
"skos:prefLabel",
value,
"xsd:string",
model);

Property pref = model.createProperty(SKOS.getURI(), "prefLabel");
assertTrue(model.contains(entityResource, pref, "Pretty Name"));
}

@Test
@DisplayName("Blank value with a non-xsd:string type should still be emitted")
void blankNonStringIsEmitted() throws Exception {
// Non-string xsd types (numbers, booleans, dates) get their own validation
// path elsewhere — the skip is intentionally narrow to xsd:string so it
// doesn't accidentally drop "0" literals or similar.
JsonNode zero = objectMapper.getNodeFactory().textNode("0");
invokePrivate(
"addTypedProperty",
new Class[] {Resource.class, String.class, JsonNode.class, String.class, Model.class},
entityResource,
"om:counter",
zero,
"xsd:integer",
model);

Property counter = model.createProperty(OM_NS, "counter");
assertTrue(model.contains(entityResource, counter));
}
}

private Object invokePrivate(String name, Class<?>[] parameterTypes, Object... args)
throws Exception {
java.lang.reflect.Method method =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
/*
* Copyright 2025 Collate
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.openmetadata.service.rdf;

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import java.lang.reflect.Field;
import java.time.Duration;
import java.util.UUID;
import org.awaitility.Awaitility;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
import org.openmetadata.schema.type.EntityRelationship;
import org.openmetadata.schema.type.Relationship;
import org.openmetadata.service.Entity;

/**
* Unit tests for {@link RdfUpdater}, specifically the glossary-term ⇔
* glossary-term {@code RELATED_TO} short-circuit. The generic relationship
* hooks unconditionally wrote {@code om:relatedTo} on top of the typed
* predicate ({@code skos:exactMatch}, {@code skos:broader}, …) emitted by
* {@link RdfRepository#addGlossaryTermRelation}, leaving a residual edge
* after a user changed the relation type from "relatedTo" to "broader".
* Verifies the short-circuit fires for the targeted shape and only for that
* shape, so other relationships (CONTAINS, OWNS, cross-entity RELATED_TO,
* etc.) still flow through the underlying repository.
*/
class RdfUpdaterTest {

private RdfRepository originalRepository;
private RdfRepository mockRepository;

@BeforeEach
void setUp() throws Exception {
mockRepository = Mockito.mock(RdfRepository.class);
when(mockRepository.isEnabled()).thenReturn(true);
originalRepository = swapRdfRepository(mockRepository);
}

@AfterEach
void tearDown() throws Exception {
swapRdfRepository(originalRepository);
}

@Nested
@DisplayName("addRelationship short-circuits glossaryTerm⇔glossaryTerm RELATED_TO")
class AddRelationship {

@Test
@DisplayName("glossaryTerm RELATED_TO glossaryTerm should NOT reach the repository")
void glossaryTermRelatedToIsShortCircuited() {
EntityRelationship rel =
new EntityRelationship()
.withFromId(UUID.randomUUID())
.withToId(UUID.randomUUID())
.withFromEntity(Entity.GLOSSARY_TERM)
.withToEntity(Entity.GLOSSARY_TERM)
.withRelationshipType(Relationship.RELATED_TO);

RdfUpdater.addRelationship(rel);

verify(mockRepository, never()).addRelationship(any());
}

@Test
@DisplayName("Cross-entity RELATED_TO (e.g. table → glossaryTerm) still flows through")
void crossEntityRelatedToIsNotShortCircuited() {
EntityRelationship rel =
new EntityRelationship()
.withFromId(UUID.randomUUID())
.withToId(UUID.randomUUID())
.withFromEntity(Entity.TABLE)
.withToEntity(Entity.GLOSSARY_TERM)
.withRelationshipType(Relationship.RELATED_TO);

RdfUpdater.addRelationship(rel);

Awaitility.await()
.atMost(Duration.ofSeconds(5))
.untilAsserted(() -> verify(mockRepository, times(1)).addRelationship(rel));
}

@Test
@DisplayName("Non-RELATED_TO between two glossary terms still flows through")
void otherRelationshipBetweenGlossaryTermsIsNotShortCircuited() {
EntityRelationship rel =
new EntityRelationship()
.withFromId(UUID.randomUUID())
.withToId(UUID.randomUUID())
.withFromEntity(Entity.GLOSSARY_TERM)
.withToEntity(Entity.GLOSSARY_TERM)
.withRelationshipType(Relationship.CONTAINS);

RdfUpdater.addRelationship(rel);

Awaitility.await()
.atMost(Duration.ofSeconds(5))
.untilAsserted(() -> verify(mockRepository, times(1)).addRelationship(rel));
}
}

@Nested
@DisplayName("removeRelationship short-circuits glossaryTerm⇔glossaryTerm RELATED_TO")
class RemoveRelationship {

@Test
@DisplayName("glossaryTerm RELATED_TO glossaryTerm should NOT reach the repository")
void glossaryTermRelatedToIsShortCircuited() {
EntityRelationship rel =
new EntityRelationship()
.withFromId(UUID.randomUUID())
.withToId(UUID.randomUUID())
.withFromEntity(Entity.GLOSSARY_TERM)
.withToEntity(Entity.GLOSSARY_TERM)
.withRelationshipType(Relationship.RELATED_TO);

RdfUpdater.removeRelationship(rel);

verify(mockRepository, never()).removeRelationship(any());
}

@Test
@DisplayName("Cross-entity RELATED_TO still flows through to repository")
void crossEntityRelatedToIsNotShortCircuited() {
EntityRelationship rel =
new EntityRelationship()
.withFromId(UUID.randomUUID())
.withToId(UUID.randomUUID())
.withFromEntity(Entity.TABLE)
.withToEntity(Entity.GLOSSARY_TERM)
.withRelationshipType(Relationship.RELATED_TO);

RdfUpdater.removeRelationship(rel);

Awaitility.await()
.atMost(Duration.ofSeconds(5))
.untilAsserted(() -> verify(mockRepository, times(1)).removeRelationship(rel));
}
}

/**
* Replace the private static {@code rdfRepository} field via reflection
* and return the previous value so tests can restore it. Required because
* RdfUpdater intentionally exposes no setter — the singleton is wired
* via {@link RdfUpdater#initialize(org.openmetadata.schema.api.configuration.rdf.RdfConfiguration)}
* which would actually connect to Fuseki.
*/
private static RdfRepository swapRdfRepository(RdfRepository replacement) throws Exception {
Field field = RdfUpdater.class.getDeclaredField("rdfRepository");
field.setAccessible(true);
RdfRepository previous = (RdfRepository) field.get(null);
field.set(null, replacement);
return previous;
}
}
Loading
Loading