Skip to content

Commit f1d79db

Browse files
pmbrullclaude
andcommitted
fix(context-memory): address review (relationship types, stable FQN, status msg, test name)
- storeRelationships: rootMemory -> Relationship.CONTAINS, parentMemory -> Relationship.HAS so the root-ancestor and direct-parent hierarchies are distinguishable. - setFullyQualifiedName: derive from the immutable name only (drop mutable primaryEntity/owner derivation that destabilized nameHash on update). - validateStatusTransition: separate "no transitions defined" from "disallowed transition". - Rename ContextMemoryResourceTest -> ContextMemoryStatusTransitionTest (pure unit test). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 79d720d commit f1d79db

2 files changed

Lines changed: 14 additions & 21 deletions

File tree

openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ContextMemoryRepository.java

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import org.openmetadata.service.util.EntityUtil;
3131
import org.openmetadata.service.util.EntityUtil.Fields;
3232
import org.openmetadata.service.util.EntityUtil.RelationIncludes;
33-
import org.openmetadata.service.util.FullyQualifiedName;
3433

3534
@Slf4j
3635
@Repository(name = "ContextMemoryRepository")
@@ -60,22 +59,10 @@ protected void clearFields(ContextMemory entity, Fields fields) {
6059

6160
@Override
6261
public void setFullyQualifiedName(ContextMemory entity) {
63-
if (entity.getPrimaryEntity() != null
64-
&& entity.getPrimaryEntity().getFullyQualifiedName() != null
65-
&& !entity.getPrimaryEntity().getFullyQualifiedName().isEmpty()) {
66-
entity.setFullyQualifiedName(
67-
FullyQualifiedName.add(
68-
entity.getPrimaryEntity().getFullyQualifiedName(), entity.getName()));
69-
return;
70-
}
71-
if (entity.getOwners() != null
72-
&& !entity.getOwners().isEmpty()
73-
&& entity.getOwners().get(0).getName() != null
74-
&& !entity.getOwners().get(0).getName().isEmpty()) {
75-
entity.setFullyQualifiedName(
76-
FullyQualifiedName.add(entity.getOwners().get(0).getName(), entity.getName()));
77-
return;
78-
}
62+
// FQN is the (immutable) memory name. Deriving it from mutable fields such as
63+
// primaryEntity or owners would change nameHash on update, risking unique-constraint
64+
// collisions and orphaned references. The link to primaryEntity/owners is captured
65+
// via the relationship table instead.
7966
entity.setFullyQualifiedName(entity.getName());
8067
}
8168

@@ -138,13 +125,15 @@ public void storeRelationships(ContextMemory entity) {
138125
Relationship.RELATED_TO);
139126
}
140127

128+
// Distinct relationship types so the root-ancestor and direct-parent hierarchies
129+
// can be resolved independently when read back from the relationship table.
141130
if (entity.getRootMemory() != null) {
142131
addRelationship(
143132
entity.getRootMemory().getId(),
144133
entity.getId(),
145134
CONTEXT_MEMORY_ENTITY,
146135
CONTEXT_MEMORY_ENTITY,
147-
Relationship.RELATED_TO);
136+
Relationship.CONTAINS);
148137
}
149138

150139
if (entity.getParentMemory() != null) {
@@ -153,7 +142,7 @@ public void storeRelationships(ContextMemory entity) {
153142
entity.getId(),
154143
CONTEXT_MEMORY_ENTITY,
155144
CONTEXT_MEMORY_ENTITY,
156-
Relationship.RELATED_TO);
145+
Relationship.HAS);
157146
}
158147
}
159148

@@ -185,7 +174,11 @@ public static void validateStatusTransition(ContextMemoryStatus from, ContextMem
185174
return; // No change
186175
}
187176
Set<ContextMemoryStatus> allowed = VALID_TRANSITIONS.get(from);
188-
if (allowed == null || !allowed.contains(to)) {
177+
if (allowed == null) {
178+
throw new BadRequestException(
179+
String.format("No transitions defined for status %s", from.value()));
180+
}
181+
if (!allowed.contains(to)) {
189182
throw new BadRequestException(
190183
String.format(
191184
"Invalid memory status transition from %s to %s. Allowed transitions from %s: %s",

openmetadata-service/src/test/java/org/openmetadata/service/resources/context/ContextMemoryResourceTest.java renamed to openmetadata-service/src/test/java/org/openmetadata/service/resources/context/ContextMemoryStatusTransitionTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import org.openmetadata.schema.entity.context.ContextMemoryStatus;
99
import org.openmetadata.service.jdbi3.ContextMemoryRepository;
1010

11-
class ContextMemoryResourceTest {
11+
class ContextMemoryStatusTransitionTest {
1212

1313
@Test
1414
void testValidStatusTransitionsAreAccepted() {

0 commit comments

Comments
 (0)