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

chore: add test to cover not sending empty array when other has data #306

Merged
merged 1 commit into from
Feb 12, 2024

Conversation

ewanharris
Copy link
Member

Description

For the Go, JS, and Python SDKs make passing no deletes or writes to the write command perform a no-op and just return an empty response

References

Part of #299
SDK PRs to come shortly

Review Checklist

  • I have clicked on "allow edits by maintainers".
  • I have added documentation for new/changed functionality in this PR or in a PR to openfga.dev [Provide a link to any relevant PRs in the references section above]
  • The correct base branch is being used, if not main
  • I have added tests to validate that the change in functionality is working as expected

sergiught
sergiught previously approved these changes Feb 9, 2024
Copy link
Member

@rhamzeh rhamzeh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed offline, we're aiming not to have the app short circuit when both Writes and Deletes are not provided, instead we want to add test and ensure that if they are an empty array, they are not sent (as fields) to the API.

So:

OpenFgaClient User Request Sent to the API
Write(writes=[tupleA], deletes=[tupleB]) Write(writes=[tupleA], deletes=[tupleB])
Write(writes=[], deletes=[tupleA]) Write(deletes=[tupleA])
Write(writes=[tupleA, deletes=[]) Write(writes=[tupleA])
Write(writes=[], deletes=[]) Write()

@ewanharris ewanharris force-pushed the feat/299-js-go-noop-empty branch from c9a600c to 39b6c48 Compare February 12, 2024 19:33
@ewanharris ewanharris changed the title feat: make write with no writes or deletes a no-op chore: add test to cover not sending empty array when other has data Feb 12, 2024
@ewanharris ewanharris force-pushed the feat/299-js-go-noop-empty branch from 39b6c48 to acf8eac Compare February 12, 2024 19:41
@ewanharris
Copy link
Member Author

@rhamzeh I've updated the PR to include a Delete in tests for dotnet, go, js, and python.

Currently this isn't really valuable in SDKs where we're not matching the body in mocks, I'll spend some time tomorrow seeing if I can improve SDKs other than python a little further to validate this better.

@rhamzeh rhamzeh added this pull request to the merge queue Feb 12, 2024
Merged via the queue into main with commit ded14d3 Feb 12, 2024
13 checks passed
@rhamzeh rhamzeh deleted the feat/299-js-go-noop-empty branch February 12, 2024 20:47
github-merge-queue bot pushed a commit to openfga/dotnet-sdk that referenced this pull request Feb 12, 2024
…44)

## Description

Adds the `Deletes` property to the test to ensure that we don't pass
this when it is empty. Given that we don't match on body this isn't
really much gain but it will help to ensure we maintain this in future
if we improve testing.

## References

Part of openfga/sdk-generator#299
Generated from openfga/sdk-generator#306

## Review Checklist
- [x] I have clicked on ["allow edits by
maintainers"](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork).
- [ ] I have added documentation for new/changed functionality in this
PR or in a PR to [openfga.dev](https://github.com/openfga/openfga.dev)
[Provide a link to any relevant PRs in the references section above]
- [x] The correct base branch is being used, if not `main`
- [ ] I have added tests to validate that the change in functionality is
working as expected
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.

3 participants