[REQ] [go-server] Services knowing about HTTP is questionable

[uykusuz]

Is your feature request related to a problem? Please describe.

Let's look at the service method signatures:

func (s *UserApiService) CreateUser(ctx context.Context, user User) (ImplResponse, error) {
}

The method returns a ImplResponse, carrying a HTTP status code. I see two problems with this.

The first one .... really, really minor: why does a method return an implementation object? It should return an interface, and ImplResponse would be an implementation of that interface.

Now to the other, bigger one.

Imo there's an architectural problem with the idea of controllers and services.

What is the idea of controllers and services?

The idea of the separation between controllers and services is (imo, pls enlighten me otherwise):

• controllers hold all the connection logic, specific to http in this case.
• services hold the business logic solely. They don't know anything about HTTP status codes. They return model classes and errors

What's the current state?

Services return response objects, carrying HTTP status codes. Making services HTTP-dependant.

What's wrong with this?

Imagine you have a REST API (openapi) and e.g. a gRPC API. The services do exactly the same, we just have different controllers.

With the current design this forces us to have three layers:

• the openapi-controller layer
• the openapi-service layer (HTTP specific)
• our actual service layer independent of HTTP knowledge.

That middle layer should not be necessary, it bloats up the code.

Describe the solution you'd like

Keeping it implementation-independent, it would be best (also just imo, please enlighten me otherwise) to have service methods like this:

func (s *UserApiService) CreateUser(ctx context.Context, user User) (interface{}, error) {
}

The interface{} is one of the generated model classes, ready for serialization.

Where error can be of a new interface type:

type Error interface {
  Error()

  IsBadUserInput()
  IsInternal()
  IsUpstream()
  // ...
}

Then the controller can check whether the returned error is of that interface and map to corresponding HTTP status codes and if not simply return a 500:

// CreateUser - Create user
func (c *UserApiController) CreateUser(w http.ResponseWriter, r *http.Request) {
	user := &User{}
	if err := json.NewDecoder(r.Body).Decode(&user); err != nil {
		w.WriteHeader(http.StatusBadRequest)
		return
	}
	result, err := c.service.CreateUser(r.Context(), *user)
	// If an error occurred, encode the error with the status code
	if err != nil {
		if knownErr, ok := err.(Error); ok {
			if knownErr.IsBadUserInput() {
				EncodeJSONResponse(err.Error(), &http.StatusBadRequest, w)
			} else if knownErr.IsInternal() {
				// ...
			}
			// aso.
		} else {	
			EncodeJSONResponse(err.Error(), &http.StatusInternalServerError, w)
		}
	} else {
		// If no error, encode the body and the result code
		EncodeJSONResponse(result, &http.StatusOK, w)
	}
}

Describe alternatives you've considered

Not alternatives, but workarounds.

One workaround is the aforementioned, three-layer-architecture, that bloats up the code.

Another one is ignoring the ImplResponse:

• either redefine ImplResponse to interface{}
• or ignore the status code in ImplResponse and only use body

[lwj5]

There's a possibility of using a type switch case in the controller, but what happens if there are 2 response code with the same type?

Any ideas?

p.s. Maybe change the issue name to go-server instead of go

[uykusuz]

@lwj5 It's just a contract on the interface. It's still the responsibility of the one who implements controller and service to choose error codes appropriately so that they make sense in the application. What we gain here is mainly a clean separation of openapi and business logic.

If there are two http status codes for one error, perhaps you can further pin it down in the controller by looking at the error more closely (i.e. some other attributes of it) or you add a new error on the business logic side.

[lwj5]

So far it seems like #9764 has addressed the err portion of this problem. There's a lot of flexibility there to customise how to handle your errors and you can define different response code based on your err.

// CreateUser - Create user
func (c *UserApiController) CreateUser(w http.ResponseWriter, r *http.Request) {
	user := &User{}
	if err := json.NewDecoder(r.Body).Decode(&user); err != nil {
		w.WriteHeader(http.StatusBadRequest)
		return
	}
	result, err := c.service.CreateUser(r.Context(), *user)
	// If an error occurred, encode the error with the status code
	if err != nil {
		// Goes to errorhandler
	} else {
		// If no error, encode the body and the result code
		EncodeJSONResponse(result, &http.StatusOK, w)
	}
}

Now the more difficult part is the

	} else {
		// If no error, encode the body and the result code
		EncodeJSONResponse(result, &http.StatusOK, w)
	}

I foresee we need another file to handle the success call for each route.

• We cannot assume that there's only 1 successful return type. Also, each type may have multiple response codes.
• We need to be able to handle cookies [go-server] Add support for cookies, functional options to Go Response object #9277
• We need to be able to handle headers

Now, would you agree that headers and cookies should be part of the logic of the servicer? If yes then...

func (s *UserApiService) CreateUser(ctx context.Context, user User) (interface{}, someFormOfHeaderCookieReturn, error) {
}

[uykusuz]

Hi @lwj5,

Thank you for the elaborate answer! Sorry for the late response, I somehow missed your answer.

So far it seems like #9764 has addressed the err portion of this problem.

Thank you for the hint! With error I can do what I want, it's general enough. The problem I see is rather the ImplResponse.

Now, would you agree that headers and cookies should be part of the logic of the servicer?

No, actually not 🙈 Remember the case again of having a grpc API and an openapi. The service is still doing the same stuff, business logic. It returns an object that has to contain all the information from which you construct return headers and cookies in the controller, since that's the whole idea of the controller: wrapping, in this case, REST. The service should not know about headers and cookies.

Perhaps we need some examples. Just one coming to my mind right now: authentication using JWT. Here the service would simply return a JWT. And the controller will decide in which header value and how to put that JWT.

Also in this case: when I express that I need a kind of access token in the endpoint, openapi will happily generate a string parameter for the corresponding servicer that shall contain that cookie. But it's just a string value. In the controller I can decide from which cookie or other header that access token is coming and pass it to the servicer in that string value.

What do you think about that?