Skip to content
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

fix: ensure existence of urn when calling BaseEntityResource::get #132

Merged
merged 15 commits into from
Nov 3, 2021

Conversation

jsdonn
Copy link
Contributor

@jsdonn jsdonn commented Oct 18, 2021

motivation: calling get() with an empty, non-null aspectNames parameter (i.e. []) results in a 404, even if the entity exists. the issue stems from the call to getInternalNonEmpty which filters out urn/aspect mappings where the value is empty (which it will be when aspectNames is empty). this results in calling get(0) on an empty map, which results in a 404.

the intended behavior should be:
urn doesn't exist - 404
urn exists -
* null aspects param - return all aspects
* empty aspects param - return base response with no aspects (i.e. only non-aspect fields)
this change checks that the urn exists in the db before calling one of the getInternal methods. additionally, because we do this exists check, we can now use getInternal instead of getInternalNonEmpty to get our intended behavior.

Checklist

  • The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
  • Links to related issues (if applicable)
  • Tests for the changes have been added/updated (if applicable)
  • Docs related to the changes have been added/updated (if applicable)

@jsdonn jsdonn requested a review from keremsahin1 October 18, 2021 23:12
@jywadhwani jywadhwani merged commit 1d6f311 into linkedin:master Nov 3, 2021
@jsdonn jsdonn deleted the exists_check branch October 27, 2022 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants