ARC-1699: Spicegen gRPC client binding #2
Conversation
thomasrichner-oviva
force-pushed
the
feature/arc-1699
branch
2 times, most recently
from
d6af8ab to
6cec415
Compare
thomasrichner-oviva
force-pushed
the
feature/arc-1699
branch
from
6cec415 to
571f5f0
Compare
| @@ -0,0 +1,15 @@ | |||
| package com.oviva.spicegen.api; | |||
|
|
|||
| public interface PermissionService { | |||
There was a problem hiding this comment.
Can extend in the future with e.g. checks etc.
There was a problem hiding this comment.
This is basically a copy of https://github.com/oviva-ag/permissions/blob/main/api/src/main/java/com/oviva/permissions/api/PermissionService.java if I understood it correctly?
| @@ -0,0 +1,120 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
There was a problem hiding this comment.
Added a full example. Testing with generated sources directly is a bit problematic, because support for generated test-sources is a bit lackluster ;)
| return permissionServiceStub.checkPermission( | ||
| com.authzed.api.v1.PermissionService.CheckPermissionRequest.newBuilder() | ||
| .setPermission(permission) | ||
| .setResource( | ||
| Core.ObjectReference.newBuilder() | ||
| .setObjectType(object.kind()) | ||
| .setObjectId(object.id()) | ||
| .build()) | ||
| .setSubject( | ||
| Core.SubjectReference.newBuilder() | ||
| .setObject( | ||
| Core.ObjectReference.newBuilder() | ||
| .setObjectType(subject.kind()) | ||
| .setObjectId(subject.id()) | ||
| .build()) | ||
| .build()) | ||
| .setConsistency( | ||
| com.authzed.api.v1.PermissionService.Consistency.newBuilder() | ||
| .setAtLeastAsFresh( | ||
| Core.ZedToken.newBuilder().setToken(consistencyToken).build()) | ||
| .build()) | ||
| .build()); |
There was a problem hiding this comment.
There's a good argument to also generate more of the permission checks ;)
This could be as short as the updates.
There was a problem hiding this comment.
Done! See #3
There was a problem hiding this comment.
Great, will also have a look once I'm done with this PR. But it already looks great!
generator/out/pom.xml
Outdated
There was a problem hiding this comment.
not needed, was just here for ease of test
thomasrichner-oviva
force-pushed
the
feature/arc-1699
branch
from
2ca7d0c to
c0ffee5
Compare
There was a problem hiding this comment.
Starts to look good left some basic comments. I noticed that many things in spicedb-binding have been copied over from permissions. That's also fine for now but I think in the future we should also add a note in the permissions repo to use the generator instead and "deprecate" things over there.
| @@ -0,0 +1,15 @@ | |||
| package com.oviva.spicegen.api; | |||
|
|
|||
| public interface PermissionService { | |||
There was a problem hiding this comment.
This is basically a copy of https://github.com/oviva-ag/permissions/blob/main/api/src/main/java/com/oviva/permissions/api/PermissionService.java if I understood it correctly?
| return permissionServiceStub.checkPermission( | ||
| com.authzed.api.v1.PermissionService.CheckPermissionRequest.newBuilder() | ||
| .setPermission(permission) | ||
| .setResource( | ||
| Core.ObjectReference.newBuilder() | ||
| .setObjectType(object.kind()) | ||
| .setObjectId(object.id()) | ||
| .build()) | ||
| .setSubject( | ||
| Core.SubjectReference.newBuilder() | ||
| .setObject( | ||
| Core.ObjectReference.newBuilder() | ||
| .setObjectType(subject.kind()) | ||
| .setObjectId(subject.id()) | ||
| .build()) | ||
| .build()) | ||
| .setConsistency( | ||
| com.authzed.api.v1.PermissionService.Consistency.newBuilder() | ||
| .setAtLeastAsFresh( | ||
| Core.ZedToken.newBuilder().setToken(consistencyToken).build()) | ||
| .build()) | ||
| .build()); |
There was a problem hiding this comment.
Great, will also have a look once I'm done with this PR. But it already looks great!
| import com.authzed.api.v1.Core; | ||
| import com.oviva.spicegen.api.UpdateRelationship; | ||
|
|
||
| public class CreateRelationshipUpdateToSpiceDBUpdateRelationshipMapper { |
There was a problem hiding this comment.
Why not just:
| public class CreateRelationshipUpdateToSpiceDBUpdateRelationshipMapper { | |
| public class UpdateRelationshipMapper { |
? :) Like all other mapper classes in this package.
|
Thanks both! I'll address Shivan's point in the follow-up PR> |
Changes:
spicedb-binding/- Moved the largely borrowedauthzed-clientimplementation from our permission library.example/- Basic example on how to use everything together.We can pull out more in the future, but let's keep it managable for now. PR is already huge.