Merge pull request #45 from Financial-Times/feature/UPPSF-517-filter-…

…by-lifecycle

Add additional filtering by lifecycle

.circleci/config.yml

@@ -2,12 +2,13 @@ version: 2
 jobs:
   build:
     docker:
-      - image: circleci/golang:1.10
-      - image: neo4j:3.2.7-enterprise
+      - image: circleci/golang:1.12
+      - image: neo4j:3.4.10-enterprise
         environment:
           NEO4J_AUTH: none
           NEO4J_HEAP_MEMORY: 256
           NEO4J_CACHE_MEMORY: 256M
+          NEO4J_ACCEPT_LICENSE_AGREEMENT: "yes"
     working_directory: /go/src/github.com/Financial-Times/public-annotations-api
     environment:
       NEO4J_TEST_URL: "http://localhost:7474/db/data/"
@@ -36,7 +37,7 @@ jobs:
       - run:
           wget --retry-connrefused --no-check-certificate -T 60 $NEO4J_TEST_URL; curl $NEO4J_TEST_URL
       - run: |
-          go test ./... -v -race -cover -coverprofile=$CIRCLE_COVERAGE_REPORT/coverage.out | go-junit-report > $CIRCLE_TEST_REPORTS/junit.xml
+          go test ./... -v -tags=integration -race -cover -coverprofile=$CIRCLE_COVERAGE_REPORT/coverage.out | go-junit-report > $CIRCLE_TEST_REPORTS/junit.xml
       - run: |
           goveralls -coverprofile=$CIRCLE_COVERAGE_REPORT/coverage.out -service=circle-ci -repotoken=$COVERALLS_TOKEN
       - store_test_results:

Dockerfile

@@ -1,4 +1,4 @@
-FROM golang:1.10-alpine
+FROM golang:1.12-alpine
 
 ENV PROJECT="public-annotations-api"
 

Dockerfile.tests

@@ -0,0 +1,18 @@
+FROM golang:1.12
+
+ENV PROJECT=public-annotations-api
+ENV REPO_PATH="github.com/Financial-Times/${PROJECT}"
+
+COPY . $GOPATH/src/${REPO_PATH}
+
+RUN echo "Fetching dependencies..." \
+&& git clone https://github.com/vishnubob/wait-for-it.git \
+&& cd wait-for-it \
+&& mv ./wait-for-it.sh $GOPATH/src/${REPO_PATH} \
+&& curl https://raw.githubusercontent.com/golang/dep/master/install.sh | sh \
+&& cd $GOPATH/src/${REPO_PATH} \
+&& $GOPATH/bin/dep ensure -vendor-only
+
+WORKDIR $GOPATH/src/${REPO_PATH}
+
+ENTRYPOINT ["./wait-for-it.sh", "neo4j:7474", "-t", "60", "--"]

Gopkg.toml

@@ -1,30 +1,3 @@
-# Gopkg.toml example
-#
-# Refer to https://github.com/golang/dep/blob/master/docs/Gopkg.toml.md
-# for detailed Gopkg.toml documentation.
-#
-# required = ["github.com/user/thing/cmd/thing"]
-# ignored = ["github.com/user/project/pkgX", "bitbucket.org/user/project/pkgA/pkgY"]
-#
-# [[constraint]]
-#   name = "github.com/user/project"
-#   version = "1.0.0"
-#
-# [[constraint]]
-#   name = "github.com/user/project2"
-#   branch = "dev"
-#   source = "github.com/myfork/project2"
-#
-# [[override]]
-#   name = "github.com/x/y"
-#   version = "2.4.0"
-#
-# [prune]
-#   non-go = false
-#   go-tests = true
-#   unused-packages = true
-
-
 [[constraint]]
   name = "github.com/Financial-Times/annotations-rw-neo4j"
   version = "2.2.1"

README.md

@@ -8,7 +8,6 @@ __Provides a public API for Annotations stored in a Neo4J graph database__
 * `go get -u github.com/Financial-Times/public-annotations-api`
 * `cd $GOPATH/src/github.com/Financial-Times/public-annotations-api`
 * `dep ensure -vendor-only`
-* `go test ./...`
 * `go install`
 * `$GOPATH/bin/public-annotations-api --neo-url={neo4jUrl} --port={port} --log-level={DEBUG|INFO|WARN|ERROR}--cache-duration{e.g. 22h10m3s}`   
 _Optional arguments are:
@@ -18,8 +17,19 @@ _Optional arguments are:
 * `curl http://localhost:8080/content/143ba45c-2fb3-35bc-b227-a6ed80b5c517/annotations | json_pp`
 * Or using [httpie](https://github.com/jkbrzt/httpie) `http GET http://localhost:8080/content/143ba45c-2fb3-35bc-b227-a6ed80b5c517/annotations`
 
+## Testing
+
+* Run unit tests only: `go test -race ./...`
+* Run unit and integration tests: 
+
+    ```
+    docker-compose -f docker-compose-tests.yml up -d --build && \
+    docker logs -f test-runner && \
+    docker-compose -f docker-compose-tests.yml down
+    ```
+
 ## Build & deployment
-Continuosly built be CircleCI. The docker image of the service is built by Dockerhub based on the git release tag. 
+Continuosly built by CircleCI. The docker image of the service is built by Dockerhub based on the git release tag. 
 To prepare a new git release, go to the repo page on GitHub and create a new release.
 * Cluster deployment:  [public-annotations-api](https://upp-k8s-jenkins.in.ft.com/job/k8s-deployment/job/apps-deployment/job/public-annotations-api-auto-deploy/)
 * CI provided by CircleCI: [public-annotations-api](https://circleci.com/gh/Financial-Times/public-annotations-api)
@@ -36,8 +46,8 @@ Returns all annotations for a given uuid of a piece of content in json format.
 This is because it will return also the parent of the brands from any brands annotations. 
 If those brands have parents, then they too will be brought into the result.
 
-* the `public-annotations-api` uses annotations lifecyle to determine which annotations are returned. If curated (tag-me) annotations (life cycle pac) for a piece of content exist, they will be returned combined with V2 annotations by default, other non-pac lifecycle annotations are omitted.
-If there are no pac life cycle annotations, non-pac annotations will be returned. The filtering described in the next paragraph relates to non-pac annotations.
+* the `public-annotations-api` uses annotations lifecycle to determine which annotations are returned. If curated (tag-me) annotations (lifecycle pac) for a piece of content exist, they will be returned combined with V2 annotations by default, other non-pac lifecycle annotations are omitted.
+If there are no pac lifecycle annotations, non-pac annotations will be returned. The filtering described in the next paragraph relates to non-pac annotations. Additional filtering by annotations lifecycle could be applied using the optional "lifecycle" query parameter.
 
 * the `public-annotations-api` will filter out less important annotations if a more important annotation is also present for the same concept.  
 _For example_, if a piece of content is annotated with a concept with "About", "Major Mentions" and "Mentions" relationships 

_ft/api.yml

@@ -28,6 +28,17 @@ paths:
           required: true
           x-example: 59439611-a23a-38ae-8615-b35a80d4e6f1
           description: UUID of a piece of content
+        - name: lifecycle
+          in: query
+          type: array
+          items:
+            type: string
+            enum:
+              - next-video
+              - v1
+              - pac
+              - v2
+          required: false
       responses:
         200:
           description: Returns the annotations if they exists.
@@ -116,9 +127,9 @@ paths:
                   - http://www.ft.com/ontology/product/Brand
                   - prefLabel: Financial Times
         400:
-          description: Bad request if the uuid path parameter is  formatted formed or missing.
+          description: Bad request if the uuid path parameter is malformed or missing, or if the lifecycle query parameter value is not valid.
         404:
-          description: Not Found if there is no annotations record for the uuid path parameter is found.
+          description: Not Found if no annotations record for the uuid path parameter is found.
         500:
           description: Internal Server Error if there was an issue processing the records.
         503:

annotations/annotations_lifecycles_filter.go

@@ -5,18 +5,55 @@ const (
 	v2Lifecycle  = "annotations-v2"
 )
 
-type lifecycleFilter struct{}
+var lifecycleMap = map[string]string{
+	"next-video": "annotations-next-video",
+	"v1":         "annotations-v1",
+	"pac":        "annotations-pac",
+	"v2":         "annotations-v2",
+}
+
+type lifecycleFilter struct {
+	lifecycles []string
+}
+
+func newLifecycleFilter(opts ...func(*lifecycleFilter)) *lifecycleFilter {
+	lf := lifecycleFilter{}
+	for _, opt := range opts {
+		opt(&lf)
+	}
 
-func newLifecycleFilter() annotationsFilter {
-	return &lifecycleFilter{}
+	return &lf
+}
+
+func withLifecycles(lifecycles []string) func(*lifecycleFilter) {
+	return func(f *lifecycleFilter) {
+		f.lifecycles = lifecycles
+	}
 }
 
 func (f *lifecycleFilter) filter(annotations []annotation, chain *annotationsFilterChain) []annotation {
 	if containsPACLifecycle(annotations) {
 		filtered := filterPACAndV2Lifecycles(annotations)
-		return chain.doNext(filtered)
+		return chain.doNext(f.applyAdditionalFiltering(filtered))
+	}
+
+	return chain.doNext(f.applyAdditionalFiltering(annotations))
+}
+
+func (f *lifecycleFilter) applyAdditionalFiltering(annotations []annotation) []annotation {
+	if len(f.lifecycles) == 0 {
+		return annotations
 	}
-	return chain.doNext(annotations)
+
+	var filtered []annotation
+	for _, annotation := range annotations {
+		for _, lc := range f.lifecycles {
+			if annotation.Lifecycle == lifecycleMap[lc] {
+				filtered = append(filtered, annotation)
+			}
+		}
+	}
+	return filtered
 }
 
 func containsPACLifecycle(annotations []annotation) bool {

annotations/annotations_lifecycles_filter_test.go

@@ -6,6 +6,11 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+const (
+	v1Lifecycle        = "annotations-v1"
+	nextVideoLifecycle = "annotations-next-video"
+)
+
 var pacAnnotationA = annotation{
 	ID:        "6bbd0457-15ab-4ddc-ab82-0cd5b8d9ce18",
 	Predicate: ABOUT,
@@ -165,3 +170,87 @@ func TestFilterOnV1V2PACAnnotations(t *testing.T) {
 	assert.Contains(t, filtered, v2AnnotationA)
 	assert.Contains(t, filtered, v2AnnotationA)
 }
+
+func TestAdditionalFilteringOnV1V2PACAnnotations(t *testing.T) {
+	tests := map[string]struct {
+		lifecycles []string
+		expected   []annotation
+	}{
+		"additional pac filtering should return only pac annotations": {
+			lifecycles: []string{"pac"},
+			expected:   []annotation{pacAnnotationA, pacAnnotationB},
+		},
+		"additional v2 filtering should return only v2 annotations": {
+			lifecycles: []string{"v2"},
+			expected:   []annotation{v2AnnotationA, v2AnnotationB},
+		},
+		"additional v1 filtering should return nil": {
+			lifecycles: []string{"v1"},
+			expected:   nil,
+		},
+		"additional next-video filtering should return nil": {
+			lifecycles: []string{"next-video"},
+			expected:   nil,
+		},
+		"additional v1&next-video filtering should return nil": {
+			lifecycles: []string{"v1", "next-video"},
+			expected:   nil,
+		},
+		"additional pac&v2 filtering should return pac&v2 annotations": {
+			lifecycles: []string{"pac", "v2"},
+			expected:   []annotation{pacAnnotationA, pacAnnotationB, v2AnnotationA, v2AnnotationB},
+		},
+		"additional pac&v1&v2&next-video filtering should return pac&v2 annotations": {
+			lifecycles: []string{"pac", "v1", "v2", "next-video"},
+			expected:   []annotation{pacAnnotationA, pacAnnotationB, v2AnnotationA, v2AnnotationB},
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			annotations := []annotation{pacAnnotationA, pacAnnotationB, v1AnnotationA, v1AnnotationB, v2AnnotationA, v2AnnotationB}
+			f := newLifecycleFilter(withLifecycles(tc.lifecycles))
+			chain := newAnnotationsFilterChain(f)
+			filtered := chain.doNext(annotations)
+
+			assert.Len(t, filtered, len(tc.expected))
+			assert.Equal(t, filtered, tc.expected)
+		})
+	}
+}
+
+func TestAdditionalFilteringNoPACAnnotations(t *testing.T) {
+	tests := map[string]struct {
+		lifecycles []string
+		expected   []annotation
+	}{
+		"additional v1 filtering should return only v1 annotations": {
+			lifecycles: []string{"v1"},
+			expected:   []annotation{v1AnnotationA, v1AnnotationB},
+		},
+		"additional v2 filtering should return only v2 annotations": {
+			lifecycles: []string{"v2"},
+			expected:   []annotation{v2AnnotationA, v2AnnotationB},
+		},
+		"additional next-video filtering should return only next-video annotations": {
+			lifecycles: []string{"next-video"},
+			expected:   []annotation{nextVideoAnnotationA, nextVideoAnnotationB},
+		},
+		"additional v1&v2 filtering should return v1&v2 annotations": {
+			lifecycles: []string{"v1", "v2"},
+			expected:   []annotation{v1AnnotationA, v1AnnotationB, v2AnnotationA, v2AnnotationB},
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			annotations := []annotation{v1AnnotationA, v1AnnotationB, v2AnnotationA, v2AnnotationB, nextVideoAnnotationA, nextVideoAnnotationB}
+			f := newLifecycleFilter(withLifecycles(tc.lifecycles))
+			chain := newAnnotationsFilterChain(f)
+			filtered := chain.doNext(annotations)
+
+			assert.Len(t, filtered, len(tc.expected))
+			assert.Equal(t, filtered, tc.expected)
+		})
+	}
+}

annotations/cypher.go

@@ -129,11 +129,7 @@ func (cd cypherDriver) read(contentUUID string) (anns annotations, found bool, e
 		}
 	}
 
-	lifecycleFilter := newLifecycleFilter()
-	predicateFilter := NewAnnotationsPredicateFilter()
-
-	chain := newAnnotationsFilterChain(lifecycleFilter, predicateFilter)
-	return chain.doNext(mappedAnnotations), found, nil
+	return mappedAnnotations, found, nil
 }
 
 func mapToResponseFormat(neoAnn neoAnnotation, env string) (annotation, error) {

annotations/cypher_test.go

@@ -1,3 +1,5 @@
+// +build integration
+
 package annotations
 
 import (
@@ -54,8 +56,6 @@ const (
 
 	v1PlatformVersion    = "v1"
 	v2PlatformVersion    = "v2"
-	v1Lifecycle          = "annotations-v1"
-	nextVideoLifecycle   = "annotations-next-video"
 	emptyPlatformVersion = ""
 
 	brandType = "http://www.ft.com/ontology/product/Brand"
@@ -328,6 +328,7 @@ func TestRetrieveNoAnnotationsWhenThereAreNonePresentExceptBrands(t *testing.T)
 
 	driver := NewCypherDriver(db, "prod")
 	anns, found, err := driver.read(contentWithNoAnnotationsUUID)
+	anns = applyDefaultFilters(anns)
 	assert.NoError(err, "Unexpected error for content %s", contentWithNoAnnotationsUUID)
 	assert.False(found, "Found annotations for content %s", contentWithNoAnnotationsUUID)
 	assert.Equal(0, len(anns), "Didn't get the same number of annotations") // Two brands, child and parent
@@ -376,13 +377,15 @@ func TestRetrieveNoAnnotationsWhenThereAreNoConceptsPresent(t *testing.T) {
 
 	driver := NewCypherDriver(db, "prod")
 	anns, found, err := driver.read(contentUUID)
+	anns = applyDefaultFilters(anns)
 	assert.NoError(err, "Unexpected error for content %s", contentUUID)
 	assert.False(found, "Found annotations for content %s", contentUUID)
 	assert.Equal(0, len(anns), "Didn't get the same number of annotations, anns=%s", anns)
 }
 
 func getAndCheckAnnotations(driver cypherDriver, contentUUID string, t *testing.T) annotations {
 	anns, found, err := driver.read(contentUUID)
+	anns = applyDefaultFilters(anns)
 	assert.NoError(t, err, "Unexpected error for content %s", contentUUID)
 	assert.True(t, found, "Found no annotations for content %s", contentUUID)
 	return anns
@@ -735,3 +738,10 @@ func count(annotationLifecycle string, db neoutils.NeoConnection) (int, error) {
 	}
 	return results[0].Count, nil
 }
+
+func applyDefaultFilters(anns []annotation) []annotation {
+	lifecycleFilter := newLifecycleFilter()
+	predicateFilter := NewAnnotationsPredicateFilter()
+	chain := newAnnotationsFilterChain(lifecycleFilter, predicateFilter)
+	return chain.doNext(anns)
+}