From a43421f2b727397751ed52edfb36900697d1d01f Mon Sep 17 00:00:00 2001 From: Callum Seabrook Date: Thu, 13 Jul 2023 21:15:03 +0100 Subject: [PATCH 1/2] Added proto layout specification --- src/proto/README.md | 101 ++++++++++++++++++ src/proto/account_conn_manager/messages.proto | 21 ---- .../grpc.proto | 19 ++-- src/proto/account_connection/messages.proto | 20 ++++ .../models.proto | 6 +- src/proto/badges/grpc.proto | 36 +++---- src/proto/badges/models.proto | 1 - 7 files changed, 152 insertions(+), 52 deletions(-) create mode 100644 src/proto/README.md delete mode 100644 src/proto/account_conn_manager/messages.proto rename src/proto/{account_conn_manager => account_connection}/grpc.proto (70%) create mode 100644 src/proto/account_connection/messages.proto rename src/proto/{account_conn_manager => account_connection}/models.proto (76%) diff --git a/src/proto/README.md b/src/proto/README.md new file mode 100644 index 0000000..a8d90c9 --- /dev/null +++ b/src/proto/README.md @@ -0,0 +1,101 @@ +# Protobuf specification format + +This file is intended to provide a guide on how to name and structure Protobuf declarations in this repository. + +It is designed to make sure that all our declarations are consistent, and that the practices are well documented, +well understood, and easy to follow, so that everyone stays on the same page as to how things are meant to work, +and the designs are not completely inconsistent. + +This guide extends from the official style guide, in that everything outlined in the official style guide is also +recommended to be used here. As such, anything included in the official style guide has been omitted here for the +sake of brevity. + +The official style guide can be found [here](https://protobuf.dev/programming-guides/style/). + +Every file in this repository should use the proto version 3 syntax. + +# ⚠️ TODOs ⚠️ + +## Service naming + +Every service should be named in PascalCase. The use of generic terms such as 'Service' or 'Manager' in the +service name should be avoided. + +## Directory structure + +Every service should have its own directory, where only its declarations live. This directory should be named the +same as the service, so the declarations are easy to find when searching through the repository. + +## File naming + +Each service is generally made up of three components: the service RPC declarations, the models that are used by the +service to model data, and the messages (events) that can be sent by the service over Kafka. + +These three components should be separated into their own files, and named as follows: +* RPC declarations: `grpc.proto`. This file should contain all service declarations, and any messages required for + the service, such as requests, responses, and error responses. +* Models: `models.proto`. This file should contain any data models used by the service, such as a user, player, or + game type. +* Messages: `messages.proto`. This file should contain any Kafka messages that may be sent by the service to indicate + that something has happened, such as a user being created, or a game being started. + +## Package naming + +Every package name in every proto file should be prefixed with the namespace `emortal`. This is for two reasons: +1. The namespace is unique, and so there is no chance of conflict with dependent packages. +2. It ensures that there is a consistent place to find all generated code. In Java, for example, all the code will + be located in the `dev.emortal.api` package, under various consistent sub-packages listed below. + +In addition, the package naming should follow this format: `emortal..`, where the component is +either `grpc`, `model`, or `message`, and the service is the name of the service that the declarations are for. + +For example, the gRPC declarations for a service called `user` would be in the package `emortal.grpc.user`. + +The service part of the package name should be in lower snake case, for the Protobuf package. Generated packages +are different, and are covered below. + +### Generated code + +The package naming for the generated code is different depending on the language. The format is as follows +for each language: +* Java: `dev.emortal.api..` +* Go: `github.com/emortalmc/proto-specs/gen/go//` + +The service part of the generated package name should be as follows for each language: +* Java: concatenate each word in the service name together, rather than separating them with underscores, as is + recommended by the Google Java style guide, which is recommended for usage in + the [Proto Best Practices](https://protobuf.dev/programming-guides/dos-donts/). +* Go: concatenate each word in the service name together, rather than separating them with underscores, as is + recommended by [Effective Go](https://go.dev/doc/effective_go#package-names). + +## Documentation + +All elements should be well documented, as advised in the [API Best Practices](https://protobuf.dev/programming-guides/api/). + +The general rule of thumb is try to document things like they are going to be read by someone who has no idea how +any of the system works, and is trying to figure it out. + +Every RPC method should have a description of the expected behaviour, and should document any errors that may occur +as part of expected behaviour, for example, invalid parameters, or a user not being found. + +## Generated code options + +### Java + +TODO - apply this naming convention everywhere + +For gRPC declarations, the option `java_outer_classname` should be set to the name of the service, in pascal case, +with the suffix "Proto" appended to it. For example, the name for a service called "User" +would be "UserProto". + +For other declarations, the option `java_multiple_files` should be used instead, which will generate a separate +Java file for each top-level declaration. + +## Errors + +Always prefer to use a pre-existing status code over a custom error type. Not only are they more consistent and +widely known, they are also a lot simpler to handle for client code, as they only have to check the status. + +If none of the pre-existing status codes fit the error that is being returned, then a custom error response should +be defined, containing an enum type with error status codes, along with any data that may be useful for clients +to debug the error. diff --git a/src/proto/account_conn_manager/messages.proto b/src/proto/account_conn_manager/messages.proto deleted file mode 100644 index e7029ba..0000000 --- a/src/proto/account_conn_manager/messages.proto +++ /dev/null @@ -1,21 +0,0 @@ -syntax = "proto3"; -package emortal.message.account_conn_manager; - -option java_package = "dev.emortal.api.message.accountconnmanager"; -option java_multiple_files = true; -option go_package = "github.com/emortalmc/proto-specs/gen/go/message/accountconnmanager"; - -import "account_conn_manager/models.proto"; - -// AccountConnectedMessage is sent when a connection is successful, not when the OAuth URL is created. -// NOTE: multiple connection types my be present (e.g. if it's the first link there might be MINECRAFT and DISCORD) -message AccountConnectedMessage { - model.account_conn_manager.ConnectionUser user = 1; - repeated model.account_conn_manager.ConnectionType connection_types = 2; - -} - -message AccountConnectionRemovedMessage { - model.account_conn_manager.ConnectionUser user = 1; - model.account_conn_manager.ConnectionType removed_connection = 2; -} diff --git a/src/proto/account_conn_manager/grpc.proto b/src/proto/account_connection/grpc.proto similarity index 70% rename from src/proto/account_conn_manager/grpc.proto rename to src/proto/account_connection/grpc.proto index 3ca9680..9da9c5c 100644 --- a/src/proto/account_conn_manager/grpc.proto +++ b/src/proto/account_connection/grpc.proto @@ -1,13 +1,13 @@ syntax = "proto3"; -package emortal.grpc.account_conn_manager; +package emortal.grpc.account_connection; -option java_package = "dev.emortal.api.grpc.accountconnmanager"; -option java_outer_classname = "AccountConnManagerProto"; -option go_package = "github.com/emortalmc/proto-specs/gen/go/grpc/accountconnmanager"; +option java_package = "dev.emortal.api.grpc.accountconnection"; +option java_outer_classname = "AccountConnectionProto"; +option go_package = "github.com/emortalmc/proto-specs/gen/go/grpc/accountconnection"; -import "account_conn_manager/models.proto"; +import "account_connection/models.proto"; -service AccountConnectionManager { +service AccountConnection { rpc GetUser(GetUserRequest) returns (GetUserResponse); rpc CreateOAuthLink(CreateOAuthLinkRequest) returns (CreateOAuthLinkResponse); } @@ -21,7 +21,7 @@ message GetUserRequest { } message GetUserResponse { - model.account_conn_manager.ConnectionUser user = 1; + model.account_connection.ConnectionUser user = 1; } // CreateOAuthLinkRequest requests an OAuth URL to link an account to another. @@ -32,7 +32,7 @@ message CreateOAuthLinkRequest { int64 source_github_id = 3; } - model.account_conn_manager.ConnectionType target_connection = 4; + model.account_connection.ConnectionType target_connection = 4; } message CreateOAuthLinkResponse { @@ -45,5 +45,6 @@ message CreateOAuthLinkErrorResponse { // CONNECTION_ALREADY_EXISTS note this is only that this user already has this connection, we don't know the target conn ID yet. CONNECTION_ALREADY_EXISTS = 1; } -} + Reason reason = 1; +} diff --git a/src/proto/account_connection/messages.proto b/src/proto/account_connection/messages.proto new file mode 100644 index 0000000..35bfb0c --- /dev/null +++ b/src/proto/account_connection/messages.proto @@ -0,0 +1,20 @@ +syntax = "proto3"; +package emortal.message.account_connection; + +option java_package = "dev.emortal.api.message.accountconnection"; +option java_multiple_files = true; +option go_package = "github.com/emortalmc/proto-specs/gen/go/message/accountconnection"; + +import "account_connection/models.proto"; + +// AccountConnectedMessage is sent when a connection is successful, not when the OAuth URL is created. +// NOTE: multiple connection types my be present (e.g. if it's the first link there might be MINECRAFT and DISCORD) +message AccountConnectedMessage { + model.account_connection.ConnectionUser user = 1; + repeated model.account_connection.ConnectionType connection_types = 2; +} + +message AccountConnectionRemovedMessage { + model.account_connection.ConnectionUser user = 1; + model.account_connection.ConnectionType removed_connection = 2; +} diff --git a/src/proto/account_conn_manager/models.proto b/src/proto/account_connection/models.proto similarity index 76% rename from src/proto/account_conn_manager/models.proto rename to src/proto/account_connection/models.proto index 5453e18..844f0a1 100644 --- a/src/proto/account_conn_manager/models.proto +++ b/src/proto/account_connection/models.proto @@ -1,9 +1,9 @@ syntax = "proto3"; -package emortal.model.account_conn_manager; +package emortal.model.account_connection; -option java_package = "dev.emortal.api.model.accountconnmanager"; +option java_package = "dev.emortal.api.model.accountconnection"; option java_multiple_files = true; -option go_package = "github.com/emortalmc/proto-specs/gen/go/model/accountconnmanager"; +option go_package = "github.com/emortalmc/proto-specs/gen/go/model/accountconnection"; message ConnectionUser { // minecraft_id of type UUID diff --git a/src/proto/badges/grpc.proto b/src/proto/badges/grpc.proto index baf4114..d36e0a0 100644 --- a/src/proto/badges/grpc.proto +++ b/src/proto/badges/grpc.proto @@ -2,12 +2,15 @@ syntax = "proto3"; package emortal.grpc.badge; option java_package = "dev.emortal.api.grpc.badge"; -option java_outer_classname = "BadgeManagerProto"; +option java_outer_classname = "BadgeManagerProto"; // TODO: Update this to "BadgeProto" option go_package = "github.com/emortalmc/proto-specs/gen/go/grpc/badge"; import "badges/models.proto"; +// TODO: Rename this to just "Badge" and update corresponding code service BadgeManager { + // GetBadges gets all the registered badges + rpc GetBadges(GetBadgesRequest) returns (GetBadgesResponse) {} rpc GetPlayerBadges(GetPlayerBadgesRequest) returns (GetPlayerBadgesResponse) {} rpc GetActivePlayerBadge(GetActivePlayerBadgeRequest) returns (GetActivePlayerBadgeResponse) {} @@ -15,9 +18,12 @@ service BadgeManager { rpc AddBadgeToPlayer(AddBadgeToPlayerRequest) returns (AddBadgeToPlayerResponse) {} rpc RemoveBadgeFromPlayer(RemoveBadgeFromPlayerRequest) returns (RemoveBadgeFromPlayerResponse) {} +} - // GetBadges gets all the registered badges - rpc GetBadges(GetBadgesRequest) returns (GetBadgesResponse) {} +message GetBadgesRequest {} + +message GetBadgesResponse { + repeated model.badge.Badge badges = 1; } message GetPlayerBadgesRequest { @@ -32,6 +38,15 @@ message GetPlayerBadgesResponse { optional string active_badge_id = 2; } +message GetActivePlayerBadgeRequest { + // player_id of type UUID + string player_id = 1; +} + +message GetActivePlayerBadgeResponse { + model.badge.Badge badge = 1; +} + message SetActivePlayerBadgeRequest { // player_id of type UUID string player_id = 1; @@ -48,15 +63,6 @@ message SetActivePlayerBadgeErrorResponse { Reason reason = 1; } -message GetActivePlayerBadgeRequest { - // player_id of type UUID - string player_id = 1; -} - -message GetActivePlayerBadgeResponse { - model.badge.Badge badge = 1; -} - message AddBadgeToPlayerRequest { // player_id of type UUID string player_id = 1; @@ -88,9 +94,3 @@ message RemoveBadgeFromPlayerErrorResponse { Reason reason = 1; } - -message GetBadgesRequest {} - -message GetBadgesResponse { - repeated model.badge.Badge badges = 1; -} diff --git a/src/proto/badges/models.proto b/src/proto/badges/models.proto index 9fb7189..0bd4213 100644 --- a/src/proto/badges/models.proto +++ b/src/proto/badges/models.proto @@ -25,4 +25,3 @@ message Badge { GuiItem gui_item = 7; } - From a5aba16b25f557dcef2a85c8d2d9615fa27a1c68 Mon Sep 17 00:00:00 2001 From: Zak Shearman <34372536+ZakShearman@users.noreply.github.com> Date: Thu, 13 Jul 2023 22:20:05 +0100 Subject: [PATCH 2/2] rewrite all docs/readme --- CONTRIBUTING.md | 19 ++++++++ docs/PROTO_STYLE.md | 78 ++++++++++++++++++++++++++++++++ src/proto/README.md | 106 +++++--------------------------------------- 3 files changed, 107 insertions(+), 96 deletions(-) create mode 100644 CONTRIBUTING.md create mode 100644 docs/PROTO_STYLE.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..3104bfa --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,19 @@ +# How to Contribute + +We'd love to accept your contributions to this project - there are just a few small guidelines you need to follow. + +## Submitting code via pull requests +* All code must be submitted via pull requests. +* We follow the [GitHub Pull Request Model](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests) for all contrubitions. +* For large bodies of work, please open an issue first to discuss the work with the team describing the feature you +would like to build and how you plan to implement it. +* All submissions will require review before being merged. +* Please follow our style guides for [**Java**](todo), [**Golang**](todo) and [**Protobuf**](docs/PROTO_STYLE.md). + +## Additional instructions for protobuf changes +Make sure to run the `gen/generate.sh` script before committing any changes. This will ensure changes to protobuf +files are reflected in the generated code. Java code is auto-generated on gradle build. + +## Project Layout + +Protobuf files are located in the `src/proto` directory. \ No newline at end of file diff --git a/docs/PROTO_STYLE.md b/docs/PROTO_STYLE.md new file mode 100644 index 0000000..dada0fa --- /dev/null +++ b/docs/PROTO_STYLE.md @@ -0,0 +1,78 @@ +# Protobuf Style Guide + +This guide extends from the official style guide, in that everything outlined in the official style guide is also +recommended to be used here. As such, anything included in the official style guide has been omitted here for the +sake of brevity. + +The official style guide can be found [here](https://protobuf.dev/programming-guides/style/). + +Every file in this repository should use the proto version 3 syntax. + +## Service naming + +* Every service should be named in PascalCase. +* The use of generic terms such as 'Service' or 'Manager' in the service name should be avoided (e.g. call it `McPlayer` not `McPlayerService`) + +## Directory structure + +* Every service should have its own directory where only its declarations live. +* This directory should be named the same as the service but in snake_cast (e.g. `mc_player`) + +## File naming + +Each service is generally made up of three components: +* service RPC declarations, requests/responses and error responses (`grpc.proto`) +* models that are used by the service to model data (`models.proto`), e.g. a user, player, or game type +* the messages (events) that can be sent by the service over Kafka (`messages.proto`), e.g. a user being created, or a game being started + +## Package naming + +### Protobuf +* The full name should be `emortal..` where the component is either `grpc`, `model`, or `message` +* The service part of the package name should be in lower snake_case + +An example of this would be for a service called McPlayer the package will be `emortal.grpc.mc_player` + +### Java +* The full name should be `dev.emortal.api..` where the component is either `grpc`, `model`, or `message` +* The service part of the package name should be in lower case with no spaces + +An example of this would be for a service called McPlayer the package will be `dev.emortal.api.grpc.mcplayer` + +### Go +* The full name should be `github.com/emortalmc/proto-specs/gen/go//` +* The service part of the package name should be in lower case with no spaces (same as Java) + +An example of this would be for a service called McPlayer the package will be `github.com/emortalmc/proto-specs/gen/go/grpc/mcplayer` + +## Documentation + +All elements should be well documented, as advised in the [API Best Practices](https://protobuf.dev/programming-guides/api/). + +The general rule of thumb is try to document things like they are going to be read by someone who has no idea how +any of the system works, and is trying to figure it out. + +## Generated Code Options + +### Java + +TODO - Apply this convention everywhere + +For gRPC declarations (`grpc.proto` file), the option `java_outer_classname` should be set to the name of the service (in pascal case), +with the suffix "Proto" appended to it. For example, the name for a service called "McPlayer" would be "McPlayerProto". + +For other declarations (`messages.proto` and `models.proto`), the option `java_multiple_files` should be used instead +which will generate a separate Java file for each message declaration. + +## Errors + +Always prefer using existing status codes over a custom error response, however, don't stretch it to a point of +ambiguity and don't use a combination of both status codes and custom error responses. Use custom error codes when: +* There are multiple errors that fit into the same status code +* There isn't a clear status code that fits the error +* You want to provide additional information or context to the error to the client + +If existing status codes aren't sufficient you should define your own error response named `ErrorResponse` +(e.g. `CreateUserErrorResponse`) and include it in the service's `grpc.proto` file. +It should be after the `Response` message. This message should contain an enum called `Reason` and be +specified in a field called `reason` at index 1. \ No newline at end of file diff --git a/src/proto/README.md b/src/proto/README.md index a8d90c9..7b3d403 100644 --- a/src/proto/README.md +++ b/src/proto/README.md @@ -1,101 +1,15 @@ -# Protobuf specification format +# EmortalMC proto-specs -This file is intended to provide a guide on how to name and structure Protobuf declarations in this repository. +This repository contains the protobuf specifications for all EmortalMC APIs. -It is designed to make sure that all our declarations are consistent, and that the practices are well documented, -well understood, and easy to follow, so that everyone stays on the same page as to how things are meant to work, -and the designs are not completely inconsistent. +## How to Contribute -This guide extends from the official style guide, in that everything outlined in the official style guide is also -recommended to be used here. As such, anything included in the official style guide has been omitted here for the -sake of brevity. +Please read out [contributing guidelines](CONTRIBUTING.md) for more information. -The official style guide can be found [here](https://protobuf.dev/programming-guides/style/). +## Project Layout -Every file in this repository should use the proto version 3 syntax. - -# ⚠️ TODOs ⚠️ - -## Service naming - -Every service should be named in PascalCase. The use of generic terms such as 'Service' or 'Manager' in the -service name should be avoided. - -## Directory structure - -Every service should have its own directory, where only its declarations live. This directory should be named the -same as the service, so the declarations are easy to find when searching through the repository. - -## File naming - -Each service is generally made up of three components: the service RPC declarations, the models that are used by the -service to model data, and the messages (events) that can be sent by the service over Kafka. - -These three components should be separated into their own files, and named as follows: -* RPC declarations: `grpc.proto`. This file should contain all service declarations, and any messages required for - the service, such as requests, responses, and error responses. -* Models: `models.proto`. This file should contain any data models used by the service, such as a user, player, or - game type. -* Messages: `messages.proto`. This file should contain any Kafka messages that may be sent by the service to indicate - that something has happened, such as a user being created, or a game being started. - -## Package naming - -Every package name in every proto file should be prefixed with the namespace `emortal`. This is for two reasons: -1. The namespace is unique, and so there is no chance of conflict with dependent packages. -2. It ensures that there is a consistent place to find all generated code. In Java, for example, all the code will - be located in the `dev.emortal.api` package, under various consistent sub-packages listed below. - -In addition, the package naming should follow this format: `emortal..`, where the component is -either `grpc`, `model`, or `message`, and the service is the name of the service that the declarations are for. - -For example, the gRPC declarations for a service called `user` would be in the package `emortal.grpc.user`. - -The service part of the package name should be in lower snake case, for the Protobuf package. Generated packages -are different, and are covered below. - -### Generated code - -The package naming for the generated code is different depending on the language. The format is as follows -for each language: -* Java: `dev.emortal.api..` -* Go: `github.com/emortalmc/proto-specs/gen/go//` - -The service part of the generated package name should be as follows for each language: -* Java: concatenate each word in the service name together, rather than separating them with underscores, as is - recommended by the Google Java style guide, which is recommended for usage in - the [Proto Best Practices](https://protobuf.dev/programming-guides/dos-donts/). -* Go: concatenate each word in the service name together, rather than separating them with underscores, as is - recommended by [Effective Go](https://go.dev/doc/effective_go#package-names). - -## Documentation - -All elements should be well documented, as advised in the [API Best Practices](https://protobuf.dev/programming-guides/api/). - -The general rule of thumb is try to document things like they are going to be read by someone who has no idea how -any of the system works, and is trying to figure it out. - -Every RPC method should have a description of the expected behaviour, and should document any errors that may occur -as part of expected behaviour, for example, invalid parameters, or a user not being found. - -## Generated code options - -### Java - -TODO - apply this naming convention everywhere - -For gRPC declarations, the option `java_outer_classname` should be set to the name of the service, in pascal case, -with the suffix "Proto" appended to it. For example, the name for a service called "User" -would be "UserProto". - -For other declarations, the option `java_multiple_files` should be used instead, which will generate a separate -Java file for each top-level declaration. - -## Errors - -Always prefer to use a pre-existing status code over a custom error type. Not only are they more consistent and -widely known, they are also a lot simpler to handle for client code, as they only have to check the status. - -If none of the pre-existing status codes fit the error that is being returned, then a custom error response should -be defined, containing an enum type with error status codes, along with any data that may be useful for clients -to debug the error. +* **Protobuf** files are located at `src/proto`. +* **Autogenerated Java code** is created by `gradlew generateProto` and run during `gradlew build`. +* **Additional Java code** is located at `src/java`. +* **Autogenerated Go code** is located at `gen/go`. +* **Additional Go code** is located at `gen/go/nongenerated`. \ No newline at end of file