-
Notifications
You must be signed in to change notification settings - Fork 15
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
add support for using hasKey from graphObjectStore #1068
base: main
Are you sure you want to change the base?
Conversation
@@ -66,3 +66,68 @@ export class InMemoryGraphObjectStore implements GraphObjectStore { | |||
return Promise.resolve(); | |||
} | |||
} | |||
|
|||
export class InMemoryGraphObjectStoreWithHasKeyImpl |
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.
Why not add hasKey
to the existing ObjectStore above?
throw new IntegrationDuplicateKeyError( | ||
`Duplicate _key detected (_key=${entity._key})`, | ||
); | ||
// if this add entities contains multiple entities we need to verify it doesn't contain any duplicates within |
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.
The logic around this new set is messy. It introduces complexity where the duplicateKeyTracker was very straightforward. Can we move this complexity to a new implementation of the duplicateKeyTracker?
It could first check the graphObjectStore and then have a temp store that would be exist while in this loop that takes the place of this new set.
It's not a blocker but seems like we could refine the approach?
if (hasHasKeyFn) { | ||
return graphObjectStore.hasKey!(_key); | ||
} else { | ||
return duplicateKeyTracker.hasKey(_key); |
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.
This PR does not modify the adding of a key to the duplicateKeyTracker
only the use of the hasKey. Follow up PR to no longer add to the duplicateKeyTracker
is coming?
I ask because we use the duplicateKeyTracker
in the above findEntity method.
Description
This PR adds support for using a
hasKey
implementation from thegraphObjectStore
rather than theDuplicateKeyTracker
if it is available.