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

feat: add new API definitions for relation metadata & assignable types #27

Merged
merged 38 commits into from
Sep 14, 2022

Conversation

jon-whit
Copy link
Contributor

@jon-whit jon-whit commented Sep 1, 2022

Description

This starts the beginning of a new OpenFGA model package v2beta1 which contains the preliminary definitions for the type info in TypeDefinitions.

References

Review Checklist

  • 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

@jon-whit jon-whit requested a review from a team as a code owner September 1, 2022 16:30
@jon-whit
Copy link
Contributor Author

jon-whit commented Sep 1, 2022

The idea here would be use to v2beta1 to support the new TypeDefinition structure and once the old structure is deprecated we can remove it strictly in favor of the v2beta1 defininitions.

If the v2beta1 definitions stick long enough for us to commit to long term maintainability, then we can cut v2beta1 under an official v2 in the future.

@rhamzeh I recognize the impact of this on SDKs and tooling, but I think it's warranted to make the model of a TypeDefinition more clear and it also supports our use case for ListObjects more accurately.

Code in the server becomes:

typedef := datastore.ReadTypeDefinition("document")
relations := typedef.GetRelations()

relation, ok := relations["viewer"]
if !ok { // relation doesn't exist }

typeinfo := relation.GetTypeInfo()

What do you think? Can we proceed with this approach?

@craigpastro
Copy link
Contributor

By the way, should we bring all the proto files to v2beta1 so we don't need to have two imports everywhere?

@jon-whit
Copy link
Contributor Author

jon-whit commented Sep 1, 2022

By the way, should we bring all the proto files to v2beta1 so we don't need to have two imports everywhere?

Not all of them. If we evolve our grpc service API definitions then we can start a new openfga.v2beta1.OpenFGAService at a later date. For now I think it makes sense to have separate API definitions since they are under different packages. If we have a shared package between the two we can create a shared package instead.

Copy link
Contributor

@craigpastro craigpastro left a comment

Choose a reason for hiding this comment

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

LGTM

@craigpastro craigpastro self-requested a review September 6, 2022 19:01
Copy link
Contributor

@craigpastro craigpastro left a comment

Choose a reason for hiding this comment

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

LGTM

@craigpastro craigpastro merged commit 98031fd into main Sep 14, 2022
@craigpastro craigpastro deleted the openfga-v2beta1 branch September 14, 2022 00:42
adriantam added a commit to openfga/js-sdk that referenced this pull request Sep 29, 2022
…elRequest`

- BREAKING: exported type `TypeDefinitions` is now `WriteAuthorizationModelRequest`
    This is only a breaking change on the SDK, not the API. It was changed to conform to the proto changes in [openfga/api](openfga/api#27).
    It makes the type name more consistent and less confusing (normally people would incorrectly assume TypeDefinitions = TypeDefinition[]).
- chore(deps): upgrade dependencies
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.

5 participants