FIX #24374 - Data Contract at Data Product level

[pmbrull]

Describe your changes:

FIX #24374

• assets inherit the data contract from the data product
• even if an asset has a data contract, if some key fields are empty: terms of service, SLA, security, they are inherited
• semantic rules are merged between asset and data contract
• if an asset has multiple data products, it won't inherit the contract

Untitled.mov

I worked on ... because ...

Type of change:

• Bug fix
• Improvement
• New feature
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation

Checklist:

• I have read the CONTRIBUTING document.
• My PR title is Fixes <issue-number>: <short explanation>
• I have commented on my code, particularly in hard-to-understand areas.
• For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

---

Summary by Gitar

• Data contract inheritance:
  • Assets inherit contract properties from parent Data Product when belonging to exactly one DP
  • Multi-DP assets skip inheritance to avoid ambiguity
• Selective field merging:
  • termsOfUse, security, and sla inherited only if empty in asset contract
  • Semantic rules merged without duplicates (asset rules take precedence)
• Schema changes:
  • Added inherited boolean flag to DataContract, termsOfUse, security, sla, and SemanticsRule
  • Migrated existing termsOfUse from string to object with content and inherited fields
• Backend implementation:
  • getEffectiveDataContract() in DataContractRepository computes inherited contract
  • Updated /dataContracts/entity endpoint returns effective contract with inherited properties
• Frontend visual indicators:
  • Inheritance icons with tooltips on all inherited fields in ContractDetail, ContractSemantics, ContractSLA
  • Edit form filters out inherited fields to prevent accidental removal via PATCH

_{This will update automatically on new commits.}

---

[github-actions]

TypeScript types have been updated based on the JSON schema changes in the PR

[gitar-bot]

Code Review 👍 Approved with suggestions 11 resolved / 14 findings

Well-implemented data contract inheritance feature with comprehensive tests. The previous findings related to TypeScript type assertions and semantic rule deduplication by name remain unresolved.

Suggestions 💡 3 suggestions

Quality: Type assertion cascades weaken TypeScript safety

📄 openmetadata-ui/src/main/resources/ui/src/components/DataContract/AddDataContract/AddDataContract.tsx:1887 📄 openmetadata-ui/src/main/resources/ui/src/components/DataContract/AddDataContract/AddDataContract.tsx:1907

The frontend code uses multiple as unknown as type assertions throughout the inheritance filtering and display logic:

const ruleWithInherited = rule as unknown as { inherited?: boolean };
const securityWithInherited = contract.security as unknown as { inherited?: boolean };
result.termsOfUse = filteredTermsOfUse;  // Implicit type assertion

These assertions bypass TypeScript's type checking and could hide bugs if the API response structure changes.

Impact: If the inherited field is renamed or restructured in the backend, the frontend will silently fail to detect inherited items.

Suggested fix: Update the generated TypeScript types to include the inherited field properly (the generated types already have it based on the diff). Then use proper type guards:

function hasInheritedFlag(obj: unknown): obj is { inherited?: boolean } {
    return typeof obj === 'object' && obj !== null && 'inherited' in obj;
}

Quality: Multiple type assertions through `unknown` reduce TypeScript safety

📄 openmetadata-ui/src/main/resources/ui/src/components/DataContract/AddDataContract/AddDataContract.tsx

The code uses as unknown as type assertions in multiple places when handling the dual format of termsOfUse (string vs object):

(formValues.termsOfUse as unknown as { content?: string })?.content
(formValues.termsOfUse as unknown as string)

While type guards are used to validate fields before accessing them, these assertions bypass TypeScript's type checking. Consider:

1. Creating a proper union type for the termsOfUse field
2. Using discriminated unions or a custom type guard that returns the properly typed value
3. Using a helper function like getTermsOfUseContent(termsOfUse: string | TermsOfUse | undefined): string | undefined

This would make the code more maintainable and catch type errors at compile time.

Edge Case: Semantic rules deduplication uses name only, ignoring rule content

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/DataContractRepository.java

In mergeContracts(), semantic rules from the data product are skipped if an entity rule has the same name, using entityRuleNames.contains(dpRule.getName()). However, this only checks the rule name, not the rule content or logic.

If an entity has a rule named "DataQuality" with one definition, and the data product has a rule named "DataQuality" with a different definition, the entity's rule wins entirely. This may be the intended behavior (as stated in the PR - "asset rules take precedence"), but there's no way to detect or warn about this conflict.

Consider logging when a rule name collision is detected so administrators can understand why certain data product rules aren't being applied.

Resolved ✅ 11 resolved

Edge Case: Unreachable code in data product iteration loop

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/DataContractRepository.java:93
The getEffectiveDataContract method has a for-loop that iterates over dataProducts, but the early return above guarantees dataProducts.size() == 1 when reaching the loop (returns early if size > 1).

The loop body also breaks on the first successful contract load, meaning it can only ever execute once. This is misleading code.

Impact: Code is functionally correct but unnecessarily complex and confusing for future maintainers.

Suggested fix: Replace the loop with a direct reference to the single data product:

EntityReference dataProductRef = dataProducts.get(0);
try {
    dataProductContract = loadEntityDataContract(dataProductRef);
    if (dataProductContract != null
        && dataProductContract.getEntityStatus() != EntityStatus.APPROVED) {
        dataProductContract = null;
    }
} catch (Exception e) {
    LOG.debug("No contract found for data product {}: {}", dataProductRef.getId(), e.getMessage());
}

Edge Case: DataProduct itself may call getEffectiveDataContract recursively

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/DataContractRepository.java:74
The code now supports Entity.DATA_PRODUCT as a valid entity type for data contracts. When getEffectiveDataContract is called on a DataProduct entity, it will call entity.getDataProducts() on the DataProduct itself.

If a DataProduct could have a reference to itself or other data products in its dataProducts field, this could lead to unexpected behavior or even infinite recursion (if A references B which references A).

Impact: Likely low risk since DataProducts probably don't have dataProducts relationships, but this is an implicit assumption that isn't enforced.

Suggested fix: Add an early return for DataProduct entities:

if (Entity.DATA_PRODUCT.equals(entity.getEntityReference().getType())) {
    return entityContract; // DataProducts don't inherit from other DataProducts
}

Bug: Missing null check for contract.termsOfUse in frontend filtering

📄 openmetadata-ui/src/main/resources/ui/src/components/DataContract/AddDataContract/AddDataContract.tsx:1892
In the AddDataContract.tsx component, when determining if termsOfUse should be included in the filtered contract, the code checks isInherited(contract.termsOfUse) but the isInherited function expects a non-null object.

While the function handles null safely (returns false for null), the logic flow in the code block starting at line 1892 does a type check after the inherited check:

if (isInherited(contract.termsOfUse)) {
    filteredTermsOfUse = undefined;
} else if (typeof contract.termsOfUse === 'object' && contract.termsOfUse !== null) {

This is correct, but the code is fragile. If termsOfUse is a string (old format), it won't match either case and will be undefined.

Impact: Low - the old string format case is handled in the else branch, but this is subtle logic that could cause issues during the migration period.

Suggested fix: Consider making the isInherited function more explicit about what it returns for different input types, or add a comment explaining the flow.

Bug: Backend modifies entityContract directly in mergeContracts

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/DataContractRepository.java:152
In DataContractRepository.mergeContracts(), the method directly modifies the entityContract parameter by calling setters like setTermsOfUse(), setSemantics(), setSecurity(), and setSla(). This mutates the original entity contract object which may have been retrieved from the database or cache.

Direct mutation of input parameters can cause:

1. Unexpected side effects if the same contract object is used elsewhere
2. Inconsistent state if the merge operation fails midway
3. Potential cache corruption if the object came from a cached source

Suggested fix: Create a deep copy of the entity contract at the start of mergeContracts() before modifying it:

private DataContract mergeContracts(
    DataContract entityContract, DataContract dataProductContract) {
  // Create a copy to avoid mutating the original
  DataContract mergedContract = JsonUtils.deepCopy(entityContract, DataContract.class);
  
  // Then use mergedContract for all modifications
  if (mergedContract.getTermsOfUse() == null && dataProductContract.getTermsOfUse() != null) {
    mergedContract.setTermsOfUse(...);
  }
  // ... rest of the merging logic
  return mergedContract;
}

Bug: formValues initialized once but filteredContract can change

📄 openmetadata-ui/src/main/resources/ui/src/components/DataContract/AddDataContract/AddDataContract.tsx:196
The formValues state is initialized with filteredContract but only on component mount:

const filteredContract = useMemo(() => {...}, [contract]);

const [formValues, setFormValues] = useState<DataContract>(
  filteredContract || ({} as DataContract)
);

If the contract prop changes (e.g., after a refresh or when inheritance status changes), filteredContract will recalculate, but formValues will retain the stale initial value. This could cause the edit form to show outdated data or lose user changes when the underlying contract updates.

Consider using a useEffect to reset formValues when filteredContract changes, or use a key prop to remount the component when the contract changes.

...and 6 more from earlier reviews

What Works Well

Strong architecture for contract inheritance with clear separation between inherited and materialized contracts. Thorough test coverage including edge cases like multi-data-product scenarios and execution status isolation. Good UX with visual inheritance indicators and proper filtering of inherited fields during editing.

Options ✅ Auto-apply

✅ Auto-apply is on Gitar will commit updates to this branch.
Display: compact Hiding non-applicable rules.

Comment with these commands to change:

✅ Auto-apply	Compact
gitar auto-apply:off	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | This comment will update automatically (Docs)}

[sonarqubecloud]

Quality Gate passed for 'open-metadata-ui'

Issues
6 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud