From d2e04099a0439c60802bf28d1b44c8733a22d621 Mon Sep 17 00:00:00 2001 From: Mo Kweon Date: Tue, 29 Jun 2021 15:18:14 -0700 Subject: [PATCH] feat: add GitHub handler and dependency injection --- k8s/kkweon-okteto/server.yaml | 2 + server/cmd/server/main.go | 14 ++++-- server/cmd/tools/tools.go | 1 + server/pkg/env/env.go | 2 + server/pkg/handlers/handlers.go | 66 +++++++++++++++++++++------- server/pkg/handlers/handlers_test.go | 44 ++++++++++++++++++- server/pkg/serv/serv.go | 13 ++++-- 7 files changed, 118 insertions(+), 24 deletions(-) diff --git a/k8s/kkweon-okteto/server.yaml b/k8s/kkweon-okteto/server.yaml index 7d0dd2d2..a99ed731 100644 --- a/k8s/kkweon-okteto/server.yaml +++ b/k8s/kkweon-okteto/server.yaml @@ -57,6 +57,8 @@ spec: value: "9000" - name: PR12ER_PROMETHEUS_PORT value: "9093" + - name: GITHUB_API_KEY + value: "${GITHUB_API_KEY}" readinessProbe: exec: diff --git a/server/cmd/server/main.go b/server/cmd/server/main.go index 7c90b0ef..667a0228 100644 --- a/server/cmd/server/main.go +++ b/server/cmd/server/main.go @@ -4,9 +4,11 @@ import ( "fmt" "net" + "github.com/codingpot/pr12er/server/internal/ghv3" "github.com/codingpot/pr12er/server/middlewares/healthserver" "github.com/codingpot/pr12er/server/middlewares/metrics" "github.com/codingpot/pr12er/server/pkg/env" + "github.com/codingpot/pr12er/server/pkg/handlers" "github.com/codingpot/pr12er/server/pkg/pr12er" "github.com/codingpot/pr12er/server/pkg/serv" grpc_logrus "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus" @@ -18,14 +20,20 @@ import ( ) func main() { - log.WithField("ServicePort", env.ServicePort).Print("gRPC server is listening") + log.WithFields( + log.Fields{ + "ServicePort": env.ServicePort, + "GITHUB_API_KEY": env.GitHubAPIKey, + }).Print("gRPC server is listening") lis, err := net.Listen("tcp", fmt.Sprintf(":%s", env.ServicePort)) if err != nil { log.WithError(err).Fatalf("failed to listen") } - s := serv.Server{} + githubService := ghv3.New(env.GitHubAPIKey) + handler := handlers.New(githubService) + s := serv.New(handler) logNewEntry := log.NewEntry(log.New()) @@ -42,7 +50,7 @@ func main() { ), ) - pr12er.RegisterPr12ErServiceServer(grpcServer, &s) + pr12er.RegisterPr12ErServiceServer(grpcServer, s) healthserver.RegisterHealthServer(grpcServer) reflection.Register(grpcServer) diff --git a/server/cmd/tools/tools.go b/server/cmd/tools/tools.go index 0cb46da0..5a699b6a 100644 --- a/server/cmd/tools/tools.go +++ b/server/cmd/tools/tools.go @@ -4,6 +4,7 @@ package tools import ( _ "github.com/daixiang0/gci" + _ "github.com/golang/mock/mockgen" _ "google.golang.org/grpc/cmd/protoc-gen-go-grpc" _ "google.golang.org/protobuf/cmd/protoc-gen-go" _ "mvdan.cc/gofumpt" diff --git a/server/pkg/env/env.go b/server/pkg/env/env.go index 7d43e6d5..9b6febfa 100644 --- a/server/pkg/env/env.go +++ b/server/pkg/env/env.go @@ -10,6 +10,8 @@ var ( ServicePort = getEnvVar("PR12ER_GRPC_PORT", "9000") // PrometheusPort is the port for Prometheus metrics (/metrics). PrometheusPort = getEnvVar("PR12ER_PROMETHEUS_PORT", "9092") + // GitHubAPIKey is the GitHub API Key used to create an issue. + GitHubAPIKey = getEnvVar("GITHUB_API_KEY", "") ) func getEnvVar(key, fallbackValue string) string { diff --git a/server/pkg/handlers/handlers.go b/server/pkg/handlers/handlers.go index 2cca0541..072fc712 100644 --- a/server/pkg/handlers/handlers.go +++ b/server/pkg/handlers/handlers.go @@ -1,15 +1,27 @@ +// Package handlers provide functions used to handle gRPC. package handlers import ( + "errors" "sort" "github.com/codingpot/pr12er/server/internal/err" + "github.com/codingpot/pr12er/server/pkg/handlers/gh" "github.com/codingpot/pr12er/server/pkg/handlers/prutils" "github.com/codingpot/pr12er/server/pkg/pr12er" ) +// Handler depends on gh.GitHubService interface. +type Handler struct { + gh gh.GitHubService +} + +func New(service gh.GitHubService) *Handler { + return &Handler{gh: service} +} + // VideosResponseFromDB converts DB proto to GetVideosResponse. -func VideosResponseFromDB(db *pr12er.Database) *pr12er.GetVideosResponse { +func (h *Handler) VideosResponseFromDB(db *pr12er.Database) *pr12er.GetVideosResponse { videos := make([]*pr12er.Video, len(db.GetPrIdToVideo())) i := 0 @@ -38,6 +50,43 @@ func VideosResponseFromDB(db *pr12er.Database) *pr12er.GetVideosResponse { } } +// DetailResponseFromDB builds DetailResponse from Database. +func (h *Handler) DetailResponseFromDB(prID int32, db *pr12er.Database) (*pr12er.GetDetailResponse, error) { + video, ok := db.GetPrIdToVideo()[prID] + if !ok { + return nil, err.ErrorPrIDNotFound{PrID: prID} + } + + return &pr12er.GetDetailResponse{ + Detail: &pr12er.Detail{ + PrId: prID, + Papers: video.GetPapers(), + }, + }, nil +} + +// Report handles when a client sends a bug report or a missing PR video report. +func (h *Handler) Report(in *pr12er.ReportRequest) (*pr12er.ReportResponse, error) { + var issue *gh.GitHubIssue + var e error + + switch in.GetType() { + case pr12er.ReportRequest_REPORT_TYPE_BUG: + issue, e = h.gh.CreateIssue("버그 리포트", in.GetBody(), []string{"bug"}) + case pr12er.ReportRequest_REPORT_TYPE_MISSING_PR_VIDEO: + issue, e = h.gh.CreateIssue("PR12 누락 동영상", in.GetBody(), []string{"data"}) + case pr12er.ReportRequest_REPORT_TYPE_UNSPECIFIED: + issue, e = h.gh.CreateIssue("기타", in.GetBody(), nil) + default: + e = errors.New("this bug report type is not supported") + } + + if issue == nil || e != nil { + return nil, e + } + return &pr12er.ReportResponse{IssueUrl: issue.URL}, e +} + // getKeywords returns all keywords by merging each paper's methods. func getKeywords(prVideo *pr12er.PrVideo) []string { var ret []string @@ -53,18 +102,3 @@ func getKeywords(prVideo *pr12er.PrVideo) []string { func getYouTubeLinkFromID(videoID string) string { return "https://youtu.be/" + videoID } - -// DetailResponseFromDB builds DetailResponse from Database. -func DetailResponseFromDB(prID int32, db *pr12er.Database) (*pr12er.GetDetailResponse, error) { - video, ok := db.GetPrIdToVideo()[prID] - if !ok { - return nil, err.ErrorPrIDNotFound{PrID: prID} - } - - return &pr12er.GetDetailResponse{ - Detail: &pr12er.Detail{ - PrId: prID, - Papers: video.GetPapers(), - }, - }, nil -} diff --git a/server/pkg/handlers/handlers_test.go b/server/pkg/handlers/handlers_test.go index c412375b..25535789 100644 --- a/server/pkg/handlers/handlers_test.go +++ b/server/pkg/handlers/handlers_test.go @@ -4,7 +4,10 @@ import ( "testing" "github.com/codingpot/pr12er/server/internal/data" + "github.com/codingpot/pr12er/server/internal/mock_gh" + "github.com/codingpot/pr12er/server/pkg/handlers/gh" "github.com/codingpot/pr12er/server/pkg/pr12er" + "github.com/golang/mock/gomock" "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" "google.golang.org/protobuf/testing/protocmp" @@ -81,7 +84,8 @@ func TestConvertToGet(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := VideosResponseFromDB(tt.args.db) + h := New(nil) + got := h.VideosResponseFromDB(tt.args.db) if got := cmp.Diff(tt.want, got, protocmp.Transform()); got != "" { t.Error(got) } @@ -140,7 +144,8 @@ func TestDetailResponseFromDB(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := DetailResponseFromDB(tt.args.prID, tt.args.db) + h := New(nil) + got, err := h.DetailResponseFromDB(tt.args.prID, tt.args.db) if tt.wantErr { assert.Error(t, err) } else { @@ -156,3 +161,38 @@ func TestDetailResponseFromDB(t *testing.T) { }) } } + +func TestHandler_Report(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + { + // GIVEN a report + in := &pr12er.ReportRequest{ + Type: pr12er.ReportRequest_REPORT_TYPE_BUG, + Body: "Bug Bug Bug", + } + + // THEN it should create a issue with the following information. + mockService := mock_gh.NewMockGitHubService(ctrl) + mockService. + EXPECT(). + CreateIssue( + gomock.Eq("버그 리포트"), + gomock.Eq("Bug Bug Bug"), + gomock.Eq([]string{"bug"}), + ). + Return(&gh.GitHubIssue{URL: "github-issue-url"}, nil) + + // THEN checks the response contains the URL. + want := &pr12er.ReportResponse{ + IssueUrl: "github-issue-url", + } + h := New(mockService) + got, err := h.Report(in) + assert.NoError(t, err) + if diff := cmp.Diff(want, got, protocmp.Transform()); diff != "" { + t.Error(diff) + } + } +} diff --git a/server/pkg/serv/serv.go b/server/pkg/serv/serv.go index 823e9c78..e09ff105 100644 --- a/server/pkg/serv/serv.go +++ b/server/pkg/serv/serv.go @@ -11,12 +11,19 @@ import ( type Server struct { pr12er.UnsafePr12ErServiceServer + h *handlers.Handler +} + +func New(handler *handlers.Handler) *Server { + return &Server{ + h: handler, + } } var _ pr12er.Pr12ErServiceServer = (*Server)(nil) func (s Server) GetDetail(_ context.Context, in *pr12er.GetDetailRequest) (*pr12er.GetDetailResponse, error) { - return handlers.DetailResponseFromDB(in.GetPrId(), &data.DB) + return s.h.DetailResponseFromDB(in.GetPrId(), &data.DB) } func (s Server) GetHello(_ context.Context, in *pr12er.HelloRequest) (*pr12er.HelloResponse, error) { @@ -24,9 +31,9 @@ func (s Server) GetHello(_ context.Context, in *pr12er.HelloRequest) (*pr12er.He } func (s Server) GetVideos(_ context.Context, _ *pr12er.GetVideosRequest) (*pr12er.GetVideosResponse, error) { - return handlers.VideosResponseFromDB(&data.DB), nil + return s.h.VideosResponseFromDB(&data.DB), nil } func (s Server) Report(_ context.Context, in *pr12er.ReportRequest) (*pr12er.ReportResponse, error) { - panic("implement me") + return s.h.Report(in) }