diff --git a/.circleci/config.yml b/.circleci/config.yml index ed6058f..f9407cc 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1,52 +1,7 @@ -version: 2 +version: 2.1 +orbs: + ft-golang-ci: financial-times/golang-ci@1 jobs: - build: - working_directory: /go/src/github.com/Financial-Times/draft-annotations-api - docker: - - image: golang:1 - environment: - GOPATH: /go - CIRCLE_TEST_REPORTS: /tmp/test-results - CIRCLE_COVERAGE_REPORT: /tmp/coverage-results - steps: - - checkout - - run: - name: External Dependencies - command: | - GO111MODULE=off go get -u github.com/mattn/goveralls - GO111MODULE=off go get -u github.com/jstemmer/go-junit-report - curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b $(go env GOPATH)/bin v1.18.0 - wget https://raw.githubusercontent.com/Financial-Times/upp-coding-standard/v1.0.0/golangci-config/.golangci.yml - - run: - name: Test Results - command: | - mkdir -p ${CIRCLE_TEST_REPORTS} - mkdir -p ${CIRCLE_COVERAGE_REPORT} - - run: - name: Go Build - command: go build -mod=readonly -v - - run: - name: Run linters - command: golangci-lint run --new-from-rev=master --config .golangci.yml - - run: - name: Run Tests - command: | - go test -mod=readonly -race -cover -coverprofile=${CIRCLE_COVERAGE_REPORT}/coverage.out ./... | go-junit-report > ${CIRCLE_TEST_REPORTS}/junit.xml - - run: - name: Upload Coverage - command: goveralls -coverprofile=${CIRCLE_COVERAGE_REPORT}/coverage.out -service=circle-ci -repotoken=${COVERALLS_TOKEN} - - store_test_results: - path: /tmp/test-results - dockerfile: - working_directory: /draft-content-api - docker: - - image: docker:18 - steps: - - checkout - - setup_docker_engine - - run: - name: Build Dockerfile - command: docker build . dredd: working_directory: /go/src/github.com/Financial-Times/draft-annotations-api docker: @@ -60,12 +15,12 @@ jobs: - image: peteclarkft/ersatz:stable steps: - checkout - - run: - name: Go Build - command: go build -mod=readonly -v - run: name: Load ersatz-fixtures.yml to ersatz image command: "curl -X POST --data-binary @_ft/ersatz-fixtures.yml -H \"Content-type: text/x-yaml\" http://localhost:9000/__configure" + - run: + name: Go Build + command: go build -mod=readonly -v - run: name: Download dredd command: | @@ -77,11 +32,17 @@ jobs: name: Dredd API Testing command: dredd workflows: - version: 2 test-and-build-docker: jobs: - - build + - ft-golang-ci/build-and-test: + name: build-and-test-project - dredd - - dockerfile: + - ft-golang-ci/docker-build: + name: build-docker-image requires: - - build + - build-and-test-project + snyk-scanning: + jobs: + - ft-golang-ci/scan: + name: scan-dependencies + context: cm-team-snyk diff --git a/annotations/annotations_api.go b/annotations/annotations_api.go index 6d4fe22..a8ff889 100644 --- a/annotations/annotations_api.go +++ b/annotations/annotations_api.go @@ -167,23 +167,23 @@ func (api *UPPAnnotationsAPI) GTG() error { apiReqURI := fmt.Sprintf(api.endpointTemplate, syntheticContentUUID) apiReq, err := http.NewRequest("GET", apiReqURI, nil) if err != nil { - return fmt.Errorf("gtg request error: %v", err.Error()) + return fmt.Errorf("GTG: %w", err) } apiReq.Header.Set(apiKeyHeader, api.apiKey) apiResp, err := api.httpClient.Do(apiReq) if err != nil { - return fmt.Errorf("gtg call error: %v", err.Error()) + return fmt.Errorf("GTG: %w", err) } defer apiResp.Body.Close() if apiResp.StatusCode != http.StatusOK { errMsgBody, err := ioutil.ReadAll(apiResp.Body) if err != nil { - return fmt.Errorf("gtg returned a non-200 HTTP status [%v]", apiResp.StatusCode) + return fmt.Errorf("status %d: %w", apiResp.StatusCode, ErrGTGNotOK) } - return fmt.Errorf("gtg returned a non-200 HTTP status [%v]: %v", apiResp.StatusCode, string(errMsgBody)) + return fmt.Errorf("status %d %s: %w", apiResp.StatusCode, string(errMsgBody), ErrGTGNotOK) } return nil } diff --git a/annotations/annotations_api_test.go b/annotations/annotations_api_test.go index d0f92b8..954a8e8 100644 --- a/annotations/annotations_api_test.go +++ b/annotations/annotations_api_test.go @@ -2,16 +2,18 @@ package annotations import ( "context" + "errors" "net" "net/http" "net/http/httptest" + "net/url" "testing" "time" "github.com/Financial-Times/go-ft-http/fthttp" tidUtils "github.com/Financial-Times/transactionid-utils-go" "github.com/husobee/vestigo" - "github.com/satori/go.uuid" + uuid "github.com/satori/go.uuid" "github.com/stretchr/testify/assert" ) @@ -34,7 +36,7 @@ func TestUnhappyAnnotationsAPIGTG(t *testing.T) { annotationsAPI := NewUPPAnnotationsAPI(testClient, annotationsServerMock.URL+"/content/%v/annotations", testAPIKey) err := annotationsAPI.GTG() - assert.EqualError(t, err, "gtg returned a non-200 HTTP status [503]: I am not happy!") + assert.True(t, errors.Is(err, ErrGTGNotOK)) } func TestAnnotationsAPIGTGWrongAPIKey(t *testing.T) { @@ -43,13 +45,15 @@ func TestAnnotationsAPIGTGWrongAPIKey(t *testing.T) { annotationsAPI := NewUPPAnnotationsAPI(testClient, annotationsServerMock.URL+"/content/%v/annotations", "a-non-existing-key") err := annotationsAPI.GTG() - assert.EqualError(t, err, "gtg returned a non-200 HTTP status [401]: unauthorized") + assert.True(t, errors.Is(err, ErrGTGNotOK)) } func TestAnnotationsAPIGTGInvalidURL(t *testing.T) { annotationsAPI := NewUPPAnnotationsAPI(testClient, ":#", testAPIKey) err := annotationsAPI.GTG() - assert.EqualError(t, err, "gtg request error: parse :: missing protocol scheme") + var urlErr *url.Error + assert.True(t, errors.As(err, &urlErr)) + assert.Equal(t, urlErr.Op, "parse") } func TestAnnotationsAPIGTGConnectionError(t *testing.T) { diff --git a/annotations/annotations_rw.go b/annotations/annotations_rw.go index f7706a0..584d593 100644 --- a/annotations/annotations_rw.go +++ b/annotations/annotations_rw.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io/ioutil" "net/http" @@ -32,6 +33,10 @@ func NewRW(client *http.Client, endpoint string) RW { return &annotationsRW{endpoint, client} } +var ErrUnexpectedStatusRead = errors.New("annotations RW returned an unexpected HTTP status code in read operation") +var ErrUnexpectedStatusWrite = errors.New("annotations RW returned an unexpected HTTP status code in write operation") +var ErrGTGNotOK = errors.New("gtg returned a non-200 HTTP status") + func (rw *annotationsRW) Read(ctx context.Context, contentUUID string) (*Annotations, string, bool, error) { tid, err := tidUtils.GetTransactionIDFromContext(ctx) @@ -72,7 +77,7 @@ func (rw *annotationsRW) Read(ctx context.Context, contentUUID string) (*Annotat case http.StatusNotFound: return nil, "", false, nil default: - return nil, "", false, fmt.Errorf("annotations RW returned an unexpected HTTP status code in read operation: %v", resp.StatusCode) + return nil, "", false, fmt.Errorf("status %d: %w", resp.StatusCode, ErrUnexpectedStatusRead) } } @@ -116,7 +121,7 @@ func (rw *annotationsRW) Write(ctx context.Context, contentUUID string, annotati newHash := resp.Header.Get(DocumentHashHeader) return newHash, nil default: - return "", fmt.Errorf("annotations RW returned an unexpected HTTP status code in write operation: %v", resp.StatusCode) + return "", fmt.Errorf("status %d: %w", resp.StatusCode, ErrUnexpectedStatusWrite) } } @@ -128,19 +133,22 @@ func (rw *annotationsRW) GTG() error { req, err := http.NewRequest("GET", rw.endpoint+"/__gtg", nil) if err != nil { log.WithError(err).Error("Error in creating the HTTP request to annotations RW GTG") - return fmt.Errorf("gtg HTTP request error: %v", err) + return fmt.Errorf("GTG: %w", err) } resp, err := rw.httpClient.Do(req) if err != nil { log.WithError(err).Error("Error making the HTTP request to annotations RW GTG") - return fmt.Errorf("gtg HTTP call error: %v", err) + return fmt.Errorf("GTG: %w", err) } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - body, _ := ioutil.ReadAll(resp.Body) - return fmt.Errorf("gtg returned unexpected status %v: %v", resp.StatusCode, string(body)) + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + return fmt.Errorf("status %d: %w", resp.StatusCode, ErrGTGNotOK) + } + return fmt.Errorf("status %d %s: %w", resp.StatusCode, string(body), ErrGTGNotOK) } return nil diff --git a/annotations/annotations_rw_test.go b/annotations/annotations_rw_test.go index 8034d05..7f6aa1d 100644 --- a/annotations/annotations_rw_test.go +++ b/annotations/annotations_rw_test.go @@ -2,10 +2,13 @@ package annotations import ( "context" + "encoding/json" + "errors" "io/ioutil" "net" "net/http" "net/http/httptest" + "net/url" "testing" "time" @@ -72,7 +75,7 @@ func TestReadAnnotationsNotFound(t *testing.T) { assert.False(t, found) } -func TestRead500Error(t *testing.T) { +func TestUnhappyReadStatus500(t *testing.T) { tid := tidUtils.NewTransactionID() s := newAnnotationsRWServerMock(t, http.MethodGet, http.StatusInternalServerError, "", "", "", tid) defer s.Close() @@ -80,7 +83,7 @@ func TestRead500Error(t *testing.T) { rw := NewRW(testClient, s.URL) ctx := tidUtils.TransactionAwareContext(context.Background(), tid) _, _, found, err := rw.Read(ctx, testContentUUID) - assert.EqualError(t, err, "annotations RW returned an unexpected HTTP status code in read operation: 500") + assert.True(t, errors.Is(err, ErrUnexpectedStatusRead)) assert.False(t, found) } @@ -90,7 +93,10 @@ func TestReadHTTPRequestError(t *testing.T) { rw := NewRW(testClient, ":#") ctx := tidUtils.TransactionAwareContext(context.Background(), tid) _, _, found, err := rw.Read(ctx, testContentUUID) - assert.EqualError(t, err, "parse :: missing protocol scheme") + + var urlError *url.Error + assert.True(t, errors.As(err, &urlError)) + assert.Equal(t, urlError.Op, "parse") assert.False(t, found) } @@ -100,7 +106,10 @@ func TestReadHTTPCallError(t *testing.T) { rw := NewRW(testClient, "") ctx := tidUtils.TransactionAwareContext(context.Background(), tid) _, _, found, err := rw.Read(ctx, testContentUUID) - assert.EqualError(t, err, "Get /drafts/content/db4daee0-2b84-465a-addb-fc8938a608db/annotations: unsupported protocol scheme \"\"") + + var urlError *url.Error + assert.True(t, errors.As(err, &urlError)) + assert.Equal(t, urlError.Op, "Get") assert.False(t, found) } @@ -112,7 +121,10 @@ func TestReadInvalidBodyError(t *testing.T) { rw := NewRW(testClient, s.URL) ctx := tidUtils.TransactionAwareContext(context.Background(), tid) _, _, found, err := rw.Read(ctx, testContentUUID) - assert.EqualError(t, err, "invalid character 'i' looking for beginning of object key string") + var jsonErr *json.SyntaxError + if assert.Error(t, err) { + assert.True(t, errors.As(err, &jsonErr)) + } assert.False(t, found) } @@ -171,7 +183,7 @@ func TestUnhappyWriteStatus500(t *testing.T) { rw := NewRW(testClient, s.URL) ctx := tidUtils.TransactionAwareContext(context.Background(), tid) _, err := rw.Write(ctx, testContentUUID, &expectedCanonicalizedAnnotations, oldHash) - assert.EqualError(t, err, "annotations RW returned an unexpected HTTP status code in write operation: 500") + assert.True(t, errors.Is(err, ErrUnexpectedStatusWrite)) } func TestWriteHTTPRequestError(t *testing.T) { @@ -180,7 +192,10 @@ func TestWriteHTTPRequestError(t *testing.T) { rw := NewRW(testClient, ":#") ctx := tidUtils.TransactionAwareContext(context.Background(), tid) _, err := rw.Write(ctx, testContentUUID, &expectedCanonicalizedAnnotations, oldHash) - assert.EqualError(t, err, "parse :: missing protocol scheme") + + var urlError *url.Error + assert.True(t, errors.As(err, &urlError)) + assert.Equal(t, urlError.Op, "parse") } func TestWriteHTTPCallError(t *testing.T) { @@ -189,7 +204,10 @@ func TestWriteHTTPCallError(t *testing.T) { rw := NewRW(testClient, "") ctx := tidUtils.TransactionAwareContext(context.Background(), tid) _, err := rw.Write(ctx, testContentUUID, &expectedCanonicalizedAnnotations, oldHash) - assert.EqualError(t, err, "Put /drafts/content/db4daee0-2b84-465a-addb-fc8938a608db/annotations: unsupported protocol scheme \"\"") + + var urlError *url.Error + assert.True(t, errors.As(err, &urlError)) + assert.Equal(t, urlError.Op, "Put") } func TestWriteMissingTID(t *testing.T) { @@ -264,13 +282,17 @@ func TestRWHappyGTG(t *testing.T) { func TestRWHTTPRequestErrorGTG(t *testing.T) { rw := NewRW(testClient, ":#") err := rw.GTG() - assert.EqualError(t, err, "gtg HTTP request error: parse :: missing protocol scheme") + var urlError *url.Error + assert.True(t, errors.As(err, &urlError)) + assert.Equal(t, urlError.Op, "parse") } func TestRWHTTPCallErrorGTG(t *testing.T) { rw := NewRW(testClient, "") err := rw.GTG() - assert.EqualError(t, err, "gtg HTTP call error: Get /__gtg: unsupported protocol scheme \"\"") + var urlError *url.Error + assert.True(t, errors.As(err, &urlError)) + assert.Equal(t, urlError.Op, "Get") } func TestRW503GTG(t *testing.T) { @@ -278,7 +300,7 @@ func TestRW503GTG(t *testing.T) { defer s.Close() rw := NewRW(testClient, s.URL) err := rw.GTG() - assert.EqualError(t, err, "gtg returned unexpected status 503: service unavailable") + assert.True(t, errors.Is(err, ErrGTGNotOK)) } func newAnnotationsRWGTGServerMock(t *testing.T, status int, body string) *httptest.Server { diff --git a/concept/read_api.go b/concept/read_api.go index 2d680a9..e63fa64 100644 --- a/concept/read_api.go +++ b/concept/read_api.go @@ -3,7 +3,9 @@ package concept import ( "context" "encoding/json" + "errors" "fmt" + "io/ioutil" "net/http" tidUtils "github.com/Financial-Times/transactionid-utils-go" @@ -32,6 +34,8 @@ func NewReadAPI(client *http.Client, endpoint string, apiKey string, batchSize i } } +var ErrUnexpectedResponse = errors.New("concept search API returned a non-200 HTTP status code") + func (search *internalConcordancesAPI) GetConceptsByIDs(ctx context.Context, conceptIDs []string) (map[string]Concept, error) { tid, err := tidUtils.GetTransactionIDFromContext(ctx) if err != nil { @@ -92,7 +96,12 @@ func (search *internalConcordancesAPI) searchConceptBatch(ctx context.Context, c defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - err = fmt.Errorf("concept search API returned a non-200 HTTP status code: %v", resp.StatusCode) + body, e := ioutil.ReadAll(resp.Body) + if e != nil { + err = fmt.Errorf("status %d: %w", resp.StatusCode, ErrUnexpectedResponse) + } else { + err = fmt.Errorf("status %d %s: %w", resp.StatusCode, string(body), ErrUnexpectedResponse) + } batchConceptsLog.WithError(err).Error("Error received from concept search API") return nil, err } diff --git a/concept/read_api_test.go b/concept/read_api_test.go index 6c89afc..677135c 100644 --- a/concept/read_api_test.go +++ b/concept/read_api_test.go @@ -3,10 +3,12 @@ package concept import ( "context" "encoding/json" + "errors" "fmt" "net" "net/http" "net/http/httptest" + "net/url" "testing" "time" @@ -14,7 +16,7 @@ import ( tidUtils "github.com/Financial-Times/transactionid-utils-go" "github.com/Pallinder/go-randomdata" "github.com/husobee/vestigo" - "github.com/satori/go.uuid" + uuid "github.com/satori/go.uuid" log "github.com/sirupsen/logrus" logTest "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/assert" @@ -93,7 +95,9 @@ func TestGetConceptsByIDsBuildingHTTPRequestError(t *testing.T) { ctx := tidUtils.TransactionAwareContext(context.Background(), tidUtils.NewTransactionID()) _, err := csAPI.GetConceptsByIDs(ctx, []string{"an-id"}) - assert.EqualError(t, err, "parse :: missing protocol scheme") + var urlError *url.Error + assert.True(t, errors.As(err, &urlError)) + assert.Equal(t, urlError.Op, "parse") } func TestGetConceptsByIDsHTTPCallError(t *testing.T) { @@ -105,7 +109,9 @@ func TestGetConceptsByIDsHTTPCallError(t *testing.T) { ctx := tidUtils.TransactionAwareContext(context.Background(), tidUtils.NewTransactionID()) _, err := csAPI.GetConceptsByIDs(ctx, []string{"an-id"}) - assert.EqualError(t, err, "Get ?ids=an-id: unsupported protocol scheme \"\"") + var urlError *url.Error + assert.True(t, errors.As(err, &urlError)) + assert.Equal(t, urlError.Op, "Get") } func TestGetConceptsByIDsNon200HTTPStatus(t *testing.T) { @@ -119,7 +125,7 @@ func TestGetConceptsByIDsNon200HTTPStatus(t *testing.T) { ctx := tidUtils.TransactionAwareContext(context.Background(), tidUtils.NewTransactionID()) _, err := csAPI.GetConceptsByIDs(ctx, []string{"an-id"}) - assert.EqualError(t, err, "concept search API returned a non-200 HTTP status code: 503") + assert.True(t, errors.Is(err, ErrUnexpectedResponse)) } func TestGetConceptsByIDsUnmarshallingPayloadError(t *testing.T) { @@ -133,7 +139,8 @@ func TestGetConceptsByIDsUnmarshallingPayloadError(t *testing.T) { ctx := tidUtils.TransactionAwareContext(context.Background(), tidUtils.NewTransactionID()) _, err := csAPI.GetConceptsByIDs(ctx, []string{"an-id"}) - assert.EqualError(t, err, "invalid character '}' looking for beginning of value") + var jsonErr *json.SyntaxError + assert.True(t, errors.As(err, &jsonErr)) } func TestHappyGTG(t *testing.T) { @@ -177,7 +184,7 @@ func TestUnhappyGTG(t *testing.T) { csAPI := NewReadAPI(testClient, s.URL, apiKey, batchSize) err := csAPI.GTG() - assert.EqualError(t, err, "concept search API returned a non-200 HTTP status code: 503") + assert.True(t, errors.Is(err, ErrUnexpectedResponse)) } func generateConcepts(n int) map[string]Concept { diff --git a/handler/handler_test.go b/handler/handler_test.go index 001b20c..ec424b5 100644 --- a/handler/handler_test.go +++ b/handler/handler_test.go @@ -688,8 +688,7 @@ func TestFetchFromAnnotationsAPIWithInvalidURL(t *testing.T) { assert.Equal(t, http.StatusInternalServerError, resp.StatusCode) assert.NoError(t, err) - assert.JSONEq(t, "{\"message\":\"Failed to read annotations: parse :: missing protocol scheme\"}", string(body)) - + assert.Contains(t, string(body), "Failed to read annotations") rw.AssertExpectations(t) aug.AssertExpectations(t) }