-
Notifications
You must be signed in to change notification settings - Fork 2
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.
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
NickAkhmetov/CAT-888 Improve combined provenance logic to safeguard against duplicate nodes/edges #3535
NickAkhmetov/CAT-888 Improve combined provenance logic to safeguard against duplicate nodes/edges #3535
Changes from 4 commits
59478e7
59d4a69
1ef2e6f
efc20c2
28ee755
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
- Improve combined provenance logic to safeguard against duplicate nodes/edges. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,18 @@ export function createProvDataURL(uuid: string, entityEndpoint: string) { | |
return `${entityEndpoint}/entities/${uuid}/provenance`; | ||
} | ||
|
||
function findDifference<T extends Record<string, unknown>, U extends Record<string, unknown>>(obj1: T, obj2: U) { | ||
const diffKeys = Object.keys({ ...obj1, ...obj2 }).reduce((acc, key) => { | ||
if (key in obj1 && key in obj2 && obj1[key] === obj2[key]) { | ||
return acc; | ||
} | ||
return [...acc, key]; | ||
}, [] as string[]); | ||
return diffKeys; | ||
} | ||
|
||
const ignoredFields = ['hubmap:last_modified_timestamp', 'hubmap:published_timestamp', 'hubmap:created_timestamp']; | ||
|
||
/** | ||
* Combines ProvData objects without overwriting properties or inserting unnecessary duplicates. | ||
* @param a the first ProvData object | ||
|
@@ -21,11 +33,41 @@ export function nonDestructiveMerge<T extends Record<string, unknown>, U extends | |
): T { | ||
const merged = { ...a } as Record<string, unknown>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we provide stricter types for the provenance data? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The types are currently provided via the generics, i.e. this function gets called with |
||
Object.entries(b).forEach(([key, value]) => { | ||
// Check for duplicate provenance edges by comparing entity and activity IDs | ||
if (value && typeof value === 'object' && 'prov:entity' in value && 'prov:activity' in value) { | ||
if ( | ||
Object.values(merged).some( | ||
(v) => | ||
typeof v === 'object' && | ||
v && | ||
'prov:entity' in v && | ||
'prov:activity' in v && | ||
v['prov:entity'] === value['prov:entity'] && | ||
v['prov:activity'] === value['prov:activity'], | ||
) | ||
) { | ||
return; | ||
} | ||
} | ||
|
||
if (merged[key]) { | ||
// Confirm that properties are actually different | ||
// Heuristic to check if the objects are the same | ||
if (JSON.stringify(merged[key]) === JSON.stringify(value)) { | ||
return; | ||
} | ||
|
||
if (merged[key] && typeof merged[key] === 'object' && value && typeof value === 'object') { | ||
// Perform a more in-depth check to see if objects are the same when excluding certain fields | ||
const difference = findDifference( | ||
merged[key] as Record<string, unknown>, | ||
value as Record<string, unknown>, | ||
).filter((diffField) => !ignoredFields.includes(diffField)); | ||
|
||
if (difference.length === 0) { | ||
return; | ||
} | ||
} | ||
|
||
merged[`${key}-${keyMod}`] = value; | ||
} else { | ||
merged[key] = value; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this avoid the assertion below?