PR comments

cmds/core-service/main.go

@@ -198,7 +198,7 @@ func createSCDServer(ctx context.Context, logger *zap.Logger) (*scd.Server, erro
 
 	return &scd.Server{
 		Store:            scdStore,
-		DssReportHandler: scd.JSONLoggingDssReportHandler{ReportLogger: logger},
+		DSSReportHandler: &scd.JSONLoggingReceivedReportHandler{ReportLogger: logger},
 		Timeout:          *timeout,
 		EnableHTTP:       *enableHTTP,
 	}, nil

pkg/logging/logging.go

@@ -77,9 +77,6 @@ func Configure(level string, format string) error {
 // WithValuesFromContext augments logger with relevant fields from ctx and returns
 // the resulting logger.
 func WithValuesFromContext(ctx context.Context, logger *zap.Logger) *zap.Logger {
-	// TODO: WithValuesFromContext is used in multiple places that might assume this
-	//  method does more than it actually does:
-	//  given it was added in 2019 we may want to check?
 	// Naive implementation for now, meant to evolve over time.
 	return logger
 }

pkg/scd/dss_report_handler.go

@@ -4,20 +4,28 @@ import (
 	"context"
 	"encoding/json"
 	"github.com/google/uuid"
+	"github.com/interuss/dss/pkg/api"
 	restapi "github.com/interuss/dss/pkg/api/scdv1"
+	dsserr "github.com/interuss/dss/pkg/errors"
 	"github.com/interuss/dss/pkg/logging"
 	"github.com/interuss/stacktrace"
 	"go.uber.org/zap"
 )
 
-// JSONLoggingDssReportHandler a DssReportHandler that simply logs the received report as JSON.
-type JSONLoggingDssReportHandler struct {
+// ReceivedReportHandler takes care of handling a DSS report received through the MakeDssReport REST handler.
+type ReceivedReportHandler interface {
+	// Handle a DSS report request. Returns the error report passed in 'req' after having set its identifier.
+	Handle(ctx context.Context, req *restapi.MakeDssReportRequest) (*restapi.ErrorReport, error)
+}
+
+// JSONLoggingReceivedReportHandler a DSSReportHandler that simply logs the received report as JSON.
+type JSONLoggingReceivedReportHandler struct {
 	// ReportLogger is the logger to which the received reports will be logged.
 	ReportLogger *zap.Logger
 }
 
 // HandleDssReport logs the received report as a JSON string to a logger.
-func (h JSONLoggingDssReportHandler) HandleDssReport(ctx context.Context, req *restapi.MakeDssReportRequest) (*restapi.ErrorReport, error) {
+func (h *JSONLoggingReceivedReportHandler) Handle(ctx context.Context, req *restapi.MakeDssReportRequest) (*restapi.ErrorReport, error) {
 	reportID, err := uuid.NewRandom()
 	if err != nil {
 		return nil, stacktrace.Propagate(err, "Failed to generate report ID")
@@ -29,8 +37,41 @@ func (h JSONLoggingDssReportHandler) HandleDssReport(ctx context.Context, req *r
 	jsonReport, err := json.Marshal(req.Body)
 	if err != nil {
 		logging.WithValuesFromContext(ctx, logging.Logger).Error("Failed to serialize DSS Report", zap.Error(err))
-		return nil, stacktrace.Propagate(err, "Failed to serialize DSS Report")
+		return nil, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Failed to serialize DSS Report")
 	}
-	h.ReportLogger.Info("Received DSS Report", zap.String("report", string(jsonReport)))
+	h.ReportLogger.Info("Received DSS Report", zap.String("reportID", reportIDStr), zap.String("report", string(jsonReport)))
 	return rVal, nil
 }
+
+// MakeDssReport creates an error report about a DSS.
+func (a *Server) MakeDssReport(ctx context.Context, req *restapi.MakeDssReportRequest,
+) restapi.MakeDssReportResponseSet {
+	if req.Auth.Error != nil {
+		resp := restapi.MakeDssReportResponseSet{}
+		setAuthError(ctx, stacktrace.Propagate(req.Auth.Error, "Auth failed"), &resp.Response401, &resp.Response403, &resp.Response500)
+		return resp
+	}
+
+	if req.BodyParseError != nil {
+		return restapi.MakeDssReportResponseSet{Response400: &restapi.ErrorResponse{
+			Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(req.BodyParseError, dsserr.BadRequest, "Malformed params"))}}
+	}
+
+	report, err := a.DSSReportHandler.Handle(ctx, req)
+
+	if err != nil {
+		err = stacktrace.Propagate(err, "Could not delete operational intent")
+		errResp := &restapi.ErrorResponse{Message: dsserr.Handle(ctx, err)}
+		switch stacktrace.GetCode(err) {
+		case dsserr.BadRequest:
+			return restapi.MakeDssReportResponseSet{Response400: errResp}
+		case dsserr.PermissionDenied:
+			return restapi.MakeDssReportResponseSet{Response403: errResp}
+		default:
+			return restapi.MakeDssReportResponseSet{Response500: &api.InternalServerErrorBody{
+				ErrorMessage: *dsserr.Handle(ctx, err)}}
+		}
+	}
+
+	return restapi.MakeDssReportResponseSet{Response201: report}
+}

pkg/scd/repos/repos.go

@@ -3,7 +3,6 @@ package repos
 import (
 	"context"
 	"github.com/golang/geo/s2"
-	restapi "github.com/interuss/dss/pkg/api/scdv1"
 	dssmodels "github.com/interuss/dss/pkg/models"
 	scdmodels "github.com/interuss/dss/pkg/scd/models"
 )
@@ -81,14 +80,7 @@ type Constraint interface {
 	DeleteConstraint(ctx context.Context, id dssmodels.ID) error
 }
 
-// DssReport takes care of handling a DSS report.
-type DssReport interface {
-	// HandleDssReport handles a DSS report request. Returns the error report passed in 'req' after having set its identifier.
-	HandleDssReport(ctx context.Context, req *restapi.MakeDssReportRequest) (*restapi.ErrorReport, error)
-}
-
 // Repository aggregates all SCD-specific repo interfaces.
-// Note that 'DssReport' is not yet part of it, while we figure exactly how we want to handle DSS reports
 type Repository interface {
 	OperationalIntent
 	Subscription

pkg/scd/server.go

@@ -2,7 +2,6 @@ package scd
 
 import (
 	"context"
-	"github.com/interuss/dss/pkg/scd/repos"
 	"time"
 
 	"github.com/interuss/dss/pkg/api"
@@ -37,34 +36,11 @@ func makeSubscribersToNotify(subscriptions []*scdmodels.Subscription) []restapi.
 // Server implements scdv1.Implementation.
 type Server struct {
 	Store            scdstore.Store
-	DssReportHandler repos.DssReport
+	DSSReportHandler ReceivedReportHandler
 	Timeout          time.Duration
 	EnableHTTP       bool
 }
 
-// MakeDssReport creates an error report about a DSS.
-func (a *Server) MakeDssReport(ctx context.Context, req *restapi.MakeDssReportRequest,
-) restapi.MakeDssReportResponseSet {
-	if req.Auth.Error != nil {
-		resp := restapi.MakeDssReportResponseSet{}
-		setAuthError(ctx, stacktrace.Propagate(req.Auth.Error, "Auth failed"), &resp.Response401, &resp.Response403, &resp.Response500)
-		return resp
-	}
-
-	if req.BodyParseError != nil {
-		return restapi.MakeDssReportResponseSet{Response400: &restapi.ErrorResponse{
-			Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(req.BodyParseError, dsserr.BadRequest, "Malformed params"))}}
-	}
-
-	resp, err := a.DssReportHandler.HandleDssReport(ctx, req)
-	if err != nil {
-		return restapi.MakeDssReportResponseSet{Response400: &restapi.ErrorResponse{
-			Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Failed to handle DSS Report"))}}
-	}
-
-	return restapi.MakeDssReportResponseSet{Response201: resp}
-}
-
 func setAuthError(ctx context.Context, authErr error, resp401, resp403 **restapi.ErrorResponse, resp500 **api.InternalServerErrorBody) {
 	switch stacktrace.GetCode(authErr) {
 	case dsserr.Unauthenticated: