Add core proto message and replace v0 usage #449
Conversation
samkim
commented
Mar 4, 2022
•
samkim
commented
- Add a copy of v0 core and namespace proto messages under core/v1
- Update interfaces and method signatures to use core where possible
- Introduce message converters (v0 -> core, core -> v0) as needed for existing v0 method calls
github-actions
bot
added
area/api v0
samkim
force-pushed
the
core-protos
branch
7 times, most recently
from
0087326
to
44e1e24
Compare
| @@ -4,7 +4,7 @@ package dispatch.v1; | |||
| option go_package = "github.com/authzed/spicedb/pkg/proto/dispatch/v1"; | |||
|
|
|||
| import "validate/validate.proto"; | |||
| import "authzed/api/v0/core.proto"; | |||
| import "core/v1/core.proto"; | |||
There was a problem hiding this comment.
Dispatch proto definitions updated to reference core messages
| import "google/protobuf/any.proto"; | ||
| import "validate/validate.proto"; | ||
|
|
||
| message RelationTuple { |
There was a problem hiding this comment.
core has copies of v0 core and v0 namespace message defintions
| @@ -1,7 +1,7 @@ | |||
| --- | |||
| version: "v1" | |||
| deps: | |||
| - "buf.build/beta/protoc-gen-validate" | |||
| - "buf.build/envoyproxy/protoc-gen-validate:bb405eae115246f0b5ccf8997136e3d8" | |||
There was a problem hiding this comment.
Pin to the specific version as of the date this file was last updated
|
|
||
| //revive:enable | ||
|
|
||
| func panicOnError(err error) { |
There was a problem hiding this comment.
Attempt at a shared error handler for all the marshal/unmarshal calls
samkim
force-pushed
the
core-protos
branch
4 times, most recently
from
09c071a
to
bee58a1
Compare
| breaking: | ||
| use: | ||
| - "WIRE" | ||
| ignore_only: | ||
| FIELD_WIRE_COMPATIBLE_TYPE: | ||
| - "dispatch/v1/dispatch.proto" # TODO: Remove after core v1 is merged |
There was a problem hiding this comment.
This ignore allows the conversion from v0 to core in the dispatch api to be merged
e2e/newenemy/newenemy_test.go
Outdated
| @@ -344,8 +345,8 @@ func checkDataNoNewEnemy(ctx context.Context, t testing.TB, schemaData []SchemaD | |||
| r1leader, r2leader := getLeaderNode(ctx, crdb[1].Conn(), exclude.Tuple), getLeaderNode(ctx, crdb[1].Conn(), direct.Tuple) | |||
| ns1Leader := getLeaderNodeForNamespace(ctx, crdb[1].Conn(), exclude.Tuple.ObjectAndRelation.Namespace) | |||
| ns2Leader := getLeaderNodeForNamespace(ctx, crdb[1].Conn(), exclude.Tuple.User.GetUserset().Namespace) | |||
| z1, _ := zookie.DecodeRevision(r1.GetRevision()) | |||
| z2, _ := zookie.DecodeRevision(r2.GetRevision()) | |||
| z1, _ := zookie.DecodeRevision(core.CoreZookie(r1.GetRevision())) | |||
There was a problem hiding this comment.
Might be worth naming these slightly differently, like V0ToCoreZookie
There was a problem hiding this comment.
I actually started with the XToY naming but it got hard to read so I landed on just the desired type as the name.
There was a problem hiding this comment.
Then maybe just ToCoreZookie? The current names are a bit ambiguous to me
pkg/proto/core/v1/util.go
Outdated
| "type.googleapis.com/impl.v1.RelationMetadata": {}, | ||
| } | ||
|
|
||
| //revive:enable |
There was a problem hiding this comment.
Moved to a separate file and updated the lint ignores for the name of the map. Both revive and stylecheck complain about the var name format.
pkg/proto/core/v1/util.go
Outdated
| } | ||
| } | ||
|
|
||
| // Core |
There was a problem hiding this comment.
Remove and add doc comments on all exported functions
samkim
force-pushed
the
core-protos
branch
2 times, most recently
from
1bd9b2f
to
8cf9d46
Compare
| func CoreRelationTupleTreeNode(treenode *v0.RelationTupleTreeNode) *RelationTupleTreeNode { | ||
| coreNode := RelationTupleTreeNode{} | ||
| bytes, err := proto.Marshal(treenode) | ||
| defer panicOnError(err) | ||
| panicOnError(err) |
There was a problem hiding this comment.
newline after each panicOnError
pkg/proto/core/v1/util.go
Outdated
| return &coreRef | ||
| } | ||
|
|
||
| // CoreNamespaceDefinition converts the input to a core NamespaceDefinition. | ||
| func CoreNamespaceDefinition(def *v0.NamespaceDefinition) *NamespaceDefinition { |
There was a problem hiding this comment.
Add To in front of all of these
There was a problem hiding this comment.
Those should've been renamed in the last push but I'll double check
- Create core manual validation file - Add go docs
