3
0

Refactor error handling

This commit is contained in:
Denis Arh 2020-12-07 20:12:10 +01:00
parent 1eba292e75
commit 5dc3fb45c8
4 changed files with 93 additions and 69 deletions

View File

@ -1,6 +1,7 @@
package scim package scim
import ( import (
"fmt"
"github.com/cortezaproject/corteza-server/system/types" "github.com/cortezaproject/corteza-server/system/types"
"net/http" "net/http"
"time" "time"
@ -45,7 +46,11 @@ func newGroupMetaResponse(u *types.Role) *metaResponse {
return rsp return rsp
} }
func newErrorResonse(httpStatus int, err error) *errorResponse { func newErrorfResponse(httpStatus int, format string, aa ...interface{}) *errorResponse {
return newErrorResponse(httpStatus, fmt.Errorf(format, aa...))
}
func newErrorResponse(httpStatus int, err error) *errorResponse {
if httpStatus == 0 { if httpStatus == 0 {
httpStatus = http.StatusInternalServerError httpStatus = http.StatusInternalServerError
} }
@ -61,3 +66,7 @@ func newErrorResonse(httpStatus int, err error) *errorResponse {
return er return er
} }
func (e *errorResponse) Error() string {
return e.Detail
}

View File

@ -45,43 +45,42 @@ func (h groupsHandler) create(w http.ResponseWriter, r *http.Request) {
payload = &groupResourceRequest{} payload = &groupResourceRequest{}
err error err error
existing *types.Role existing *types.Role
code = http.StatusBadRequest
) )
if err = payload.decodeJSON(r.Body); err != nil { if err = payload.decodeJSON(r.Body); err != nil {
sendError(w, newErrorResonse(code, err)) sendError(w, newErrorResponse(http.StatusBadRequest, err))
return return
} }
{ {
// do we need to upsert? // do we need to upsert?
if payload.ExternalId != nil { if payload.ExternalId != nil {
existing, code, err = h.lookupByExternalId(ctx, *payload.ExternalId) existing, err = h.lookupByExternalId(ctx, *payload.ExternalId)
if err != nil && code != http.StatusNotFound { if err != nil {
sendError(w, newErrorResonse(code, err)) sendError(w, err)
return return
} }
} else if *payload.Name != "" { } else if *payload.Name != "" {
existing, err = svc.FindByName(*payload.Name) existing, err = svc.FindByName(*payload.Name)
if err != nil && !errors.Is(err, service.RoleErrNotFound()) { if err != nil && !errors.Is(err, service.RoleErrNotFound()) {
sendError(w, newErrorResonse(http.StatusInternalServerError, err)) sendError(w, err)
return return
} }
} }
} }
res, code, err := h.save(ctx, payload, existing) res, err := h.save(ctx, payload, existing)
if err != nil { if err != nil {
sendError(w, newErrorResonse(code, err)) sendError(w, err)
return return
} }
code = http.StatusOK status := http.StatusOK
if res.UpdatedAt == nil { if res.UpdatedAt == nil {
code = http.StatusCreated status = http.StatusCreated
} }
send(w, code, newGroupResourceResponse(res)) send(w, status, newGroupResourceResponse(res))
} }
func (h groupsHandler) replace(w http.ResponseWriter, r *http.Request) { func (h groupsHandler) replace(w http.ResponseWriter, r *http.Request) {
@ -94,25 +93,25 @@ func (h groupsHandler) replace(w http.ResponseWriter, r *http.Request) {
) )
if err := payload.decodeJSON(r.Body); err != nil { if err := payload.decodeJSON(r.Body); err != nil {
sendError(w, newErrorResonse(http.StatusBadRequest, err)) sendError(w, newErrorResponse(http.StatusBadRequest, err))
return return
} }
res, code, err := h.save(ctx, payload, existing) res, err := h.save(ctx, payload, existing)
if err != nil { if err != nil {
sendError(w, newErrorResonse(code, err)) sendError(w, err)
return return
} }
code = http.StatusOK status := http.StatusOK
if res.UpdatedAt == nil { if res.UpdatedAt == nil {
code = http.StatusCreated status = http.StatusCreated
} }
send(w, code, newGroupResourceResponse(res)) send(w, status, newGroupResourceResponse(res))
} }
func (h groupsHandler) save(ctx context.Context, req *groupResourceRequest, existing *types.Role) (res *types.Role, code int, err error) { func (h groupsHandler) save(ctx context.Context, req *groupResourceRequest, existing *types.Role) (res *types.Role, err error) {
var ( var (
svc = h.svc.With(ctx) svc = h.svc.With(ctx)
) )
@ -133,10 +132,10 @@ func (h groupsHandler) save(ctx context.Context, req *groupResourceRequest, exis
} }
if err != nil { if err != nil {
return nil, http.StatusInternalServerError, err return nil, newErrorResponse(http.StatusInternalServerError, err)
} }
return res, 0, nil return res, nil
} }
func (h groupsHandler) delete(w http.ResponseWriter, r *http.Request) { func (h groupsHandler) delete(w http.ResponseWriter, r *http.Request) {
@ -151,7 +150,7 @@ func (h groupsHandler) delete(w http.ResponseWriter, r *http.Request) {
} }
if err := svc.Delete(res.ID); err != nil { if err := svc.Delete(res.ID); err != nil {
sendError(w, newErrorResonse(http.StatusBadRequest, err)) sendError(w, newErrorResponse(http.StatusBadRequest, err))
} else { } else {
w.WriteHeader(http.StatusNoContent) w.WriteHeader(http.StatusNoContent)
} }
@ -166,23 +165,27 @@ func (h groupsHandler) lookup(ctx context.Context, id string, w http.ResponseWri
) )
if h.externalIdAsPrimary { if h.externalIdAsPrimary {
role, code, err := h.lookupByExternalId(ctx, id) res, err := h.lookupByExternalId(ctx, id)
if err != nil { if err != nil {
sendError(w, newErrorResonse(code, err)) sendError(w, err)
return nil return nil
} }
return role if res == nil {
sendError(w, newErrorResponse(http.StatusNotFound, fmt.Errorf("group not found")))
}
return res
} else { } else {
groupId, err := strconv.ParseUint(id, 10, 64) id, err := strconv.ParseUint(id, 10, 64)
if err != nil || groupId == 0 { if err != nil || id == 0 {
sendError(w, newErrorResonse(http.StatusBadRequest, err)) sendError(w, newErrorResponse(http.StatusBadRequest, err))
return nil return nil
} }
role, err := svc.FindByID(groupId) role, err := svc.FindByID(id)
if err != nil { if err != nil {
sendError(w, newErrorResonse(http.StatusBadRequest, err)) sendError(w, newErrorResponse(http.StatusBadRequest, err))
return nil return nil
} }
@ -190,23 +193,23 @@ func (h groupsHandler) lookup(ctx context.Context, id string, w http.ResponseWri
} }
} }
func (h groupsHandler) lookupByExternalId(ctx context.Context, id string) (r *types.Role, code int, err error) { func (h groupsHandler) lookupByExternalId(ctx context.Context, id string) (r *types.Role, err error) {
spew.Dump(id) spew.Dump(id)
if h.externalIdValidator != nil && !h.externalIdValidator.MatchString(id) { if h.externalIdValidator != nil && !h.externalIdValidator.MatchString(id) {
return nil, http.StatusBadRequest, fmt.Errorf("invalid external ID") return nil, newErrorfResponse(http.StatusBadRequest, "invalid external ID")
} }
rr, _, err := h.svc.With(ctx).Find(types.RoleFilter{Labels: map[string]string{groupLabel_SCIM_externalId: id}}) rr, _, err := h.svc.With(ctx).Find(types.RoleFilter{Labels: map[string]string{groupLabel_SCIM_externalId: id}})
if err != nil { if err != nil {
return nil, http.StatusInternalServerError, err return nil, newErrorResponse(http.StatusInternalServerError, err)
} }
switch len(rr) { switch len(rr) {
case 0: case 0:
return nil, http.StatusNotFound, fmt.Errorf("group not found") return nil, nil
case 1: case 1:
return rr[0], 0, nil return rr[0], nil
default: default:
return nil, http.StatusPreconditionFailed, fmt.Errorf("more than one group matches this externalId") return nil, newErrorfResponse(http.StatusPreconditionFailed, "more than one group matches this externalId")
} }
} }

View File

@ -14,6 +14,14 @@ func send(w http.ResponseWriter, status int, payload interface{}) {
} }
} }
func sendError(w http.ResponseWriter, err *errorResponse) { func sendError(w http.ResponseWriter, err error) {
send(w, err.Status, err) var (
status = http.StatusInternalServerError
)
if er, ok := err.(*errorResponse); ok {
status = er.Status
}
send(w, status, err)
} }

View File

@ -53,39 +53,39 @@ func (h usersHandler) create(w http.ResponseWriter, r *http.Request) {
) )
if err = payload.decodeJSON(r.Body); err != nil { if err = payload.decodeJSON(r.Body); err != nil {
sendError(w, newErrorResonse(code, err)) sendError(w, newErrorResponse(code, err))
return return
} }
{ {
// do we need to upsert? // do we need to upsert?
if payload.ExternalId != nil { if payload.ExternalId != nil {
existing, code, err = h.lookupByExternalId(ctx, *payload.ExternalId) existing, err = h.lookupByExternalId(ctx, *payload.ExternalId)
if err != nil && code != http.StatusNotFound { if err != nil {
sendError(w, newErrorResonse(code, err)) sendError(w, newErrorResponse(code, err))
return return
} }
} else if email := payload.Emails.getFirst(); email != "" { } else if email := payload.Emails.getFirst(); email != "" {
existing, err = svc.FindByEmail(email) existing, err = svc.FindByEmail(email)
if err != nil && !errors.Is(err, service.UserErrNotFound()) { if err != nil && !errors.Is(err, service.UserErrNotFound()) {
sendError(w, newErrorResonse(http.StatusInternalServerError, err)) sendError(w, newErrorResponse(http.StatusInternalServerError, err))
return return
} }
} }
} }
res, code, err := h.save(ctx, payload, existing) res, err := h.save(ctx, payload, existing)
if err != nil { if err != nil {
sendError(w, newErrorResonse(code, err)) sendError(w, err)
return return
} }
code = http.StatusOK status := http.StatusOK
if res.UpdatedAt == nil { if res.UpdatedAt == nil {
code = http.StatusCreated status = http.StatusCreated
} }
send(w, code, newUserResourceResponse(res)) send(w, status, newUserResourceResponse(res))
} }
func (h usersHandler) replace(w http.ResponseWriter, r *http.Request) { func (h usersHandler) replace(w http.ResponseWriter, r *http.Request) {
@ -98,25 +98,25 @@ func (h usersHandler) replace(w http.ResponseWriter, r *http.Request) {
) )
if err := payload.decodeJSON(r.Body); err != nil { if err := payload.decodeJSON(r.Body); err != nil {
sendError(w, newErrorResonse(http.StatusBadRequest, err)) sendError(w, newErrorResponse(http.StatusBadRequest, err))
return return
} }
res, code, err := h.save(ctx, payload, existing) res, err := h.save(ctx, payload, existing)
if err != nil { if err != nil {
sendError(w, newErrorResonse(code, err)) sendError(w, err)
return return
} }
code = http.StatusOK status := http.StatusOK
if res.UpdatedAt == nil { if res.UpdatedAt == nil {
code = http.StatusCreated status = http.StatusCreated
} }
send(w, code, newUserResourceResponse(res)) send(w, status, newUserResourceResponse(res))
} }
func (h usersHandler) save(ctx context.Context, req *userResourceRequest, existing *types.User) (res *types.User, code int, err error) { func (h usersHandler) save(ctx context.Context, req *userResourceRequest, existing *types.User) (res *types.User, err error) {
var ( var (
svc = h.svc.With(ctx) svc = h.svc.With(ctx)
) )
@ -137,7 +137,7 @@ func (h usersHandler) save(ctx context.Context, req *userResourceRequest, existi
} }
if err != nil { if err != nil {
return nil, http.StatusInternalServerError, err return nil, err
} }
if req.Password != nil && *req.Password != "" { if req.Password != nil && *req.Password != "" {
@ -147,7 +147,7 @@ func (h usersHandler) save(ctx context.Context, req *userResourceRequest, existi
} }
} }
return res, 0, nil return res, nil
} }
func (h usersHandler) delete(w http.ResponseWriter, r *http.Request) { func (h usersHandler) delete(w http.ResponseWriter, r *http.Request) {
@ -162,7 +162,7 @@ func (h usersHandler) delete(w http.ResponseWriter, r *http.Request) {
} }
if err := svc.Delete(res.ID); err != nil { if err := svc.Delete(res.ID); err != nil {
sendError(w, newErrorResonse(http.StatusBadRequest, err)) sendError(w, newErrorResponse(http.StatusBadRequest, err))
} else { } else {
w.WriteHeader(http.StatusNoContent) w.WriteHeader(http.StatusNoContent)
} }
@ -177,23 +177,27 @@ func (h usersHandler) lookup(ctx context.Context, id string, w http.ResponseWrit
) )
if h.externalIdAsPrimary { if h.externalIdAsPrimary {
role, code, err := h.lookupByExternalId(ctx, id) res, err := h.lookupByExternalId(ctx, id)
if err != nil { if err != nil {
sendError(w, newErrorResonse(code, err)) sendError(w, err)
return nil return nil
} }
return role if res == nil {
sendError(w, newErrorResponse(http.StatusNotFound, fmt.Errorf("user not found")))
}
return res
} else { } else {
groupId, err := strconv.ParseUint(id, 10, 64) groupId, err := strconv.ParseUint(id, 10, 64)
if err != nil || groupId == 0 { if err != nil || groupId == 0 {
sendError(w, newErrorResonse(http.StatusBadRequest, err)) sendError(w, newErrorResponse(http.StatusBadRequest, err))
return nil return nil
} }
role, err := svc.FindByID(groupId) role, err := svc.FindByID(groupId)
if err != nil { if err != nil {
sendError(w, newErrorResonse(http.StatusBadRequest, err)) sendError(w, newErrorResponse(http.StatusBadRequest, err))
return nil return nil
} }
@ -201,23 +205,23 @@ func (h usersHandler) lookup(ctx context.Context, id string, w http.ResponseWrit
} }
} }
func (h usersHandler) lookupByExternalId(ctx context.Context, id string) (r *types.User, code int, err error) { func (h usersHandler) lookupByExternalId(ctx context.Context, id string) (r *types.User, err error) {
spew.Dump(id) spew.Dump(id)
if h.externalIdValidator != nil && !h.externalIdValidator.MatchString(id) { if h.externalIdValidator != nil && !h.externalIdValidator.MatchString(id) {
return nil, http.StatusBadRequest, fmt.Errorf("invalid external ID") return nil, newErrorfResponse(http.StatusBadRequest, "invalid external ID")
} }
rr, _, err := h.svc.With(ctx).Find(types.UserFilter{Labels: map[string]string{groupLabel_SCIM_externalId: id}}) rr, _, err := h.svc.With(ctx).Find(types.UserFilter{Labels: map[string]string{groupLabel_SCIM_externalId: id}})
if err != nil { if err != nil {
return nil, http.StatusInternalServerError, err return nil, newErrorResponse(http.StatusInternalServerError, err)
} }
switch len(rr) { switch len(rr) {
case 0: case 0:
return nil, http.StatusNotFound, fmt.Errorf("user not found") return nil, nil
case 1: case 1:
return rr[0], 0, nil return rr[0], nil
default: default:
return nil, http.StatusPreconditionFailed, fmt.Errorf("more than one user matches this externalId") return nil, newErrorfResponse(http.StatusPreconditionFailed, "more than one user matches this externalId")
} }
} }