Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Returns etag in the getVersion response header #416

Merged
merged 7 commits into from
Mar 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 31 additions & 26 deletions api/versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/ONSdigital/dp-api-clients-go/v2/headers"
errs "github.com/ONSdigital/dp-dataset-api/apierrors"
"github.com/ONSdigital/dp-dataset-api/models"
dpresponse "github.com/ONSdigital/dp-net/v2/handlers/response"
dphttp "github.com/ONSdigital/dp-net/v2/http"
dprequest "github.com/ONSdigital/dp-net/v2/request"
"github.com/ONSdigital/log.go/v2/log"
Expand Down Expand Up @@ -137,13 +138,13 @@ func (api *DatasetAPI) getVersion(w http.ResponseWriter, r *http.Request) {
vars := mux.Vars(r)
datasetID := vars["dataset_id"]
edition := vars["edition"]
version := vars["version"]
logData := log.Data{"dataset_id": datasetID, "edition": edition, "version": version}
versionNumber := vars["version"]
logData := log.Data{"dataset_id": datasetID, "edition": edition, "version": versionNumber}

b, getVersionErr := func() ([]byte, error) {
v, getVersionErr := func() (*models.Version, error) {
authorised := api.authenticate(r, logData)

versionId, err := models.ParseAndValidateVersionNumber(ctx, version)
versionId, err := models.ParseAndValidateVersionNumber(ctx, versionNumber)
if err != nil {
log.Error(ctx, "getVersion endpoint: invalid version", err, logData)
return nil, err
Expand All @@ -164,44 +165,38 @@ func (api *DatasetAPI) getVersion(w http.ResponseWriter, r *http.Request) {
return nil, err
}

results, err := api.dataStore.Backend.GetVersion(ctx, datasetID, edition, versionId, state)
version, err := api.dataStore.Backend.GetVersion(ctx, datasetID, edition, versionId, state)
if err != nil {
log.Error(ctx, "failed to find version for dataset edition", err, logData)
return nil, err
}

results.Links.Self.HRef = results.Links.Version.HRef
version.Links.Self.HRef = version.Links.Version.HRef

if err = models.CheckState("version", results.State); err != nil {
log.Error(ctx, "unpublished version has an invalid state", err, log.Data{"state": results.State})
if err = models.CheckState("version", version.State); err != nil {
log.Error(ctx, "unpublished version has an invalid state", err, log.Data{"state": version.State})
return nil, errs.ErrResourceState
}

// Only the download service should not have access to the public/private download
// fields
if r.Header.Get(downloadServiceToken) != api.downloadServiceToken {
if results.Downloads != nil {
if results.Downloads.CSV != nil {
results.Downloads.CSV.Private = ""
results.Downloads.CSV.Public = ""
if version.Downloads != nil {
if version.Downloads.CSV != nil {
version.Downloads.CSV.Private = ""
version.Downloads.CSV.Public = ""
}
if results.Downloads.XLS != nil {
results.Downloads.XLS.Private = ""
results.Downloads.XLS.Public = ""
if version.Downloads.XLS != nil {
version.Downloads.XLS.Private = ""
version.Downloads.XLS.Public = ""
}
if results.Downloads.CSVW != nil {
results.Downloads.CSVW.Private = ""
results.Downloads.CSVW.Public = ""
if version.Downloads.CSVW != nil {
version.Downloads.CSVW.Private = ""
version.Downloads.CSVW.Public = ""
}
}
}

b, err := json.Marshal(results)
if err != nil {
log.Error(ctx, "failed to marshal version resource into bytes", err, logData)
return nil, err
}
return b, nil
return version, nil
}()

if getVersionErr != nil {
Expand All @@ -210,7 +205,17 @@ func (api *DatasetAPI) getVersion(w http.ResponseWriter, r *http.Request) {
}

setJSONContentType(w)
_, err := w.Write(b)
if v.ETag != "" {
dpresponse.SetETag(w, v.ETag)
}

versionBytes, err := json.Marshal(v)
if err != nil {
log.Error(ctx, "failed to marshal version resource into bytes", err, logData)
handleVersionAPIErr(ctx, err, w, logData)
}

_, err = w.Write(versionBytes)
if err != nil {
log.Error(ctx, "failed writing bytes to response", err, logData)
handleVersionAPIErr(ctx, err, w, logData)
Expand Down
72 changes: 54 additions & 18 deletions api/versions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,16 @@ func TestGetVersionsReturnsError(t *testing.T) {

func TestGetVersionReturnsOK(t *testing.T) {
t.Parallel()
Convey("A successful request to get version returns 200 OK response", t, func() {
Convey("Given a version", t, func() {
version := &models.Version{
State: models.EditionConfirmedState,
Links: &models.VersionLinks{
Self: &models.LinkObject{},
Version: &models.LinkObject{
HRef: "href",
},
},
}
r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123-456/editions/678/versions/1", nil)

w := httptest.NewRecorder()
Expand All @@ -242,30 +251,57 @@ func TestGetVersionReturnsOK(t *testing.T) {
CheckEditionExistsFunc: func(ctx context.Context, datasetID, editionID, state string) error {
return nil
},
GetVersionFunc: func(ctx context.Context, datasetID, editionID string, version int, state string) (*models.Version, error) {
return &models.Version{
State: models.EditionConfirmedState,
Links: &models.VersionLinks{
Self: &models.LinkObject{},
Version: &models.LinkObject{
HRef: "href",
},
},
}, nil
GetVersionFunc: func(ctx context.Context, datasetID, editionID string, versionNumber int, state string) (*models.Version, error) {
return version, nil
},
}

datasetPermissions := getAuthorisationHandlerMock()
permissions := getAuthorisationHandlerMock()
api := GetAPIWithCMDMocks(mockedDataStore, &mocks.DownloadsGeneratorMock{}, datasetPermissions, permissions)
api.Router.ServeHTTP(w, r)

So(w.Code, ShouldEqual, http.StatusOK)
So(datasetPermissions.Required.Calls, ShouldEqual, 1)
So(permissions.Required.Calls, ShouldEqual, 0)
So(len(mockedDataStore.CheckDatasetExistsCalls()), ShouldEqual, 1)
So(len(mockedDataStore.CheckEditionExistsCalls()), ShouldEqual, 1)
So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 1)
Convey("With an etag", func() {
version.ETag = "version-etag"
Convey("When we call the GET version endpoint", func() {
api.Router.ServeHTTP(w, r)

Convey("Then it returns a 200 OK", func() {
So(w.Code, ShouldEqual, http.StatusOK)
})
Convey("And the etag is returned in the response header", func() {
So(w.Header().Get("Etag"), ShouldEqual, version.ETag)
})

Convey("And the relevant calls have been made", func() {
So(datasetPermissions.Required.Calls, ShouldEqual, 1)
So(permissions.Required.Calls, ShouldEqual, 0)
So(len(mockedDataStore.CheckDatasetExistsCalls()), ShouldEqual, 1)
So(len(mockedDataStore.CheckEditionExistsCalls()), ShouldEqual, 1)
So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 1)
})
})
})
Convey("Without an etag", func() {
version.ETag = ""
Convey("When we call the GET version endpoint", func() {
api.Router.ServeHTTP(w, r)

Convey("Then it returns a 200 OK", func() {
So(w.Code, ShouldEqual, http.StatusOK)
})
Convey("And no etag is returned in the response header", func() {
So(w.Header().Get("Etag"), ShouldBeEmpty)
})

Convey("And the relevant calls have been made", func() {
So(datasetPermissions.Required.Calls, ShouldEqual, 1)
So(permissions.Required.Calls, ShouldEqual, 0)
So(len(mockedDataStore.CheckDatasetExistsCalls()), ShouldEqual, 1)
So(len(mockedDataStore.CheckEditionExistsCalls()), ShouldEqual, 1)
So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 1)
})
})
})
})
}

Expand Down
13 changes: 7 additions & 6 deletions dimension/dimension.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/ONSdigital/dp-dataset-api/models"
"github.com/ONSdigital/dp-dataset-api/store"
"github.com/ONSdigital/dp-dataset-api/utils"
dpresponse "github.com/ONSdigital/dp-net/v2/handlers/response"
dphttp "github.com/ONSdigital/dp-net/v2/http"
dprequest "github.com/ONSdigital/dp-net/v2/request"
"github.com/ONSdigital/log.go/v2/log"
Expand Down Expand Up @@ -75,7 +76,7 @@ func (s *Store) GetDimensionsHandler(w http.ResponseWriter, r *http.Request, lim
}

log.Info(ctx, "successfully get dimensions for an instance resource", logData)
setETag(w, instance.ETag)
dpresponse.SetETag(w, instance.ETag)
return dimensions, totalCount, nil
}

Expand Down Expand Up @@ -129,7 +130,7 @@ func (s *Store) GetUniqueDimensionAndOptionsHandler(w http.ResponseWriter, r *ht
}

log.Info(ctx, "successfully get unique dimension options for an instance resource", logData)
setETag(w, instance.ETag)
dpresponse.SetETag(w, instance.ETag)
return slicedOptions, totalCount, nil
}

Expand Down Expand Up @@ -168,7 +169,7 @@ func (s *Store) AddHandler(w http.ResponseWriter, r *http.Request) {
}
log.Info(ctx, "added dimension to instance resource", logData)

setETag(w, newETag)
dpresponse.SetETag(w, newETag)
}

// PatchDimensionsHandler represents adding multiple dimensions to a specific instance
Expand Down Expand Up @@ -208,7 +209,7 @@ func (s *Store) PatchDimensionsHandler(w http.ResponseWriter, r *http.Request) {

// set content type and write response body
setJSONPatchContentType(w)
setETag(w, newETag)
dpresponse.SetETag(w, newETag)
writeBody(ctx, w, b, logData)
log.Info(ctx, "successfully patched dimensions of an instance resource", logData)
}
Expand Down Expand Up @@ -433,7 +434,7 @@ func (s *Store) PatchOptionHandler(w http.ResponseWriter, r *http.Request) {

// set content type and write response body
setJSONPatchContentType(w)
setETag(w, newETag)
dpresponse.SetETag(w, newETag)
writeBody(ctx, w, b, logData)
log.Info(ctx, "successfully patched dimension option of an instance resource", logData)
}
Expand Down Expand Up @@ -511,5 +512,5 @@ func (s *Store) AddNodeIDHandler(w http.ResponseWriter, r *http.Request) {

logData["action"] = AddDimensionAction
log.Info(ctx, "added node id to dimension of an instance resource", logData)
setETag(w, newETag)
dpresponse.SetETag(w, newETag)
}
4 changes: 0 additions & 4 deletions dimension/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,6 @@ func setJSONPatchContentType(w http.ResponseWriter) {
w.Header().Add("Content-Type", "application/json-patch+json")
}

func setETag(w http.ResponseWriter, eTag string) {
w.Header().Add("ETag", eTag)
}

func writeBody(ctx context.Context, w http.ResponseWriter, b []byte, data log.Data) {
if _, err := w.Write(b); err != nil {
log.Error(ctx, "failed to write response body", err, data)
Expand Down
3 changes: 2 additions & 1 deletion instance/dimensions.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

errs "github.com/ONSdigital/dp-dataset-api/apierrors"
"github.com/ONSdigital/dp-dataset-api/models"
dpresponse "github.com/ONSdigital/dp-net/v2/handlers/response"
dphttp "github.com/ONSdigital/dp-net/v2/http"
"github.com/ONSdigital/log.go/v2/log"
"github.com/gorilla/mux"
Expand Down Expand Up @@ -107,5 +108,5 @@ func (s *Store) UpdateDimension(w http.ResponseWriter, r *http.Request) {

log.Info(ctx, "updated instance dimension: request successful", logData)

setETag(w, newETag)
dpresponse.SetETag(w, newETag)
}
3 changes: 2 additions & 1 deletion instance/event.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

errs "github.com/ONSdigital/dp-dataset-api/apierrors"
"github.com/ONSdigital/dp-dataset-api/models"
dpresponse "github.com/ONSdigital/dp-net/v2/handlers/response"
"github.com/ONSdigital/log.go/v2/log"
"github.com/gorilla/mux"
)
Expand Down Expand Up @@ -73,5 +74,5 @@ func (s *Store) AddEvent(w http.ResponseWriter, r *http.Request) {
}

log.Info(ctx, "add instance event: request successful", data)
setETag(w, newETag)
dpresponse.SetETag(w, newETag)
}
5 changes: 3 additions & 2 deletions instance/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

errs "github.com/ONSdigital/dp-dataset-api/apierrors"
"github.com/ONSdigital/dp-dataset-api/models"
dpresponse "github.com/ONSdigital/dp-net/v2/handlers/response"
dphttp "github.com/ONSdigital/dp-net/v2/http"
"github.com/ONSdigital/log.go/v2/log"
"github.com/gorilla/mux"
Expand Down Expand Up @@ -56,7 +57,7 @@ func (s *Store) UpdateObservations(w http.ResponseWriter, r *http.Request) {
}

log.Info(ctx, "update imported observations: request successful", logData)
setETag(w, newETag)
dpresponse.SetETag(w, newETag)
}

// UpdateImportTask updates any task in the request body against an instance
Expand Down Expand Up @@ -190,7 +191,7 @@ func (s *Store) UpdateImportTask(w http.ResponseWriter, r *http.Request) {
}

log.Info(ctx, "updateImportTask endpoint: request successful", logData)
setETag(w, eTag)
dpresponse.SetETag(w, eTag)
}

func unmarshalImportTasks(reader io.Reader) (*models.InstanceImportTasks, error) {
Expand Down
15 changes: 6 additions & 9 deletions instance/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,15 @@ import (
"github.com/ONSdigital/dp-dataset-api/models"
"github.com/ONSdigital/dp-dataset-api/mongo"
"github.com/ONSdigital/dp-dataset-api/store"
dpresponse "github.com/ONSdigital/dp-net/v2/handlers/response"
dphttp "github.com/ONSdigital/dp-net/v2/http"
"github.com/ONSdigital/log.go/v2/log"
"github.com/gorilla/mux"
"github.com/pkg/errors"
uuid "github.com/satori/go.uuid"
)

//Store provides a backend for instances
// Store provides a backend for instances
type Store struct {
store.Storer
Host string
Expand All @@ -40,7 +41,7 @@ func (e taskError) Error() string {
return ""
}

//GetList returns a list of instances, the total count of instances that match the query parameters and an error
// GetList returns a list of instances, the total count of instances that match the query parameters and an error
func (s *Store) GetList(w http.ResponseWriter, r *http.Request, limit int, offset int) (interface{}, int, error) {
ctx := r.Context()
stateFilterQuery := r.URL.Query().Get("state")
Expand Down Expand Up @@ -125,7 +126,7 @@ func (s *Store) Get(w http.ResponseWriter, r *http.Request) {
}

setJSONContentType(w)
setETag(w, instance.ETag)
dpresponse.SetETag(w, instance.ETag)
writeBody(ctx, w, b, logData)
log.Info(ctx, "get instance: request successful", logData)
}
Expand Down Expand Up @@ -169,7 +170,7 @@ func (s *Store) Add(w http.ResponseWriter, r *http.Request) {
}

setJSONContentType(w)
setETag(w, instance.ETag)
dpresponse.SetETag(w, instance.ETag)
w.WriteHeader(http.StatusCreated)
writeBody(ctx, w, b, logData)

Expand Down Expand Up @@ -305,7 +306,7 @@ func (s *Store) Update(w http.ResponseWriter, r *http.Request) {
}

setJSONContentType(w)
setETag(w, newETag)
dpresponse.SetETag(w, newETag)
w.WriteHeader(http.StatusOK)
writeBody(ctx, w, b, logData)

Expand Down Expand Up @@ -454,10 +455,6 @@ func getIfMatch(r *http.Request) string {
return ifMatch
}

func setETag(w http.ResponseWriter, eTag string) {
w.Header().Set("ETag", eTag)
}

func writeBody(ctx context.Context, w http.ResponseWriter, b []byte, logData log.Data) {
if _, err := w.Write(b); err != nil {
log.Fatal(ctx, "failed to write http response body", err, logData)
Expand Down