Refactored protocol file structures and message names (#130)

haikalpribadi · web-flow · commit 32b98993572d · 2021-03-19T16:26:07.000Z
## What is the goal of this PR?

The previous file structure did not communicate clearly the architecture of our protocol definitions. We have Grakn Core specific protocol definitions, as well as Grakn Cluster specific ones. These protocol files and their message names were also not named accurately based on their domains. For example, `database.proto` existed twice in Cluster and Core protobuf packages. We also split messages that should be two different message types, such as `ClusterDatabase` and `ClusterDatabaseManager`.

## What are the changes implemented in this PR?

- Renamed `//protobuf/cluster/cluster.proto` to `cluster_server.proto`
- Renamed `//cluster/database.proto` to `//cluster/cluster_database.proto`
- Renamed `grakn.protocol.cluster.Database` to `ClusterDatabase`
- Split `ClusterDatabaseManager` out from `ClusterDatabase`
- Moved `//protobuf/cluster` out to its own package `//cluster`
- Split `//protobuf` into `//common` and `//core
- Set package address of all protocol definitions and their generated files to `grakn.protocol`
diff --git a/cluster/BUILD b/cluster/BUILD
@@ -20,19 +20,22 @@ package(default_visibility = ["//visibility:public"])
 load("@graknlabs_dependencies//tool/checkstyle:rules.bzl", "checkstyle_test")
 
 proto_library(
-    name = "grakn-cluster-proto",
-    srcs = [":grakn_cluster.proto"],
-    deps = [":cluster-proto", ":database-proto"],
+    name = "service-proto",
+    srcs = [":cluster_service.proto"],
+    deps = [
+        ":server-proto",
+        ":database-proto"
+    ],
 )
 
 proto_library(
-    name = "cluster-proto",
-    srcs = [":cluster.proto"],
+    name = "server-proto",
+    srcs = [":cluster_server.proto"],
 )
 
 proto_library(
     name = "database-proto",
-    srcs = [":database.proto"],
+    srcs = [":cluster_database.proto"],
 )
 
 checkstyle_test(
diff --git a/cluster/cluster_database.proto b/cluster/cluster_database.proto
@@ -17,36 +17,39 @@
 
 syntax = "proto3";
 
-option java_package = "grakn.protocol.cluster";
-option java_outer_classname = "DatabaseProto";
+option java_package = "grakn.protocol";
+option java_outer_classname = "ClusterDatabaseProto";
 
-package grakn.protocol.cluster;
+package grakn.protocol;
 
-message Database {
-
-    string name = 1;
-    repeated Replica replicas = 2;
-
-    message Replica {
-        string address = 1;
-        bool primary = 2;
-        bool preferred = 3;
-        int64 term = 4;
-    }
+message ClusterDatabaseManager {
 
     message Get {
         message Req {
             string name = 1;
         }
         message Res {
-            Database database = 1;
+            ClusterDatabase database = 1;
         }
     }
 
     message All {
         message Req {}
         message Res {
-            repeated Database databases = 1;
+            repeated ClusterDatabase databases = 1;
         }
     }
 }
+
+message ClusterDatabase {
+
+    string name = 1;
+    repeated Replica replicas = 2;
+
+    message Replica {
+        string address = 1;
+        bool primary = 2;
+        bool preferred = 3;
+        int64 term = 4;
+    }
+}
diff --git a/cluster/cluster_server.proto b/cluster/cluster_server.proto
@@ -17,16 +17,20 @@
 
 syntax = "proto3";
 
-option java_package = "grakn.protocol.cluster";
-option java_outer_classname = "ClusterProto";
+option java_package = "grakn.protocol";
+option java_outer_classname = "ClusterServerProto";
 
-package grakn.protocol.cluster;
+package grakn.protocol;
 
-message Cluster {
-    message Servers {
+message ServerManager {
+    message All {
         message Req {}
         message Res {
-            repeated string addresses = 1;
+            repeated Server servers = 1;
         }
     }
 }
+
+message Server {
+    string address = 1;
+}
diff --git a/cluster/cluster_service.proto b/cluster/cluster_service.proto
@@ -17,18 +17,18 @@
 
 syntax = "proto3";
 
-option java_package = "grakn.protocol.cluster";
-option java_outer_classname = "GraknClusterProto";
+option java_package = "grakn.protocol";
+option java_outer_classname = "ClusterServiceProto";
 
-import "protobuf/cluster/cluster.proto";
-import "protobuf/cluster/database.proto";
+import "cluster/cluster_server.proto";
+import "cluster/cluster_database.proto";
 
-package grakn.protocol.cluster;
+package grakn.protocol;
 
 service GraknCluster {
 
-    rpc cluster_servers (Cluster.Servers.Req) returns (Cluster.Servers.Res);
-    rpc database_get (Database.Get.Req) returns (Database.Get.Res);
-    rpc database_all (Database.All.Req) returns (Database.All.Res);
+    rpc servers_all (ServerManager.All.Req) returns (ServerManager.All.Res);
+    rpc databases_get (ClusterDatabaseManager.Get.Req) returns (ClusterDatabaseManager.Get.Res);
+    rpc databases_all (ClusterDatabaseManager.All.Req) returns (ClusterDatabaseManager.All.Res);
 
 }
diff --git a/common/BUILD b/common/BUILD
@@ -30,21 +30,6 @@ proto_library(
     srcs = ["concept.proto"],
 )
 
-proto_library(
-    name = "database-proto",
-    srcs = ["database.proto"],
-)
-
-proto_library(
-    name = "grakn-proto",
-    srcs = ["grakn.proto"],
-    deps = [
-        ":database-proto",
-        ":session-proto",
-        ":transaction-proto",
-    ],
-)
-
 proto_library(
     name = "logic-proto",
     srcs = ["logic.proto"]
@@ -90,8 +75,6 @@ filegroup(
     srcs = [
         "answer.proto",
         "concept.proto",
-        "database.proto",
-        "grakn.proto",
         "options.proto",
         "query.proto",
         "session.proto",
diff --git a/common/answer.proto b/common/answer.proto
@@ -20,7 +20,7 @@ syntax = "proto3";
 option java_package = "grakn.protocol";
 option java_outer_classname = "AnswerProto";
 
-import "protobuf/concept.proto";
+import "common/concept.proto";
 
 package grakn.protocol;
 
diff --git a/common/concept.proto b/common/concept.proto
diff --git a/common/logic.proto b/common/logic.proto
diff --git a/common/options.proto b/common/options.proto
@@ -36,7 +36,7 @@ message Options {
         bool explain = 3;
     }
     oneof parallel_opt {
-      bool parallel = 4;
+        bool parallel = 4;
     }
     oneof batch_size_opt {
         int32 batch_size = 5;
diff --git a/common/query.proto b/common/query.proto
@@ -20,8 +20,8 @@ syntax = "proto3";
 option java_package = "grakn.protocol";
 option java_outer_classname = "QueryProto";
 
-import "protobuf/answer.proto";
-import "protobuf/options.proto";
+import "common/answer.proto";
+import "common/options.proto";
 
 package grakn.protocol;
 
diff --git a/common/session.proto b/common/session.proto
@@ -20,7 +20,7 @@ syntax = "proto3";
 option java_package = "grakn.protocol";
 option java_outer_classname = "SessionProto";
 
-import "protobuf/options.proto";
+import "common/options.proto";
 
 package grakn.protocol;
 
diff --git a/common/transaction.proto b/common/transaction.proto
@@ -20,10 +20,10 @@ syntax = "proto3";
 option java_package = "grakn.protocol";
 option java_outer_classname = "TransactionProto";
 
-import "protobuf/concept.proto";
-import "protobuf/logic.proto";
-import "protobuf/options.proto";
-import "protobuf/query.proto";
+import "common/concept.proto";
+import "common/logic.proto";
+import "common/options.proto";
+import "common/query.proto";
 
 package grakn.protocol;
 
diff --git a/core/BUILD b/core/BUILD
@@ -0,0 +1,54 @@
+#
+# Copyright (C) 2021 Grakn Labs
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as
+# published by the Free Software Foundation, either version 3 of the
+# License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <https://www.gnu.org/licenses/>.
+#
+
+package(default_visibility = ["//visibility:public"])
+
+load("@graknlabs_dependencies//tool/checkstyle:rules.bzl", "checkstyle_test")
+
+proto_library(
+    name = "database-proto",
+    srcs = ["core_database.proto"],
+)
+
+proto_library(
+    name = "service-proto",
+    srcs = ["core_service.proto"],
+    deps = [
+        "//core:database-proto",
+        "//common:session-proto",
+        "//common:transaction-proto",
+    ],
+)
+
+# TODO: This should eventually be removed
+# At the moment, this exists to support Node.js libraries.
+# When it becomes a real rule, we should extract the .src_files from the above proto_library rules.
+# For now, this is required to get the source files.
+filegroup(
+    name = "proto-raw-buffers",
+    srcs = [
+        "core_database.proto",
+        "core_service.proto",
+    ]
+)
+
+checkstyle_test(
+    name = "checkstyle",
+    include = glob(["*"]),
+    license_type = "agpl",
+    size = "small",
+)
diff --git a/core/core_database.proto b/core/core_database.proto
@@ -18,12 +18,12 @@
 syntax = "proto3";
 
 option java_package = "grakn.protocol";
-option java_outer_classname = "DatabaseProto";
+option java_outer_classname = "CoreDatabaseProto";
 
 package grakn.protocol;
 
-message DatabaseManager {
-    
+message CoreDatabaseManager {
+
     message Contains {
         message Req {
             string name = 1;
@@ -48,7 +48,7 @@ message DatabaseManager {
     }
 }
 
-message Database {
+message CoreDatabase {
 
     message Schema {
         message Req {
diff --git a/core/core_service.proto b/core/core_service.proto
@@ -18,24 +18,24 @@
 syntax = "proto3";
 
 option java_package = "grakn.protocol";
-option java_outer_classname = "GraknProto";
+option java_outer_classname = "CoreServiceProto";
 
-import "protobuf/database.proto";
-import "protobuf/session.proto";
-import "protobuf/transaction.proto";
+import "core/core_database.proto";
+import "common/session.proto";
+import "common/transaction.proto";
 
 package grakn.protocol;
 
-service Grakn {
+service GraknCore {
 
     // Database Manager API
-    rpc databases_contains (DatabaseManager.Contains.Req) returns (DatabaseManager.Contains.Res);
-    rpc databases_create (DatabaseManager.Create.Req) returns (DatabaseManager.Create.Res);
-    rpc databases_all (DatabaseManager.All.Req) returns (DatabaseManager.All.Res);
+    rpc databases_contains (CoreDatabaseManager.Contains.Req) returns (CoreDatabaseManager.Contains.Res);
+    rpc databases_create (CoreDatabaseManager.Create.Req) returns (CoreDatabaseManager.Create.Res);
+    rpc databases_all (CoreDatabaseManager.All.Req) returns (CoreDatabaseManager.All.Res);
 
     // Database API
-    rpc database_schema (Database.Schema.Req) returns (Database.Schema.Res);
-    rpc database_delete (Database.Delete.Req) returns (Database.Delete.Res);
+    rpc database_schema (CoreDatabase.Schema.Req) returns (CoreDatabase.Schema.Res);
+    rpc database_delete (CoreDatabase.Delete.Req) returns (CoreDatabase.Delete.Res);
 
     // Session API
     rpc session_open (Session.Open.Req) returns (Session.Open.Res);
diff --git a/dependencies/graknlabs/repositories.bzl b/dependencies/graknlabs/repositories.bzl
@@ -21,5 +21,5 @@ def graknlabs_dependencies():
     git_repository(
         name = "graknlabs_dependencies",
         remote = "https://github.com/graknlabs/dependencies",
-        commit = "16e0f136723d929dfa6b5de72c1b95d7ac4cadd4", # sync-marker: do not remove this comment, this is used for sync-dependencies by @graknlabs_dependencies
+        commit = "d49062d3cfe3f2ee1d9b33fa181ce9e9792d9f15", # sync-marker: do not remove this comment, this is used for sync-dependencies by @graknlabs_dependencies
     )
diff --git a/grpc/java/BUILD b/grpc/java/BUILD
@@ -26,30 +26,24 @@ load("@graknlabs_dependencies//distribution:deployment.bzl", "deployment")
 java_grpc_compile(
     name = "protocol-src",
     deps = [
-        "//protobuf:answer-proto",
-        "//protobuf:concept-proto",
-        "//protobuf:database-proto",
-        "//protobuf:grakn-proto",
-        "//protobuf:logic-proto",
-        "//protobuf:options-proto",
-        "//protobuf:query-proto",
-        "//protobuf:session-proto",
-        "//protobuf:transaction-proto",
-    ]
-)
-
-java_grpc_compile(
-    name = "protocol-cluster-src",
-    deps = [
-        "//protobuf/cluster:grakn-cluster-proto",
-        "//protobuf/cluster:cluster-proto",
-        "//protobuf/cluster:database-proto"
+        "//cluster:service-proto",
+        "//cluster:server-proto",
+        "//cluster:database-proto",
+        "//common:answer-proto",
+        "//common:concept-proto",
+        "//common:logic-proto",
+        "//common:options-proto",
+        "//common:query-proto",
+        "//common:session-proto",
+        "//common:transaction-proto",
+        "//core:database-proto",
+        "//core:service-proto",
     ]
 )
 
 java_library(
     name = "protocol",
-    srcs = [":protocol-src", ":protocol-cluster-src"],
+    srcs = [":protocol-src"],
     deps = [
         "@maven//:com_google_guava_guava",
         "@maven//:com_google_protobuf_protobuf_java",
diff --git a/grpc/nodejs/BUILD b/grpc/nodejs/BUILD
diff --git a/grpc/python/BUILD b/grpc/python/BUILD

Original file line number	Diff line number	Diff line change
`@@ -36,7 +36,7 @@ message Options {`
`36`	`36`	`bool explain = 3;`
`37`	`37`	`}`
`38`	`38`	`oneof parallel_opt {`
`39`		`- bool parallel = 4;`
	`39`	`+ bool parallel = 4;`
`40`	`40`	`}`
`41`	`41`	`oneof batch_size_opt {`
`42`	`42`	`int32 batch_size = 5;`