From 1fb16a4d8d89dc7aced51c9fea1df0b879fbf8fd Mon Sep 17 00:00:00 2001 From: pjdufour-truss Date: Thu, 9 May 2019 20:38:51 +0000 Subject: [PATCH] trace ids --- cmd/milmove/main.go | 2 + pkg/auth/authentication/client_cert.go | 7 +- pkg/auth/cookie.go | 29 +++-- pkg/auth/session.go | 12 +- pkg/handlers/contexts.go | 46 ++++++-- pkg/handlers/dpsapi/authentication.go | 15 ++- pkg/handlers/internalapi/backup_contacts.go | 26 ++-- pkg/handlers/internalapi/documents.go | 23 ++-- .../internalapi/dps_auth_cookie_url.go | 11 +- pkg/handlers/internalapi/duty_stations.go | 8 +- pkg/handlers/internalapi/entitlements.go | 13 +- .../internalapi/generic_move_documents.go | 13 +- pkg/handlers/internalapi/logger.go | 15 +++ pkg/handlers/internalapi/move_documents.go | 23 ++-- pkg/handlers/internalapi/move_queue_items.go | 7 +- pkg/handlers/internalapi/moves.go | 65 +++++----- .../internalapi/moving_expense_documents.go | 13 +- pkg/handlers/internalapi/office.go | 61 +++++----- pkg/handlers/internalapi/orders.go | 37 +++--- .../internalapi/personally_procured_move.go | 65 +++++----- .../personally_procured_move_attachments.go | 19 ++- .../personally_procured_move_estimate.go | 13 +- .../personally_procured_move_incentive.go | 13 +- .../personally_procured_move_sit_estimate.go | 9 +- pkg/handlers/internalapi/service_members.go | 29 +++-- pkg/handlers/internalapi/shipments.go | 76 +++++++----- .../internalapi/signed_certifications.go | 17 ++- .../internalapi/transportation_offices.go | 3 +- pkg/handlers/internalapi/uploads.go | 48 ++++---- pkg/handlers/internalapi/users.go | 19 ++- pkg/handlers/ordersapi/get_orders.go | 19 +-- .../get_orders_by_issuer_and_orders_num.go | 17 ++- .../ordersapi/index_orders_for_member.go | 17 ++- pkg/handlers/ordersapi/post_revision.go | 29 +++-- .../ordersapi/post_revision_to_orders.go | 25 ++-- pkg/handlers/publicapi/access_code.go | 5 +- .../publicapi/generic_move_documents.go | 21 ++-- pkg/handlers/publicapi/invoices.go | 19 ++- pkg/handlers/publicapi/logger.go | 15 +++ pkg/handlers/publicapi/move_documents.go | 45 ++++--- pkg/handlers/publicapi/service_agents.go | 33 +++--- pkg/handlers/publicapi/shipment_line_items.go | 100 ++++++++-------- pkg/handlers/publicapi/shipments.go | 111 +++++++++--------- pkg/handlers/publicapi/shipments_test.go | 6 +- pkg/handlers/publicapi/storage_in_transits.go | 107 ++++++++--------- pkg/handlers/publicapi/tariff400ng_items.go | 5 +- .../transportation_service_provider.go | 15 ++- .../transportation_service_provider_test.go | 6 +- pkg/logging/context.go | 19 +++ pkg/logging/logger.go | 10 -- pkg/middleware/context_logger.go | 24 ++++ pkg/middleware/context_logger_test.go | 41 +++++++ pkg/middleware/logger.go | 6 + pkg/middleware/middleware_test.go | 17 +++ pkg/middleware/request_logger.go | 14 ++- pkg/middleware/trace.go | 26 ++++ pkg/middleware/trace_test.go | 23 ++++ pkg/trace/context.go | 23 ++++ 58 files changed, 917 insertions(+), 618 deletions(-) create mode 100644 pkg/handlers/internalapi/logger.go create mode 100644 pkg/handlers/publicapi/logger.go create mode 100644 pkg/logging/context.go delete mode 100644 pkg/logging/logger.go create mode 100644 pkg/middleware/context_logger.go create mode 100644 pkg/middleware/context_logger_test.go create mode 100644 pkg/middleware/trace.go create mode 100644 pkg/middleware/trace_test.go create mode 100644 pkg/trace/context.go diff --git a/cmd/milmove/main.go b/cmd/milmove/main.go index 23a478dab86c..57fb4b00563b 100644 --- a/cmd/milmove/main.go +++ b/cmd/milmove/main.go @@ -591,6 +591,8 @@ func serveFunction(cmd *cobra.Command, args []string) error { root := goji.NewMux() root.Use(middleware.Recovery(logger)) + root.Use(middleware.Trace(logger)) // injects http request trace id + root.Use(middleware.ContextLogger("milmove_trace_id", logger)) // injects http request logger root.Use(sessionCookieMiddleware) root.Use(middleware.RequestLogger(logger)) diff --git a/pkg/auth/authentication/client_cert.go b/pkg/auth/authentication/client_cert.go index d0c1f6616e48..936b73f9287c 100644 --- a/pkg/auth/authentication/client_cert.go +++ b/pkg/auth/authentication/client_cert.go @@ -23,7 +23,12 @@ func SetClientCertInRequestContext(r *http.Request, clientCert *models.ClientCer // ClientCertFromRequestContext gets the reference to the ClientCert stored in the request.Context() func ClientCertFromRequestContext(r *http.Request) *models.ClientCert { - if clientCert, ok := r.Context().Value(clientCertContextKey).(*models.ClientCert); ok { + return ClientCertFromContext(r.Context()) +} + +// ClientCertFromContext gets the reference to the ClientCert stored in the request.Context() +func ClientCertFromContext(ctx context.Context) *models.ClientCert { + if clientCert, ok := ctx.Value(clientCertContextKey).(*models.ClientCert); ok { return clientCert } return nil diff --git a/pkg/auth/cookie.go b/pkg/auth/cookie.go index bfb358692265..a1bcc1398885 100644 --- a/pkg/auth/cookie.go +++ b/pkg/auth/cookie.go @@ -12,6 +12,8 @@ import ( beeline "github.com/honeycombio/beeline-go" "github.com/pkg/errors" "go.uber.org/zap" + + "github.com/transcom/mymove/pkg/logging" ) // ApplicationServername is a collection of all the servernames for the application @@ -234,15 +236,25 @@ func ApplicationName(hostname string, appnames ApplicationServername) (Applicati } // SessionCookieMiddleware handle serializing and de-serializing the session between the user_session cookie and the request context -func SessionCookieMiddleware(logger Logger, secret string, noSessionTimeout bool, appnames ApplicationServername, useSecureCookie bool) func(next http.Handler) http.Handler { - logger.Info("Creating session", +func SessionCookieMiddleware(serverLogger Logger, secret string, noSessionTimeout bool, appnames ApplicationServername, useSecureCookie bool) func(next http.Handler) http.Handler { + serverLogger.Info("Creating session", zap.String("milServername", appnames.MilServername), zap.String("officeServername", appnames.OfficeServername), zap.String("tspServername", appnames.TspServername), zap.String("adminServername", appnames.AdminServername)) return func(next http.Handler) http.Handler { - mw := func(w http.ResponseWriter, r *http.Request) { - ctx, span := beeline.StartSpan(r.Context(), "SessionCookieMiddleware") + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + + ctx := r.Context() + + var logger Logger + if requestLogger, ok := logging.FromContext(ctx).(Logger); ok { + logger = requestLogger + } else { + logger = serverLogger + } + + ctx, span := beeline.StartSpan(ctx, "SessionCookieMiddleware") defer span.Send() // Set up the new session object @@ -265,18 +277,15 @@ func SessionCookieMiddleware(logger Logger, secret string, noSessionTimeout bool session.ApplicationName = appName session.Hostname = strings.ToLower(hostname) - // And put the session info into the request context - ctx = SetSessionInRequestContext(r.WithContext(ctx), &session) - // And update the cookie. May get over-ridden later WriteSessionCookie(w, &session, secret, noSessionTimeout, logger, useSecureCookie) span.AddTraceField("auth.application_name", session.ApplicationName) span.AddTraceField("auth.hostname", session.Hostname) - next.ServeHTTP(w, r.WithContext(ctx)) + // And put the session info into the request context + next.ServeHTTP(w, r.WithContext(SetSessionInContext(ctx, &session))) - } - return http.HandlerFunc(mw) + }) } } diff --git a/pkg/auth/session.go b/pkg/auth/session.go index a37466ea8157..c395bc9a04be 100644 --- a/pkg/auth/session.go +++ b/pkg/auth/session.go @@ -67,9 +67,19 @@ func SetSessionInRequestContext(r *http.Request, session *Session) context.Conte return context.WithValue(r.Context(), sessionContextKey, session) } +// SetSessionInContext modifies the context to add the session data. +func SetSessionInContext(ctx context.Context, session *Session) context.Context { + return context.WithValue(ctx, sessionContextKey, session) +} + // SessionFromRequestContext gets the reference to the Session stored in the request.Context() func SessionFromRequestContext(r *http.Request) *Session { - if session, ok := r.Context().Value(sessionContextKey).(*Session); ok { + return SessionFromContext(r.Context()) +} + +// SessionFromContext gets the reference to the Session stored in the request.Context() +func SessionFromContext(ctx context.Context) *Session { + if session, ok := ctx.Value(sessionContextKey).(*Session); ok { return session } return nil diff --git a/pkg/handlers/contexts.go b/pkg/handlers/contexts.go index fc5dab720283..84076ab1c9fd 100644 --- a/pkg/handlers/contexts.go +++ b/pkg/handlers/contexts.go @@ -2,15 +2,18 @@ package handlers import ( "context" + "net/http" "github.com/go-openapi/runtime/middleware" "github.com/gobuffalo/pop" "github.com/gobuffalo/validate" "go.uber.org/zap" + "github.com/transcom/mymove/pkg/auth" "github.com/transcom/mymove/pkg/db/sequence" "github.com/transcom/mymove/pkg/dpsauth" "github.com/transcom/mymove/pkg/iws" + "github.com/transcom/mymove/pkg/logging" "github.com/transcom/mymove/pkg/logging/hnyzap" "github.com/transcom/mymove/pkg/notifications" "github.com/transcom/mymove/pkg/route" @@ -21,7 +24,12 @@ import ( // HandlerContext provides access to all the contextual references needed by individual handlers type HandlerContext interface { DB() *pop.Connection - Logger() Logger + SessionAndLoggerFromContext(ctx context.Context) (*auth.Session, Logger) + SessionAndLoggerFromRequest(r *http.Request) (*auth.Session, Logger) + SessionFromRequest(r *http.Request) *auth.Session + SessionFromContext(ctx context.Context) *auth.Session + LoggerFromContext(ctx context.Context) Logger + LoggerFromRequest(r *http.Request) Logger HoneyZapLogger() *hnyzap.Logger FileStorer() storage.FileStorer SetFileStorer(storer storage.FileStorer) @@ -75,16 +83,38 @@ func NewHandlerContext(db *pop.Connection, logger Logger) HandlerContext { } } -// DB returns a POP db connection for the context -func (hctx *handlerContext) DB() *pop.Connection { - return hctx.db +func (hctx *handlerContext) SessionAndLoggerFromRequest(r *http.Request) (*auth.Session, Logger) { + return hctx.SessionAndLoggerFromContext(r.Context()) +} + +func (hctx *handlerContext) SessionAndLoggerFromContext(ctx context.Context) (*auth.Session, Logger) { + return auth.SessionFromContext(ctx), hctx.LoggerFromContext(ctx) +} + +func (hctx *handlerContext) SessionFromRequest(r *http.Request) *auth.Session { + return auth.SessionFromContext(r.Context()) +} + +func (hctx *handlerContext) SessionFromContext(ctx context.Context) *auth.Session { + return auth.SessionFromContext(ctx) } -// Logger returns the logger to use in this context -func (hctx *handlerContext) Logger() Logger { +func (hctx *handlerContext) LoggerFromRequest(r *http.Request) Logger { + return hctx.LoggerFromContext(r.Context()) +} + +func (hctx *handlerContext) LoggerFromContext(ctx context.Context) Logger { + if logger, ok := logging.FromContext(ctx).(Logger); ok { + return logger + } return hctx.logger } +// DB returns a POP db connection for the context +func (hctx *handlerContext) DB() *pop.Connection { + return hctx.db +} + // HoneyZapLogger returns the logger capable of writing to Honeycomb to use in this context func (hctx *handlerContext) HoneyZapLogger() *hnyzap.Logger { if zapLogger, ok := hctx.logger.(*zap.Logger); ok { @@ -96,13 +126,13 @@ func (hctx *handlerContext) HoneyZapLogger() *hnyzap.Logger { // RespondAndTraceError uses Honeycomb to trace errors and then passes response to the standard ResponseForError func (hctx *handlerContext) RespondAndTraceError(ctx context.Context, err error, msg string, fields ...zap.Field) middleware.Responder { hctx.HoneyZapLogger().TraceError(ctx, msg, fields...) - return ResponseForError(hctx.Logger(), err) + return ResponseForError(hctx.LoggerFromContext(ctx), err) } // RespondAndTraceVErrors uses Honeycomb to trace errors and then passes response to the standard ResponseForVErrors func (hctx *handlerContext) RespondAndTraceVErrors(ctx context.Context, verrs *validate.Errors, err error, msg string, fields ...zap.Field) middleware.Responder { hctx.HoneyZapLogger().TraceError(ctx, msg, fields...) - return ResponseForVErrors(hctx.Logger(), verrs, err) + return ResponseForVErrors(hctx.LoggerFromContext(ctx), verrs, err) } // FileStorer returns the storage to use in the current context diff --git a/pkg/handlers/dpsapi/authentication.go b/pkg/handlers/dpsapi/authentication.go index 492970f73476..388c487327ce 100644 --- a/pkg/handlers/dpsapi/authentication.go +++ b/pkg/handlers/dpsapi/authentication.go @@ -34,9 +34,14 @@ var affiliationMap = map[models.ServiceMemberAffiliation]dpsmessages.Affiliation // Handle returns user information given an encrypted token func (h GetUserHandler) Handle(params dps.GetUserParams) middleware.Responder { - clientCert := authentication.ClientCertFromRequestContext(params.HTTPRequest) + + ctx := params.HTTPRequest.Context() + + logger := h.LoggerFromContext(ctx) + + clientCert := authentication.ClientCertFromContext(ctx) if clientCert == nil || !clientCert.AllowDpsAuthAPI { - h.Logger().Info("Client certificate is not authorized to access this API") + logger.Info("Client certificate is not authorized to access this API") return dps.NewGetUserUnauthorized() } @@ -44,7 +49,7 @@ func (h GetUserHandler) Handle(params dps.GetUserParams) middleware.Responder { token := params.Token loginGovID, err := dpsauth.CookieToLoginGovID(token, dpsParams.CookieSecret) if err != nil { - h.Logger().Error("Extracting user ID from token", zap.Error(err)) + logger.Error("Extracting user ID from token", zap.Error(err)) switch err.(type) { case *dpsauth.ErrInvalidCookie: @@ -57,9 +62,9 @@ func (h GetUserHandler) Handle(params dps.GetUserParams) middleware.Responder { if err != nil { switch e := err.(type) { case *errUserMissingData: - h.Logger().Error("Fetching user data from user ID", zap.Error(err), zap.String("user", e.userID.String())) + logger.Error("Fetching user data from user ID", zap.Error(err), zap.String("user", e.userID.String())) default: - h.Logger().Error("Fetching user data from user ID", zap.Error(err)) + logger.Error("Fetching user data from user ID", zap.Error(err)) } return dps.NewGetUserInternalServerError() diff --git a/pkg/handlers/internalapi/backup_contacts.go b/pkg/handlers/internalapi/backup_contacts.go index c4865bd0f04f..b4b5ae0d75b5 100644 --- a/pkg/handlers/internalapi/backup_contacts.go +++ b/pkg/handlers/internalapi/backup_contacts.go @@ -7,7 +7,6 @@ import ( "github.com/gofrs/uuid" "github.com/honeycombio/beeline-go" - "github.com/transcom/mymove/pkg/auth" backupop "github.com/transcom/mymove/pkg/gen/internalapi/internaloperations/backup_contacts" "github.com/transcom/mymove/pkg/gen/internalmessages" "github.com/transcom/mymove/pkg/handlers" @@ -35,16 +34,19 @@ type CreateBackupContactHandler struct { // Handle ... creates a new BackupContact from a request payload func (h CreateBackupContactHandler) Handle(params backupop.CreateServiceMemberBackupContactParams) middleware.Responder { - ctx, span := beeline.StartSpan(params.HTTPRequest.Context(), reflect.TypeOf(h).Name()) + + ctx := params.HTTPRequest.Context() + + ctx, span := beeline.StartSpan(ctx, reflect.TypeOf(h).Name()) defer span.Send() // User should always be populated by middleware - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromContext(ctx) /* #nosec UUID is pattern matched by swagger which checks the format */ serviceMemberID, _ := uuid.FromString(params.ServiceMemberID.String()) serviceMember, err := models.FetchServiceMemberForUser(ctx, h.DB(), session, serviceMemberID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } newContact, verrs, err := serviceMember.CreateBackupContact(h.DB(), @@ -53,7 +55,7 @@ func (h CreateBackupContactHandler) Handle(params backupop.CreateServiceMemberBa params.CreateBackupContactPayload.Telephone, models.BackupContactPermission(params.CreateBackupContactPayload.Permission)) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } contactPayload := payloadForBackupContactModel(newContact) @@ -70,13 +72,13 @@ func (h IndexBackupContactsHandler) Handle(params backupop.IndexServiceMemberBac ctx, span := beeline.StartSpan(params.HTTPRequest.Context(), reflect.TypeOf(h).Name()) defer span.Send() - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) /* #nosec UUID is pattern matched by swagger which checks the format */ serviceMemberID, _ := uuid.FromString(params.ServiceMemberID.String()) serviceMember, err := models.FetchServiceMemberForUser(ctx, h.DB(), session, serviceMemberID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } contacts := serviceMember.BackupContacts @@ -98,12 +100,12 @@ type ShowBackupContactHandler struct { // Handle retrieves a backup contact in the system belonging to the logged in user given backup contact ID func (h ShowBackupContactHandler) Handle(params backupop.ShowServiceMemberBackupContactParams) middleware.Responder { // User should always be populated by middleware - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) /* #nosec UUID is pattern matched by swagger which checks the format */ contactID, _ := uuid.FromString(params.BackupContactID.String()) contact, err := models.FetchBackupContact(h.DB(), session, contactID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } contactPayload := payloadForBackupContactModel(contact) @@ -118,12 +120,12 @@ type UpdateBackupContactHandler struct { // Handle ... updates a BackupContact from a request payload func (h UpdateBackupContactHandler) Handle(params backupop.UpdateServiceMemberBackupContactParams) middleware.Responder { // User should always be populated by middleware - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) /* #nosec UUID is pattern matched by swagger which checks the format */ contactID, _ := uuid.FromString(params.BackupContactID.String()) contact, err := models.FetchBackupContact(h.DB(), session, contactID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } contact.Name = *params.UpdateServiceMemberBackupContactPayload.Name @@ -132,7 +134,7 @@ func (h UpdateBackupContactHandler) Handle(params backupop.UpdateServiceMemberBa contact.Permission = models.BackupContactPermission(params.UpdateServiceMemberBackupContactPayload.Permission) if verrs, err := h.DB().ValidateAndUpdate(&contact); verrs.HasAny() || err != nil { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } contactPayload := payloadForBackupContactModel(contact) diff --git a/pkg/handlers/internalapi/documents.go b/pkg/handlers/internalapi/documents.go index 4c14e1b2e3ff..71f131aecee2 100644 --- a/pkg/handlers/internalapi/documents.go +++ b/pkg/handlers/internalapi/documents.go @@ -9,7 +9,6 @@ import ( "github.com/honeycombio/beeline-go" - auth "github.com/transcom/mymove/pkg/auth" documentop "github.com/transcom/mymove/pkg/gen/internalapi/internaloperations/documents" "github.com/transcom/mymove/pkg/gen/internalmessages" "github.com/transcom/mymove/pkg/handlers" @@ -47,17 +46,17 @@ func (h CreateDocumentHandler) Handle(params documentop.CreateDocumentParams) mi ctx, span := beeline.StartSpan(params.HTTPRequest.Context(), reflect.TypeOf(h).Name()) defer span.Send() - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) serviceMemberID, err := uuid.FromString(params.DocumentPayload.ServiceMemberID.String()) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } // Fetch to check auth serviceMember, err := models.FetchServiceMemberForUser(ctx, h.DB(), session, serviceMemberID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } newDocument := models.Document{ @@ -66,17 +65,17 @@ func (h CreateDocumentHandler) Handle(params documentop.CreateDocumentParams) mi verrs, err := h.DB().ValidateAndCreate(&newDocument) if err != nil { - h.Logger().Info("DB Insertion", zap.Error(err)) + logger.Info("DB Insertion", zap.Error(err)) return documentop.NewCreateDocumentInternalServerError() } else if verrs.HasAny() { - h.Logger().Error("Could not save document", zap.String("errors", verrs.Error())) + logger.Error("Could not save document", zap.String("errors", verrs.Error())) return documentop.NewCreateDocumentBadRequest() } - h.Logger().Info("created a document with id: ", zap.Any("new_document_id", newDocument.ID)) + logger.Info("created a document with id", zap.Any("new_document_id", newDocument.ID)) documentPayload, err := payloadForDocumentModel(h.FileStorer(), newDocument) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return documentop.NewCreateDocumentCreated().WithPayload(documentPayload) } @@ -92,21 +91,21 @@ func (h ShowDocumentHandler) Handle(params documentop.ShowDocumentParams) middle ctx, span := beeline.StartSpan(params.HTTPRequest.Context(), reflect.TypeOf(h).Name()) defer span.Send() - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) documentID, err := uuid.FromString(params.DocumentID.String()) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } document, err := models.FetchDocument(ctx, h.DB(), session, documentID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } documentPayload, err := payloadForDocumentModel(h.FileStorer(), document) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return documentop.NewShowDocumentOK().WithPayload(documentPayload) diff --git a/pkg/handlers/internalapi/dps_auth_cookie_url.go b/pkg/handlers/internalapi/dps_auth_cookie_url.go index 2d1aafc28fe0..692eb49d7ef8 100644 --- a/pkg/handlers/internalapi/dps_auth_cookie_url.go +++ b/pkg/handlers/internalapi/dps_auth_cookie_url.go @@ -9,7 +9,6 @@ import ( "github.com/gofrs/uuid" "go.uber.org/zap" - "github.com/transcom/mymove/pkg/auth" "github.com/transcom/mymove/pkg/dpsauth" "github.com/transcom/mymove/pkg/gen/internalapi/internaloperations/dps_auth" "github.com/transcom/mymove/pkg/gen/internalmessages" @@ -40,7 +39,7 @@ func (h DPSAuthGetCookieURLHandler) Handle(params dps_auth.GetCookieURLParams) m // launch this feature, all service members should be able to access this endpoint. // Important: Only DPS users should ever be allowed to set parameters though (for testing). // Service members should never be allowed to set params and only be allowed to use the default params. - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) if !session.IsDpsUser() { return dps_auth.NewGetCookieURLForbidden() } @@ -48,7 +47,7 @@ func (h DPSAuthGetCookieURLHandler) Handle(params dps_auth.GetCookieURLParams) m dpsParams := h.DPSAuthParams() url, err := url.Parse(fmt.Sprintf("%s://%s:%d%s", dpsParams.SDDCProtocol, dpsParams.SDDCHostname, dpsParams.SDDCPort, dpsauth.SetCookiePath)) if err != nil { - h.Logger().Error("Parsing cookie URL", zap.Error(err)) + logger.Error("Parsing cookie URL", zap.Error(err)) return dps_auth.NewGetCookieURLInternalServerError() } @@ -56,9 +55,9 @@ func (h DPSAuthGetCookieURLHandler) Handle(params dps_auth.GetCookieURLParams) m if err != nil { switch e := err.(type) { case *errUserMissing: - h.Logger().Error("Generating token for cookie URL", zap.Error(err), zap.String("user", e.userID.String())) + logger.Error("Generating token for cookie URL", zap.Error(err), zap.String("user", e.userID.String())) default: - h.Logger().Error("Generating token for cookie URL", zap.Error(err)) + logger.Error("Generating token for cookie URL", zap.Error(err)) } return dps_auth.NewGetCookieURLInternalServerError() @@ -84,7 +83,7 @@ func (h DPSAuthGetCookieURLHandler) generateToken(params dps_auth.GetCookieURLPa dpsRedirectURL = *params.DpsRedirectURL } - session := auth.SessionFromRequestContext(params.HTTPRequest) + session := h.SessionFromRequest(params.HTTPRequest) user, err := models.GetUser(h.DB(), session.UserID) if err != nil { return "", &errUserMissing{userID: session.UserID} diff --git a/pkg/handlers/internalapi/duty_stations.go b/pkg/handlers/internalapi/duty_stations.go index fd0e8380cc16..4e799bfb0994 100644 --- a/pkg/handlers/internalapi/duty_stations.go +++ b/pkg/handlers/internalapi/duty_stations.go @@ -41,12 +41,12 @@ type SearchDutyStationsHandler struct { // Handle returns a list of stations based on the search query func (h SearchDutyStationsHandler) Handle(params stationop.SearchDutyStationsParams) middleware.Responder { - var stations models.DutyStations - var err error - stations, err = models.FindDutyStations(h.DB(), params.Search) + logger := h.LoggerFromRequest(params.HTTPRequest) + + stations, err := models.FindDutyStations(h.DB(), params.Search) if err != nil { - h.Logger().Error("Finding duty stations", zap.Error(err)) + logger.Error("Finding duty stations", zap.Error(err)) return stationop.NewSearchDutyStationsInternalServerError() } diff --git a/pkg/handlers/internalapi/entitlements.go b/pkg/handlers/internalapi/entitlements.go index 0da86f1e415b..3ebde126a303 100644 --- a/pkg/handlers/internalapi/entitlements.go +++ b/pkg/handlers/internalapi/entitlements.go @@ -10,7 +10,6 @@ import ( "github.com/honeycombio/beeline-go" - "github.com/transcom/mymove/pkg/auth" entitlementop "github.com/transcom/mymove/pkg/gen/internalapi/internaloperations/entitlements" "github.com/transcom/mymove/pkg/handlers" "github.com/transcom/mymove/pkg/models" @@ -27,21 +26,21 @@ func (h ValidateEntitlementHandler) Handle(params entitlementop.ValidateEntitlem ctx, span := beeline.StartSpan(params.HTTPRequest.Context(), reflect.TypeOf(h).Name()) defer span.Send() - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) moveID, _ := uuid.FromString(params.MoveID.String()) // Fetch move, orders, serviceMember and PPM move, err := models.FetchMove(h.DB(), session, moveID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } orders, err := models.FetchOrderForUser(h.DB(), session, move.OrdersID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } serviceMember, err := models.FetchServiceMemberForUser(ctx, h.DB(), session, orders.ServiceMemberID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } // Return 404 if there's no PPM or Shipment, or if there is no Rank @@ -58,10 +57,10 @@ func (h ValidateEntitlementHandler) Handle(params entitlementop.ValidateEntitlem smEntitlement, err := models.GetEntitlement(*serviceMember.Rank, orders.HasDependents, orders.SpouseHasProGear) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } if weightEstimate > int64(smEntitlement) { - return handlers.ResponseForConflictErrors(h.Logger(), fmt.Errorf("your estimated weight of %s lbs is above your weight entitlement of %s lbs. \n You will only be paid for the weight you move up to your weight entitlement", humanize.Comma(weightEstimate), humanize.Comma(int64(smEntitlement)))) + return handlers.ResponseForConflictErrors(logger, fmt.Errorf("your estimated weight of %s lbs is above your weight entitlement of %s lbs. \n You will only be paid for the weight you move up to your weight entitlement", humanize.Comma(weightEstimate), humanize.Comma(int64(smEntitlement)))) } return entitlementop.NewValidateEntitlementOK() diff --git a/pkg/handlers/internalapi/generic_move_documents.go b/pkg/handlers/internalapi/generic_move_documents.go index 14aa8d7458e9..9d1fa14dce46 100644 --- a/pkg/handlers/internalapi/generic_move_documents.go +++ b/pkg/handlers/internalapi/generic_move_documents.go @@ -7,7 +7,6 @@ import ( "github.com/gofrs/uuid" "github.com/honeycombio/beeline-go" - "github.com/transcom/mymove/pkg/auth" movedocop "github.com/transcom/mymove/pkg/gen/internalapi/internaloperations/move_docs" "github.com/transcom/mymove/pkg/gen/internalmessages" "github.com/transcom/mymove/pkg/handlers" @@ -46,14 +45,14 @@ func (h CreateGenericMoveDocumentHandler) Handle(params movedocop.CreateGenericM ctx, span := beeline.StartSpan(params.HTTPRequest.Context(), reflect.TypeOf(h).Name()) defer span.Send() - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) // #nosec UUID is pattern matched by swagger and will be ok moveID, _ := uuid.FromString(params.MoveID.String()) // Validate that this move belongs to the current user move, err := models.FetchMove(h.DB(), session, moveID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } payload := params.CreateGenericMoveDocumentPayload @@ -69,7 +68,7 @@ func (h CreateGenericMoveDocumentHandler) Handle(params movedocop.CreateGenericM converted := uuid.Must(uuid.FromString(id.String())) upload, fetchUploadErr := models.FetchUpload(ctx, h.DB(), session, converted) if fetchUploadErr != nil { - return handlers.ResponseForError(h.Logger(), fetchUploadErr) + return handlers.ResponseForError(logger, fetchUploadErr) } uploads = append(uploads, upload) } @@ -81,7 +80,7 @@ func (h CreateGenericMoveDocumentHandler) Handle(params movedocop.CreateGenericM // Enforce that the ppm's move_id matches our move ppm, fetchPPMErr := models.FetchPersonallyProcuredMove(h.DB(), session, id) if fetchPPMErr != nil { - return handlers.ResponseForError(h.Logger(), fetchPPMErr) + return handlers.ResponseForError(logger, fetchPPMErr) } if ppm.MoveID != moveID { return movedocop.NewCreateGenericMoveDocumentBadRequest() @@ -99,12 +98,12 @@ func (h CreateGenericMoveDocumentHandler) Handle(params movedocop.CreateGenericM *move.SelectedMoveType) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } newPayload, err := payloadForGenericMoveDocumentModel(h.FileStorer(), *newMoveDocument) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return movedocop.NewCreateGenericMoveDocumentOK().WithPayload(newPayload) } diff --git a/pkg/handlers/internalapi/logger.go b/pkg/handlers/internalapi/logger.go new file mode 100644 index 000000000000..fad6a39e8904 --- /dev/null +++ b/pkg/handlers/internalapi/logger.go @@ -0,0 +1,15 @@ +package internalapi + +import ( + "go.uber.org/zap" +) + +// Logger is a logger interface for middleware +type Logger interface { + Debug(msg string, fields ...zap.Field) + Info(msg string, fields ...zap.Field) + Error(msg string, fields ...zap.Field) + Warn(msg string, fields ...zap.Field) + Fatal(msg string, fields ...zap.Field) + WithOptions(options ...zap.Option) *zap.Logger +} diff --git a/pkg/handlers/internalapi/move_documents.go b/pkg/handlers/internalapi/move_documents.go index dad1fc819a43..85b15e729908 100644 --- a/pkg/handlers/internalapi/move_documents.go +++ b/pkg/handlers/internalapi/move_documents.go @@ -6,7 +6,6 @@ import ( "github.com/pkg/errors" - "github.com/transcom/mymove/pkg/auth" movedocop "github.com/transcom/mymove/pkg/gen/internalapi/internaloperations/move_docs" "github.com/transcom/mymove/pkg/gen/internalmessages" "github.com/transcom/mymove/pkg/handlers" @@ -87,26 +86,26 @@ type IndexMoveDocumentsHandler struct { // Handle handles the request func (h IndexMoveDocumentsHandler) Handle(params movedocop.IndexMoveDocumentsParams) middleware.Responder { // #nosec User should always be populated by middleware - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) // #nosec UUID is pattern matched by swagger and will be ok moveID, _ := uuid.FromString(params.MoveID.String()) // Validate that this move belongs to the current user move, err := models.FetchMove(h.DB(), session, moveID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } moveDocs, err := move.FetchAllMoveDocumentsForMove(h.DB()) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } moveDocumentsPayload := make(internalmessages.MoveDocuments, len(moveDocs)) for i, doc := range moveDocs { moveDocumentPayload, err := payloadForMoveDocumentExtractor(h.FileStorer(), doc) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } moveDocumentsPayload[i] = moveDocumentPayload } @@ -122,14 +121,14 @@ type UpdateMoveDocumentHandler struct { // Handle ... updates a move document from a request payload func (h UpdateMoveDocumentHandler) Handle(params movedocop.UpdateMoveDocumentParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) moveDocID, _ := uuid.FromString(params.MoveDocumentID.String()) // Fetch move document from move id moveDoc, err := models.FetchMoveDocument(h.DB(), session, moveDocID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } payload := params.UpdateMoveDocument @@ -148,12 +147,12 @@ func (h UpdateMoveDocumentHandler) Handle(params movedocop.UpdateMoveDocumentPar if newStatus != moveDoc.Status { err = moveDoc.AttemptTransition(newStatus) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } if newStatus == models.MoveDocumentStatusOK && moveDoc.MoveDocumentType == models.MoveDocumentTypeSHIPMENTSUMMARY { if moveDoc.PersonallyProcuredMoveID == nil { - return handlers.ResponseForError(h.Logger(), errors.New("No PPM loaded for Approved Move Doc")) + return handlers.ResponseForError(logger, errors.New("No PPM loaded for Approved Move Doc")) } ppm := &moveDoc.PersonallyProcuredMove @@ -163,7 +162,7 @@ func (h UpdateMoveDocumentHandler) Handle(params movedocop.UpdateMoveDocumentPar if ppm.Status != models.PPMStatusCOMPLETED { completeErr := ppm.Complete() if completeErr != nil { - return handlers.ResponseForError(h.Logger(), completeErr) + return handlers.ResponseForError(logger, completeErr) } } } @@ -202,12 +201,12 @@ func (h UpdateMoveDocumentHandler) Handle(params movedocop.UpdateMoveDocumentPar verrs, err := models.SaveMoveDocument(h.DB(), moveDoc, saveAction) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } moveDocPayload, err := payloadForMoveDocument(h.FileStorer(), *moveDoc) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return movedocop.NewUpdateMoveDocumentOK().WithPayload(moveDocPayload) } diff --git a/pkg/handlers/internalapi/move_queue_items.go b/pkg/handlers/internalapi/move_queue_items.go index b5ada6e1f1ca..9218ad3d8047 100644 --- a/pkg/handlers/internalapi/move_queue_items.go +++ b/pkg/handlers/internalapi/move_queue_items.go @@ -5,7 +5,6 @@ import ( "github.com/go-openapi/swag" "go.uber.org/zap" - "github.com/transcom/mymove/pkg/auth" queueop "github.com/transcom/mymove/pkg/gen/internalapi/internaloperations/queues" "github.com/transcom/mymove/pkg/gen/internalmessages" "github.com/transcom/mymove/pkg/handlers" @@ -39,7 +38,7 @@ type ShowQueueHandler struct { // Handle retrieves a list of all MoveQueueItems in the system in the moves queue func (h ShowQueueHandler) Handle(params queueop.ShowQueueParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) if !session.IsOfficeUser() { return queueop.NewShowQueueForbidden() @@ -49,8 +48,8 @@ func (h ShowQueueHandler) Handle(params queueop.ShowQueueParams) middleware.Resp MoveQueueItems, err := models.GetMoveQueueItems(h.DB(), lifecycleState) if err != nil { - h.Logger().Error("Loading Queue", zap.String("State", lifecycleState), zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("Loading Queue", zap.String("State", lifecycleState), zap.Error(err)) + return handlers.ResponseForError(logger, err) } MoveQueueItemPayloads := make([]*internalmessages.MoveQueueItem, len(MoveQueueItems)) diff --git a/pkg/handlers/internalapi/moves.go b/pkg/handlers/internalapi/moves.go index 58f0547e2c7d..99843b522603 100644 --- a/pkg/handlers/internalapi/moves.go +++ b/pkg/handlers/internalapi/moves.go @@ -14,7 +14,6 @@ import ( "go.uber.org/zap" "github.com/transcom/mymove/pkg/assets" - "github.com/transcom/mymove/pkg/auth" "github.com/transcom/mymove/pkg/awardqueue" moveop "github.com/transcom/mymove/pkg/gen/internalapi/internaloperations/moves" "github.com/transcom/mymove/pkg/gen/internalmessages" @@ -73,7 +72,7 @@ type ShowMoveHandler struct { // Handle retrieves a move in the system belonging to the logged in user given move ID func (h ShowMoveHandler) Handle(params moveop.ShowMoveParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) /* #nosec UUID is pattern matched by swagger which checks the format */ moveID, _ := uuid.FromString(params.MoveID.String()) @@ -81,17 +80,17 @@ func (h ShowMoveHandler) Handle(params moveop.ShowMoveParams) middleware.Respond // Validate that this move belongs to the current user move, err := models.FetchMove(h.DB(), session, moveID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } // Fetch orders for authorized user orders, err := models.FetchOrderForUser(h.DB(), session, move.OrdersID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } movePayload, err := payloadForMoveModel(h.FileStorer(), orders, *move) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return moveop.NewShowMoveOK().WithPayload(movePayload) } @@ -103,19 +102,19 @@ type PatchMoveHandler struct { // Handle ... patches a Move from a request payload func (h PatchMoveHandler) Handle(params moveop.PatchMoveParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) /* #nosec UUID is pattern matched by swagger which checks the format */ moveID, _ := uuid.FromString(params.MoveID.String()) // Validate that this move belongs to the current user move, err := models.FetchMove(h.DB(), session, moveID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } // Fetch orders for authorized user orders, err := models.FetchOrderForUser(h.DB(), session, move.OrdersID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } payload := params.PatchMovePayload newSelectedMoveType := payload.SelectedMoveType @@ -129,11 +128,11 @@ func (h PatchMoveHandler) Handle(params moveop.PatchMoveParams) middleware.Respo verrs, err := h.DB().ValidateAndUpdate(move) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } movePayload, err := payloadForMoveModel(h.FileStorer(), orders, *move) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return moveop.NewPatchMoveCreated().WithPayload(movePayload) } @@ -148,7 +147,7 @@ func (h SubmitMoveHandler) Handle(params moveop.SubmitMoveForApprovalParams) mid ctx, span := beeline.StartSpan(params.HTTPRequest.Context(), reflect.TypeOf(h).Name()) defer span.Send() - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) /* #nosec UUID is pattern matched by swagger which checks the format */ moveID, _ := uuid.FromString(params.MoveID.String()) @@ -156,7 +155,7 @@ func (h SubmitMoveHandler) Handle(params moveop.SubmitMoveForApprovalParams) mid move, err := models.FetchMove(h.DB(), session, moveID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } submitDate := time.Time(*params.SubmitMoveForApprovalPayload.PpmSubmitDate) @@ -166,22 +165,22 @@ func (h SubmitMoveHandler) Handle(params moveop.SubmitMoveForApprovalParams) mid h.HoneyZapLogger().TraceError(ctx, "Failed to change move status to submit", zap.String("move_id", moveID.String()), zap.String("move_status", string(move.Status))) - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } // Transaction to save move and dependencies verrs, err := models.SaveMoveDependencies(h.DB(), move) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } err = h.NotificationSender().SendNotification( ctx, - notifications.NewMoveSubmitted(h.DB(), h.Logger(), session, moveID), + notifications.NewMoveSubmitted(h.DB(), logger, session, moveID), ) if err != nil { - h.Logger().Error("problem sending email to user", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("problem sending email to user", zap.Error(err)) + return handlers.ResponseForError(logger, err) } if len(move.Shipments) > 0 { @@ -190,7 +189,7 @@ func (h SubmitMoveHandler) Handle(params moveop.SubmitMoveForApprovalParams) mid movePayload, err := payloadForMoveModel(h.FileStorer(), move.Orders, *move) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return moveop.NewSubmitMoveForApprovalOK().WithPayload(movePayload) } @@ -202,7 +201,7 @@ type ShowMoveDatesSummaryHandler struct { // Handle returns a summary of the dates in the move process. func (h ShowMoveDatesSummaryHandler) Handle(params moveop.ShowMoveDatesSummaryParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) moveDate := time.Time(params.MoveDate) moveID, _ := uuid.FromString(params.MoveID.String()) @@ -210,12 +209,12 @@ func (h ShowMoveDatesSummaryHandler) Handle(params moveop.ShowMoveDatesSummaryPa // Validate that this move belongs to the current user _, err := models.FetchMove(h.DB(), session, moveID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } summary, err := calculateMoveDatesFromMove(h.DB(), h.Planner(), moveID, moveDate) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } moveDatesSummary := &internalmessages.MoveDatesSummary{ @@ -239,27 +238,27 @@ type ShowShipmentSummaryWorksheetHandler struct { // Handle returns a generated PDF func (h ShowShipmentSummaryWorksheetHandler) Handle(params moveop.ShowShipmentSummaryWorksheetParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) moveID, _ := uuid.FromString(params.MoveID.String()) - ppmComputer := paperwork.NewSSWPPMComputer(rateengine.NewRateEngine(h.DB(), h.Logger())) + ppmComputer := paperwork.NewSSWPPMComputer(rateengine.NewRateEngine(h.DB(), logger)) ssfd, err := models.FetchDataShipmentSummaryWorksheetFormData(h.DB(), session, moveID) if err != nil { - h.Logger().Error("Error fetching data for SSW", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("Error fetching data for SSW", zap.Error(err)) + return handlers.ResponseForError(logger, err) } ssfd.PreparationDate = time.Time(params.PreparationDate) ssfd.Obligations, err = ppmComputer.ComputeObligations(ssfd, h.Planner()) if err != nil { - h.Logger().Error("Error calculating obligations ", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("Error calculating obligations ", zap.Error(err)) + return handlers.ResponseForError(logger, err) } page1Data, page2Data, err := models.FormatValuesShipmentSummaryWorksheet(ssfd) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } formFiller := paperwork.NewFormFiller() @@ -269,14 +268,14 @@ func (h ShowShipmentSummaryWorksheetHandler) Handle(params moveop.ShowShipmentSu page1Template, err := assets.Asset(page1Layout.TemplateImagePath) if err != nil { - h.Logger().Error("Error reading template file", zap.String("asset", page1Layout.TemplateImagePath), zap.Error(err)) + logger.Error("Error reading template file", zap.String("asset", page1Layout.TemplateImagePath), zap.Error(err)) return moveop.NewShowShipmentSummaryWorksheetInternalServerError() } page1Reader := bytes.NewReader(page1Template) err = formFiller.AppendPage(page1Reader, page1Layout.FieldsLayout, page1Data) if err != nil { - h.Logger().Error("Error appending page to PDF", zap.Error(err)) + logger.Error("Error appending page to PDF", zap.Error(err)) return moveop.NewShowShipmentSummaryWorksheetInternalServerError() } @@ -285,21 +284,21 @@ func (h ShowShipmentSummaryWorksheetHandler) Handle(params moveop.ShowShipmentSu page2Template, err := assets.Asset(page2Layout.TemplateImagePath) if err != nil { - h.Logger().Error("Error reading template file", zap.String("asset", page2Layout.TemplateImagePath), zap.Error(err)) + logger.Error("Error reading template file", zap.String("asset", page2Layout.TemplateImagePath), zap.Error(err)) return moveop.NewShowShipmentSummaryWorksheetInternalServerError() } page2Reader := bytes.NewReader(page2Template) err = formFiller.AppendPage(page2Reader, page2Layout.FieldsLayout, page2Data) if err != nil { - h.Logger().Error("Error appending page to PDF", zap.Error(err)) + logger.Error("Error appending page to PDF", zap.Error(err)) return moveop.NewShowShipmentSummaryWorksheetInternalServerError() } buf := new(bytes.Buffer) err = formFiller.Output(buf) if err != nil { - h.Logger().Error("Error writing out PDF", zap.Error(err)) + logger.Error("Error writing out PDF", zap.Error(err)) return moveop.NewShowShipmentSummaryWorksheetInternalServerError() } diff --git a/pkg/handlers/internalapi/moving_expense_documents.go b/pkg/handlers/internalapi/moving_expense_documents.go index 8efe61b84def..432ab477cee5 100644 --- a/pkg/handlers/internalapi/moving_expense_documents.go +++ b/pkg/handlers/internalapi/moving_expense_documents.go @@ -7,7 +7,6 @@ import ( "github.com/gofrs/uuid" "github.com/honeycombio/beeline-go" - "github.com/transcom/mymove/pkg/auth" movedocop "github.com/transcom/mymove/pkg/gen/internalapi/internaloperations/move_docs" "github.com/transcom/mymove/pkg/gen/internalmessages" "github.com/transcom/mymove/pkg/handlers" @@ -49,14 +48,14 @@ func (h CreateMovingExpenseDocumentHandler) Handle(params movedocop.CreateMoving ctx, span := beeline.StartSpan(params.HTTPRequest.Context(), reflect.TypeOf(h).Name()) defer span.Send() - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) // #nosec UUID is pattern matched by swagger and will be ok moveID, _ := uuid.FromString(params.MoveID.String()) // Validate that this move belongs to the current user move, err := models.FetchMove(h.DB(), session, moveID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } payload := params.CreateMovingExpenseDocumentPayload @@ -72,7 +71,7 @@ func (h CreateMovingExpenseDocumentHandler) Handle(params movedocop.CreateMoving converted := uuid.Must(uuid.FromString(id.String())) upload, fetchUploadErr := models.FetchUpload(ctx, h.DB(), session, converted) if fetchUploadErr != nil { - return handlers.ResponseForError(h.Logger(), fetchUploadErr) + return handlers.ResponseForError(logger, fetchUploadErr) } uploads = append(uploads, upload) } @@ -84,7 +83,7 @@ func (h CreateMovingExpenseDocumentHandler) Handle(params movedocop.CreateMoving // Enforce that the ppm's move_id matches our move ppm, fetchPPMErr := models.FetchPersonallyProcuredMove(h.DB(), session, id) if fetchPPMErr != nil { - return handlers.ResponseForError(h.Logger(), fetchPPMErr) + return handlers.ResponseForError(logger, fetchPPMErr) } if ppm.MoveID != moveID { return movedocop.NewCreateMovingExpenseDocumentBadRequest() @@ -107,12 +106,12 @@ func (h CreateMovingExpenseDocumentHandler) Handle(params movedocop.CreateMoving ) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } newPayload, err := payloadForMovingExpenseDocumentModel(h.FileStorer(), *newMovingExpenseDocument) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return movedocop.NewCreateMovingExpenseDocumentOK().WithPayload(newPayload) } diff --git a/pkg/handlers/internalapi/office.go b/pkg/handlers/internalapi/office.go index 91f06a4ca22a..7aefd56a3892 100644 --- a/pkg/handlers/internalapi/office.go +++ b/pkg/handlers/internalapi/office.go @@ -9,7 +9,6 @@ import ( beeline "github.com/honeycombio/beeline-go" "go.uber.org/zap" - "github.com/transcom/mymove/pkg/auth" officeop "github.com/transcom/mymove/pkg/gen/internalapi/internaloperations/office" "github.com/transcom/mymove/pkg/handlers" "github.com/transcom/mymove/pkg/models" @@ -23,7 +22,7 @@ type ApproveMoveHandler struct { // Handle ... approves a Move from a request payload func (h ApproveMoveHandler) Handle(params officeop.ApproveMoveParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) if !session.IsOfficeUser() { return officeop.NewApproveMoveForbidden() @@ -32,12 +31,12 @@ func (h ApproveMoveHandler) Handle(params officeop.ApproveMoveParams) middleware moveID, _ := uuid.FromString(params.MoveID.String()) move, err := models.FetchMove(h.DB(), session, moveID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } // Don't approve Move if orders are incomplete orders, ordersErr := models.FetchOrder(h.DB(), move.OrdersID) if ordersErr != nil { - return handlers.ResponseForError(h.Logger(), ordersErr) + return handlers.ResponseForError(logger, ordersErr) } if orders.IsComplete() != true { return officeop.NewApprovePPMBadRequest() @@ -45,20 +44,20 @@ func (h ApproveMoveHandler) Handle(params officeop.ApproveMoveParams) middleware err = move.Approve() if err != nil { - h.Logger().Info("Attempted to approve move, got invalid transition", zap.Error(err), zap.String("move_status", string(move.Status))) - return handlers.ResponseForError(h.Logger(), err) + logger.Info("Attempted to approve move, got invalid transition", zap.Error(err), zap.String("move_status", string(move.Status))) + return handlers.ResponseForError(logger, err) } verrs, err := h.DB().ValidateAndUpdate(move) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } // TODO: Save and/or update the move association status' (PPM, Reimbursement, Orders) a la Cancel handler movePayload, err := payloadForMoveModel(h.FileStorer(), move.Orders, *move) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return officeop.NewApproveMoveOK().WithPayload(movePayload) } @@ -74,7 +73,7 @@ func (h CancelMoveHandler) Handle(params officeop.CancelMoveParams) middleware.R ctx, span := beeline.StartSpan(params.HTTPRequest.Context(), reflect.TypeOf(h).Name()) defer span.Send() - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) if !session.IsOfficeUser() { return officeop.NewCancelMoveForbidden() } @@ -84,35 +83,35 @@ func (h CancelMoveHandler) Handle(params officeop.CancelMoveParams) middleware.R move, err := models.FetchMove(h.DB(), session, moveID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } // Canceling move will result in canceled associated PPMs err = move.Cancel(*params.CancelMove.CancelReason) if err != nil { - h.Logger().Error("Attempted to cancel move, got invalid transition", zap.Error(err), zap.String("move_status", string(move.Status))) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("Attempted to cancel move, got invalid transition", zap.Error(err), zap.String("move_status", string(move.Status))) + return handlers.ResponseForError(logger, err) } // Save move, orders, and PPMs statuses verrs, err := models.SaveMoveDependencies(h.DB(), move) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } err = h.NotificationSender().SendNotification( ctx, - notifications.NewMoveCanceled(h.DB(), h.Logger(), session, moveID), + notifications.NewMoveCanceled(h.DB(), logger, session, moveID), ) if err != nil { - h.Logger().Error("problem sending email to user", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("problem sending email to user", zap.Error(err)) + return handlers.ResponseForError(logger, err) } movePayload, err := payloadForMoveModel(h.FileStorer(), move.Orders, *move) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return officeop.NewCancelMoveOK().WithPayload(movePayload) } @@ -127,7 +126,7 @@ func (h ApprovePPMHandler) Handle(params officeop.ApprovePPMParams) middleware.R ctx, span := beeline.StartSpan(params.HTTPRequest.Context(), reflect.TypeOf(h).Name()) defer span.Send() - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) if !session.IsOfficeUser() { return officeop.NewApprovePPMForbidden() } @@ -137,7 +136,7 @@ func (h ApprovePPMHandler) Handle(params officeop.ApprovePPMParams) middleware.R ppm, err := models.FetchPersonallyProcuredMove(h.DB(), session, ppmID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } moveID := ppm.MoveID var approveDate time.Time @@ -146,27 +145,27 @@ func (h ApprovePPMHandler) Handle(params officeop.ApprovePPMParams) middleware.R } err = ppm.Approve(approveDate) if err != nil { - h.Logger().Error("Attempted to approve PPM, got invalid transition", zap.Error(err), zap.String("move_status", string(ppm.Status))) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("Attempted to approve PPM, got invalid transition", zap.Error(err), zap.String("move_status", string(ppm.Status))) + return handlers.ResponseForError(logger, err) } verrs, err := h.DB().ValidateAndUpdate(ppm) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } err = h.NotificationSender().SendNotification( ctx, - notifications.NewMoveApproved(h.DB(), h.Logger(), session, moveID), + notifications.NewMoveApproved(h.DB(), logger, session, moveID), ) if err != nil { - h.Logger().Error("problem sending email to user", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("problem sending email to user", zap.Error(err)) + return handlers.ResponseForError(logger, err) } ppmPayload, err := payloadForPPMModel(h.FileStorer(), *ppm) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return officeop.NewApprovePPMOK().WithPayload(ppmPayload) } @@ -178,7 +177,7 @@ type ApproveReimbursementHandler struct { // Handle ... approves a Reimbursement from a request payload func (h ApproveReimbursementHandler) Handle(params officeop.ApproveReimbursementParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) if !session.IsOfficeUser() { return officeop.NewApproveReimbursementForbidden() @@ -189,18 +188,18 @@ func (h ApproveReimbursementHandler) Handle(params officeop.ApproveReimbursement reimbursement, err := models.FetchReimbursement(h.DB(), session, reimbursementID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } err = reimbursement.Approve() if err != nil { - h.Logger().Error("Attempted to approve, got invalid transition", zap.Error(err), zap.String("reimbursement_status", string(reimbursement.Status))) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("Attempted to approve, got invalid transition", zap.Error(err), zap.String("reimbursement_status", string(reimbursement.Status))) + return handlers.ResponseForError(logger, err) } verrs, err := h.DB().ValidateAndUpdate(reimbursement) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } reimbursementPayload := payloadForReimbursementModel(reimbursement) diff --git a/pkg/handlers/internalapi/orders.go b/pkg/handlers/internalapi/orders.go index fe9908b6d238..7cce28ce0517 100644 --- a/pkg/handlers/internalapi/orders.go +++ b/pkg/handlers/internalapi/orders.go @@ -8,7 +8,6 @@ import ( "github.com/gofrs/uuid" "github.com/honeycombio/beeline-go" - "github.com/transcom/mymove/pkg/auth" ordersop "github.com/transcom/mymove/pkg/gen/internalapi/internaloperations/orders" "github.com/transcom/mymove/pkg/gen/internalmessages" "github.com/transcom/mymove/pkg/handlers" @@ -68,26 +67,26 @@ func (h CreateOrdersHandler) Handle(params ordersop.CreateOrdersParams) middlewa ctx, span := beeline.StartSpan(params.HTTPRequest.Context(), reflect.TypeOf(h).Name()) defer span.Send() - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) payload := params.CreateOrders serviceMemberID, err := uuid.FromString(payload.ServiceMemberID.String()) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } serviceMember, err := models.FetchServiceMemberForUser(ctx, h.DB(), session, serviceMemberID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } stationID, err := uuid.FromString(payload.NewDutyStationID.String()) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } dutyStation, err := models.FetchDutyStation(ctx, h.DB(), stationID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } var deptIndicator *string @@ -111,18 +110,18 @@ func (h CreateOrdersHandler) Handle(params ordersop.CreateOrdersParams) middlewa payload.Sac, deptIndicator) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } newMove, verrs, err := newOrder.CreateNewMove(h.DB(), nil) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } newOrder.Moves = append(newOrder.Moves, *newMove) orderPayload, err := payloadForOrdersModel(h.FileStorer(), newOrder) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return ordersop.NewCreateOrdersCreated().WithPayload(orderPayload) } @@ -134,17 +133,17 @@ type ShowOrdersHandler struct { // Handle retrieves orders in the system belonging to the logged in user given order ID func (h ShowOrdersHandler) Handle(params ordersop.ShowOrdersParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) // #nosec swagger verifies uuid format orderID, _ := uuid.FromString(params.OrdersID.String()) order, err := models.FetchOrderForUser(h.DB(), session, orderID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } orderPayload, err := payloadForOrdersModel(h.FileStorer(), order) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return ordersop.NewShowOrdersOK().WithPayload(orderPayload) } @@ -160,25 +159,25 @@ func (h UpdateOrdersHandler) Handle(params ordersop.UpdateOrdersParams) middlewa ctx, span := beeline.StartSpan(params.HTTPRequest.Context(), reflect.TypeOf(h).Name()) defer span.Send() - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) orderID, err := uuid.FromString(params.OrdersID.String()) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } order, err := models.FetchOrderForUser(h.DB(), session, orderID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } payload := params.UpdateOrders stationID, err := uuid.FromString(payload.NewDutyStationID.String()) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } dutyStation, err := models.FetchDutyStation(ctx, h.DB(), stationID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } order.OrdersNumber = payload.OrdersNumber @@ -201,12 +200,12 @@ func (h UpdateOrdersHandler) Handle(params ordersop.UpdateOrdersParams) middlewa verrs, err := models.SaveOrder(h.DB(), &order) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } orderPayload, err := payloadForOrdersModel(h.FileStorer(), order) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return ordersop.NewUpdateOrdersOK().WithPayload(orderPayload) } diff --git a/pkg/handlers/internalapi/personally_procured_move.go b/pkg/handlers/internalapi/personally_procured_move.go index 5ed50b8a3b0d..be13f820c2f8 100644 --- a/pkg/handlers/internalapi/personally_procured_move.go +++ b/pkg/handlers/internalapi/personally_procured_move.go @@ -8,7 +8,6 @@ import ( "github.com/gofrs/uuid" "go.uber.org/zap" - "github.com/transcom/mymove/pkg/auth" ppmop "github.com/transcom/mymove/pkg/gen/internalapi/internaloperations/ppm" "github.com/transcom/mymove/pkg/gen/internalmessages" "github.com/transcom/mymove/pkg/handlers" @@ -76,14 +75,14 @@ type CreatePersonallyProcuredMoveHandler struct { // Handle is the handler func (h CreatePersonallyProcuredMoveHandler) Handle(params ppmop.CreatePersonallyProcuredMoveParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) // #nosec UUID is pattern matched by swagger and will be ok moveID, _ := uuid.FromString(params.MoveID.String()) // Validate that this move belongs to the current user move, err := models.FetchMove(h.DB(), session, moveID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } payload := params.CreatePersonallyProcuredMovePayload @@ -108,12 +107,12 @@ func (h CreatePersonallyProcuredMoveHandler) Handle(params ppmop.CreatePersonall advance) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } ppmPayload, err := payloadForPPMModel(h.FileStorer(), *newPPM) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return ppmop.NewCreatePersonallyProcuredMoveCreated().WithPayload(ppmPayload) } @@ -126,14 +125,14 @@ type IndexPersonallyProcuredMovesHandler struct { // Handle handles the request func (h IndexPersonallyProcuredMovesHandler) Handle(params ppmop.IndexPersonallyProcuredMovesParams) middleware.Responder { // #nosec User should always be populated by middleware - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) // #nosec UUID is pattern matched by swagger and will be ok moveID, _ := uuid.FromString(params.MoveID.String()) // Validate that this move belongs to the current user move, err := models.FetchMove(h.DB(), session, moveID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } // The given move does belong to the current user. @@ -142,7 +141,7 @@ func (h IndexPersonallyProcuredMovesHandler) Handle(params ppmop.IndexPersonally for i, ppm := range ppms { ppmPayload, err := payloadForPPMModel(h.FileStorer(), ppm) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } ppmsPayload[i] = ppmPayload } @@ -228,7 +227,7 @@ type PatchPersonallyProcuredMoveHandler struct { // Handle is the handler func (h PatchPersonallyProcuredMoveHandler) Handle(params ppmop.PatchPersonallyProcuredMoveParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) // #nosec UUID is pattern matched by swagger and will be ok moveID, _ := uuid.FromString(params.MoveID.String()) @@ -237,41 +236,41 @@ func (h PatchPersonallyProcuredMoveHandler) Handle(params ppmop.PatchPersonallyP ppm, err := models.FetchPersonallyProcuredMove(h.DB(), session, ppmID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } if ppm.MoveID != moveID { - h.Logger().Info("Move ID for PPM does not match requested PPM Move ID", zap.String("requested move_id", moveID.String()), zap.String("actual move_id", ppm.MoveID.String())) + logger.Info("Move ID for PPM does not match requested PPM Move ID", zap.String("requested move_id", moveID.String()), zap.String("actual move_id", ppm.MoveID.String())) return ppmop.NewPatchPersonallyProcuredMoveBadRequest() } - needsEstimatesRecalculated := h.ppmNeedsEstimatesRecalculated(ppm, params.PatchPersonallyProcuredMovePayload) + needsEstimatesRecalculated := h.ppmNeedsEstimatesRecalculated(ppm, params.PatchPersonallyProcuredMovePayload, logger) patchPPMWithPayload(ppm, params.PatchPersonallyProcuredMovePayload) if needsEstimatesRecalculated { - err = h.updateEstimates(ppm) + err = h.updateEstimates(ppm, logger) if err != nil { - h.Logger().Error("Unable to set calculated fields on PPM", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("Unable to set calculated fields on PPM", zap.Error(err)) + return handlers.ResponseForError(logger, err) } } verrs, err := models.SavePersonallyProcuredMove(h.DB(), ppm) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } ppmPayload, err := payloadForPPMModel(h.FileStorer(), *ppm) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return ppmop.NewPatchPersonallyProcuredMoveOK().WithPayload(ppmPayload) } // ppmNeedsEstimatesRecalculated determines whether the fields that comprise // the PPM incentive and SIT estimate calculations have changed, necessitating a recalculation -func (h PatchPersonallyProcuredMoveHandler) ppmNeedsEstimatesRecalculated(ppm *models.PersonallyProcuredMove, patch *internalmessages.PatchPersonallyProcuredMovePayload) bool { +func (h PatchPersonallyProcuredMoveHandler) ppmNeedsEstimatesRecalculated(ppm *models.PersonallyProcuredMove, patch *internalmessages.PatchPersonallyProcuredMovePayload, logger Logger) bool { originPtr := patch.PickupPostalCode destinationPtr := patch.DestinationPostalCode weightPtr := patch.WeightEstimate @@ -298,7 +297,7 @@ func (h PatchPersonallyProcuredMoveHandler) ppmNeedsEstimatesRecalculated(ppm *m needsUpdate := valuesOK && valuesChanged if needsUpdate { - h.Logger().Info("updating PPM calculated fields", + logger.Info("updating PPM calculated fields", zap.String("originZip", origin), zap.String("destinationZip", destination), zap.Int64("weight", weight), @@ -317,7 +316,7 @@ type SubmitPersonallyProcuredMoveHandler struct { // Handle Submits a PPM to change its status to SUBMITTED func (h SubmitPersonallyProcuredMoveHandler) Handle(params ppmop.SubmitPersonallyProcuredMoveParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) // #nosec UUID is pattern matched by swagger and will be ok ppmID, _ := uuid.FromString(params.PersonallyProcuredMoveID.String()) @@ -325,7 +324,7 @@ func (h SubmitPersonallyProcuredMoveHandler) Handle(params ppmop.SubmitPersonall ppm, err := models.FetchPersonallyProcuredMove(h.DB(), session, ppmID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } var submitDate time.Time @@ -336,13 +335,13 @@ func (h SubmitPersonallyProcuredMoveHandler) Handle(params ppmop.SubmitPersonall verrs, err := models.SavePersonallyProcuredMove(h.DB(), ppm) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } ppmPayload, err := payloadForPPMModel(h.FileStorer(), *ppm) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return ppmop.NewSubmitPersonallyProcuredMoveOK().WithPayload(ppmPayload) @@ -390,14 +389,14 @@ func dateForComparison(previousValue, newValue *time.Time) (value time.Time, val return value, false, false } -func (h PatchPersonallyProcuredMoveHandler) updateEstimates(ppm *models.PersonallyProcuredMove) error { - re := rateengine.NewRateEngine(h.DB(), h.Logger()) +func (h PatchPersonallyProcuredMoveHandler) updateEstimates(ppm *models.PersonallyProcuredMove, logger Logger) error { + re := rateengine.NewRateEngine(h.DB(), logger) daysInSIT := 0 if ppm.HasSit != nil && *ppm.HasSit && ppm.DaysInStorage != nil { daysInSIT = int(*ppm.DaysInStorage) } - lhDiscount, sitDiscount, err := models.PPMDiscountFetch(h.DB(), h.Logger(), *ppm.PickupPostalCode, *ppm.DestinationPostalCode, *ppm.OriginalMoveDate) + lhDiscount, sitDiscount, err := models.PPMDiscountFetch(h.DB(), logger, *ppm.PickupPostalCode, *ppm.DestinationPostalCode, *ppm.OriginalMoveDate) if err != nil { return err } @@ -444,29 +443,29 @@ type RequestPPMPaymentHandler struct { // Handle is the handler func (h RequestPPMPaymentHandler) Handle(params ppmop.RequestPPMPaymentParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) // #nosec UUID is pattern matched by swagger and will be ok ppmID, _ := uuid.FromString(params.PersonallyProcuredMoveID.String()) ppm, err := models.FetchPersonallyProcuredMove(h.DB(), session, ppmID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } err = ppm.RequestPayment() if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } verrs, err := models.SavePersonallyProcuredMove(h.DB(), ppm) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } ppmPayload, err := payloadForPPMModel(h.FileStorer(), *ppm) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return ppmop.NewRequestPPMPaymentOK().WithPayload(ppmPayload) @@ -543,7 +542,7 @@ type RequestPPMExpenseSummaryHandler struct { // Handle is the handler func (h RequestPPMExpenseSummaryHandler) Handle(params ppmop.RequestPPMExpenseSummaryParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) ppmID, _ := uuid.FromString(params.PersonallyProcuredMoveID.String()) @@ -551,7 +550,7 @@ func (h RequestPPMExpenseSummaryHandler) Handle(params ppmop.RequestPPMExpenseSu status := models.MoveDocumentStatusOK moveDocsExpense, err := models.FetchMovingExpenseDocuments(h.DB(), session, ppmID, &status) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } expenseSummaryPayload := buildExpenseSummaryPayload(moveDocsExpense) diff --git a/pkg/handlers/internalapi/personally_procured_move_attachments.go b/pkg/handlers/internalapi/personally_procured_move_attachments.go index 516f4cdf9261..a48d3ac108a4 100644 --- a/pkg/handlers/internalapi/personally_procured_move_attachments.go +++ b/pkg/handlers/internalapi/personally_procured_move_attachments.go @@ -5,7 +5,6 @@ import ( "github.com/gofrs/uuid" "go.uber.org/zap" - "github.com/transcom/mymove/pkg/auth" ppmop "github.com/transcom/mymove/pkg/gen/internalapi/internaloperations/ppm" "github.com/transcom/mymove/pkg/handlers" "github.com/transcom/mymove/pkg/models" @@ -20,19 +19,19 @@ type CreatePersonallyProcuredMoveAttachmentsHandler struct { // Handle is the handler func (h CreatePersonallyProcuredMoveAttachmentsHandler) Handle(params ppmop.CreatePPMAttachmentsParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) // #nosec UUID is pattern matched by swagger and will be ok ppmID, _ := uuid.FromString(params.PersonallyProcuredMoveID.String()) ppm, err := models.FetchPersonallyProcuredMove(h.DB(), session, ppmID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } err = h.DB().Load(ppm, "Move.Orders.UploadedOrders.Uploads") if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } moveDocs, err := ppm.FetchMoveDocumentsForTypes(h.DB(), params.DocTypes) @@ -44,10 +43,10 @@ func (h CreatePersonallyProcuredMoveAttachmentsHandler) Handle(params ppmop.Crea } // Init our tools - loader := uploader.NewUploader(h.DB(), h.Logger(), h.FileStorer()) - generator, err := paperwork.NewGenerator(h.DB(), h.Logger(), loader) + loader := uploader.NewUploader(h.DB(), logger, h.FileStorer()) + generator, err := paperwork.NewGenerator(h.DB(), logger, loader) if err != nil { - h.Logger().Error("failed to initialize generator", zap.Error(err)) + logger.Error("failed to initialize generator", zap.Error(err)) return ppmop.NewCreatePPMAttachmentsInternalServerError() } @@ -65,19 +64,19 @@ func (h CreatePersonallyProcuredMoveAttachmentsHandler) Handle(params ppmop.Crea // Convert to PDF and merge into single PDF mergedPdf, err := generator.CreateMergedPDFUpload(uploads) if err != nil { - h.Logger().Error("failed to merge PDF files", zap.Error(err)) + logger.Error("failed to merge PDF files", zap.Error(err)) return ppmop.NewCreatePPMAttachmentsUnprocessableEntity() } // Upload merged PDF to S3 and return Upload object pdfUpload, verrs, err := loader.CreateUpload(session.UserID, &mergedPdf, uploader.AllowedTypesPDF) if verrs.HasAny() || err != nil { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } url, err := loader.PresignedURL(pdfUpload) if err != nil { - h.Logger().Error("failed to get presigned url", zap.Error(err)) + logger.Error("failed to get presigned url", zap.Error(err)) return ppmop.NewCreatePPMAttachmentsInternalServerError() } diff --git a/pkg/handlers/internalapi/personally_procured_move_estimate.go b/pkg/handlers/internalapi/personally_procured_move_estimate.go index 75197a0c1e8f..fde60d910e1a 100644 --- a/pkg/handlers/internalapi/personally_procured_move_estimate.go +++ b/pkg/handlers/internalapi/personally_procured_move_estimate.go @@ -21,21 +21,24 @@ type ShowPPMEstimateHandler struct { // Handle calculates a PPM reimbursement range. func (h ShowPPMEstimateHandler) Handle(params ppmop.ShowPPMEstimateParams) middleware.Responder { - engine := rateengine.NewRateEngine(h.DB(), h.Logger()) + + logger := h.LoggerFromRequest(params.HTTPRequest) + + engine := rateengine.NewRateEngine(h.DB(), logger) lhDiscount, _, err := models.PPMDiscountFetch(h.DB(), - h.Logger(), + logger, params.OriginZip, params.DestinationZip, time.Time(params.OriginalMoveDate), ) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } distanceMiles, err := h.Planner().Zip5TransitDistance(params.OriginZip, params.DestinationZip) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } cost, err := engine.ComputePPM(unit.Pound(params.WeightEstimate), @@ -49,7 +52,7 @@ func (h ShowPPMEstimateHandler) Handle(params ppmop.ShowPPMEstimateParams) middl ) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } min := cost.GCC.MultiplyFloat64(0.95) diff --git a/pkg/handlers/internalapi/personally_procured_move_incentive.go b/pkg/handlers/internalapi/personally_procured_move_incentive.go index 262889046c8b..a08ff1205d6f 100644 --- a/pkg/handlers/internalapi/personally_procured_move_incentive.go +++ b/pkg/handlers/internalapi/personally_procured_move_incentive.go @@ -8,7 +8,6 @@ import ( "github.com/go-openapi/runtime/middleware" "github.com/go-openapi/swag" - "github.com/transcom/mymove/pkg/auth" ppmop "github.com/transcom/mymove/pkg/gen/internalapi/internaloperations/ppm" "github.com/transcom/mymove/pkg/gen/internalmessages" "github.com/transcom/mymove/pkg/handlers" @@ -23,26 +22,26 @@ type ShowPPMIncentiveHandler struct { // Handle calculates a PPM reimbursement range. func (h ShowPPMIncentiveHandler) Handle(params ppmop.ShowPPMIncentiveParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) if !session.IsOfficeUser() { return ppmop.NewShowPPMIncentiveForbidden() } - engine := rateengine.NewRateEngine(h.DB(), h.Logger()) + engine := rateengine.NewRateEngine(h.DB(), logger) lhDiscount, _, err := models.PPMDiscountFetch(h.DB(), - h.Logger(), + logger, params.OriginZip, params.DestinationZip, time.Time(params.OriginalMoveDate), ) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } distanceMiles, err := h.Planner().Zip5TransitDistance(params.OriginZip, params.DestinationZip) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } cost, err := engine.ComputePPM(unit.Pound(params.Weight), @@ -56,7 +55,7 @@ func (h ShowPPMIncentiveHandler) Handle(params ppmop.ShowPPMIncentiveParams) mid ) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } gcc := cost.GCC diff --git a/pkg/handlers/internalapi/personally_procured_move_sit_estimate.go b/pkg/handlers/internalapi/personally_procured_move_sit_estimate.go index 3381ef5e713b..dc59698866fa 100644 --- a/pkg/handlers/internalapi/personally_procured_move_sit_estimate.go +++ b/pkg/handlers/internalapi/personally_procured_move_sit_estimate.go @@ -22,24 +22,25 @@ type ShowPPMSitEstimateHandler struct { // Handle calculates SIT charge and retrieves SIT discount rate. // It returns the discount rate applied to relevant SIT charge. func (h ShowPPMSitEstimateHandler) Handle(params ppmop.ShowPPMSitEstimateParams) middleware.Responder { - engine := rateengine.NewRateEngine(h.DB(), h.Logger()) + logger := h.LoggerFromRequest(params.HTTPRequest) + engine := rateengine.NewRateEngine(h.DB(), logger) sitZip3 := rateengine.Zip5ToZip3(params.DestinationZip) cwtWeight := unit.Pound(params.WeightEstimate).ToCWT() originalMoveDateTime := time.Time(params.OriginalMoveDate) lhDiscount, sitDiscount, err := models.PPMDiscountFetch(h.DB(), - h.Logger(), + logger, params.OriginZip, params.DestinationZip, time.Time(params.OriginalMoveDate), ) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } sitComputation, err := engine.SitCharge(cwtWeight, int(params.DaysInStorage), sitZip3, originalMoveDateTime, true) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } // Swagger returns int64 when using the integer type diff --git a/pkg/handlers/internalapi/service_members.go b/pkg/handlers/internalapi/service_members.go index 549b6c0ced12..baaf05a84d1b 100644 --- a/pkg/handlers/internalapi/service_members.go +++ b/pkg/handlers/internalapi/service_members.go @@ -10,7 +10,6 @@ import ( "github.com/honeycombio/beeline-go" "go.uber.org/zap" - "github.com/transcom/mymove/pkg/auth" servicememberop "github.com/transcom/mymove/pkg/gen/internalapi/internaloperations/service_members" "github.com/transcom/mymove/pkg/gen/internalmessages" "github.com/transcom/mymove/pkg/handlers" @@ -68,7 +67,9 @@ type CreateServiceMemberHandler struct { // Handle ... creates a new ServiceMember from a request payload func (h CreateServiceMemberHandler) Handle(params servicememberop.CreateServiceMemberParams) middleware.Responder { - ctx, span := beeline.StartSpan(params.HTTPRequest.Context(), reflect.TypeOf(h).Name()) + ctx := params.HTTPRequest.Context() + + ctx, span := beeline.StartSpan(ctx, reflect.TypeOf(h).Name()) defer span.Send() residentialAddress := addressModelFromPayload(params.CreateServiceMemberPayload.ResidentialAddress) @@ -102,7 +103,7 @@ func (h CreateServiceMemberHandler) Handle(params servicememberop.CreateServiceM } // User should always be populated by middleware - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromContext(ctx) // Create a new serviceMember for an authenticated user newServiceMember := models.ServiceMember{ @@ -148,7 +149,7 @@ func (h CreateServiceMemberHandler) Handle(params servicememberop.CreateServiceM // And return serviceMemberPayload := payloadForServiceMemberModel(h.FileStorer(), newServiceMember) responder := servicememberop.NewCreateServiceMemberCreated().WithPayload(serviceMemberPayload) - return handlers.NewCookieUpdateResponder(params.HTTPRequest, h.CookieSecret(), h.NoSessionTimeout(), h.Logger(), responder, h.UseSecureCookie()) + return handlers.NewCookieUpdateResponder(params.HTTPRequest, h.CookieSecret(), h.NoSessionTimeout(), logger, responder, h.UseSecureCookie()) } // ShowServiceMemberHandler returns a serviceMember for a user and service member ID @@ -159,10 +160,12 @@ type ShowServiceMemberHandler struct { // Handle retrieves a service member in the system belonging to the logged in user given service member ID func (h ShowServiceMemberHandler) Handle(params servicememberop.ShowServiceMemberParams) middleware.Responder { - ctx, span := beeline.StartSpan(params.HTTPRequest.Context(), reflect.TypeOf(h).Name()) + ctx := params.HTTPRequest.Context() + + ctx, span := beeline.StartSpan(ctx, reflect.TypeOf(h).Name()) defer span.Send() - session := auth.SessionFromRequestContext(params.HTTPRequest) + session := h.SessionFromContext(ctx) serviceMemberID, _ := uuid.FromString(params.ServiceMemberID.String()) @@ -185,10 +188,12 @@ type PatchServiceMemberHandler struct { // Handle ... patches a new ServiceMember from a request payload func (h PatchServiceMemberHandler) Handle(params servicememberop.PatchServiceMemberParams) middleware.Responder { - ctx, span := beeline.StartSpan(params.HTTPRequest.Context(), reflect.TypeOf(h).Name()) + ctx := params.HTTPRequest.Context() + + ctx, span := beeline.StartSpan(ctx, reflect.TypeOf(h).Name()) defer span.Send() - session := auth.SessionFromRequestContext(params.HTTPRequest) + session := h.SessionFromContext(ctx) serviceMemberID, _ := uuid.FromString(params.ServiceMemberID.String()) @@ -304,10 +309,12 @@ type ShowServiceMemberOrdersHandler struct { // Handle retrieves orders for a service member func (h ShowServiceMemberOrdersHandler) Handle(params servicememberop.ShowServiceMemberOrdersParams) middleware.Responder { - ctx, span := beeline.StartSpan(params.HTTPRequest.Context(), reflect.TypeOf(h).Name()) + ctx := params.HTTPRequest.Context() + + ctx, span := beeline.StartSpan(ctx, reflect.TypeOf(h).Name()) defer span.Send() - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromContext(ctx) serviceMemberID, _ := uuid.FromString(params.ServiceMemberID.String()) serviceMember, err := models.FetchServiceMemberForUser(ctx, h.DB(), session, serviceMemberID) @@ -322,7 +329,7 @@ func (h ShowServiceMemberOrdersHandler) Handle(params servicememberop.ShowServic orderPayload, err := payloadForOrdersModel(h.FileStorer(), order) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return servicememberop.NewShowServiceMemberOrdersOK().WithPayload(orderPayload) } diff --git a/pkg/handlers/internalapi/shipments.go b/pkg/handlers/internalapi/shipments.go index 626c73cd4702..1e662724738e 100644 --- a/pkg/handlers/internalapi/shipments.go +++ b/pkg/handlers/internalapi/shipments.go @@ -1,6 +1,7 @@ package internalapi import ( + "reflect" "time" "github.com/facebookgo/clock" @@ -9,10 +10,13 @@ import ( "github.com/gofrs/uuid" "go.uber.org/zap" + "github.com/honeycombio/beeline-go" + "github.com/transcom/mymove/pkg/auth" shipmentop "github.com/transcom/mymove/pkg/gen/internalapi/internaloperations/shipments" "github.com/transcom/mymove/pkg/gen/internalmessages" "github.com/transcom/mymove/pkg/handlers" + "github.com/transcom/mymove/pkg/logging" "github.com/transcom/mymove/pkg/models" invoiceop "github.com/transcom/mymove/pkg/services/invoice" ) @@ -139,14 +143,15 @@ type CreateShipmentHandler struct { // Handle is the handler func (h CreateShipmentHandler) Handle(params shipmentop.CreateShipmentParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) + // #nosec UUID is pattern matched by swagger and will be ok moveID, _ := uuid.FromString(params.MoveID.String()) // Validate that this move belongs to the current user move, err := models.FetchMove(h.DB(), session, moveID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } payload := params.Shipment @@ -198,18 +203,18 @@ func (h CreateShipmentHandler) Handle(params shipmentop.CreateShipmentParams) mi Market: &market, } if err = updateShipmentDatesWithPayload(h, &newShipment, params.Shipment); err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } verrs, err := models.SaveShipmentAndAddresses(h.DB(), &newShipment) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } shipmentPayload, err := payloadForShipmentModel(newShipment) if err != nil { - h.Logger().Error("Error in shipment payload: ", zap.Error(err)) + logger.Error("Error in shipment payload: ", zap.Error(err)) } return shipmentop.NewCreateShipmentCreated().WithPayload(shipmentPayload) @@ -322,19 +327,19 @@ type PatchShipmentHandler struct { // Handle is the handler func (h PatchShipmentHandler) Handle(params shipmentop.PatchShipmentParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) // #nosec UUID is pattern matched by swagger and will be ok shipmentID, _ := uuid.FromString(params.ShipmentID.String()) shipment, err := models.FetchShipment(h.DB(), session, shipmentID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } patchShipmentWithPayload(shipment, params.Shipment) if err = updateShipmentDatesWithPayload(h, shipment, params.Shipment); err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } // Premove survey info can only be edited by office users or TSPs @@ -345,12 +350,12 @@ func (h PatchShipmentHandler) Handle(params shipmentop.PatchShipmentParams) midd verrs, err := models.SaveShipmentAndAddresses(h.DB(), shipment) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } shipmentPayload, err := payloadForShipmentModel(*shipment) if err != nil { - h.Logger().Error("Error in shipment payload: ", zap.Error(err)) + logger.Error("Error in shipment payload: ", zap.Error(err)) } return shipmentop.NewPatchShipmentOK().WithPayload(shipmentPayload) @@ -389,19 +394,23 @@ type GetShipmentHandler struct { // Handle is the handler func (h GetShipmentHandler) Handle(params shipmentop.GetShipmentParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + ctx := params.HTTPRequest.Context() + + logger := logging.FromContext(ctx).(Logger) + + session := auth.SessionFromContext(ctx) // #nosec UUID is pattern matched by swagger and will be ok shipmentID, _ := uuid.FromString(params.ShipmentID.String()) shipment, err := models.FetchShipment(h.DB(), session, shipmentID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } shipmentPayload, err := payloadForShipmentModel(*shipment) if err != nil { - h.Logger().Error("Error in shipment payload: ", zap.Error(err)) + logger.Error("Error in shipment payload: ", zap.Error(err)) } return shipmentop.NewGetShipmentOK().WithPayload(shipmentPayload) @@ -414,7 +423,13 @@ type ApproveHHGHandler struct { // Handle is the handler func (h ApproveHHGHandler) Handle(params shipmentop.ApproveHHGParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + + ctx := params.HTTPRequest.Context() + + ctx, span := beeline.StartSpan(ctx, reflect.TypeOf(h).Name()) + defer span.Send() + + session, logger := h.SessionAndLoggerFromContext(ctx) if !session.IsOfficeUser() { return shipmentop.NewApproveHHGForbidden() } @@ -424,21 +439,21 @@ func (h ApproveHHGHandler) Handle(params shipmentop.ApproveHHGParams) middleware shipment, err := models.FetchShipment(h.DB(), session, shipmentID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } err = shipment.Approve() if err != nil { - h.Logger().Error("Attempted to approve HHG, got invalid transition", zap.Error(err), zap.String("shipment_status", string(shipment.Status))) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("Attempted to approve HHG, got invalid transition", zap.Error(err), zap.String("shipment_status", string(shipment.Status))) + return handlers.ResponseForError(logger, err) } verrs, err := h.DB().ValidateAndUpdate(shipment) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } shipmentPayload, err := payloadForShipmentModel(*shipment) if err != nil { - h.Logger().Error("Error in shipment payload: ", zap.Error(err)) + logger.Error("Error in shipment payload: ", zap.Error(err)) } return shipmentop.NewApproveHHGOK().WithPayload(shipmentPayload) @@ -451,7 +466,8 @@ type ShipmentInvoiceHandler struct { // Handle is the handler func (h ShipmentInvoiceHandler) Handle(params shipmentop.CreateAndSendHHGInvoiceParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) if !session.IsOfficeUser() { return shipmentop.NewCreateAndSendHHGInvoiceForbidden() } @@ -460,10 +476,10 @@ func (h ShipmentInvoiceHandler) Handle(params shipmentop.CreateAndSendHHGInvoice shipmentID, _ := uuid.FromString(params.ShipmentID.String()) shipment, err := invoiceop.FetchShipmentForInvoice{DB: h.DB()}.Call(shipmentID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } if shipment.Status != models.ShipmentStatusDELIVERED && shipment.Status != models.ShipmentStatusCOMPLETED { - h.Logger().Error("Shipment status not in delivered state.") + logger.Error("Shipment status not in delivered state.") return shipmentop.NewCreateAndSendHHGInvoicePreconditionFailed() } @@ -471,7 +487,7 @@ func (h ShipmentInvoiceHandler) Handle(params shipmentop.CreateAndSendHHGInvoice //if invoices exists and at least one is either in process or has succeeded then return 409 existingInvoices, err := models.FetchInvoicesForShipment(h.DB(), shipmentID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } for _, invoice := range existingInvoices { //if an invoice has started, is in process or has been submitted successfully then throw err @@ -483,39 +499,39 @@ func (h ShipmentInvoiceHandler) Handle(params shipmentop.CreateAndSendHHGInvoice approver, err := models.FetchOfficeUserByID(h.DB(), session.OfficeUserID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } // before processing the invoice, save it in an in process state var invoice models.Invoice verrs, err := invoiceop.CreateInvoice{DB: h.DB(), Clock: clock.New()}.Call(*approver, &invoice, shipment) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } invoice858CString, verrs, err := invoiceop.ProcessInvoice{ DB: h.DB(), - Logger: h.Logger(), + Logger: logger, GexSender: h.GexSender(), SendProductionInvoice: h.SendProductionInvoice(), ICNSequencer: h.ICNSequencer(), }.Call(&invoice, shipment) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } // Send invoice to S3 for storage if response from GEX is successful fs := h.FileStorer() verrs, err = invoiceop.StoreInvoice858C{ DB: h.DB(), - Logger: h.Logger(), + Logger: logger, Storer: &fs, }.Call(*invoice858CString, &invoice, session.UserID) if verrs.HasAny() { - h.Logger().Error("Failed to store invoice record to s3, with validation errors", zap.Error(verrs)) + logger.Error("Failed to store invoice record to s3, with validation errors", zap.Error(verrs)) } if err != nil { - h.Logger().Error("Failed to store invoice record to s3, with error", zap.Error(err)) + logger.Error("Failed to store invoice record to s3, with error", zap.Error(err)) } payload := payloadForInvoiceModel(&invoice) diff --git a/pkg/handlers/internalapi/signed_certifications.go b/pkg/handlers/internalapi/signed_certifications.go index 57b8cae66390..87a4222789f1 100644 --- a/pkg/handlers/internalapi/signed_certifications.go +++ b/pkg/handlers/internalapi/signed_certifications.go @@ -6,7 +6,6 @@ import ( "github.com/go-openapi/runtime/middleware" "github.com/gofrs/uuid" - "github.com/transcom/mymove/pkg/auth" certop "github.com/transcom/mymove/pkg/gen/internalapi/internaloperations/certification" "github.com/transcom/mymove/pkg/gen/internalmessages" "github.com/transcom/mymove/pkg/handlers" @@ -41,7 +40,7 @@ type CreateSignedCertificationHandler struct { // Handle creates a new SignedCertification from a request payload func (h CreateSignedCertificationHandler) Handle(params certop.CreateSignedCertificationParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) // User should always be populated by middleware moveID, _ := uuid.FromString(params.MoveID.String()) payload := params.CreateSignedCertificationPayload @@ -52,7 +51,7 @@ func (h CreateSignedCertificationHandler) Handle(params certop.CreateSignedCerti if err == nil { _, err = models.FetchPersonallyProcuredMove(h.DB(), session, ppmID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } } } @@ -62,7 +61,7 @@ func (h CreateSignedCertificationHandler) Handle(params certop.CreateSignedCerti if err == nil { _, err = models.FetchShipment(h.DB(), session, shipmentID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } } } @@ -75,7 +74,7 @@ func (h CreateSignedCertificationHandler) Handle(params certop.CreateSignedCerti move, err := models.FetchMove(h.DB(), session, moveID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } newSignedCertification, verrs, err := move.CreateSignedCertification(h.DB(), @@ -87,7 +86,7 @@ func (h CreateSignedCertificationHandler) Handle(params certop.CreateSignedCerti shipmentID, ptrCertType) if verrs.HasAny() || err != nil { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } signedCertificationPayload := payloadForSignedCertificationModel(*newSignedCertification) @@ -101,12 +100,12 @@ type IndexSignedCertificationsHandler struct { // Handle gets a list of SignedCertifications for a move func (h IndexSignedCertificationsHandler) Handle(params certop.IndexSignedCertificationParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) moveID, _ := uuid.FromString(params.MoveID.String()) _, err := models.FetchMove(h.DB(), session, moveID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } signedCertifications, err := models.FetchSignedCertifications(h.DB(), session, moveID) @@ -115,7 +114,7 @@ func (h IndexSignedCertificationsHandler) Handle(params certop.IndexSignedCertif signedCertificationsPayload = append(signedCertificationsPayload, payloadForSignedCertificationModel(*sc)) } if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return certop.NewIndexSignedCertificationOK().WithPayload(signedCertificationsPayload) } diff --git a/pkg/handlers/internalapi/transportation_offices.go b/pkg/handlers/internalapi/transportation_offices.go index afe282435183..fffd56b45333 100644 --- a/pkg/handlers/internalapi/transportation_offices.go +++ b/pkg/handlers/internalapi/transportation_offices.go @@ -38,10 +38,11 @@ type ShowDutyStationTransportationOfficeHandler struct { // Handle retrieves the transportation office in the system for a given duty station ID func (h ShowDutyStationTransportationOfficeHandler) Handle(params transportationofficeop.ShowDutyStationTransportationOfficeParams) middleware.Responder { + logger := h.LoggerFromRequest(params.HTTPRequest) dutyStationID, _ := uuid.FromString(params.DutyStationID.String()) transportationOffice, err := models.FetchDutyStationTransportationOffice(h.DB(), dutyStationID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } transportationOfficePayload := payloadForTransportationOfficeModel(transportationOffice) diff --git a/pkg/handlers/internalapi/uploads.go b/pkg/handlers/internalapi/uploads.go index 09e38da7514e..a6738800bb42 100644 --- a/pkg/handlers/internalapi/uploads.go +++ b/pkg/handlers/internalapi/uploads.go @@ -11,7 +11,6 @@ import ( "github.com/honeycombio/beeline-go" "go.uber.org/zap" - "github.com/transcom/mymove/pkg/auth" uploadop "github.com/transcom/mymove/pkg/gen/internalapi/internaloperations/uploads" "github.com/transcom/mymove/pkg/gen/internalmessages" "github.com/transcom/mymove/pkg/handlers" @@ -39,32 +38,37 @@ type CreateUploadHandler struct { // Handle creates a new Upload from a request payload func (h CreateUploadHandler) Handle(params uploadop.CreateUploadParams) middleware.Responder { - ctx, span := beeline.StartSpan(params.HTTPRequest.Context(), reflect.TypeOf(h).Name()) + ctx := params.HTTPRequest.Context() + + session, logger := h.SessionAndLoggerFromContext(ctx) + + ctx, span := beeline.StartSpan(ctx, reflect.TypeOf(h).Name()) defer span.Send() file, ok := params.File.(*runtime.File) if !ok { - h.Logger().Error("This should always be a runtime.File, something has changed in go-swagger.") + logger.Error("This should always be a runtime.File, something has changed in go-swagger.") return uploadop.NewCreateUploadInternalServerError() } - h.Logger().Info("File name and size: ", zap.String("name", file.Header.Filename), zap.Int64("size", file.Header.Size)) - - // User should always be populated by middleware - session := auth.SessionFromRequestContext(params.HTTPRequest) + logger.Info( + "File name and size", + zap.String("name", file.Header.Filename), + zap.Int64("size", file.Header.Size), + ) var docID *uuid.UUID if params.DocumentID != nil { documentID, err := uuid.FromString(params.DocumentID.String()) if err != nil { - h.Logger().Info("Badly formed UUID for document", zap.String("document_id", params.DocumentID.String()), zap.Error(err)) + logger.Info("Badly formed UUID for document", zap.String("document_id", params.DocumentID.String()), zap.Error(err)) return uploadop.NewCreateUploadBadRequest() } // Fetch document to ensure user has access to it document, docErr := models.FetchDocument(ctx, h.DB(), session, documentID) if docErr != nil { - return handlers.ResponseForError(h.Logger(), docErr) + return handlers.ResponseForError(logger, docErr) } docID = &document.ID } @@ -72,25 +76,25 @@ func (h CreateUploadHandler) Handle(params uploadop.CreateUploadParams) middlewa // Read the incoming data into a temporary afero.File for consumption aFile, err := h.FileStorer().TempFileSystem().Create(file.Header.Filename) if err != nil { - h.Logger().Error("Error opening afero file.", zap.Error(err)) + logger.Error("Error opening afero file.", zap.Error(err)) return uploadop.NewCreateUploadInternalServerError() } _, err = io.Copy(aFile, file.Data) if err != nil { - h.Logger().Error("Error copying incoming data into afero file.", zap.Error(err)) + logger.Error("Error copying incoming data into afero file.", zap.Error(err)) return uploadop.NewCreateUploadInternalServerError() } - uploader := uploaderpkg.NewUploader(h.DB(), h.Logger(), h.FileStorer()) + uploader := uploaderpkg.NewUploader(h.DB(), logger, h.FileStorer()) newUpload, verrs, err := uploader.CreateUploadForDocument(docID, session.UserID, aFile, uploaderpkg.AllowedTypesServiceMember) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } url, err := uploader.PresignedURL(newUpload) if err != nil { - h.Logger().Error("failed to get presigned url", zap.Error(err)) + logger.Error("failed to get presigned url", zap.Error(err)) return uploadop.NewCreateUploadInternalServerError() } uploadPayload := payloadForUploadModel(*newUpload, url) @@ -108,17 +112,17 @@ func (h DeleteUploadHandler) Handle(params uploadop.DeleteUploadParams) middlewa ctx, span := beeline.StartSpan(params.HTTPRequest.Context(), reflect.TypeOf(h).Name()) defer span.Send() - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) uploadID, _ := uuid.FromString(params.UploadID.String()) upload, err := models.FetchUpload(ctx, h.DB(), session, uploadID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } - uploader := uploaderpkg.NewUploader(h.DB(), h.Logger(), h.FileStorer()) + uploader := uploaderpkg.NewUploader(h.DB(), logger, h.FileStorer()) if err = uploader.DeleteUpload(&upload); err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return uploadop.NewDeleteUploadNoContent() @@ -136,18 +140,18 @@ func (h DeleteUploadsHandler) Handle(params uploadop.DeleteUploadsParams) middle defer span.Send() // User should always be populated by middleware - session := auth.SessionFromRequestContext(params.HTTPRequest) - uploader := uploaderpkg.NewUploader(h.DB(), h.Logger(), h.FileStorer()) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) + uploader := uploaderpkg.NewUploader(h.DB(), logger, h.FileStorer()) for _, uploadID := range params.UploadIds { uuid, _ := uuid.FromString(uploadID.String()) upload, err := models.FetchUpload(ctx, h.DB(), session, uuid) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } if err = uploader.DeleteUpload(&upload); err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } } diff --git a/pkg/handlers/internalapi/users.go b/pkg/handlers/internalapi/users.go index c2ec9ab5d40e..1bede1d64738 100644 --- a/pkg/handlers/internalapi/users.go +++ b/pkg/handlers/internalapi/users.go @@ -8,7 +8,6 @@ import ( "github.com/pkg/errors" "go.uber.org/zap" - "github.com/transcom/mymove/pkg/auth" userop "github.com/transcom/mymove/pkg/gen/internalapi/internaloperations/users" "github.com/transcom/mymove/pkg/gen/internalmessages" "github.com/transcom/mymove/pkg/handlers" @@ -26,7 +25,7 @@ func (h ShowLoggedInUserHandler) Handle(params userop.ShowLoggedInUserParams) mi ctx, span := beeline.StartSpan(params.HTTPRequest.Context(), reflect.TypeOf(h).Name()) defer span.Send() - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) if !session.IsServiceMember() { userPayload := internalmessages.LoggedInUserPayload{ @@ -39,7 +38,7 @@ func (h ShowLoggedInUserHandler) Handle(params userop.ShowLoggedInUserParams) mi // Load Servicemember and first level associations serviceMember, err := models.FetchServiceMemberForUser(ctx, h.DB(), session, session.ServiceMemberID) if err != nil { - h.Logger().Error("Error retrieving service_member", zap.Error(err)) + logger.Error("Error retrieving service_member", zap.Error(err)) return userop.NewShowLoggedInUserUnauthorized() } @@ -48,7 +47,7 @@ func (h ShowLoggedInUserHandler) Handle(params userop.ShowLoggedInUserParams) mi // Fetch associations on duty station dutyStation, err := models.FetchDutyStation(ctx, h.DB(), *serviceMember.DutyStationID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } serviceMember.DutyStation = dutyStation @@ -57,11 +56,11 @@ func (h ShowLoggedInUserHandler) Handle(params userop.ShowLoggedInUserParams) mi if err != nil { if errors.Cause(err) != models.ErrFetchNotFound { // The absence of an office shouldn't render the entire request a 404 - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } // We might not have Transportation Office data for a Duty Station, and that's ok if errors.Cause(err) != models.ErrFetchNotFound { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } } serviceMember.DutyStation.TransportationOffice = transportationOffice @@ -71,7 +70,7 @@ func (h ShowLoggedInUserHandler) Handle(params userop.ShowLoggedInUserParams) mi if len(serviceMember.Orders) > 0 { orders, err := models.FetchOrderForUser(h.DB(), session, serviceMember.Orders[0].ID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } serviceMember.Orders[0] = orders @@ -79,11 +78,11 @@ func (h ShowLoggedInUserHandler) Handle(params userop.ShowLoggedInUserParams) mi if err != nil { if errors.Cause(err) != models.ErrFetchNotFound { // The absence of an office shouldn't render the entire request a 404 - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } // We might not have Transportation Office data for a Duty Station, and that's ok if errors.Cause(err) != models.ErrFetchNotFound { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } } serviceMember.Orders[0].NewDutyStation.TransportationOffice = newDutyStationTransportationOffice @@ -94,7 +93,7 @@ func (h ShowLoggedInUserHandler) Handle(params userop.ShowLoggedInUserParams) mi // TODO: load advances on all ppms for the latest order's move ppm, err := models.FetchPersonallyProcuredMove(h.DB(), session, serviceMember.Orders[0].Moves[0].PersonallyProcuredMoves[0].ID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } serviceMember.Orders[0].Moves[0].PersonallyProcuredMoves[0].Advance = ppm.Advance } diff --git a/pkg/handlers/ordersapi/get_orders.go b/pkg/handlers/ordersapi/get_orders.go index fa1302055d2f..593f9eb4fb84 100644 --- a/pkg/handlers/ordersapi/get_orders.go +++ b/pkg/handlers/ordersapi/get_orders.go @@ -18,31 +18,36 @@ type GetOrdersHandler struct { // Handle (GetOrdersHandler) responds to GET /orders/{uuid} func (h GetOrdersHandler) Handle(params ordersoperations.GetOrdersParams) middleware.Responder { - clientCert := authentication.ClientCertFromRequestContext(params.HTTPRequest) + + ctx := params.HTTPRequest.Context() + + logger := h.LoggerFromContext(ctx) + + clientCert := authentication.ClientCertFromContext(ctx) if clientCert == nil { - return handlers.ResponseForError(h.Logger(), errors.WithMessage(models.ErrUserUnauthorized, "No client certificate provided")) + return handlers.ResponseForError(logger, errors.WithMessage(models.ErrUserUnauthorized, "No client certificate provided")) } if !clientCert.AllowOrdersAPI { - return handlers.ResponseForError(h.Logger(), errors.WithMessage(models.ErrFetchForbidden, "Not permitted to access this API")) + return handlers.ResponseForError(logger, errors.WithMessage(models.ErrFetchForbidden, "Not permitted to access this API")) } id, err := uuid.FromString(params.UUID.String()) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } orders, err := models.FetchElectronicOrderByID(h.DB(), id) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } if !verifyOrdersReadAccess(orders.Issuer, clientCert) { - return handlers.ResponseForError(h.Logger(), errors.WithMessage(models.ErrFetchForbidden, "Not permitted to read Orders from this issuer")) + return handlers.ResponseForError(logger, errors.WithMessage(models.ErrFetchForbidden, "Not permitted to read Orders from this issuer")) } ordersPayload, err := payloadForElectronicOrderModel(orders) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return ordersoperations.NewGetOrdersOK().WithPayload(ordersPayload) diff --git a/pkg/handlers/ordersapi/get_orders_by_issuer_and_orders_num.go b/pkg/handlers/ordersapi/get_orders_by_issuer_and_orders_num.go index 84ffdd4304de..a5a5c03144ca 100644 --- a/pkg/handlers/ordersapi/get_orders_by_issuer_and_orders_num.go +++ b/pkg/handlers/ordersapi/get_orders_by_issuer_and_orders_num.go @@ -17,25 +17,30 @@ type GetOrdersByIssuerAndOrdersNumHandler struct { // Handle (GetOrdersByIssuerAndOrdersNumHandler) responds to GET /issuers/{issuer}/orders/{ordersNum} func (h GetOrdersByIssuerAndOrdersNumHandler) Handle(params ordersoperations.GetOrdersByIssuerAndOrdersNumParams) middleware.Responder { - clientCert := authentication.ClientCertFromRequestContext(params.HTTPRequest) + + ctx := params.HTTPRequest.Context() + + logger := h.LoggerFromContext(ctx) + + clientCert := authentication.ClientCertFromContext(ctx) if clientCert == nil { - return handlers.ResponseForError(h.Logger(), errors.WithMessage(models.ErrUserUnauthorized, "No client certificate provided")) + return handlers.ResponseForError(logger, errors.WithMessage(models.ErrUserUnauthorized, "No client certificate provided")) } if !clientCert.AllowOrdersAPI { - return handlers.ResponseForError(h.Logger(), errors.WithMessage(models.ErrFetchForbidden, "Not permitted to access this API")) + return handlers.ResponseForError(logger, errors.WithMessage(models.ErrFetchForbidden, "Not permitted to access this API")) } if !verifyOrdersReadAccess(models.Issuer(params.Issuer), clientCert) { - return handlers.ResponseForError(h.Logger(), errors.WithMessage(models.ErrFetchForbidden, "Not permitted to read orders from this issuer")) + return handlers.ResponseForError(logger, errors.WithMessage(models.ErrFetchForbidden, "Not permitted to read orders from this issuer")) } orders, err := models.FetchElectronicOrderByIssuerAndOrdersNum(h.DB(), params.Issuer, params.OrdersNum) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } ordersPayload, err := payloadForElectronicOrderModel(orders) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return ordersoperations.NewGetOrdersByIssuerAndOrdersNumOK().WithPayload(ordersPayload) diff --git a/pkg/handlers/ordersapi/index_orders_for_member.go b/pkg/handlers/ordersapi/index_orders_for_member.go index edee776a5656..a2524689b164 100644 --- a/pkg/handlers/ordersapi/index_orders_for_member.go +++ b/pkg/handlers/ordersapi/index_orders_for_member.go @@ -18,30 +18,35 @@ type IndexOrdersForMemberHandler struct { // Handle (IndexOrdersForMemberHandler) responds to GET /edipis/{edipi}/orders func (h IndexOrdersForMemberHandler) Handle(params ordersoperations.IndexOrdersForMemberParams) middleware.Responder { - clientCert := authentication.ClientCertFromRequestContext(params.HTTPRequest) + + ctx := params.HTTPRequest.Context() + + logger := h.LoggerFromContext(ctx) + + clientCert := authentication.ClientCertFromContext(ctx) if clientCert == nil { - return handlers.ResponseForError(h.Logger(), errors.WithMessage(models.ErrUserUnauthorized, "No client certificate provided")) + return handlers.ResponseForError(logger, errors.WithMessage(models.ErrUserUnauthorized, "No client certificate provided")) } if !clientCert.AllowOrdersAPI { - return handlers.ResponseForError(h.Logger(), errors.WithMessage(models.ErrFetchForbidden, "Not permitted to access this API")) + return handlers.ResponseForError(logger, errors.WithMessage(models.ErrFetchForbidden, "Not permitted to access this API")) } allowedIssuers := clientCert.GetAllowedOrdersIssuersRead() if len(allowedIssuers) == 0 { - return handlers.ResponseForError(h.Logger(), errors.WithMessage(models.ErrFetchForbidden, "Not permitted to read any Orders")) + return handlers.ResponseForError(logger, errors.WithMessage(models.ErrFetchForbidden, "Not permitted to read any Orders")) } orders, err := models.FetchElectronicOrdersByEdipiAndIssuers(h.DB(), params.Edipi, allowedIssuers) if err == models.ErrFetchNotFound { return ordersoperations.NewIndexOrdersForMemberOK().WithPayload([]*ordersmessages.Orders{}) } else if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } ordersPayloads := make([]*ordersmessages.Orders, len(orders)) for i, o := range orders { payload, err := payloadForElectronicOrderModel(o) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } ordersPayloads[i] = payload } diff --git a/pkg/handlers/ordersapi/post_revision.go b/pkg/handlers/ordersapi/post_revision.go index 564872134cad..2956404ceef6 100644 --- a/pkg/handlers/ordersapi/post_revision.go +++ b/pkg/handlers/ordersapi/post_revision.go @@ -25,18 +25,23 @@ type PostRevisionHandler struct { // Handle (params ordersoperations.PostRevisionParams) responds to POST /orders func (h PostRevisionHandler) Handle(params ordersoperations.PostRevisionParams) middleware.Responder { - ctx, span := beeline.StartSpan(params.HTTPRequest.Context(), reflect.TypeOf(h).Name()) + + ctx := params.HTTPRequest.Context() + + logger := h.LoggerFromContext(ctx) + + ctx, span := beeline.StartSpan(ctx, reflect.TypeOf(h).Name()) defer span.Send() clientCert := authentication.ClientCertFromRequestContext(params.HTTPRequest) if clientCert == nil { - return handlers.ResponseForError(h.Logger(), errors.WithMessage(models.ErrUserUnauthorized, "No client certificate provided")) + return handlers.ResponseForError(logger, errors.WithMessage(models.ErrUserUnauthorized, "No client certificate provided")) } if !clientCert.AllowOrdersAPI { - return handlers.ResponseForError(h.Logger(), errors.WithMessage(models.ErrWriteForbidden, "Not permitted to access this API")) + return handlers.ResponseForError(logger, errors.WithMessage(models.ErrWriteForbidden, "Not permitted to access this API")) } if !verifyOrdersWriteAccess(models.Issuer(params.Issuer), clientCert) { - return handlers.ResponseForError(h.Logger(), errors.WithMessage(models.ErrWriteForbidden, "Not permitted to write Orders from this issuer")) + return handlers.ResponseForError(logger, errors.WithMessage(models.ErrWriteForbidden, "Not permitted to write Orders from this issuer")) } var edipi string @@ -50,7 +55,7 @@ func (h PostRevisionHandler) Handle(params ordersoperations.PostRevisionParams) } matchReasonCode, edipiNum, _, _, err := rbsPersonLookup.GetPersonUsingSSN(iwsParams) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } switch matchReasonCode { case iws.MatchReasonCodeLimited: @@ -61,10 +66,10 @@ func (h PostRevisionHandler) Handle(params ordersoperations.PostRevisionParams) edipi = fmt.Sprintf("%010d", edipiNum) case iws.MatchReasonCodeMultiple: // more than one EDIPI for this SSN! Uhh... how to choose? FWIW it's unlikely but not impossible to encounter this in the wild - return handlers.ResponseForError(h.Logger(), errors.WithMessage(models.ErrFetchNotFound, "DMDC IWS matched multiple EDIPIs for this SSN")) + return handlers.ResponseForError(logger, errors.WithMessage(models.ErrFetchNotFound, "DMDC IWS matched multiple EDIPIs for this SSN")) case iws.MatchReasonCodeNone: // No match: fail - return handlers.ResponseForError(h.Logger(), errors.WithMessage(models.ErrFetchNotFound, "DMDC IWS matched no EDIPI for this SSN")) + return handlers.ResponseForError(logger, errors.WithMessage(models.ErrFetchNotFound, "DMDC IWS matched no EDIPI for this SSN")) } } else if len(params.MemberID) == 10 { edipi = params.MemberID @@ -89,10 +94,10 @@ func (h PostRevisionHandler) Handle(params ordersoperations.PostRevisionParams) newRevision = toElectronicOrdersRevision(orders, params.Revision) verrs, err = models.CreateElectronicOrderWithRevision(ctx, h.DB(), orders, newRevision) } else if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } else if orders.Edipi != edipi { return handlers.ResponseForError( - h.Logger(), + logger, errors.WithMessage( models.ErrWriteConflict, fmt.Sprintf("Cannot POST Revision for EDIPI %s to Electronic Orders with OrdersNum %s from Issuer %s: the existing orders are issued to EDIPI %s", edipi, params.OrdersNum, params.Issuer, orders.Edipi))) @@ -102,7 +107,7 @@ func (h PostRevisionHandler) Handle(params ordersoperations.PostRevisionParams) // SeqNum collision if r.SeqNum == int(*params.Revision.SeqNum) { return handlers.ResponseForError( - h.Logger(), + logger, errors.WithMessage( models.ErrWriteConflict, fmt.Sprintf("Cannot POST Revision with SeqNum %d for EDIPI %s to Electronic Orders with OrdersNum %s from Issuer %s: a Revision with that SeqNum already exists in those Orders", r.SeqNum, edipi, params.OrdersNum, params.Issuer))) @@ -113,13 +118,13 @@ func (h PostRevisionHandler) Handle(params ordersoperations.PostRevisionParams) verrs, err = models.CreateElectronicOrdersRevision(ctx, h.DB(), newRevision) } if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } orders.Revisions = append(orders.Revisions, *newRevision) orderPayload, err := payloadForElectronicOrderModel(orders) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return ordersoperations.NewPostRevisionCreated().WithPayload(orderPayload) } diff --git a/pkg/handlers/ordersapi/post_revision_to_orders.go b/pkg/handlers/ordersapi/post_revision_to_orders.go index 9621c3c863b6..9949ecdb07f4 100644 --- a/pkg/handlers/ordersapi/post_revision_to_orders.go +++ b/pkg/handlers/ordersapi/post_revision_to_orders.go @@ -22,36 +22,41 @@ type PostRevisionToOrdersHandler struct { // Handle (params ordersoperations.PostRevisionToOrdersParams) responds to POST /orders/{uuid} func (h PostRevisionToOrdersHandler) Handle(params ordersoperations.PostRevisionToOrdersParams) middleware.Responder { - ctx, span := beeline.StartSpan(params.HTTPRequest.Context(), reflect.TypeOf(h).Name()) + + ctx := params.HTTPRequest.Context() + + logger := h.LoggerFromContext(ctx) + + ctx, span := beeline.StartSpan(ctx, reflect.TypeOf(h).Name()) defer span.Send() - clientCert := authentication.ClientCertFromRequestContext(params.HTTPRequest) + clientCert := authentication.ClientCertFromContext(ctx) if clientCert == nil { - return handlers.ResponseForError(h.Logger(), errors.WithMessage(models.ErrUserUnauthorized, "No client certificate provided")) + return handlers.ResponseForError(logger, errors.WithMessage(models.ErrUserUnauthorized, "No client certificate provided")) } if !clientCert.AllowOrdersAPI { - return handlers.ResponseForError(h.Logger(), errors.WithMessage(models.ErrWriteForbidden, "Not permitted to access this API")) + return handlers.ResponseForError(logger, errors.WithMessage(models.ErrWriteForbidden, "Not permitted to access this API")) } id, err := uuid.FromString(params.UUID.String()) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } orders, err := models.FetchElectronicOrderByID(h.DB(), id) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } if !verifyOrdersWriteAccess(orders.Issuer, clientCert) { - return handlers.ResponseForError(h.Logger(), errors.WithMessage(models.ErrWriteForbidden, "Not permitted to write Orders from this issuer")) + return handlers.ResponseForError(logger, errors.WithMessage(models.ErrWriteForbidden, "Not permitted to write Orders from this issuer")) } for _, r := range orders.Revisions { // SeqNum collision if r.SeqNum == int(*params.Revision.SeqNum) { return handlers.ResponseForError( - h.Logger(), + logger, errors.WithMessage( models.ErrWriteConflict, fmt.Sprintf("Cannot POST Revision with SeqNum %d to Orders %s: a Revision with that SeqNum already exists in those Orders", r.SeqNum, params.UUID))) @@ -61,14 +66,14 @@ func (h PostRevisionToOrdersHandler) Handle(params ordersoperations.PostRevision newRevision := toElectronicOrdersRevision(orders, params.Revision) verrs, err := models.CreateElectronicOrdersRevision(ctx, h.DB(), newRevision) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } orders.Revisions = append(orders.Revisions, *newRevision) orderPayload, err := payloadForElectronicOrderModel(orders) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return ordersoperations.NewPostRevisionToOrdersCreated().WithPayload(orderPayload) } diff --git a/pkg/handlers/publicapi/access_code.go b/pkg/handlers/publicapi/access_code.go index 7340dca104fb..a306a45ae133 100644 --- a/pkg/handlers/publicapi/access_code.go +++ b/pkg/handlers/publicapi/access_code.go @@ -5,7 +5,6 @@ import ( "github.com/go-openapi/runtime/middleware" - "github.com/transcom/mymove/pkg/auth" "github.com/transcom/mymove/pkg/gen/apimessages" accesscodeop "github.com/transcom/mymove/pkg/gen/restapi/apioperations/accesscode" "github.com/transcom/mymove/pkg/handlers" @@ -40,7 +39,7 @@ func payloadForAccessCodeModel(accessCode models.AccessCode) *apimessages.Access // Handle accepts the code - validates the access code func (h ValidateAccessCodeHandler) Handle(params accesscodeop.ValidateAccessCodeParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) if session == nil { return accesscodeop.NewValidateAccessCodeUnauthorized() @@ -53,7 +52,7 @@ func (h ValidateAccessCodeHandler) Handle(params accesscodeop.ValidateAccessCode var validateAccessCodePayload *apimessages.ValidateAccessCodePayload if !valid { - h.Logger().Warn("Access code not valid") + logger.Warn("Access code not valid") validateAccessCodePayload = &apimessages.ValidateAccessCodePayload{ Valid: &valid, } diff --git a/pkg/handlers/publicapi/generic_move_documents.go b/pkg/handlers/publicapi/generic_move_documents.go index 5835cab30a80..857d747ade8b 100644 --- a/pkg/handlers/publicapi/generic_move_documents.go +++ b/pkg/handlers/publicapi/generic_move_documents.go @@ -9,7 +9,6 @@ import ( beeline "github.com/honeycombio/beeline-go" "go.uber.org/zap" - "github.com/transcom/mymove/pkg/auth" "github.com/transcom/mymove/pkg/gen/apimessages" movedocop "github.com/transcom/mymove/pkg/gen/restapi/apioperations/move_docs" "github.com/transcom/mymove/pkg/handlers" @@ -75,27 +74,27 @@ func (h CreateGenericMoveDocumentHandler) Handle(params movedocop.CreateGenericM ctx, span := beeline.StartSpan(params.HTTPRequest.Context(), reflect.TypeOf(h).Name()) defer span.Send() - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) // Verify that the TSP user is authorized to update move doc shipmentID, _ := uuid.FromString(params.ShipmentID.String()) _, shipment, err := models.FetchShipmentForVerifiedTSPUser(h.DB(), session.TspUserID, shipmentID) if err != nil { if err.Error() == "USER_UNAUTHORIZED" { - h.Logger().Error("DB Query", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("DB Query", zap.Error(err)) + return handlers.ResponseForError(logger, err) } if err.Error() == "FETCH_FORBIDDEN" { - h.Logger().Error("DB Query", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("DB Query", zap.Error(err)) + return handlers.ResponseForError(logger, err) } - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } // Fetch move move, err := models.FetchMove(h.DB(), session, shipment.Move.ID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } payload := params.CreateGenericMoveDocumentPayload @@ -111,7 +110,7 @@ func (h CreateGenericMoveDocumentHandler) Handle(params movedocop.CreateGenericM converted := uuid.Must(uuid.FromString(id.String())) upload, fetchUploadErr := models.FetchUpload(ctx, h.DB(), session, converted) if fetchUploadErr != nil { - return handlers.ResponseForError(h.Logger(), fetchUploadErr) + return handlers.ResponseForError(logger, fetchUploadErr) } uploads = append(uploads, upload) } @@ -125,12 +124,12 @@ func (h CreateGenericMoveDocumentHandler) Handle(params movedocop.CreateGenericM *shipment.Move.SelectedMoveType) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } newPayload, err := payloadForGenericMoveDocumentModel(h.FileStorer(), *newMoveDocument, shipmentID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return movedocop.NewCreateGenericMoveDocumentOK().WithPayload(newPayload) } diff --git a/pkg/handlers/publicapi/invoices.go b/pkg/handlers/publicapi/invoices.go index e9f6fd9edfa7..b0f23c9f9a05 100644 --- a/pkg/handlers/publicapi/invoices.go +++ b/pkg/handlers/publicapi/invoices.go @@ -5,7 +5,6 @@ import ( "github.com/gofrs/uuid" "go.uber.org/zap" - "github.com/transcom/mymove/pkg/auth" "github.com/transcom/mymove/pkg/gen/apimessages" accessorialop "github.com/transcom/mymove/pkg/gen/restapi/apioperations/accessorials" "github.com/transcom/mymove/pkg/handlers" @@ -45,7 +44,7 @@ type GetInvoiceHandler struct { // Handle returns a specified invoice func (h GetInvoiceHandler) Handle(params accessorialop.GetInvoiceParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) if session == nil { return accessorialop.NewGetInvoiceUnauthorized() @@ -56,17 +55,17 @@ func (h GetInvoiceHandler) Handle(params accessorialop.GetInvoiceParams) middlew invoice, err := models.FetchInvoice(h.DB(), session, invoiceID) if err != nil { if err == models.ErrFetchNotFound { - h.Logger().Warn("Invoice not found", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Warn("Invoice not found", zap.Error(err)) + return handlers.ResponseForError(logger, err) } else if err == models.ErrFetchForbidden { - h.Logger().Error("User not permitted to access invoice", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("User not permitted to access invoice", zap.Error(err)) + return handlers.ResponseForError(logger, err) } else if err == models.ErrUserUnauthorized { - h.Logger().Error("User not authorized to access invoice", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("User not authorized to access invoice", zap.Error(err)) + return handlers.ResponseForError(logger, err) } - h.Logger().Error("Error fetching invoice", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("Error fetching invoice", zap.Error(err)) + return handlers.ResponseForError(logger, err) } payload := payloadForInvoiceModel(invoice) diff --git a/pkg/handlers/publicapi/logger.go b/pkg/handlers/publicapi/logger.go new file mode 100644 index 000000000000..f6e6f417e0c0 --- /dev/null +++ b/pkg/handlers/publicapi/logger.go @@ -0,0 +1,15 @@ +package publicapi + +import ( + "go.uber.org/zap" +) + +// Logger is a logger interface for middleware +type Logger interface { + Debug(msg string, fields ...zap.Field) + Info(msg string, fields ...zap.Field) + Error(msg string, fields ...zap.Field) + Warn(msg string, fields ...zap.Field) + Fatal(msg string, fields ...zap.Field) + //WithOptions(options ...zap.Option) *zap.Logger +} diff --git a/pkg/handlers/publicapi/move_documents.go b/pkg/handlers/publicapi/move_documents.go index 890c18dae288..74e4773feab0 100644 --- a/pkg/handlers/publicapi/move_documents.go +++ b/pkg/handlers/publicapi/move_documents.go @@ -5,7 +5,6 @@ import ( "github.com/gofrs/uuid" "go.uber.org/zap" - "github.com/transcom/mymove/pkg/auth" "github.com/transcom/mymove/pkg/gen/apimessages" movedocop "github.com/transcom/mymove/pkg/gen/restapi/apioperations/move_docs" "github.com/transcom/mymove/pkg/handlers" @@ -19,39 +18,39 @@ type IndexMoveDocumentsHandler struct { // Handle handles the request func (h IndexMoveDocumentsHandler) Handle(params movedocop.IndexMoveDocumentsParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) // Verify that the TSP user is authorized to update move doc shipmentID, _ := uuid.FromString(params.ShipmentID.String()) _, shipment, err := models.FetchShipmentForVerifiedTSPUser(h.DB(), session.TspUserID, shipmentID) if err != nil { if err.Error() == "USER_UNAUTHORIZED" { - h.Logger().Error("DB Query", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("DB Query", zap.Error(err)) + return handlers.ResponseForError(logger, err) } if err.Error() == "FETCH_FORBIDDEN" { - h.Logger().Error("DB Query", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("DB Query", zap.Error(err)) + return handlers.ResponseForError(logger, err) } - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } // Validate that this move belongs to the current user move, err := models.FetchMove(h.DB(), session, shipment.Move.ID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } moveDocs, err := move.FetchAllMoveDocumentsForMove(h.DB()) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } moveDocumentsPayload := make(apimessages.MoveDocuments, len(moveDocs)) for i, doc := range moveDocs { documentPayload, err := payloadForDocumentModel(h.FileStorer(), doc.Document) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } moveDocumentPayload := apimessages.MoveDocumentPayload{ ID: handlers.FmtUUID(doc.ID), @@ -63,7 +62,7 @@ func (h IndexMoveDocumentsHandler) Handle(params movedocop.IndexMoveDocumentsPar Notes: handlers.FmtStringPtr(doc.Notes), } if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } moveDocumentsPayload[i] = &moveDocumentPayload } @@ -79,35 +78,35 @@ type UpdateMoveDocumentHandler struct { // Handle ... updates a move document from a request payload func (h UpdateMoveDocumentHandler) Handle(params movedocop.UpdateMoveDocumentParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) // Verify that the TSP user is authorized to update move doc shipmentID, _ := uuid.FromString(params.ShipmentID.String()) _, _, err := models.FetchShipmentForVerifiedTSPUser(h.DB(), session.TspUserID, shipmentID) if err != nil { if err.Error() == "USER_UNAUTHORIZED" { - h.Logger().Error("DB Query", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("DB Query", zap.Error(err)) + return handlers.ResponseForError(logger, err) } if err.Error() == "FETCH_FORBIDDEN" { - h.Logger().Error("DB Query", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("DB Query", zap.Error(err)) + return handlers.ResponseForError(logger, err) } - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } // Fetch move document from move doc id moveDocID, _ := uuid.FromString(params.MoveDocumentID.String()) moveDoc, err := models.FetchMoveDocument(h.DB(), session, moveDocID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } if moveDoc.ShipmentID == nil { - h.Logger().Error("Move document is not associated to a shipment.") + logger.Error("Move document is not associated to a shipment.") return movedocop.NewCreateGenericMoveDocumentForbidden() } if *moveDoc.ShipmentID != shipmentID { - h.Logger().Error("Move doc shipment ID does not match shipment ID.") + logger.Error("Move doc shipment ID does not match shipment ID.") return movedocop.NewCreateGenericMoveDocumentForbidden() } @@ -124,7 +123,7 @@ func (h UpdateMoveDocumentHandler) Handle(params movedocop.UpdateMoveDocumentPar if newStatus != moveDoc.Status { err = moveDoc.AttemptTransition(newStatus) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } } @@ -132,12 +131,12 @@ func (h UpdateMoveDocumentHandler) Handle(params movedocop.UpdateMoveDocumentPar verrs, err := models.SaveMoveDocument(h.DB(), moveDoc, saveAction) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } moveDocPayload, err := payloadForGenericMoveDocumentModel(h.FileStorer(), *moveDoc, shipmentID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } return movedocop.NewUpdateMoveDocumentOK().WithPayload(moveDocPayload) } diff --git a/pkg/handlers/publicapi/service_agents.go b/pkg/handlers/publicapi/service_agents.go index c98de6c0b44a..f6cabc9f49ca 100644 --- a/pkg/handlers/publicapi/service_agents.go +++ b/pkg/handlers/publicapi/service_agents.go @@ -6,7 +6,6 @@ import ( "github.com/gofrs/uuid" "go.uber.org/zap" - "github.com/transcom/mymove/pkg/auth" "github.com/transcom/mymove/pkg/gen/apimessages" serviceagentop "github.com/transcom/mymove/pkg/gen/restapi/apioperations/service_agents" "github.com/transcom/mymove/pkg/handlers" @@ -39,7 +38,7 @@ type IndexServiceAgentsHandler struct { // Handle returns a list of service agents - checks that currently logged in user is authorized to act for the TSP assigned the shipment func (h IndexServiceAgentsHandler) Handle(params serviceagentop.IndexServiceAgentsParams) middleware.Responder { var serviceAgents []models.ServiceAgent - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) shipmentID, _ := uuid.FromString(params.ShipmentID.String()) @@ -47,29 +46,29 @@ func (h IndexServiceAgentsHandler) Handle(params serviceagentop.IndexServiceAgen // TODO (2018_08_27 cgilmer): Find a way to check Shipment belongs to TSP without 2 queries tspUser, err := models.FetchTspUserByID(h.DB(), session.TspUserID) if err != nil { - h.Logger().Error("DB Query", zap.Error(err)) + logger.Error("DB Query", zap.Error(err)) return serviceagentop.NewIndexServiceAgentsForbidden() } shipment, err := models.FetchShipmentByTSP(h.DB(), tspUser.TransportationServiceProviderID, shipmentID) if err != nil { - h.Logger().Error("DB Query", zap.Error(err)) + logger.Error("DB Query", zap.Error(err)) return serviceagentop.NewIndexServiceAgentsBadRequest() } serviceAgents, err = models.FetchServiceAgentsByTSP(h.DB(), tspUser.TransportationServiceProviderID, shipment.ID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } } else if session.IsOfficeUser() { shipment, err := models.FetchShipment(h.DB(), session, shipmentID) if err != nil { - h.Logger().Error("DB Query", zap.Error(err)) + logger.Error("DB Query", zap.Error(err)) return serviceagentop.NewIndexServiceAgentsBadRequest() } serviceAgents, err = models.FetchServiceAgentsOnShipment(h.DB(), shipment.ID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } } else { return serviceagentop.NewIndexServiceAgentsForbidden() @@ -89,20 +88,20 @@ type CreateServiceAgentHandler struct { // Handle creates a new ServiceAgent from a request payload - checks that currently logged in user is authorized to act for the TSP assigned the shipment func (h CreateServiceAgentHandler) Handle(params serviceagentop.CreateServiceAgentParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) shipmentID, _ := uuid.FromString(params.ShipmentID.String()) // TODO (rebecca): Find a way to check Shipment belongs to TSP without 2 queries tspUser, err := models.FetchTspUserByID(h.DB(), session.TspUserID) if err != nil { - h.Logger().Error("DB Query", zap.Error(err)) + logger.Error("DB Query", zap.Error(err)) return serviceagentop.NewCreateServiceAgentForbidden() } shipment, err := models.FetchShipmentByTSP(h.DB(), tspUser.TransportationServiceProviderID, shipmentID) if err != nil { - h.Logger().Error("DB Query", zap.Error(err)) + logger.Error("DB Query", zap.Error(err)) return serviceagentop.NewCreateServiceAgentBadRequest() } @@ -120,7 +119,7 @@ func (h CreateServiceAgentHandler) Handle(params serviceagentop.CreateServiceAge payload.PhoneIsPreferred, payload.Notes) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } serviceAgentPayload := payloadForServiceAgentModel(newServiceAgent) @@ -137,7 +136,7 @@ func (h PatchServiceAgentHandler) Handle(params serviceagentop.PatchServiceAgent var serviceAgent *models.ServiceAgent var err error - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) shipmentID, _ := uuid.FromString(params.ShipmentID.String()) serviceAgentID, _ := uuid.FromString(params.ServiceAgentID.String()) @@ -145,23 +144,23 @@ func (h PatchServiceAgentHandler) Handle(params serviceagentop.PatchServiceAgent if session.IsTspUser() { tspUser, fetchTspUserByIDErr := models.FetchTspUserByID(h.DB(), session.TspUserID) if fetchTspUserByIDErr != nil { - h.Logger().Error("DB Query", zap.Error(fetchTspUserByIDErr)) + logger.Error("DB Query", zap.Error(fetchTspUserByIDErr)) return serviceagentop.NewPatchServiceAgentForbidden() } serviceAgent, err = models.FetchServiceAgentByTSP(h.DB(), tspUser.TransportationServiceProviderID, shipmentID, serviceAgentID) if err != nil { - h.Logger().Error("DB Query", zap.Error(err)) + logger.Error("DB Query", zap.Error(err)) return serviceagentop.NewPatchServiceAgentBadRequest() } } else if session.IsOfficeUser() { serviceAgent, err = models.FetchServiceAgentForOffice(h.DB(), shipmentID, serviceAgentID) if err != nil { - h.Logger().Error("DB Query", zap.Error(err)) + logger.Error("DB Query", zap.Error(err)) return serviceagentop.NewPatchServiceAgentBadRequest() } } else { - h.Logger().Error("Non office or TSP user attempted to patch service agent") + logger.Error("Non office or TSP user attempted to patch service agent") return serviceagentop.NewPatchServiceAgentForbidden() } // Update the Service Agent @@ -182,7 +181,7 @@ func (h PatchServiceAgentHandler) Handle(params serviceagentop.PatchServiceAgent verrs, err := h.DB().ValidateAndSave(serviceAgent) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } serviceAgentPayload := payloadForServiceAgentModel(*serviceAgent) diff --git a/pkg/handlers/publicapi/shipment_line_items.go b/pkg/handlers/publicapi/shipment_line_items.go index ae8434fe7d6a..859f0847aa69 100644 --- a/pkg/handlers/publicapi/shipment_line_items.go +++ b/pkg/handlers/publicapi/shipment_line_items.go @@ -82,7 +82,7 @@ type GetShipmentLineItemsHandler struct { handlers.HandlerContext } -func (h GetShipmentLineItemsHandler) recalculateShipmentLineItems(shipmentLineItems models.ShipmentLineItems, shipmentID uuid.UUID, session *auth.Session) (bool, middleware.Responder) { +func (h GetShipmentLineItemsHandler) recalculateShipmentLineItems(shipmentLineItems models.ShipmentLineItems, shipmentID uuid.UUID, session *auth.Session, logger Logger) (bool, middleware.Responder) { update := false // If there is a shipment line item with an invoice do not run the recalculate function @@ -98,18 +98,18 @@ func (h GetShipmentLineItemsHandler) recalculateShipmentLineItems(shipmentLineIt // Only returning ShipmentLineItems that are approved and have no InvoiceID shipment, err := invoice.FetchShipmentForInvoice{DB: h.DB()}.Call(shipmentID) if err != nil { - h.Logger().Error("Error fetching Shipment for re-pricing line items for shipment", zap.Error(err)) + logger.Error("Error fetching Shipment for re-pricing line items for shipment", zap.Error(err)) return update, accessorialop.NewGetShipmentLineItemsInternalServerError() } // Run re-calculation process update, err = shipmentop.ProcessRecalculateShipment{ DB: h.DB(), - Logger: h.Logger(), + Logger: logger, }.Call(&shipment, shipmentLineItems, h.Planner()) if err != nil { - h.Logger().Error("Error re-pricing line items for shipment", zap.Error(err)) + logger.Error("Error re-pricing line items for shipment", zap.Error(err)) return update, accessorialop.NewGetShipmentLineItemsInternalServerError() } @@ -119,7 +119,7 @@ func (h GetShipmentLineItemsHandler) recalculateShipmentLineItems(shipmentLineIt // Handle returns a specified shipment line item func (h GetShipmentLineItemsHandler) Handle(params accessorialop.GetShipmentLineItemsParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) shipmentID := uuid.Must(uuid.FromString(params.ShipmentID.String())) @@ -127,8 +127,8 @@ func (h GetShipmentLineItemsHandler) Handle(params accessorialop.GetShipmentLine // Check that the TSP user can access the shipment _, _, err := models.FetchShipmentForVerifiedTSPUser(h.DB(), session.TspUserID, shipmentID) if err != nil { - h.Logger().Error("Error fetching shipment for TSP user", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("Error fetching shipment for TSP user", zap.Error(err)) + return handlers.ResponseForError(logger, err) } } else if !session.IsOfficeUser() { return accessorialop.NewGetShipmentLineItemsForbidden() @@ -136,18 +136,18 @@ func (h GetShipmentLineItemsHandler) Handle(params accessorialop.GetShipmentLine shipmentLineItems, err := models.FetchLineItemsByShipmentID(h.DB(), &shipmentID) if err != nil { - h.Logger().Error("Error fetching line items for shipment", zap.Error(err)) + logger.Error("Error fetching line items for shipment", zap.Error(err)) return accessorialop.NewGetShipmentLineItemsInternalServerError() } - update, recalculateError := h.recalculateShipmentLineItems(shipmentLineItems, shipmentID, session) + update, recalculateError := h.recalculateShipmentLineItems(shipmentLineItems, shipmentID, session, logger) if recalculateError != nil { return recalculateError } if update { shipmentLineItems, err = models.FetchLineItemsByShipmentID(h.DB(), &shipmentID) if err != nil { - h.Logger().Error("Error fetching line items for shipment after re-calculation", + logger.Error("Error fetching line items for shipment after re-calculation", zap.Error(err)) return accessorialop.NewGetShipmentLineItemsInternalServerError() } @@ -164,7 +164,7 @@ type CreateShipmentLineItemHandler struct { // Handle handles the request func (h CreateShipmentLineItemHandler) Handle(params accessorialop.CreateShipmentLineItemParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) shipmentID := uuid.Must(uuid.FromString(params.ShipmentID.String())) var shipment *models.Shipment @@ -176,14 +176,14 @@ func (h CreateShipmentLineItemHandler) Handle(params accessorialop.CreateShipmen // Check that the TSP user can access the shipment _, shipment, err = models.FetchShipmentForVerifiedTSPUser(h.DB(), session.TspUserID, shipmentID) if err != nil { - h.Logger().Error("Error fetching shipment for TSP user", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("Error fetching shipment for TSP user", zap.Error(err)) + return handlers.ResponseForError(logger, err) } } else if session.IsOfficeUser() { shipment, err = models.FetchShipment(h.DB(), session, shipmentID) if err != nil { - h.Logger().Error("Error fetching shipment for office user", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("Error fetching shipment for office user", zap.Error(err)) + return handlers.ResponseForError(logger, err) } } else { return accessorialop.NewCreateShipmentLineItemForbidden() @@ -192,7 +192,7 @@ func (h CreateShipmentLineItemHandler) Handle(params accessorialop.CreateShipmen tariff400ngItemID := uuid.Must(uuid.FromString(params.Payload.Tariff400ngItemID.String())) tariff400ngItem, err := models.FetchTariff400ngItem(h.DB(), tariff400ngItemID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } if !tariff400ngItem.RequiresPreApproval { @@ -242,8 +242,8 @@ func (h CreateShipmentLineItemHandler) Handle(params accessorialop.CreateShipmen ) if verrs.HasAny() || err != nil { - h.Logger().Error("Error fetching shipment line items for shipment", zap.Error(err)) - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + logger.Error("Error fetching shipment line items for shipment", zap.Error(err)) + return handlers.ResponseForVErrors(logger, verrs, err) } payload := payloadForShipmentLineItemModel(shipmentLineItem) return accessorialop.NewCreateShipmentLineItemCreated().WithPayload(payload) @@ -256,14 +256,14 @@ type UpdateShipmentLineItemHandler struct { // Handle updates a specified shipment line item func (h UpdateShipmentLineItemHandler) Handle(params accessorialop.UpdateShipmentLineItemParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) shipmentLineItemID := uuid.Must(uuid.FromString(params.ShipmentLineItemID.String())) var shipment *models.Shipment // Fetch shipment line item shipmentLineItem, err := models.FetchShipmentLineItemByID(h.DB(), &shipmentLineItemID) if err != nil { - h.Logger().Error("Error fetching shipment line item for shipment", zap.Error(err)) + logger.Error("Error fetching shipment line item for shipment", zap.Error(err)) return accessorialop.NewUpdateShipmentLineItemInternalServerError() } @@ -272,14 +272,14 @@ func (h UpdateShipmentLineItemHandler) Handle(params accessorialop.UpdateShipmen // Check that the TSP user can access the shipment _, shipment, err = models.FetchShipmentForVerifiedTSPUser(h.DB(), session.TspUserID, shipmentLineItem.ShipmentID) if err != nil { - h.Logger().Error("Error fetching shipment for TSP user", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("Error fetching shipment for TSP user", zap.Error(err)) + return handlers.ResponseForError(logger, err) } } else if session.IsOfficeUser() { shipment, err = models.FetchShipment(h.DB(), session, shipmentLineItem.ShipmentID) if err != nil { - h.Logger().Error("Error fetching shipment for office user", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("Error fetching shipment for office user", zap.Error(err)) + return handlers.ResponseForError(logger, err) } } else { return accessorialop.NewUpdateShipmentLineItemForbidden() @@ -292,10 +292,10 @@ func (h UpdateShipmentLineItemHandler) Handle(params accessorialop.UpdateShipmen canUpdate35A := tariff400ngItem.Code == "35A" && shipmentLineItem.EstimateAmountCents != nil && shipmentLineItem.InvoiceID == nil if !tariff400ngItem.RequiresPreApproval { - h.Logger().Error("Error: tariff400ng item " + tariff400ngItem.Code + " does not require pre-approval") + logger.Error("Error: tariff400ng item " + tariff400ngItem.Code + " does not require pre-approval") return accessorialop.NewUpdateShipmentLineItemForbidden() } else if shipmentLineItem.Status == models.ShipmentLineItemStatusAPPROVED && !canUpdate35A { - h.Logger().Error("Error: cannot update shipment line item if status is approved (or status is invoiced for tariff400ng item 35A)") + logger.Error("Error: cannot update shipment line item if status is approved (or status is invoiced for tariff400ng item 35A)") return accessorialop.NewUpdateShipmentLineItemUnprocessableEntity() } @@ -342,17 +342,17 @@ func (h UpdateShipmentLineItemHandler) Handle(params accessorialop.UpdateShipmen &shipmentLineItem, ) if verrs.HasAny() || err != nil { - h.Logger().Error("Error fetching shipment line items for shipment", zap.Error(err)) - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + logger.Error("Error fetching shipment line items for shipment", zap.Error(err)) + return handlers.ResponseForVErrors(logger, verrs, err) } if (shipmentLineItem.Status == models.ShipmentLineItemStatusCONDITIONALLYAPPROVED || shipmentLineItem.Status == models.ShipmentLineItemStatusAPPROVED) && shipmentLineItem.ActualAmountCents != nil { // If shipment is delivered, price single shipment line item if shipmentLineItem.Shipment.Status == models.ShipmentStatusDELIVERED { - engine := rateengine.NewRateEngine(h.DB(), h.Logger()) + engine := rateengine.NewRateEngine(h.DB(), logger) err = engine.PricePreapprovalRequest(&shipmentLineItem) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } } @@ -360,7 +360,7 @@ func (h UpdateShipmentLineItemHandler) Handle(params accessorialop.UpdateShipmen if shipmentLineItem.Status == models.ShipmentLineItemStatusCONDITIONALLYAPPROVED { err = shipmentLineItem.Approve() if err != nil { - h.Logger().Error("Error approving shipment line item for shipment", zap.Error(err)) + logger.Error("Error approving shipment line item for shipment", zap.Error(err)) return accessorialop.NewApproveShipmentLineItemForbidden() } } @@ -377,7 +377,7 @@ func (h UpdateShipmentLineItemHandler) Handle(params accessorialop.UpdateShipmen // Conditionally approve the shipment line item err = shipmentLineItem.ConditionallyApprove() if err != nil { - h.Logger().Error("Error conditionally approving shipment line item for shipment", zap.Error(err)) + logger.Error("Error conditionally approving shipment line item for shipment", zap.Error(err)) return accessorialop.NewApproveShipmentLineItemForbidden() } } @@ -396,18 +396,18 @@ type DeleteShipmentLineItemHandler struct { // Handle deletes a specified shipment line item func (h DeleteShipmentLineItemHandler) Handle(params accessorialop.DeleteShipmentLineItemParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) // Fetch shipment line item first shipmentLineItemID := uuid.Must(uuid.FromString(params.ShipmentLineItemID.String())) shipmentLineItem, err := models.FetchShipmentLineItemByID(h.DB(), &shipmentLineItemID) if err != nil { if errors.Cause(err) == sql.ErrNoRows { - h.Logger().Error("Error shipment line item for shipment not found", zap.Error(err)) + logger.Error("Error shipment line item for shipment not found", zap.Error(err)) return accessorialop.NewDeleteShipmentLineItemNotFound() } - h.Logger().Error("Error fetching shipment line item for shipment", zap.Error(err)) + logger.Error("Error fetching shipment line item for shipment", zap.Error(err)) return accessorialop.NewDeleteShipmentLineItemInternalServerError() } @@ -421,14 +421,14 @@ func (h DeleteShipmentLineItemHandler) Handle(params accessorialop.DeleteShipmen // Check that the TSP user can access the shipment _, _, fetchShipmentForVerifiedTSPUserErr := models.FetchShipmentForVerifiedTSPUser(h.DB(), session.TspUserID, shipmentID) if fetchShipmentForVerifiedTSPUserErr != nil { - h.Logger().Error("Error fetching shipment for TSP user", zap.Error(fetchShipmentForVerifiedTSPUserErr)) - return handlers.ResponseForError(h.Logger(), fetchShipmentForVerifiedTSPUserErr) + logger.Error("Error fetching shipment for TSP user", zap.Error(fetchShipmentForVerifiedTSPUserErr)) + return handlers.ResponseForError(logger, fetchShipmentForVerifiedTSPUserErr) } } else if session.IsOfficeUser() { _, fetchShipmentErr := models.FetchShipment(h.DB(), session, shipmentID) if fetchShipmentErr != nil { - h.Logger().Error("Error fetching shipment for office user", zap.Error(fetchShipmentErr)) - return handlers.ResponseForError(h.Logger(), fetchShipmentErr) + logger.Error("Error fetching shipment for office user", zap.Error(fetchShipmentErr)) + return handlers.ResponseForError(logger, fetchShipmentErr) } } else { return accessorialop.NewDeleteShipmentLineItemForbidden() @@ -437,8 +437,8 @@ func (h DeleteShipmentLineItemHandler) Handle(params accessorialop.DeleteShipmen // Delete the shipment line item err = h.DB().Destroy(&shipmentLineItem) if err != nil { - h.Logger().Error("Error deleting shipment line item for shipment", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("Error deleting shipment line item for shipment", zap.Error(err)) + return handlers.ResponseForError(logger, err) } payload := payloadForShipmentLineItemModel(&shipmentLineItem) @@ -452,14 +452,14 @@ type ApproveShipmentLineItemHandler struct { // Handle returns a specified shipment func (h ApproveShipmentLineItemHandler) Handle(params accessorialop.ApproveShipmentLineItemParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) var shipment *models.Shipment shipmentLineItemID := uuid.Must(uuid.FromString(params.ShipmentLineItemID.String())) shipmentLineItem, err := models.FetchShipmentLineItemByID(h.DB(), &shipmentLineItemID) if err != nil { - h.Logger().Error("Error fetching line items for shipment", zap.Error(err)) + logger.Error("Error fetching line items for shipment", zap.Error(err)) return accessorialop.NewApproveShipmentLineItemInternalServerError() } @@ -468,11 +468,11 @@ func (h ApproveShipmentLineItemHandler) Handle(params accessorialop.ApproveShipm if shipmentLineItem.Tariff400ngItem.RequiresPreApproval && session.IsOfficeUser() { shipment, err = models.FetchShipment(h.DB(), session, shipmentLineItem.ShipmentID) if err != nil { - h.Logger().Error("Error fetching shipment for office user", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("Error fetching shipment for office user", zap.Error(err)) + return handlers.ResponseForError(logger, err) } } else { - h.Logger().Error("Error does not require pre-approval for shipment") + logger.Error("Error does not require pre-approval for shipment") return accessorialop.NewApproveShipmentLineItemForbidden() } shipmentLineItem.Shipment = *shipment @@ -481,24 +481,24 @@ func (h ApproveShipmentLineItemHandler) Handle(params accessorialop.ApproveShipm // Conditionally approve the shipment line item err = shipmentLineItem.ConditionallyApprove() if err != nil { - h.Logger().Error("Error conditionally approving shipment line item for shipment", zap.Error(err)) + logger.Error("Error conditionally approving shipment line item for shipment", zap.Error(err)) return accessorialop.NewApproveShipmentLineItemForbidden() } } else { // Approve the shipment line item err = shipmentLineItem.Approve() if err != nil { - h.Logger().Error("Error approving shipment line item for shipment", zap.Error(err)) + logger.Error("Error approving shipment line item for shipment", zap.Error(err)) return accessorialop.NewApproveShipmentLineItemForbidden() } } // If shipment is delivered and line item is approved, price single shipment line item if shipmentLineItem.Shipment.Status == models.ShipmentStatusDELIVERED && shipmentLineItem.Status == models.ShipmentLineItemStatusAPPROVED { - engine := rateengine.NewRateEngine(h.DB(), h.Logger()) + engine := rateengine.NewRateEngine(h.DB(), logger) err = engine.PricePreapprovalRequest(&shipmentLineItem) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } } diff --git a/pkg/handlers/publicapi/shipments.go b/pkg/handlers/publicapi/shipments.go index 44ec0c9430e2..26f6693afbc1 100644 --- a/pkg/handlers/publicapi/shipments.go +++ b/pkg/handlers/publicapi/shipments.go @@ -15,7 +15,6 @@ import ( "github.com/gofrs/uuid" "go.uber.org/zap" - "github.com/transcom/mymove/pkg/auth" "github.com/transcom/mymove/pkg/gen/apimessages" shipmentop "github.com/transcom/mymove/pkg/gen/restapi/apioperations/shipments" "github.com/transcom/mymove/pkg/handlers" @@ -118,21 +117,21 @@ type IndexShipmentsHandler struct { // Handle retrieves a list of all shipments func (h IndexShipmentsHandler) Handle(params shipmentop.IndexShipmentsParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) // TODO: (cgilmer 2018_07_25) This is an extra query we don't need to run on every request. Put the // TransportationServiceProviderID into the session object after refactoring the session code to be more readable. // See original commits in https://github.com/transcom/mymove/pull/802 tspUser, err := models.FetchTspUserByID(h.DB(), session.TspUserID) if err != nil { - h.Logger().Error("DB Query", zap.Error(err)) + logger.Error("DB Query", zap.Error(err)) return shipmentop.NewIndexShipmentsForbidden() } shipments, err := models.FetchShipmentsByTSP(h.DB(), tspUser.TransportationServiceProviderID, params.Status, params.OrderBy) if err != nil { - h.Logger().Error("DB Query", zap.Error(err)) + logger.Error("DB Query", zap.Error(err)) return shipmentop.NewIndexShipmentsBadRequest() } @@ -153,30 +152,30 @@ func (h GetShipmentHandler) Handle(params shipmentop.GetShipmentParams) middlewa var shipment *models.Shipment var err error shipmentID, _ := uuid.FromString(params.ShipmentID.String()) - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) if session.IsTspUser() { // Check that the TSP user can access the shipment tspUser, fetchTspByIDErr := models.FetchTspUserByID(h.DB(), session.TspUserID) if fetchTspByIDErr != nil { - h.Logger().Error("Error retrieving authenticated TSP user", zap.Error(fetchTspByIDErr)) + logger.Error("Error retrieving authenticated TSP user", zap.Error(fetchTspByIDErr)) return shipmentop.NewGetShipmentForbidden() } shipment, err = models.FetchShipmentByTSP(h.DB(), tspUser.TransportationServiceProviderID, shipmentID) if err != nil { - h.Logger().Error("Error fetching shipment for TSP user", zap.Error(err)) + logger.Error("Error fetching shipment for TSP user", zap.Error(err)) return shipmentop.NewGetShipmentForbidden() } } else if session.IsOfficeUser() { shipment, err = models.FetchShipment(h.DB(), session, shipmentID) if err != nil { - h.Logger().Error("Error fetching shipment for office user", zap.Error(err)) + logger.Error("Error fetching shipment for office user", zap.Error(err)) return shipmentop.NewGetShipmentForbidden() } } else if session.IsServiceMember() { shipment, err = models.FetchShipment(h.DB(), session, shipmentID) if err != nil { - h.Logger().Error("Error fetching shipment for service member", zap.Error(err)) + logger.Error("Error fetching shipment for service member", zap.Error(err)) if err == models.ErrFetchForbidden { return shipmentop.NewGetShipmentForbidden() } @@ -197,7 +196,7 @@ type GetShipmentInvoicesHandler struct { // Handle accepts the shipment ID - returns list of associated invoices func (h GetShipmentInvoicesHandler) Handle(params shipmentop.GetShipmentInvoicesParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) shipmentID, _ := uuid.FromString(params.ShipmentID.String()) @@ -207,21 +206,21 @@ func (h GetShipmentInvoicesHandler) Handle(params shipmentop.GetShipmentInvoices // See original commits in https://github.com/transcom/mymove/pull/802 tspUser, err := models.FetchTspUserByID(h.DB(), session.TspUserID) if err != nil { - h.Logger().Error("DB Query", zap.Error(err)) + logger.Error("DB Query", zap.Error(err)) return shipmentop.NewGetShipmentInvoicesForbidden() } // Make sure TSP has access to this shipment _, err = models.FetchShipmentByTSP(h.DB(), tspUser.TransportationServiceProviderID, shipmentID) if err != nil { - h.Logger().Error("DB Query", zap.Error(err)) + logger.Error("DB Query", zap.Error(err)) return shipmentop.NewGetShipmentInvoicesForbidden() } } invoices, err := models.FetchInvoicesForShipment(h.DB(), shipmentID) if err != nil { - h.Logger().Error("DB Query", zap.Error(err)) + logger.Error("DB Query", zap.Error(err)) return shipmentop.NewGetShipmentInvoicesBadRequest() } @@ -236,7 +235,7 @@ type AcceptShipmentHandler struct { // Handle accepts the shipment - checks that currently logged in user is authorized to act for the TSP assigned the shipment func (h AcceptShipmentHandler) Handle(params shipmentop.AcceptShipmentParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) shipmentID, _ := uuid.FromString(params.ShipmentID.String()) @@ -245,7 +244,7 @@ func (h AcceptShipmentHandler) Handle(params shipmentop.AcceptShipmentParams) mi // See original commits in https://github.com/transcom/mymove/pull/802 tspUser, err := models.FetchTspUserByID(h.DB(), session.TspUserID) if err != nil { - h.Logger().Error("DB Query", zap.Error(err)) + logger.Error("DB Query", zap.Error(err)) return shipmentop.NewAcceptShipmentForbidden() } @@ -253,15 +252,15 @@ func (h AcceptShipmentHandler) Handle(params shipmentop.AcceptShipmentParams) mi shipment, shipmentOffer, verrs, err := models.AcceptShipmentForTSP(h.DB(), tspUser.TransportationServiceProviderID, shipmentID) if err != nil || verrs.HasAny() { if err == models.ErrFetchNotFound { - h.Logger().Error("DB Query", zap.Error(err)) + logger.Error("DB Query", zap.Error(err)) return shipmentop.NewAcceptShipmentBadRequest() } else if err == models.ErrInvalidTransition { - h.Logger().Info("Attempted to accept shipment, got invalid transition", zap.Error(err), zap.String("shipment_status", string(shipment.Status))) - h.Logger().Info("Attempted to accept shipment offer, got invalid transition", zap.Error(err), zap.Bool("shipment_offer_accepted", *shipmentOffer.Accepted)) + logger.Info("Attempted to accept shipment, got invalid transition", zap.Error(err), zap.String("shipment_status", string(shipment.Status))) + logger.Info("Attempted to accept shipment offer, got invalid transition", zap.Error(err), zap.Bool("shipment_offer_accepted", *shipmentOffer.Accepted)) return shipmentop.NewAcceptShipmentConflict() } else { - h.Logger().Error("Unknown Error", zap.Error(err)) - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + logger.Error("Unknown Error", zap.Error(err)) + return handlers.ResponseForVErrors(logger, verrs, err) } } @@ -276,7 +275,7 @@ type TransportShipmentHandler struct { // Handle updates the shipment with pack and pickup dates and weights and puts it in-transit - checks that currently logged in user is authorized to act for the TSP assigned the shipment func (h TransportShipmentHandler) Handle(params shipmentop.TransportShipmentParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) shipmentID, _ := uuid.FromString(params.ShipmentID.String()) @@ -285,13 +284,13 @@ func (h TransportShipmentHandler) Handle(params shipmentop.TransportShipmentPara // See original commits in https://github.com/transcom/mymove/pull/802 tspUser, err := models.FetchTspUserByID(h.DB(), session.TspUserID) if err != nil { - h.Logger().Error("DB Query", zap.Error(err)) + logger.Error("DB Query", zap.Error(err)) return shipmentop.NewTransportShipmentForbidden() } shipment, err := models.FetchShipmentByTSP(h.DB(), tspUser.TransportationServiceProviderID, shipmentID) if err != nil { - h.Logger().Error("DB Query", zap.Error(err)) + logger.Error("DB Query", zap.Error(err)) return shipmentop.NewTransportShipmentBadRequest() } @@ -299,14 +298,14 @@ func (h TransportShipmentHandler) Handle(params shipmentop.TransportShipmentPara err = shipment.Pack(actualPackDate) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } actualPickupDate := (time.Time)(*params.Payload.ActualPickupDate) err = shipment.Transport(actualPickupDate) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } shipment.NetWeight = handlers.PoundPtrFromInt64Ptr(params.Payload.NetWeight) @@ -321,7 +320,7 @@ func (h TransportShipmentHandler) Handle(params shipmentop.TransportShipmentPara verrs, err := h.DB().ValidateAndUpdate(shipment) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } sp := payloadForShipmentModel(*shipment) return shipmentop.NewTransportShipmentOK().WithPayload(sp) @@ -334,7 +333,7 @@ type DeliverShipmentHandler struct { // Handle delivers the shipment - checks that currently logged in user is authorized to act for the TSP assigned the shipment func (h DeliverShipmentHandler) Handle(params shipmentop.DeliverShipmentParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) shipmentID, _ := uuid.FromString(params.ShipmentID.String()) @@ -343,18 +342,18 @@ func (h DeliverShipmentHandler) Handle(params shipmentop.DeliverShipmentParams) // See original commits in https://github.com/transcom/mymove/pull/802 tspUser, err := models.FetchTspUserByID(h.DB(), session.TspUserID) if err != nil { - h.Logger().Error("DB Query", zap.Error(err)) + logger.Error("DB Query", zap.Error(err)) return shipmentop.NewDeliverShipmentForbidden() } shipment, err := models.FetchShipmentByTSP(h.DB(), tspUser.TransportationServiceProviderID, shipmentID) if err != nil { - h.Logger().Error("DB Query", zap.Error(err)) + logger.Error("DB Query", zap.Error(err)) return shipmentop.NewDeliverShipmentBadRequest() } actualDeliveryDate := (time.Time)(*params.Payload.ActualDeliveryDate) - engine := rateengine.NewRateEngine(h.DB(), h.Logger()) + engine := rateengine.NewRateEngine(h.DB(), logger) verrs, err := shipmentservice.DeliverAndPriceShipment{ DB: h.DB(), @@ -363,7 +362,7 @@ func (h DeliverShipmentHandler) Handle(params shipmentop.DeliverShipmentParams) }.Call(actualDeliveryDate, shipment) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } sp := payloadForShipmentModel(*shipment) @@ -377,7 +376,7 @@ type CompletePmSurveyHandler struct { // Handle completes a pre-moves survey - checks that currently logged in user is authorized to act for the TSP assigned the shipment func (h CompletePmSurveyHandler) Handle(params shipmentop.CompletePmSurveyParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) shipmentID, _ := uuid.FromString(params.ShipmentID.String()) @@ -386,13 +385,13 @@ func (h CompletePmSurveyHandler) Handle(params shipmentop.CompletePmSurveyParams // See original commits in https://github.com/transcom/mymove/pull/802 tspUser, err := models.FetchTspUserByID(h.DB(), session.TspUserID) if err != nil { - h.Logger().Error("DB Query", zap.Error(err)) + logger.Error("DB Query", zap.Error(err)) return shipmentop.NewCompletePmSurveyForbidden() } shipment, err := models.FetchShipmentByTSP(h.DB(), tspUser.TransportationServiceProviderID, shipmentID) if err != nil { - h.Logger().Error("DB Query", zap.Error(err)) + logger.Error("DB Query", zap.Error(err)) return shipmentop.NewCompletePmSurveyBadRequest() } @@ -402,7 +401,7 @@ func (h CompletePmSurveyHandler) Handle(params shipmentop.CompletePmSurveyParams verrs, err := models.SaveShipment(h.DB(), shipment) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } sp := payloadForShipmentModel(*shipment) @@ -566,25 +565,25 @@ func (h PatchShipmentHandler) Handle(params shipmentop.PatchShipmentParams) midd var shipment *models.Shipment var err error shipmentID, _ := uuid.FromString(params.ShipmentID.String()) - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) // authorization if session.IsTspUser() { // Check that the TSP user can access the shipment tspUser, fetchTspByIDErr := models.FetchTspUserByID(h.DB(), session.TspUserID) if fetchTspByIDErr != nil { - h.Logger().Error("Error retrieving authenticated TSP user", zap.Error(fetchTspByIDErr)) + logger.Error("Error retrieving authenticated TSP user", zap.Error(fetchTspByIDErr)) return shipmentop.NewGetShipmentForbidden() } shipment, err = models.FetchShipmentByTSP(h.DB(), tspUser.TransportationServiceProviderID, shipmentID) if err != nil { - h.Logger().Error("Error fetching shipment for TSP user", zap.Error(err)) + logger.Error("Error fetching shipment for TSP user", zap.Error(err)) return shipmentop.NewPatchShipmentBadRequest() } } else if session.IsOfficeUser() { shipment, err = models.FetchShipment(h.DB(), session, shipmentID) if err != nil { - h.Logger().Error("Error fetching shipment for office user", zap.Error(err)) + logger.Error("Error fetching shipment for office user", zap.Error(err)) return shipmentop.NewPatchShipmentBadRequest() } } else { @@ -595,7 +594,7 @@ func (h PatchShipmentHandler) Handle(params shipmentop.PatchShipmentParams) midd verrs, err := models.SaveShipmentAndAddresses(h.DB(), shipment) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } shipmentPayload := payloadForShipmentModel(*shipment) @@ -610,61 +609,61 @@ type CreateGovBillOfLadingHandler struct { // Handle generates the GBL PDF & uploads it as a document associated to a move doc, shipment and move func (h CreateGovBillOfLadingHandler) Handle(params shipmentop.CreateGovBillOfLadingParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) // Verify that the TSP user is authorized to update move doc shipmentID, _ := uuid.FromString(params.ShipmentID.String()) tspUser, shipment, err := models.FetchShipmentForVerifiedTSPUser(h.DB(), session.TspUserID, shipmentID) if err != nil { if err.Error() == "USER_UNAUTHORIZED" { - h.Logger().Error("DB Query", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("DB Query", zap.Error(err)) + return handlers.ResponseForError(logger, err) } if err.Error() == "FETCH_FORBIDDEN" { - h.Logger().Error("DB Query", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("DB Query", zap.Error(err)) + return handlers.ResponseForError(logger, err) } - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } // Don't allow GBL generation for shipments that already have a GBL move document extantGBLS, _ := models.FetchMoveDocumentsByTypeForShipment(h.DB(), session, models.MoveDocumentTypeGOVBILLOFLADING, shipmentID) if len(extantGBLS) > 0 { - return handlers.ResponseForCustomErrors(h.Logger(), fmt.Errorf("there is already a Bill of Lading for this shipment"), http.StatusBadRequest) + return handlers.ResponseForCustomErrors(logger, fmt.Errorf("there is already a Bill of Lading for this shipment"), http.StatusBadRequest) } // Don't allow GBL generation for incomplete orders orders, ordersErr := models.FetchOrder(h.DB(), shipment.Move.OrdersID) if ordersErr != nil { - return handlers.ResponseForError(h.Logger(), ordersErr) + return handlers.ResponseForError(logger, ordersErr) } if orders.IsCompleteForGBL() != true { - return handlers.ResponseForCustomErrors(h.Logger(), fmt.Errorf("the move is missing some information from the JPPSO. Please contact the JPPSO"), http.StatusExpectationFailed) + return handlers.ResponseForCustomErrors(logger, fmt.Errorf("the move is missing some information from the JPPSO. Please contact the JPPSO"), http.StatusExpectationFailed) } // Create PDF for GBL gbl, err := models.FetchGovBillOfLadingFormValues(h.DB(), shipmentID) if err != nil { // TODO: (andrea) Pass info of exactly what is missing in custom error message - h.Logger().Error("Failed retrieving the GBL data.", zap.Error(err)) + logger.Error("Failed retrieving the GBL data.", zap.Error(err)) return shipmentop.NewCreateGovBillOfLadingExpectationFailed() } formLayout := paperwork.Form1203Layout template, err := paperworkservice.MakeFormTemplate(gbl, gbl.GBLNumber1, formLayout, services.GBL) if err != nil { - h.Logger().Error(errors.Cause(err).Error(), zap.Error(errors.Cause(err))) + logger.Error(errors.Cause(err).Error(), zap.Error(errors.Cause(err))) } gblFile, err := h.pdfFormCreator.CreateForm(template) if err != nil { - h.Logger().Error(errors.Cause(err).Error(), zap.Error(errors.Cause(err))) + logger.Error(errors.Cause(err).Error(), zap.Error(errors.Cause(err))) return shipmentop.NewCreateGovBillOfLadingInternalServerError() } - uploader := uploaderpkg.NewUploader(h.DB(), h.Logger(), h.FileStorer()) + uploader := uploaderpkg.NewUploader(h.DB(), logger, h.FileStorer()) upload, verrs, err := uploader.CreateUpload(*tspUser.UserID, &gblFile, uploaderpkg.AllowedTypesPDF) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } uploads := []models.Upload{*upload} @@ -679,12 +678,12 @@ func (h CreateGovBillOfLadingHandler) Handle(params shipmentop.CreateGovBillOfLa models.SelectedMoveType(apimessages.SelectedMoveTypeHHG), ) if err != nil || verrs.HasAny() { - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + return handlers.ResponseForVErrors(logger, verrs, err) } documentPayload, err := payloadForDocumentModel(h.FileStorer(), doc.Document) if err != nil { - h.Logger().Error("Error fetching document for gbl doc", zap.Error(err)) + logger.Error("Error fetching document for gbl doc", zap.Error(err)) return shipmentop.NewCreateGovBillOfLadingInternalServerError() } diff --git a/pkg/handlers/publicapi/shipments_test.go b/pkg/handlers/publicapi/shipments_test.go index e39f3f4b6554..8765e224a939 100644 --- a/pkg/handlers/publicapi/shipments_test.go +++ b/pkg/handlers/publicapi/shipments_test.go @@ -14,8 +14,6 @@ import ( "github.com/go-openapi/strfmt" "github.com/go-openapi/swag" - "github.com/transcom/mymove/pkg/auth" - "github.com/transcom/mymove/pkg/gen/apimessages" shipmentop "github.com/transcom/mymove/pkg/gen/restapi/apioperations/shipments" "github.com/transcom/mymove/pkg/handlers" @@ -161,11 +159,11 @@ func (suite *HandlerSuite) TestGetShipmentHandlerWhereSessionServiceMemberIDDoes ShipmentID: strfmt.UUID(shipment.ID.String()), } - session := auth.SessionFromRequestContext(params.HTTPRequest) - handler := GetShipmentHandler{handlers.NewHandlerContext(suite.DB(), suite.TestLogger())} response := handler.Handle(params) + session := handler.SessionFromRequest(params.HTTPRequest) + suite.NotEqual(session.ServiceMemberID, shipment.ServiceMemberID) suite.Assertions.IsType(&shipmentop.GetShipmentForbidden{}, response) } diff --git a/pkg/handlers/publicapi/storage_in_transits.go b/pkg/handlers/publicapi/storage_in_transits.go index 5aaaefa43cb8..50abb5f504ad 100644 --- a/pkg/handlers/publicapi/storage_in_transits.go +++ b/pkg/handlers/publicapi/storage_in_transits.go @@ -9,7 +9,6 @@ import ( "github.com/gofrs/uuid" "go.uber.org/zap" - "github.com/transcom/mymove/pkg/auth" "github.com/transcom/mymove/pkg/gen/apimessages" sitop "github.com/transcom/mymove/pkg/gen/restapi/apioperations/storage_in_transits" "github.com/transcom/mymove/pkg/handlers" @@ -54,18 +53,18 @@ type IndexStorageInTransitHandler struct { // This is meant to return a list of storage in transits using their shipment ID. func (h IndexStorageInTransitHandler) Handle(params sitop.IndexStorageInTransitsParams) middleware.Responder { shipmentID, err := uuid.FromString(params.ShipmentID.String()) - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) if err != nil { - h.Logger().Error("UUID Parsing", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("UUID Parsing", zap.Error(err)) + return handlers.ResponseForError(logger, err) } storageInTransits, err := h.storageInTransitIndexer.IndexStorageInTransits(shipmentID, session) if err != nil { - h.Logger().Error(fmt.Sprintf("SITs Retrieval failed for shipment: %s", shipmentID), zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error(fmt.Sprintf("SITs Retrieval failed for shipment: %s", shipmentID), zap.Error(err)) + return handlers.ResponseForError(logger, err) } storageInTransitsList := make(apimessages.StorageInTransits, len(storageInTransits)) @@ -88,13 +87,13 @@ type CreateStorageInTransitHandler struct { func (h CreateStorageInTransitHandler) Handle(params sitop.CreateStorageInTransitParams) middleware.Responder { payload := params.StorageInTransit shipmentID, err := uuid.FromString(params.ShipmentID.String()) - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) newStorageInTransit, verrs, err := h.storageInTransitCreator.CreateStorageInTransit(*payload, shipmentID, session) if verrs.HasAny() || err != nil { - h.Logger().Error(fmt.Sprintf("SIT Creation failed for shipment: %s", shipmentID), zap.Error(err), zap.Error(verrs)) - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + logger.Error(fmt.Sprintf("SIT Creation failed for shipment: %s", shipmentID), zap.Error(err), zap.Error(verrs)) + return handlers.ResponseForVErrors(logger, verrs, err) } storageInTransitPayload := payloadForStorageInTransitModel(newStorageInTransit) @@ -112,22 +111,23 @@ type ApproveStorageInTransitHandler struct { // This is meant to set the status for a storage in transit to approved, save the authorization notes that // support that status, save the authorization date, and return the saved object in a payload. func (h ApproveStorageInTransitHandler) Handle(params sitop.ApproveStorageInTransitParams) middleware.Responder { + + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) + payload := params.StorageInTransitApprovalPayload shipmentID, err := uuid.FromString(params.ShipmentID.String()) storageInTransitID, err := uuid.FromString(params.StorageInTransitID.String()) if err != nil { - h.Logger().Error("UUID Parsing", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("UUID Parsing", zap.Error(err)) + return handlers.ResponseForError(logger, err) } - session := auth.SessionFromRequestContext(params.HTTPRequest) - storageInTransit, verrs, err := h.storageInTransitApprover.ApproveStorageInTransit(*payload, shipmentID, session, storageInTransitID) if err != nil || verrs.HasAny() { - h.Logger().Error(fmt.Sprintf("SIT approval failed for ID: %s on shipment: %s", storageInTransitID, shipmentID), zap.Error(err), zap.Error(verrs)) - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + logger.Error(fmt.Sprintf("SIT approval failed for ID: %s on shipment: %s", storageInTransitID, shipmentID), zap.Error(err), zap.Error(verrs)) + return handlers.ResponseForVErrors(logger, verrs, err) } returnPayload := payloadForStorageInTransitModel(storageInTransit) @@ -145,22 +145,23 @@ type DenyStorageInTransitHandler struct { // This is meant to set the status for a storage in transit to denied, save the supporting authorization notes, // and return the saved object in a payload. func (h DenyStorageInTransitHandler) Handle(params sitop.DenyStorageInTransitParams) middleware.Responder { + + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) + payload := params.StorageInTransitApprovalPayload shipmentID, err := uuid.FromString(params.ShipmentID.String()) storageInTransitID, err := uuid.FromString(params.StorageInTransitID.String()) if err != nil { - h.Logger().Error("UUID Parsing", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("UUID Parsing", zap.Error(err)) + return handlers.ResponseForError(logger, err) } - session := auth.SessionFromRequestContext(params.HTTPRequest) - storageInTransit, verrs, err := h.storageInTransitDenier.DenyStorageInTransit(*payload, shipmentID, session, storageInTransitID) if err != nil || verrs.HasAny() { - h.Logger().Error(fmt.Sprintf("SIT denial failed for ID: %s on shipment: %s", storageInTransitID, shipmentID), zap.Error(err), zap.Error(verrs)) - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + logger.Error(fmt.Sprintf("SIT denial failed for ID: %s on shipment: %s", storageInTransitID, shipmentID), zap.Error(err), zap.Error(verrs)) + return handlers.ResponseForVErrors(logger, verrs, err) } returnPayload := payloadForStorageInTransitModel(storageInTransit) @@ -177,20 +178,20 @@ type InSitStorageInTransitHandler struct { // Handle handles the handling // This is meant to set the status for a storage in transit to 'in SIT' and return the saved object in a payload. func (h InSitStorageInTransitHandler) Handle(params sitop.InSitStorageInTransitParams) middleware.Responder { + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) shipmentID, err := uuid.FromString(params.ShipmentID.String()) storageInTransitID, err := uuid.FromString(params.StorageInTransitID.String()) if err != nil { - h.Logger().Error("UUID Parsing", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("UUID Parsing", zap.Error(err)) + return handlers.ResponseForError(logger, err) } inSitPayload := params.StorageInTransitInSitPayload - session := auth.SessionFromRequestContext(params.HTTPRequest) storageInTransit, verrs, err := h.storageInTransitInSITPlacer.PlaceIntoSITStorageInTransit(*inSitPayload, shipmentID, session, storageInTransitID) if err != nil || verrs.HasAny() { - h.Logger().Error(fmt.Sprintf("Place into SIT failed for ID: %s on shipment: %s", storageInTransitID, shipmentID), zap.Error(err), zap.Error(verrs)) - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + logger.Error(fmt.Sprintf("Place into SIT failed for ID: %s on shipment: %s", storageInTransitID, shipmentID), zap.Error(err), zap.Error(verrs)) + return handlers.ResponseForVErrors(logger, verrs, err) } returnPayload := payloadForStorageInTransitModel(storageInTransit) @@ -209,21 +210,20 @@ type DeliverStorageInTransitHandler struct { func (h DeliverStorageInTransitHandler) Handle(params sitop.DeliverStorageInTransitParams) middleware.Responder { // TODO: it looks like from the wireframes for the delivery status change form that this will also need to edit // delivery address(es) and the actual delivery date. + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) shipmentID, err := uuid.FromString(params.ShipmentID.String()) storageInTransitID, err := uuid.FromString(params.StorageInTransitID.String()) if err != nil { - h.Logger().Error("UUID Parsing", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("UUID Parsing", zap.Error(err)) + return handlers.ResponseForError(logger, err) } - session := auth.SessionFromRequestContext(params.HTTPRequest) - storageInTransit, verrs, err := h.deliverStorageInTransit.DeliverStorageInTransit(shipmentID, session, storageInTransitID) if err != nil || verrs.HasAny() { - h.Logger().Error(fmt.Sprintf("SIT delivery failed for ID: %s on shipment: %s", storageInTransitID, shipmentID), zap.Error(err), zap.Error(verrs)) - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + logger.Error(fmt.Sprintf("SIT delivery failed for ID: %s on shipment: %s", storageInTransitID, shipmentID), zap.Error(err), zap.Error(verrs)) + return handlers.ResponseForVErrors(logger, verrs, err) } returnPayload := payloadForStorageInTransitModel(storageInTransit) @@ -241,22 +241,21 @@ type ReleaseStorageInTransitHandler struct { // This is meant to set the status of a storage in transit to released, save the actual date that supports that, // and return the saved object in a payload. func (h ReleaseStorageInTransitHandler) Handle(params sitop.ReleaseStorageInTransitParams) middleware.Responder { + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) shipmentID, err := uuid.FromString(params.ShipmentID.String()) storageInTransitID, err := uuid.FromString(params.StorageInTransitID.String()) payload := params.StorageInTransitOnReleasePayload if err != nil { - h.Logger().Error("UUID Parsing", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("UUID Parsing", zap.Error(err)) + return handlers.ResponseForError(logger, err) } - session := auth.SessionFromRequestContext(params.HTTPRequest) - storageInTransit, verrs, err := h.releaseStorageInTransit.ReleaseStorageInTransit(*payload, shipmentID, session, storageInTransitID) if err != nil || verrs.HasAny() { - h.Logger().Error(fmt.Sprintf("Release SIT failed for ID: %s on shipment: %s", storageInTransitID, shipmentID), zap.Error(err), zap.Error(verrs)) - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + logger.Error(fmt.Sprintf("Release SIT failed for ID: %s on shipment: %s", storageInTransitID, shipmentID), zap.Error(err), zap.Error(verrs)) + return handlers.ResponseForVErrors(logger, verrs, err) } returnPayload := payloadForStorageInTransitModel(storageInTransit) @@ -274,21 +273,21 @@ type PatchStorageInTransitHandler struct { // This is meant to edit a storage in transit object based on provided parameters and return the saved object // in a payload func (h PatchStorageInTransitHandler) Handle(params sitop.PatchStorageInTransitParams) middleware.Responder { + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) payload := params.StorageInTransit shipmentID, err := uuid.FromString(params.ShipmentID.String()) storageInTransitID, err := uuid.FromString(params.StorageInTransitID.String()) if err != nil { - h.Logger().Error("UUID Parsing", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("UUID Parsing", zap.Error(err)) + return handlers.ResponseForError(logger, err) } - session := auth.SessionFromRequestContext(params.HTTPRequest) storageInTransit, verrs, err := h.patchStorageInTransit.PatchStorageInTransit(*payload, shipmentID, storageInTransitID, session) if err != nil || verrs.HasAny() { - h.Logger().Error(fmt.Sprintf("Patch SIT failed for ID: %s on shipment: %s", storageInTransitID, shipmentID), zap.Error(err), zap.Error(verrs)) - return handlers.ResponseForVErrors(h.Logger(), verrs, err) + logger.Error(fmt.Sprintf("Patch SIT failed for ID: %s on shipment: %s", storageInTransitID, shipmentID), zap.Error(err), zap.Error(verrs)) + return handlers.ResponseForVErrors(logger, verrs, err) } storageInTransitPayload := payloadForStorageInTransitModel(storageInTransit) @@ -305,20 +304,19 @@ type GetStorageInTransitHandler struct { // Handle handles the handling // This is meant to fetch a single storage in transit using its shipment and object IDs func (h GetStorageInTransitHandler) Handle(params sitop.GetStorageInTransitParams) middleware.Responder { + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) storageInTransitID, err := uuid.FromString(params.StorageInTransitID.String()) shipmentID, err := uuid.FromString(params.ShipmentID.String()) if err != nil { - h.Logger().Error("UUID Parsing", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("UUID Parsing", zap.Error(err)) + return handlers.ResponseForError(logger, err) } - session := auth.SessionFromRequestContext(params.HTTPRequest) - storageInTransit, err := h.storageInTransitFetcher.FetchStorageInTransitByID(storageInTransitID, shipmentID, session) if err != nil { - h.Logger().Error("DB Query", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("DB Query", zap.Error(err)) + return handlers.ResponseForError(logger, err) } storageInTransitPayload := payloadForStorageInTransitModel(storageInTransit) @@ -334,21 +332,20 @@ type DeleteStorageInTransitHandler struct { // Handle handles the handling // This is meant to delete a storage in transit object using its own shipment and object IDs func (h DeleteStorageInTransitHandler) Handle(params sitop.DeleteStorageInTransitParams) middleware.Responder { + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) storageInTransitID, err := uuid.FromString(params.StorageInTransitID.String()) shipmentID, err := uuid.FromString(params.ShipmentID.String()) if err != nil { - h.Logger().Error("UUID Parsing", zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error("UUID Parsing", zap.Error(err)) + return handlers.ResponseForError(logger, err) } - session := auth.SessionFromRequestContext(params.HTTPRequest) - err = h.deleteStorageInTransit.DeleteStorageInTransit(shipmentID, storageInTransitID, session) if err != nil { - h.Logger().Error(fmt.Sprintf("Deleting SIT failed for id: %s on shipment: %s", storageInTransitID, shipmentID), zap.Error(err)) - return handlers.ResponseForError(h.Logger(), err) + logger.Error(fmt.Sprintf("Deleting SIT failed for id: %s on shipment: %s", storageInTransitID, shipmentID), zap.Error(err)) + return handlers.ResponseForError(logger, err) } return sitop.NewDeleteStorageInTransitOK() diff --git a/pkg/handlers/publicapi/tariff400ng_items.go b/pkg/handlers/publicapi/tariff400ng_items.go index 74d591e4f74c..505b3cb63fa0 100644 --- a/pkg/handlers/publicapi/tariff400ng_items.go +++ b/pkg/handlers/publicapi/tariff400ng_items.go @@ -4,7 +4,6 @@ import ( "github.com/go-openapi/runtime/middleware" "go.uber.org/zap" - "github.com/transcom/mymove/pkg/auth" "github.com/transcom/mymove/pkg/gen/apimessages" accessorialop "github.com/transcom/mymove/pkg/gen/restapi/apioperations/accessorials" "github.com/transcom/mymove/pkg/handlers" @@ -48,7 +47,7 @@ type GetTariff400ngItemsHandler struct { // Handle returns a list of 400ng items func (h GetTariff400ngItemsHandler) Handle(params accessorialop.GetTariff400ngItemsParams) middleware.Responder { - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) if session == nil { return accessorialop.NewGetTariff400ngItemsUnauthorized() @@ -57,7 +56,7 @@ func (h GetTariff400ngItemsHandler) Handle(params accessorialop.GetTariff400ngIt // params.RequiresPreApproval has a default so we don't need to nil-check it items, err := models.FetchTariff400ngItems(h.DB(), *params.RequiresPreApproval) if err != nil { - h.Logger().Error("Error fetching 400ng items", zap.Error(err)) + logger.Error("Error fetching 400ng items", zap.Error(err)) return accessorialop.NewGetTariff400ngItemsInternalServerError() } payload := payloadForTariff400ngItemModels(items) diff --git a/pkg/handlers/publicapi/transportation_service_provider.go b/pkg/handlers/publicapi/transportation_service_provider.go index 8431112d1789..2d14a009761b 100644 --- a/pkg/handlers/publicapi/transportation_service_provider.go +++ b/pkg/handlers/publicapi/transportation_service_provider.go @@ -6,7 +6,6 @@ import ( "github.com/gofrs/uuid" "go.uber.org/zap" - "github.com/transcom/mymove/pkg/auth" "github.com/transcom/mymove/pkg/gen/apimessages" tspop "github.com/transcom/mymove/pkg/gen/restapi/apioperations/transportation_service_provider" "github.com/transcom/mymove/pkg/handlers" @@ -40,32 +39,32 @@ type GetTransportationServiceProviderHandler struct { func (h GetTransportationServiceProviderHandler) Handle(params tspop.GetTransportationServiceProviderParams) middleware.Responder { var shipment *models.Shipment var err error - session := auth.SessionFromRequestContext(params.HTTPRequest) + session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest) shipmentID, _ := uuid.FromString(params.ShipmentID.String()) if session.IsTspUser() { // TODO (2018_08_27 cgilmer): Find a way to check Shipment belongs to TSP without 2 queries tspUser, fetchTSPByUserErr := models.FetchTspUserByID(h.DB(), session.TspUserID) if fetchTSPByUserErr != nil { - h.Logger().Error("DB Query", zap.Error(fetchTSPByUserErr)) + logger.Error("DB Query", zap.Error(fetchTSPByUserErr)) // return tspop.NewGetTransporationServiceProviderForbidden() } shipment, err = models.FetchShipmentByTSP(h.DB(), tspUser.TransportationServiceProviderID, shipmentID) if err != nil { - handlers.ResponseForError(h.Logger(), err) + handlers.ResponseForError(logger, err) return tspop.NewGetTransportationServiceProviderBadRequest() } } else if session.IsOfficeUser() { shipment, err = models.FetchShipment(h.DB(), session, shipmentID) if err != nil { - handlers.ResponseForError(h.Logger(), err) + handlers.ResponseForError(logger, err) return tspop.NewGetTransportationServiceProviderBadRequest() } } else if session.IsServiceMember() { shipment, err = models.FetchShipment(h.DB(), session, shipmentID) if err != nil { - handlers.ResponseForError(h.Logger(), err) + handlers.ResponseForError(logger, err) if err == models.ErrFetchForbidden { return tspop.NewGetTransportationServiceProviderForbidden() } @@ -77,12 +76,12 @@ func (h GetTransportationServiceProviderHandler) Handle(params tspop.GetTranspor transportationServiceProviderID := shipment.CurrentTransportationServiceProviderID() if transportationServiceProviderID == uuid.Nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } transportationServiceProvider, err := models.FetchTransportationServiceProvider(h.DB(), transportationServiceProviderID) if err != nil { - return handlers.ResponseForError(h.Logger(), err) + return handlers.ResponseForError(logger, err) } transportationServiceProviderPayload := payloadForTransportationServiceProviderModel(*transportationServiceProvider) diff --git a/pkg/handlers/publicapi/transportation_service_provider_test.go b/pkg/handlers/publicapi/transportation_service_provider_test.go index 8555c9314a23..dc354b7b76cd 100644 --- a/pkg/handlers/publicapi/transportation_service_provider_test.go +++ b/pkg/handlers/publicapi/transportation_service_provider_test.go @@ -6,8 +6,6 @@ import ( "github.com/go-openapi/strfmt" - "github.com/transcom/mymove/pkg/auth" - tspop "github.com/transcom/mymove/pkg/gen/restapi/apioperations/transportation_service_provider" "github.com/transcom/mymove/pkg/handlers" "github.com/transcom/mymove/pkg/models" @@ -60,11 +58,11 @@ func (suite *HandlerSuite) TestGetTransportationServiceProviderHandlerWhereSessi ShipmentID: strfmt.UUID(shipment.ID.String()), } - session := auth.SessionFromRequestContext(params.HTTPRequest) - handler := GetTransportationServiceProviderHandler{handlers.NewHandlerContext(suite.DB(), suite.TestLogger())} response := handler.Handle(params) + session := handler.SessionFromRequest(params.HTTPRequest) + suite.NotEqual(session.ServiceMemberID, shipment.ServiceMemberID) suite.Assertions.IsType(&tspop.GetTransportationServiceProviderForbidden{}, response) } diff --git a/pkg/logging/context.go b/pkg/logging/context.go new file mode 100644 index 000000000000..87890e13e9cf --- /dev/null +++ b/pkg/logging/context.go @@ -0,0 +1,19 @@ +package logging + +import ( + "context" +) + +type contextKey int + +var loggerContextKey = contextKey(1) + +// NewContext returns a new context with a logger. +func NewContext(ctx context.Context, logger interface{}) context.Context { + return context.WithValue(ctx, loggerContextKey, logger) +} + +// FromContext returns a logger associated with a context, if any. +func FromContext(ctx context.Context) interface{} { + return ctx.Value(loggerContextKey) +} diff --git a/pkg/logging/logger.go b/pkg/logging/logger.go deleted file mode 100644 index 73a19d1dba9c..000000000000 --- a/pkg/logging/logger.go +++ /dev/null @@ -1,10 +0,0 @@ -package logging - -import ( - "go.uber.org/zap" -) - -// Logger is an interface that describes the logging requirements of this package. -type Logger interface { - Info(msg string, fields ...zap.Field) -} diff --git a/pkg/middleware/context_logger.go b/pkg/middleware/context_logger.go new file mode 100644 index 000000000000..363973882bad --- /dev/null +++ b/pkg/middleware/context_logger.go @@ -0,0 +1,24 @@ +package middleware + +import ( + "net/http" + + "go.uber.org/zap" + + "github.com/transcom/mymove/pkg/logging" + "github.com/transcom/mymove/pkg/trace" +) + +// ContextLogger returns a handler that injects the logger into the request context. +func ContextLogger(field string, original Logger) func(next http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + if id := trace.FromContext(ctx); len(id) > 0 { + next.ServeHTTP(w, r.WithContext(logging.NewContext(ctx, original.With(zap.String(field, id))))) + } else { + next.ServeHTTP(w, r.WithContext(logging.NewContext(ctx, original))) + } + }) + } +} diff --git a/pkg/middleware/context_logger_test.go b/pkg/middleware/context_logger_test.go new file mode 100644 index 000000000000..bd761644d463 --- /dev/null +++ b/pkg/middleware/context_logger_test.go @@ -0,0 +1,41 @@ +package middleware + +import ( + "bytes" + "net/http" + "net/http/httptest" + "strings" + + //"github.com/stretchr/testify/suite" + "go.uber.org/zap" + "go.uber.org/zap/zapcore" + //"go.uber.org/zap/zaptest" +) + +func (suite *testSuite) TestContextLoggerWithoutTrace() { + buf := bytes.NewBuffer(make([]byte, 0)) + // Create logger that writes to the buffer instead of stdout/stderr + logger := suite.logger.WithOptions(zap.WrapCore(func(c zapcore.Core) zapcore.Core { + return zapcore.NewCore( + zapcore.NewConsoleEncoder(zap.NewDevelopmentEncoderConfig()), + zapcore.AddSync(buf), + zapcore.DebugLevel, + ) + })) + mw := ContextLogger("", logger) + rr := httptest.NewRecorder() + suite.do(mw, suite.log, rr, httptest.NewRequest("GET", testURL, nil)) + suite.Equal(http.StatusOK, rr.Code, errStatusCode) // check status code + out := strings.TrimSpace(string(buf.Bytes())) // remove trailing new line + suite.NotEmpty(out, "log was empty") + lines := strings.Split(out, "\n") + suite.Len(lines, 2) // test that there are 2 log lines (info message and error message) + parts := strings.Split(lines[0], "\t") + suite.Len(parts, 3) + suite.Equal(parts[1], "INFO") + suite.Equal(parts[2], "Placeholder for info message") + parts = strings.Split(lines[1], "\t") + suite.Len(parts, 3) + suite.Equal(parts[1], "ERROR") + suite.Equal(parts[2], "Placeholder for error message") +} diff --git a/pkg/middleware/logger.go b/pkg/middleware/logger.go index 2e384663de64..346920306c7d 100644 --- a/pkg/middleware/logger.go +++ b/pkg/middleware/logger.go @@ -11,5 +11,11 @@ type Logger interface { Error(msg string, fields ...zap.Field) Warn(msg string, fields ...zap.Field) Fatal(msg string, fields ...zap.Field) + With(fields ...zap.Field) *zap.Logger WithOptions(options ...zap.Option) *zap.Logger } + +// InfoLogger is a logger interface with the Info method. +type InfoLogger interface { + Info(msg string, fields ...zap.Field) +} diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/middleware_test.go index 47412870a658..a8e7318a44e8 100644 --- a/pkg/middleware/middleware_test.go +++ b/pkg/middleware/middleware_test.go @@ -10,6 +10,9 @@ import ( "github.com/stretchr/testify/suite" "go.uber.org/zap" "go.uber.org/zap/zaptest" + + "github.com/transcom/mymove/pkg/logging" + "github.com/transcom/mymove/pkg/trace" ) const ( @@ -29,6 +32,8 @@ type testSuite struct { ok http.HandlerFunc reflect http.HandlerFunc panic http.HandlerFunc + trace http.HandlerFunc + log http.HandlerFunc } func TestSuite(t *testing.T) { @@ -53,6 +58,18 @@ func TestSuite(t *testing.T) { panic: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { panic(errors.New("foobar")) }), + trace: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + traceID := trace.FromContext(r.Context()) + w.WriteHeader(http.StatusOK) + fmt.Fprint(w, traceID) + }), + log: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if l, ok := logging.FromContext(r.Context()).(Logger); ok { + l.Info("Placeholder for info message") + l.Error("Placeholder for error message") + w.WriteHeader(http.StatusOK) + } + }), } suite.Run(t, ts) diff --git a/pkg/middleware/request_logger.go b/pkg/middleware/request_logger.go index eea5d8f3000b..2e3eaf86e7ce 100644 --- a/pkg/middleware/request_logger.go +++ b/pkg/middleware/request_logger.go @@ -10,15 +10,25 @@ import ( "go.uber.org/zap" "github.com/transcom/mymove/pkg/auth" + "github.com/transcom/mymove/pkg/logging" ) // RequestLogger returns a middleware that logs requests. // The middleware trys to use the logger from the context. // If the request context has no handler, then falls back to the server logger. -func RequestLogger(logger Logger) func(inner http.Handler) http.Handler { +func RequestLogger(serverLogger Logger) func(inner http.Handler) http.Handler { return func(inner http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + var logger InfoLogger + if requestLogger, ok := logging.FromContext(ctx).(InfoLogger); ok { + logger = requestLogger + } else { + logger = serverLogger + } + fields := []zap.Field{ zap.String("accepted-language", r.Header.Get("accepted-language")), zap.Int64("content-length", r.ContentLength), @@ -46,7 +56,7 @@ func RequestLogger(logger Logger) func(inner http.Handler) http.Handler { } } - if session := auth.SessionFromRequestContext(r); session != nil { + if session := auth.SessionFromContext(ctx); session != nil { if session.UserID != uuid.Nil { fields = append(fields, zap.String("user-id", session.UserID.String())) } diff --git a/pkg/middleware/trace.go b/pkg/middleware/trace.go new file mode 100644 index 000000000000..ea3a2907c61f --- /dev/null +++ b/pkg/middleware/trace.go @@ -0,0 +1,26 @@ +package middleware + +import ( + "net/http" + + "github.com/gofrs/uuid" + + "github.com/pkg/errors" + + "github.com/transcom/mymove/pkg/trace" +) + +// Trace returns a trace middleware that injects a unique trace id into every request. +func Trace(logger Logger) func(next http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + id, err := uuid.NewV4() + if err != nil { + logger.Error(errors.Wrap(err, "error creating trace id").Error()) + next.ServeHTTP(w, r) + } else { + next.ServeHTTP(w, r.WithContext(trace.NewContext(r.Context(), id.String()))) + } + }) + } +} diff --git a/pkg/middleware/trace_test.go b/pkg/middleware/trace_test.go new file mode 100644 index 000000000000..d17a265f64ba --- /dev/null +++ b/pkg/middleware/trace_test.go @@ -0,0 +1,23 @@ +package middleware + +import ( + "fmt" + "io/ioutil" + "net/http" + "net/http/httptest" + + "github.com/gofrs/uuid" +) + +func (suite *testSuite) TestTrace() { + mw := Trace(suite.logger) + rr := httptest.NewRecorder() + suite.do(mw, suite.trace, rr, httptest.NewRequest("GET", testURL, nil)) + suite.Equal(http.StatusOK, rr.Code, errStatusCode) // check status code + body, err := ioutil.ReadAll(rr.Body) + suite.Nil(err) // check that you could read full body + suite.NotEmpty(string(body)) // check that handler returned the trace id + id, err := uuid.FromString(string(body)) + suite.Nil(err, "failed to parse UUID") + suite.logger.Debug(fmt.Sprintf("Trace ID: %s", id)) +} diff --git a/pkg/trace/context.go b/pkg/trace/context.go new file mode 100644 index 000000000000..cac1ba0e507e --- /dev/null +++ b/pkg/trace/context.go @@ -0,0 +1,23 @@ +package trace + +import ( + "context" +) + +type contextKey int + +var traceIDContextKey = contextKey(1) + +// NewContext returns a new context with a logger. +func NewContext(ctx context.Context, traceID string) context.Context { + return context.WithValue(ctx, traceIDContextKey, traceID) +} + +// FromContext returns a logger associated with a context, if any. +func FromContext(ctx context.Context) string { + str, ok := ctx.Value(traceIDContextKey).(string) + if !ok { + return "" + } + return str +}