Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate Protocol class into CoreProtocol, ServerProtocol, and ClientProtocol #1698

Closed
hlbarber opened this issue Sep 2, 2022 · 3 comments
Closed

Comments

@hlbarber
Copy link
Contributor

hlbarber commented Sep 2, 2022

At the time of writing, there is a single Protocol class which holds both client and server methods, despite it living inside the codegen folder. The codegen folder is supposed to be server agnostic.

We should separate Protocol interface into CoreProtocol, ServerProtocol, and ClientProtocol interfaces.

  • CoreProtocol holds methods common to both the server and client.
  • ServerProtocol holds an instance of CoreProtocol and lives in the codegen-server.
  • ClientProtocol should work similarly to ServerProtocol - encapsulating all client specific methods and living in an appropriate space.

As a result, the classes implementing Protocol should also be separated. For example:

  • class CoreRestJsonProtocol: CoreProtocol
  • class ServerRestJsonProtocol: ServerProtocol
  • class ClientRestJsonProtocol: ClientProtocol

and methods should be added to Server{Protocol}Protocol and Client{Protocol}Protocol to retrieve Core{Protocol}Protocol.

@hlbarber
Copy link
Contributor Author

hlbarber commented Sep 2, 2022

Related progress towards this goal: #1697

@hlbarber
Copy link
Contributor Author

hlbarber commented Sep 5, 2022

The ServerProtocol class has been introduced in #1693. Further work needs to be done to move existing methods from the implementations of Protocol onto the implementations of ServerProtocol.

@hlbarber
Copy link
Contributor Author

hlbarber commented Sep 8, 2022

and methods should be added to Server{Protocol}Protocol and Client{Protocol}Protocol to retrieve Core{Protocol}Protocol.

Rather than have this be the case, it's probably more ergonomic to have ServerProtocol : CoreProtocol and Server{Protocol}Protocol : Core{Protocol}Protocol, ServerProtocol. Similarly, ClientProtocol: CoreProtocol and Client{Protocol}Protocol: Core{Protocol}Protocol, ClientProtocol.

Changes have been made in #1693 to do this for the server side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant