Merge branch 'release/1.23.0'

Makefile

@@ -8,6 +8,8 @@ GIT_COMMIT=$(shell git rev-parse HEAD)
 VERSION ?= $(shell git tag --points-at HEAD | grep ^v | head -n 1)
 LDFLAGS=-ldflags "-X main.BuildTime=$(BUILD_TIME) -X main.GitCommit=$(GIT_COMMIT) -X main.Version=$(VERSION)"
 
+export GRAPH_DRIVER_TYPE?=neo4j
+export GRAPH_ADDR?=bolt://localhost:7687
 export ENABLE_PRIVATE_ENDPOINTS?=true
 
 .PHONY: all
@@ -24,7 +26,7 @@ build:
 
 .PHONY: debug
 debug:
-	GRAPH_DRIVER_TYPE="neo4j" GRAPH_ADDR="bolt://localhost:7687" HUMAN_LOG=1 go run -race $(LDFLAGS) main.go
+	HUMAN_LOG=1 go run -race $(LDFLAGS) main.go
 
 .PHONY: acceptance-publishing
 acceptance-publishing: build

go.mod

@@ -3,13 +3,13 @@ module github.com/ONSdigital/dp-dataset-api
 go 1.13
 
 require (
-	github.com/ONSdigital/dp-api-clients-go v1.28.0
+	github.com/ONSdigital/dp-api-clients-go v1.30.0
 	github.com/ONSdigital/dp-authorisation v0.1.0
-	github.com/ONSdigital/dp-graph/v2 v2.1.3
+	github.com/ONSdigital/dp-graph/v2 v2.3.0
 	github.com/ONSdigital/dp-healthcheck v1.0.5
 	github.com/ONSdigital/dp-kafka v1.1.7
 	github.com/ONSdigital/dp-mongodb v1.4.0
-	github.com/ONSdigital/dp-net v1.0.8
+	github.com/ONSdigital/dp-net v1.0.9
 	github.com/ONSdigital/go-ns v0.0.0-20200205115900-a11716f93bad
 	github.com/ONSdigital/log.go v1.0.1
 	github.com/frankban/quicktest v1.9.0 // indirect

models/dataset.go

@@ -6,7 +6,9 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"net/url"
 	"strconv"
+	"strings"
 	"time"
 
 	errs "github.com/ONSdigital/dp-dataset-api/apierrors"
@@ -398,6 +400,27 @@ func (ed *EditionUpdate) PublishLinks(ctx context.Context, host string, versionL
 	return nil
 }
 
+func ValidateDataset(ctx context.Context, dataset *Dataset) error {
+	var invalidFields []string
+	if dataset.URI != "" {
+		dataset.URI = strings.TrimSpace(dataset.URI)
+		datasetURI, err := url.Parse(dataset.URI)
+		if err != nil {
+			invalidFields = append(invalidFields, "URI")
+			log.Event(ctx, "error parsing URI", log.ERROR, log.Error(err))
+		} else {
+			if !strings.Contains(datasetURI.Path, "/datasets") || !strings.Contains(datasetURI.Path, dataset.ID) {
+				fmt.Println("hello")
+				invalidFields = append(invalidFields, "URI")
+			}
+		}
+	}
+	if invalidFields != nil {
+		return fmt.Errorf("invalid fields: %v", invalidFields)
+	}
+	return nil
+}
+
 // ValidateVersion checks the content of the version structure
 func ValidateVersion(version *Version) error {
 

models/dataset_test.go

@@ -13,6 +13,13 @@ import (
 	. "github.com/smartystreets/goconvey/convey"
 )
 
+func createDataset() Dataset {
+	return Dataset{
+		ID:  "123",
+		URI: "http://localhost:22000/datasets/123",
+	}
+}
+
 var testContext = context.Background()
 
 func TestCreateDataset(t *testing.T) {
@@ -135,6 +142,63 @@ func TestCreateVersion(t *testing.T) {
 	})
 }
 
+func TestValidateDataset(t *testing.T) {
+	t.Parallel()
+
+	Convey("Unsuccessful validation (false) returned", t, func() {
+
+		Convey("when dataset.URI is unable to be parsed into url format", func() {
+			dataset := createDataset()
+			dataset.URI = ":foo"
+			fmt.Println(dataset.URI)
+			validationErr := ValidateDataset(testContext, &dataset)
+			So(validationErr, ShouldNotBeNil)
+			So(validationErr.Error(), ShouldResemble, errors.New("invalid fields: [URI]").Error())
+		})
+
+		Convey("when dataset.URI does not contain 'datasets' path", func() {
+			dataset := createDataset()
+			dataset.URI = "/123"
+			validationErr := ValidateDataset(testContext, &dataset)
+			So(validationErr, ShouldNotBeNil)
+			So(validationErr.Error(), ShouldResemble, errors.New("invalid fields: [URI]").Error())
+		})
+
+		Convey("when dataset.URI does not contain 'id' path", func() {
+			dataset := createDataset()
+			dataset.URI = "http://localhost:22000/datasets"
+			validationErr := ValidateDataset(testContext, &dataset)
+			So(validationErr, ShouldNotBeNil)
+			So(validationErr.Error(), ShouldResemble, errors.New("invalid fields: [URI]").Error())
+		})
+
+	})
+
+	Convey("Successful validation (true) returned", t, func() {
+
+		Convey("when dataset.URI contains its path in appropriate url format ", func() {
+			dataset := createDataset()
+			dataset.ID = "123"
+			dataset.URI = "http://localhost:22000/datasets/123/breadcrumbs"
+			validationErr := ValidateDataset(testContext, &dataset)
+			So(validationErr, ShouldBeNil)
+		})
+	})
+
+	Convey("Successful validation (true) returned", t, func() {
+
+		Convey("when dataset.URI contains whitespace it should not return an error ", func() {
+			dataset := createDataset()
+			dataset.ID = "123"
+			dataset.URI = "  http://localhost:22000/datasets/123/breadcrumbs  "
+			validationErr := ValidateDataset(testContext, &dataset)
+			So(validationErr, ShouldBeNil)
+			So(dataset.URI, ShouldEqual, "http://localhost:22000/datasets/123/breadcrumbs")
+		})
+	})
+
+}
+
 func TestValidateVersion(t *testing.T) {
 	t.Parallel()
 	Convey("Successfully return without any errors", t, func() {

mongo/dataset_store.go

@@ -441,6 +441,11 @@ func createDatasetUpdateQuery(ctx context.Context, id string, dataset *models.Da
 		updates["next.uri"] = dataset.URI
 	}
 
+	if err := models.ValidateDataset(ctx, dataset); err != nil {
+		log.Event(ctx, "failed validation check to create dataset", log.ERROR, log.Error(err))
+		return nil
+	}
+
 	log.Event(ctx, "built update query for dataset resource", log.INFO, log.Data{"dataset_id": id, "dataset": dataset, "updates": updates})
 
 	return updates

mongo/dataset_test.go

@@ -197,7 +197,7 @@ func TestDatasetUpdateQuery(t *testing.T) {
 			"next.release_frequency":        "yearly",
 			"next.theme":                    "construction",
 			"next.title":                    "CPI",
-			"next.uri":                      "http://ons.gov.uk/dataset/123/landing-page",
+			"next.uri":                      "http://ons.gov.uk/datasets/123/landing-page",
 		}
 
 		dataset := &models.Dataset{
@@ -225,7 +225,7 @@ func TestDatasetUpdateQuery(t *testing.T) {
 			ReleaseFrequency: "yearly",
 			Theme:            "construction",
 			Title:            "CPI",
-			URI:              "http://ons.gov.uk/dataset/123/landing-page",
+			URI:              "http://ons.gov.uk/datasets/123/landing-page",
 		}
 
 		selector := createDatasetUpdateQuery(testContext, "123", dataset, models.CreatedState)

service/initialise.go

@@ -60,13 +60,13 @@ func (e *ExternalServiceList) GetProducer(ctx context.Context, cfg *config.Confi
 }
 
 // GetGraphDB returns a graphDB (only if observation and private endpoint are enabled)
-func (e *ExternalServiceList) GetGraphDB(ctx context.Context) (store.GraphDB, error) {
-	graphDB, err := e.Init.DoGetGraphDB(ctx)
+func (e *ExternalServiceList) GetGraphDB(ctx context.Context) (store.GraphDB, Closer, error) {
+	graphDB, graphDBErrorConsumer, err := e.Init.DoGetGraphDB(ctx)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	e.Graph = true
-	return graphDB, nil
+	return graphDB, graphDBErrorConsumer, nil
 }
 
 // GetMongoDB returns a mongodb health client and dataset mongo object
@@ -104,8 +104,15 @@ func (e *Init) DoGetKafkaProducer(ctx context.Context, cfg *config.Configuration
 }
 
 // DoGetGraphDB creates a new GraphDB
-func (e *Init) DoGetGraphDB(ctx context.Context) (store.GraphDB, error) {
-	return graph.New(ctx, graph.Subsets{Observation: true, Instance: true})
+func (e *Init) DoGetGraphDB(ctx context.Context) (store.GraphDB, Closer, error) {
+	graphDB, err := graph.New(ctx, graph.Subsets{Observation: true, Instance: true})
+	if err != nil {
+		return nil, nil, err
+	}
+
+	graphDBErrorConsumer := graph.NewLoggingErrorConsumer(ctx, graphDB.ErrorChan())
+
+	return graphDB, graphDBErrorConsumer, nil
 }
 
 // DoGetMongoDB returns a MongoDB

service/interfaces.go

@@ -13,13 +13,14 @@ import (
 //go:generate moq -out mock/initialiser.go -pkg mock . Initialiser
 //go:generate moq -out mock/server.go -pkg mock . HTTPServer
 //go:generate moq -out mock/healthcheck.go -pkg mock . HealthChecker
+//go:generate moq -out mock/closer.go -pkg mock . Closer
 
 // Initialiser defines the methods to initialise external services
 type Initialiser interface {
 	DoGetHTTPServer(bindAddr string, router http.Handler) HTTPServer
 	DoGetHealthCheck(cfg *config.Configuration, buildTime, gitCommit, version string) (HealthChecker, error)
 	DoGetKafkaProducer(ctx context.Context, cfg *config.Configuration) (kafka.IProducer, error)
-	DoGetGraphDB(ctx context.Context) (store.GraphDB, error)
+	DoGetGraphDB(ctx context.Context) (store.GraphDB, Closer, error)
 	DoGetMongoDB(ctx context.Context, cfg *config.Configuration) (store.MongoDB, error)
 }
 
@@ -36,3 +37,8 @@ type HealthChecker interface {
 	Stop()
 	AddCheck(name string, checker healthcheck.Checker) (err error)
 }
+
+// Closer defines the required methods for a closable resource
+type Closer interface {
+	Close(ctx context.Context) error
+}

service/service.go

@@ -36,6 +36,7 @@ type Service struct {
 	config                    *config.Configuration
 	serviceList               *ExternalServiceList
 	graphDB                   store.GraphDB
+	graphDBErrorConsumer      Closer
 	mongoDB                   store.MongoDB
 	generateDownloadsProducer kafka.IProducer
 	identityClient            *clientsidentity.Client
@@ -78,6 +79,11 @@ func (svc *Service) SetGraphDB(graphDB store.GraphDB) {
 	svc.graphDB = graphDB
 }
 
+// SetGraphDBErrorConsumer sets the graphDB error consumer for a service
+func (svc *Service) SetGraphDBErrorConsumer(graphDBErrorConsumer Closer) {
+	svc.graphDBErrorConsumer = graphDBErrorConsumer
+}
+
 // Run the service
 func (svc *Service) Run(ctx context.Context, buildTime, gitCommit, version string, svcErrors chan error) (err error) {
 
@@ -95,7 +101,7 @@ func (svc *Service) Run(ctx context.Context, buildTime, gitCommit, version strin
 			"EnablePrivateEndpoints":    svc.config.EnablePrivateEndpoints,
 		})
 	} else {
-		svc.graphDB, err = svc.serviceList.GetGraphDB(ctx)
+		svc.graphDB, svc.graphDBErrorConsumer, err = svc.serviceList.GetGraphDB(ctx)
 		if err != nil {
 			log.Event(ctx, "failed to initialise graph driver", log.FATAL, log.Error(err))
 			return err
@@ -260,6 +266,11 @@ func (svc *Service) Close(ctx context.Context) error {
 				log.Event(shutdownContext, "failed to close graph db", log.ERROR, log.Error(err))
 				hasShutdownError = true
 			}
+
+			if err := svc.graphDBErrorConsumer.Close(shutdownContext); err != nil {
+				log.Event(shutdownContext, "failed to close graph db error consumer", log.ERROR, log.Error(err))
+				hasShutdownError = true
+			}
 		}
 	}()