cloudfoundry / korifi

Cloud Foundry on Kubernetes
Apache License 2.0
308 stars 60 forks source link

[Chore]: Experiment with inlining an action in the respective handler that is using it #1058

Open georgethebeatle opened 2 years ago

georgethebeatle commented 2 years ago

Background

There is an actions package that contains certain objects implementing the same Invoke method. There are several reasons why these actions look weird:

All this is confusing and is not clearly communicating the code intent. It also makes code navigation harder, because one has to go check what is passed to the handler's constructor in main.go in order to find the action implementation (usually a :GoImplements is enough.

Action to take

Impact

Simpler code structure

Dev Notes

No response

tcdowney commented 2 years ago

With actions we're following some of the patterns established by the Cloud Controller v3 API code where we found them valuable (notes on the action pattern). Kind of similar to the "service layer" pattern.

The idea was the handlers should be relatively thin, mostly concerned with handling the request itself and delivering responses, but not concerning itself with the actual "business logic" if it could avoid it. Essentially the "controller" layer in MVC.

The repositories were meant to be more "data object" oriented and handle the concerns of interacting with the underlying data store (K8s API / CRD YAML) and converting those into objects that are closer to the CF representations that the API can work with.

It's not necessarily the case with many of our endpoints, but our actions allow for shared behaviors between multiple front-doors. For example, both the manifest and POST /v3/processes/:guid/actions/scale can scale a CFProcess and can share similar business logic. In Cloud Controller they were even more necessary since we had the manifest, v2, and v3 endpoints all manipulating the same underlying constructs but through different API front doors.

I just wanted to share some background around why we introduced them and how I've personally found value in this pattern in the past, but I'll make sure others on the team see this issue and the draft PR so they can give their feedback as well.

@cloudfoundry/cf-k8s