Standardize the naming of GRPC Functions/Commands and Message Types

[CMCDragonkai]

The naming of GRPC functions/commands and message types is getting confusing. We need to have a standard way of naming with:

• Prefix - use this for hierarchical grouping
• Name - use this for the main intention
• Suffix - use this for creating variants

This is a renaming procedure, no new functionality should be done. But we may identify things that are missing or types that can be merged together.

Do note that all GRPC commands requires an input message type and an output message type.

Tasks

1. • Identify all major prefixes
2. • Identify major variants
3. • Rename all commands and types
4. • Change the usage of the functions across the rest of the codebase

[joshuakarp]

Also adding my previous comment from nodes CLI MR: https://gitlab.com/MatrixAI/Engineering/Polykey/js-polykey/-/merge_requests/198#note_633812954

I've noticed that client.proto seems to be using snake_case for its field names, whereas agent.proto is using camelCase. After chatting with @CMCDragonkai, we're gonna follow Google's Protobuf style guide and:

• use UpperCamelCase for RPC services and message names (e.g. VaultsList, NodesPing, StatusMessage)
• use snake_case for field names (e.g. node_id)

[CMCDragonkai]

The prefix of GRPC commands should be used to group the relevant calls together.

We have a natural grouping of calls by default which are the domains:

NotificationsX
GestaltsX
AgentX
...

Where X is the actual call done for that particular domain.

Furthermore X itself should be structured with "object-verb" order. Note that "subject" is assumed to be the caller itself. So in linguistic typology we are using SOV order. https://en.wikipedia.org/wiki/Subject%E2%80%93object%E2%80%93verb

Relevant message types should also be structured in the same way.

For variants of the same call, use a suffix. Like:

NotificationXY
NotificationXZ

For example:

NotificationsRead
NotificationsReadAll

We also need to standardize on the relevant side-effectul verbs/actions. Here are some ideas from RESTful terminology:

• Get - get one thing, e.g. NotificationsGet
• Put - update a particular thing, replace it, or to create if not exist e.g. GestaltsBlahPut
• Post - I don't actually like this call I would always replace this with create
• Patch - partial update
• Delete

I also like CRUD in general:

• Create
• Read
• Update
• Delete

With 2 additions:

• ReadAll - for reading multiple of a thing
• Patch - for partial update

In that case, it is possible to also have

• Upsert
• Replace
• Put
• Patch

Different domains may have domain specific action terminology. Make sure that everybody drafts these up during their MR refactoring.

@emmacasolin @DrFacepalm @tegefaulkes @joshuakarp @scottmmorris

[CMCDragonkai]

Another thing is because GRPC calls are always one to one from Client to Server.

The calls are always named with respect to the Client. The Client is the "subject" in the SOV order.

So for example, if the server is meant to receive a notification from the client. Then the call should not be named ReceiveNotification, as that would imply "ClientReceiveNotification", but instead it is actually "ClientSendNotification" we want to mean, and therefore the call should be named SendNotification.

[CMCDragonkai]

Interesting that SOV word order is the most common in all human languages.

[CMCDragonkai]

There are some message types like VaultsVaultMessage or NodesNodeMessage.

This is fine for now to represent the input/query message to refer to a vault or node.

We can address other naming issues later. For now as long as the prefix is the domains, this will make it clearer.

[tegefaulkes]

While including the domains inside the GRPC functions makes sense, I'm not sure it does for the message names.
In the case of NodeMessage it contains information specific to a node. It can be used in function calls inside the identities domain or the nodes domain and as such. is not really specific to a domain.
Also, it is always used in the context of a grpc call. Such as NodesGetNode(NodeMessage).

So I think keeping message names like NodeMessage or VaultMessage is the better option here.

[CMCDragonkai]

Good point. What we can do is do a hierarchy/taxonomy of message types. Begin with the basic primitive messages that are used by everything. And then domain specific message types. We can figure this out after sessions MR fixes gets merged.

…

On 8/13/21 1:00 PM, Brian Botha wrote: While including the domains inside the GRPC functions makes sense, I'm not sure it does for the message names. In the case of |NodeMessage| it contains information specific to a node. It can be used in function calls inside the identities domain or the nodes domain and as such. is not really specific to a domain. Also, it is always used in the context of a grpc call. Such as |NodesGetNode(NodeMessage)|. So I think keeping message names like |NodeMessage| or |VaultMessage| is the better option here. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#218 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAE4OHOV7PEJXKQQL6QPT5LT4SDDRANCNFSM5A7FPHSQ>.

[CMCDragonkai]

I've noticed that the naming of the command files aren't standardised atm.

For example:

commandAgentLock.ts
commandPingNode.ts

• We already categorize them by directories
• There's a common command prefix being used

Let's simplify this. For example:

.
├── agent
│   ├── commandAgentLockAll.ts
│   ├── commandAgentLock.ts
│   ├── commandAgentUnlock.ts
│   ├── commandStartAgent.ts
│   ├── commandStatusAgent.ts
│   ├── commandStopAgent.ts
│   └── index.ts

Can instead be:

.
├── agent
│   ├── lockAll.ts
│   ├── lock.ts
│   ├── unlock.ts
│   ├── start.ts
│   ├── status.ts
│   ├── stop.ts
│   └── index.ts

[CMCDragonkai]

Ok it seems that file structure in the bin has changed.

What's left is cleaning up all remaining GRPC and Protobuf types. We'll do a once over at the end and we can close this issue and move this standard into the wiki.

[tegefaulkes]

Protobuf definitions have been updated in MR https://gitlab.com/MatrixAI/Engineering/Polykey/js-polykey/-/merge_requests/210.

Protobuf definitions have been updated to follow the following standard.

• Serivce and message names are UpperCamelCase and message fields are now snake_case. So cases where a field was nodeId->node_id, this results in the message getters and setters having correct capitalisation. getNodeid->getNodeId
• Message fields have been updated to be more specific. eg id->node_id or vault_id where relevent.
• In some cases where one message field took different kinds of data E.G. VaultMessage, the field was replaced with a oneof field. This means the kind of data can be know vaultMessage.getNameOrIdCase().
• service names now follow SOV order. So services like KeysChangePassword->KeysPasswordChange. Most services have changed like this so keep an eye out for it.

[CMCDragonkai]

This is done now @emmacasolin @joshuakarp please rebase your nodes CLI MR once you start it.

@scottmmorris since @tegefaulkes is working with you on the EFS and vaults, then it's also time to rebase the vaults refactoring.

[CMCDragonkai]

Important to keep this standard as we move forward!