Skip to content
33 changes: 15 additions & 18 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,29 +1,26 @@
# Package configuration
# Config
PROJECT = code-annotation
COMMANDS = server/cmd/code-annotation
COMMANDS = cli/server

# Including ci Makefile
MAKEFILE = Makefile.main
CI_REPOSITORY = https://github.com/src-d/ci.git
CI_FOLDER = .ci

# Tools
YARN = yarn
REMOVE = rm -rf

SERVER_URL ?= /api
API_PORT ?= 8080
HOST ?= 127.0.0.1
PORT ?= 8080
SERVER_URL ?= //$(HOST):$(PORT)

# Including ci Makefile
CI_REPOSITORY ?= https://github.com/src-d/ci.git
CI_PATH ?= $(shell pwd)/.ci
MAKEFILE := $(CI_PATH)/Makefile.main
$(MAKEFILE):
@git clone --quiet $(CI_REPOSITORY) $(CI_FOLDER); \
cp $(CI_FOLDER)/$(MAKEFILE) .;

git clone --quiet --depth 1 $(CI_REPOSITORY) $(CI_PATH);
-include $(MAKEFILE)

# set enviroment variables from .env file
include .env
export $(shell sed 's/=.*//' .env)

# Tools
YARN = yarn

dependencies-frontend: dependencies
$(YARN) install

Expand All @@ -38,7 +35,7 @@ build: dependencies-frontend

## Compiles the dashboard assets, and serve the dashboard through its API
serve: build
go run server/cmd/code-annotation/*
go run cli/server/server.go

gorun:
go run server/cmd/code-annotation/*
go run cli/server/server.go
File renamed without changes.
97 changes: 97 additions & 0 deletions cli/server/server.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
package main

import (
"fmt"
"net/http"

"github.com/src-d/code-annotation/server/handler"
"github.com/src-d/code-annotation/server/repository"
"github.com/src-d/code-annotation/server/service"

"github.com/go-chi/chi"
"github.com/go-chi/chi/middleware"
"github.com/go-chi/cors"
"github.com/kelseyhightower/envconfig"
)

type appConfig struct {
Host string `envconfig:"HOST"`
Port int `envconfig:"PORT" default:"8080"`
UIDomain string `envconfig:"UI_DOMAIN" default:"http://127.0.0.1:8080"`
}

func main() {
// main configuration
var conf appConfig
envconfig.MustProcess("", &conf)

// create repos
userRepo := &repository.Users{}

// create services
var oauthConfig service.OAuthConfig
envconfig.MustProcess("oauth", &oauthConfig)
oauth := service.NewOAuth(oauthConfig.ClientID, oauthConfig.ClientSecret)

var jwtConfig service.JWTConfig
envconfig.MustProcess("jwt", &jwtConfig)
jwt := service.NewJWT(jwtConfig.SigningKey)

// cors options
corsOptions := cors.Options{
AllowedOrigins: []string{"*"},
AllowedMethods: []string{"GET", "POST", "PUT", "OPTIONS"},
AllowedHeaders: []string{"Location", "Authorization", "Content-Type"},
}

// loger
logger := service.NewLogger()

r := chi.NewRouter()

ms := []func(http.Handler) http.Handler{
middleware.Recoverer,
cors.New(corsOptions).Handler,
middleware.RequestLogger(&middleware.DefaultLogFormatter{Logger: logger}),
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better write as in documentation:

r := chi.NewRouter()
r.Use(middleware.Recoverer)
r.Use(cors.New(corsOptions).Handler)
r.Use(middleware.RequestLogger(&middleware.DefaultLogFormatter{Logger: logger}))

r.Get("/login", GetHandler(handler.Login(oauth)))
r.Get("/oauth-callback", GetHandler(handler.OAuthCallback(oauth, jwt, userRepo, conf.UIDomain, logger)))

r.Route("/api", func(r chi.Router) {
  r.Use(jwt.Middleware)
  ...
})

not because current code is wrong, but because it isn't that easy to understand.
Like I need to scan all code below this line for ms.

If you want to exclude static (though I don't see why, I think it should be logged also and FrontendStatics may panic also). You can wrap it in another r.Group. But r.Use, r.Use is much more readable than array.


msProtected := append(ms, jwt.Middleware)

r.Group(func(r chi.Router) {
r.Use(ms...)

r.Get("/login", GetHandler(handler.Login(oauth)))
r.Get("/oauth-callback", GetHandler(handler.OAuthCallback(oauth, jwt, userRepo, conf.UIDomain, logger)))
})

r.Route("/api", func(r chi.Router) {
r.Use(msProtected...)

r.Get("/me", GetHandler(handler.Me(userRepo)))

r.Route("/experiments/{experimentId}", func(r chi.Router) {

r.Get("/", GetHandler(handler.GetExperimentDetails()))

r.Route("/assignments", func(r chi.Router) {

r.Get("/", GetHandler(handler.GetAssignmentsForUserExperiment()))
r.Put("/{assignmentId}", GetHandler(handler.SaveAssignment()))
})

r.Get("/file-pairs/{pairId}", GetHandler(handler.GetFilePairDetails()))
})
})

r.Get("/*", handler.FrontendStatics)

logger.Info("running...")
http.ListenAndServe(fmt.Sprintf("%s:%d", conf.Host, conf.Port), r)
}

func GetHandler(rp handler.RequestProcessFunc) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
response, errors := rp(r)
handler.Write(w, r, response, errors)
}
}
80 changes: 0 additions & 80 deletions server/cmd/code-annotation/main.go

This file was deleted.

71 changes: 71 additions & 0 deletions server/handler/assignments.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package handler

import (
"encoding/json"
"io/ioutil"
"net/http"
"strconv"

"github.com/src-d/code-annotation/server/repository"
"github.com/src-d/code-annotation/server/serializer"

"github.com/go-chi/chi"
)

func GetAssignmentsForUserExperiment() RequestProcessFunc {
return func(r *http.Request) (resp *serializer.Response, errs []*serializer.Error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/golang/go/wiki/CodeReviewComments#named-result-parameters

also you lose all the profit of go type checking.

consider code:

func(r *http.Request) (resp *serializer.Response, errs []*serializer.Error) {
  ...a lot of code...
  if somecheck {
    resp = postResponse()
  }
  ...a lot of code...
  resp = getResponse()
  return
}

code above compiles and produces wrong response.

this signature and always return new value avoids the problem:

func(r *http.Request) (*serializer.Response, []*serializer.Error) {

experimentID, err := strconv.Atoi(chi.URLParam(r, "experimentId"))
if err != nil {
errs = append(errs, &serializer.Error{500, "Wrong format in experiment ID sent", err.Error()})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry I don't remember in which document it's described. But go vet won't allow this code with warning like composite literal uses unkeyed fields https://golang.org/cmd/vet/#hdr-Unkeyed_composite_literals

And it makes a lot of sense!
If you change the order of fields (last 2) in your struct - code still compiles but works incorrectly.
Also it will be very painful to add a new optional field.

return
}

userID, err := strconv.Atoi("1")
if err != nil {
errs = append(errs, &serializer.Error{500, "Wrong format in user ID sent", err.Error()})
return
}

assignments, err := repository.GetAssignmentsFor(userID, experimentID)
if err == repository.ErrNoAssignmentsInitialized {
assignments, err = repository.CreateAssignmentsFor(userID, experimentID)
}
if err != nil {
errs = append(errs, &serializer.Error{500, "No assignments retrieved", err.Error()})
return
}

resp = serializer.NewAssignmentsResponse(assignments)
return
}
}

func SaveAssignment() RequestProcessFunc {
return func(r *http.Request) (resp *serializer.Response, errs []*serializer.Error) {
assignmentId, err := strconv.Atoi(chi.URLParam(r, "assignmentId"))
if err != nil {
errs = append(errs, &serializer.Error{500, "Wrong format in assignment ID sent", err.Error()})
return
}

var assignmentRequest serializer.AssignmentRequest
body, err := ioutil.ReadAll(r.Body)
defer r.Body.Close()
if err == nil {
err = json.Unmarshal(body, &assignmentRequest)
}
if err != nil {
errs = append(errs, &serializer.Error{500, "Payload could not be read", err.Error()})
return
}

assignmentCandidate := serializer.NewAssignmentsFromRequest(&assignmentRequest)
if err := repository.UpdateAssignment(assignmentId, assignmentCandidate); err != nil {
errs = append(errs, &serializer.Error{500, "Answer could not be saved", err.Error()})
return
}

resp = serializer.NewCountResponse(1)
return
}
}
33 changes: 20 additions & 13 deletions server/handler/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,48 +6,55 @@ import (

"github.com/sirupsen/logrus"
"github.com/src-d/code-annotation/server/repository"
"github.com/src-d/code-annotation/server/serializer"
"github.com/src-d/code-annotation/server/service"
)

// Login handler redirects user to oauth provider
func Login(oAuth *service.OAuth) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
func Login(oAuth *service.OAuth) RequestProcessFunc {
return func(r *http.Request) (resp *serializer.Response, errs []*serializer.Error) {
url := oAuth.MakeAuthURL()
http.Redirect(w, r, url, http.StatusTemporaryRedirect)
resp = serializer.NewTemporalRedirect(url)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the point of using multiple layers of logic for it if you actually just do http.Redirect anyway?

return
}
}

// OAuthCallback makes exchange with oauth provider, gets&creates user and redirects to index page with JWT token
func OAuthCallback(oAuth *service.OAuth, jwt *service.JWT, userRepo *repository.Users, uiDomain string) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
func OAuthCallback(
oAuth *service.OAuth,
jwt *service.JWT,
userRepo *repository.Users,
uiDomain string,
logger logrus.FieldLogger,
) RequestProcessFunc {
return func(r *http.Request) (resp *serializer.Response, errs []*serializer.Error) {
if err := oAuth.ValidateState(r.FormValue("state")); err != nil {
writeResponse(w, respErr(http.StatusBadRequest, err.Error()))
errs = append(errs, &serializer.Error{500, "state could not be validated", err.Error()})
return
}

code := r.FormValue("code")
user, err := oAuth.GetUser(r.Context(), code)
if err != nil {
logrus.Errorf("oauth get user error: %s", err)
// FIXME can it be not server error? for wrong code
writeResponse(w, respInternalErr())
errs = append(errs, &serializer.Error{500, "oauth get user error", err.Error()})
return
}

// FIXME with real repo we need to check does user exists already or not
if err := userRepo.Create(user); err != nil {
logrus.Errorf("can't create user: %s", err)
writeResponse(w, respInternalErr())
errs = append(errs, &serializer.Error{500, "can't create user", err.Error()})
return
}

token, err := jwt.MakeToken(user)
if err != nil {
logrus.Errorf("make jwt token error: %s", err)
writeResponse(w, respInternalErr())
errs = append(errs, &serializer.Error{500, "make jwt token error", err.Error()})
return
}

url := fmt.Sprintf("%s/?token=%s", uiDomain, token)
http.Redirect(w, r, url, http.StatusTemporaryRedirect)
resp = serializer.NewTemporalRedirect(url)
return
}
}
Loading