-
Notifications
You must be signed in to change notification settings - Fork 21
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
Implement Dummy API for Project Understanding #445
Changes from all commits
171aca9
5fdfdbc
b1bcdc7
3f38f34
2587139
106acf7
91bb9b9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
package models | ||
|
||
import ( | ||
"errors" | ||
"fmt" | ||
) | ||
|
||
func RequiredFieldError(field string) error { | ||
return errors.New(field + " is required") | ||
} | ||
|
||
func MinLengthError(field string, min int) error { | ||
return errors.New(fmt.Sprintf("%s must be at least %d characters", field, min)) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
package models | ||
|
||
type HelloRequest struct { | ||
|
||
// New nickname to say hello | ||
Nickname string `json:"nickname"` | ||
} | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
package models | ||
|
||
type HelloResponse struct { | ||
|
||
// Welcome message | ||
Message string `json:"message"` | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
package models | ||
|
||
func (r *HelloRequest) Validate() error { | ||
if r.Nickname == "" { | ||
return RequiredFieldError("nickname") | ||
} | ||
|
||
if len(r.Nickname) < 2 { | ||
return MinLengthError("nickname", 2) | ||
} | ||
|
||
return nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
package core | ||
|
||
import ( | ||
"github.com/yorkie-team/codepair/backend/internal/core/hello" | ||
"github.com/yorkie-team/codepair/backend/internal/infra/database/mongo" | ||
) | ||
|
||
type Handlers struct { | ||
Hello *hello.Handler | ||
} | ||
|
||
// NewHandlers creates a new handlers. | ||
func NewHandlers() *Handlers { | ||
// Repositories | ||
helloRepository := mongo.NewHelloRepository() | ||
|
||
kokodak marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// Services | ||
helloService := hello.NewService(helloRepository) | ||
|
||
// Handlers | ||
helloHandler := hello.NewHandler(helloService) | ||
|
||
return &Handlers{ | ||
Hello: helloHandler, | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
package hello | ||
|
||
import ( | ||
"fmt" | ||
|
||
"github.com/labstack/echo/v4" | ||
|
||
"github.com/yorkie-team/codepair/backend/api/codepair/v1/models" | ||
"github.com/yorkie-team/codepair/backend/internal/transport/http" | ||
) | ||
|
||
type Handler struct { | ||
helloService *Service | ||
} | ||
|
||
// NewHandler creates a new handler for hello. | ||
func NewHandler(service *Service) *Handler { | ||
return &Handler{ | ||
helloService: service, | ||
} | ||
} | ||
|
||
// HelloCodePair returns a hello message for a given CodePairVisitor. | ||
func (h *Handler) HelloCodePair(e echo.Context) error { | ||
req := new(models.HelloRequest) | ||
|
||
if err := http.BindAndValidateRequest(e, req); err != nil { | ||
return fmt.Errorf("%w", err) | ||
} | ||
|
||
helloMessage, err := h.helloService.HelloCodePair(e, CodePairVisitor{ | ||
Nickname: req.Nickname, | ||
}) | ||
if err != nil { | ||
return fmt.Errorf("%w", http.NewErrorResponse(e, err)) | ||
} | ||
|
||
return fmt.Errorf("%w", http.NewOkResponse(e, models.HelloResponse{ | ||
Message: helloMessage, | ||
})) | ||
} | ||
kokodak marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
package hello | ||
|
||
// CodePairVisitor is a visitor for CodePair | ||
type CodePairVisitor struct { | ||
// Nickname is the nickname of the visitor | ||
Nickname string | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
package hello | ||
|
||
type Repository interface { | ||
// ReadHelloMessageFor reads a hello message for a given CodePairVisitor | ||
ReadHelloMessageFor(codePairVisitor CodePairVisitor) (string, error) | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,28 @@ | ||||||
package hello | ||||||
|
||||||
import ( | ||||||
"github.com/labstack/echo/v4" | ||||||
"github.com/yorkie-team/codepair/backend/internal/transport/http" | ||||||
) | ||||||
|
||||||
type Service struct { | ||||||
helloRepository Repository | ||||||
} | ||||||
|
||||||
// NewService creates a new service for hello. | ||||||
func NewService(repository Repository) *Service { | ||||||
return &Service{ | ||||||
helloRepository: repository, | ||||||
} | ||||||
} | ||||||
|
||||||
// HelloCodePair returns a hello message for a given CodePairVisitor | ||||||
func (s *Service) HelloCodePair(e echo.Context, codePairVisitor CodePairVisitor) (string, error) { | ||||||
helloMessage, err := s.helloRepository.ReadHelloMessageFor(codePairVisitor) | ||||||
if err != nil { | ||||||
e.Logger().Fatal(err) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replace Fatal logging with Error level logging. Using Apply this diff: - e.Logger().Fatal(err)
+ e.Logger().Error(err) 📝 Committable suggestion
Suggested change
|
||||||
return "", http.ErrInternalServerError | ||||||
} | ||||||
|
||||||
return helloMessage, nil | ||||||
devleejb marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,14 @@ | ||||||||||||||||||||
package mongo | ||||||||||||||||||||
|
||||||||||||||||||||
import "github.com/yorkie-team/codepair/backend/internal/core/hello" | ||||||||||||||||||||
|
||||||||||||||||||||
type HelloRepository struct{} | ||||||||||||||||||||
|
||||||||||||||||||||
// NewHelloRepository creates a new HelloRepository. | ||||||||||||||||||||
func NewHelloRepository() HelloRepository { | ||||||||||||||||||||
return HelloRepository{} | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
func (h HelloRepository) ReadHelloMessageFor(codePairVisitor hello.CodePairVisitor) (string, error) { | ||||||||||||||||||||
return "Hello, " + codePairVisitor.Nickname + "!", nil | ||||||||||||||||||||
} | ||||||||||||||||||||
Comment on lines
+12
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add input validation and error handling. The current implementation could be improved with:
Consider this safer implementation: func (h HelloRepository) ReadHelloMessageFor(codePairVisitor hello.CodePairVisitor) (string, error) {
+ if codePairVisitor.Nickname == "" {
+ return "", errors.New("invalid visitor: nickname is required")
+ }
return "Hello, " + codePairVisitor.Nickname + "!", nil
} 📝 Committable suggestion
Suggested change
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,11 @@ | ||
package server | ||
|
||
import ( | ||
"fmt" | ||
"net/http" | ||
|
||
"github.com/labstack/echo/v4" | ||
"github.com/yorkie-team/codepair/backend/internal/core" | ||
) | ||
|
||
func RegisterRoutes(e *echo.Echo) { | ||
e.GET("/", func(c echo.Context) error { | ||
err := c.String(http.StatusOK, "Hello, World!") | ||
return fmt.Errorf("error: %w", err) | ||
}) | ||
// RegisterRoutes registers routes for the server. | ||
func RegisterRoutes(e *echo.Echo, handlers *core.Handlers) { | ||
e.POST("/hello", handlers.Hello.HelloCodePair) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
package http | ||
|
||
import ( | ||
"errors" | ||
|
||
"github.com/yorkie-team/codepair/backend/api/codepair/v1/models" | ||
) | ||
|
||
// ConvertErrorToResponse converts an error to a response. | ||
func ConvertErrorToResponse(err error) models.HttpExceptionResponse { | ||
switch { | ||
case errors.Is(err, ErrInternalServerError): | ||
return NewInternalServerErrorResponse() | ||
default: | ||
return NewInvalidJSONErrorResponse() | ||
} | ||
} | ||
kokodak marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
package http | ||
|
||
import ( | ||
nethttp "net/http" | ||
|
||
"github.com/yorkie-team/codepair/backend/api/codepair/v1/models" | ||
) | ||
|
||
func newHTTPExceptionResponse(statusCode int, message string) models.HttpExceptionResponse { | ||
return models.HttpExceptionResponse{ | ||
StatusCode: statusCode, | ||
Message: message, | ||
} | ||
} | ||
|
||
// NewInvalidJSONErrorResponse creates a HttpExceptionResponse that represents an invalid JSON error. | ||
func NewInvalidJSONErrorResponse() models.HttpExceptionResponse { | ||
return newHTTPExceptionResponse(nethttp.StatusBadRequest, "Invalid JSON") | ||
} | ||
|
||
// NewValidationErrorResponse creates a HttpExceptionResponse that represents a validation error. | ||
func NewValidationErrorResponse(reason string) models.HttpExceptionResponse { | ||
return newHTTPExceptionResponse(nethttp.StatusBadRequest, reason) | ||
} | ||
|
||
// NewInternalServerErrorResponse creates a HttpExceptionResponse that represents a internal server error. | ||
func NewInternalServerErrorResponse() models.HttpExceptionResponse { | ||
return newHTTPExceptionResponse(nethttp.StatusInternalServerError, "Internal Server Error") | ||
} | ||
kokodak marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package http | ||
|
||
import "errors" | ||
|
||
var ( | ||
// ErrInternalServerError is an internal server error | ||
ErrInternalServerError = errors.New("internal server error") | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
package http | ||
|
||
import ( | ||
"fmt" | ||
nethttp "net/http" | ||
|
||
"github.com/labstack/echo/v4" | ||
) | ||
|
||
type request interface { | ||
Validate() error | ||
} | ||
|
||
// BindAndValidateRequest binds and validates the request. | ||
// If the request is invalid, it returns an error response. | ||
func BindAndValidateRequest(e echo.Context, req request) error { | ||
if err := e.Bind(req); err != nil { | ||
return fmt.Errorf("failed to bind request: %w", e.JSON(nethttp.StatusBadRequest, NewInvalidJSONErrorResponse())) | ||
} | ||
if err := req.Validate(); err != nil { | ||
return fmt.Errorf("validation failed: %w", e.JSON(nethttp.StatusBadRequest, NewValidationErrorResponse(err.Error()))) | ||
} | ||
return nil | ||
} | ||
kokodak marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// NewErrorResponse handles the creation and response of an error. | ||
// It converts the provided error into a response structure and sends it as a JSON response. | ||
// The response status code is determined by the error's associated status. | ||
func NewErrorResponse(e echo.Context, err error) error { | ||
resp := ConvertErrorToResponse(err) | ||
return fmt.Errorf("returning error response as JSON: %w", e.JSON(resp.StatusCode, ConvertErrorToResponse(err))) | ||
} | ||
kokodak marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// NewOkResponse sends a JSON response with a status code of 200. | ||
func NewOkResponse(e echo.Context, resp interface{}) error { | ||
return fmt.Errorf("returning success response as JSON: %w", e.JSON(nethttp.StatusOK, resp)) | ||
} | ||
kokodak marked this conversation as resolved.
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about writing a Request, Response, and Validator of the same model (
Hello
) in the same file? It seems unnecessary to write the Request and Response in different files.Maybe it would be better to write them together in the router file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally agree with @blurfx's suggestion, as this style of micro-separating so-called DTOs is not a common approach in Golang and may lead to an unnecessary proliferation of files for DTO management.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One important thing to keep in mind is that all request and response types are auto-generated from the OpenAPI specification.
Validator
Since these files are auto-generated, declaring validation functions within the same file is risky, as they might be overwritten during regeneration.
Type Placement
Moving these types to their corresponding packages is quite challenging, as it is auto-generated. Additionally, the directory structure is similar to yorkie. I believe these types correspond to files like
admin.pb.go
andyorkie.pb.go
.DTO
I’m unsure how we should manage request and response types for REST APIs in Go. Could you provide an example or some guidance on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn’t realize these files were auto-generated by OpenAPI generation. If that’s the case, I think it’s great!