Merge pull request #31 from Financial-Times/fix/restrict-predicates

Unsupported predicates
Financial-Times · Sep 8, 2017 · 4a3a669 · 4a3a669
2 parents d4b2b08 + 355cf3a
commit 4a3a669
Show file tree

Hide file tree

Showing 9 changed files with 222 additions and 108 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -0,0 +1,71 @@
+version: 2
+jobs:
+  build:
+    docker:
+      - image: circleci/golang:1.8
+      - image: neo4j:$NEO4J_VERSION
+        environment:
+          NEO4J_AUTH: none
+          NEO4J_HEAP_MEMORY: 256
+          NEO4J_CACHE_MEMORY: 256M
+
+    working_directory: /go/src/github.com/Financial-Times/annotations-rw-neo4j
+    environment:
+          NEO4J_TEST_URL: "http://localhost:7474/db/data/"
+          CIRCLE_TEST_REPORTS: /tmp/test-reports
+          CIRCLE_ARTIFACTS: /tmp/artifacts
+
+    steps:
+      - checkout
+      - run:
+          name: External Dependencies
+          command: |
+            go get github.com/mattn/goveralls
+            go get -u github.com/jstemmer/go-junit-report
+            go get -u github.com/kardianos/govendor
+      - run:
+          name: Create test folder
+          command: mkdir -p $CIRCLE_TEST_REPORTS
+      - run:
+          name: Govendor Sync
+          command: govendor sync -v
+      - run:
+          name: Go Build
+          command: go build -v
+      - run:
+          wget --retry-connrefused --no-check-certificate -T 60 $NEO4J_TEST_URL; curl $NEO4J_TEST_URL
+      - run:
+          wget https://raw.githubusercontent.com/Financial-Times/cookiecutter-upp-golang/master/coverage.sh && chmod +x coverage.sh
+      - run: |
+          mkdir -p $CIRCLE_TEST_REPORTS/golang
+          mkdir -p $CIRCLE_ARTIFACTS
+          govendor test -race -v +local | go-junit-report > $CIRCLE_TEST_REPORTS/golang/junit.xml
+          ./coverage.sh
+      - run: |
+          goveralls -coverprofile=$CIRCLE_ARTIFACTS/coverage.out -service=circle-ci -repotoken=$COVERALLS_TOKEN
+      - store_test_results:
+          path: /tmp/test-reports
+
+      - store_artifacts:
+          path: /tmp/artifacts
+          destination: build
+
+  docker_build:
+    working_directory: /annotations-rw-neo4j
+    docker:
+      - image: docker:1.12.6-git
+    steps:
+      - checkout
+      - setup_docker_engine
+      - run:
+          name: Build Dockerfile
+          command: docker build .
+
+workflows:
+  version: 2
+  tests_and_docker:
+    jobs:
+      - build
+      - docker_build:
+          requires:
+            - build
diff --git a/annotations/cypher.go b/annotations/cypher.go
@@ -2,6 +2,7 @@ package annotations
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"regexp"
 	"time"
@@ -14,6 +15,8 @@ import (
 
 var uuidExtractRegex = regexp.MustCompile(".*/([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})$")
 
+var UnsupportedPredicateErr = errors.New("Unsupported predicate")
+
 // Service interface. Compatible with the baserwftapp service EXCEPT for
 // 1) the Write function, which has signature Write(thing interface{}) error...
 // 2) the DecodeJson function, which has signature DecodeJSON(*json.Decoder) (thing interface{}, identity string, err error)
@@ -108,12 +111,13 @@ func (s service) Delete(contentUUID string, annotationLifecycle string) (bool, e
 
 //Write a set of annotations associated with a piece of content. Any annotations
 //already there will be removed
-func (s service) Write(contentUUID string, annotationLifecycle string, platformVersion string, tid string, thing interface{}) (err error) {
+func (s service) Write(contentUUID string, annotationLifecycle string, platformVersion string, tid string, thing interface{}) error {
 	annotationsToWrite := thing.(Annotations)
 
 	if contentUUID == "" {
 		return fmt.Errorf("%s Content uuid is required", tid)
 	}
+
 	if err := validateAnnotations(&annotationsToWrite); err != nil {
 		log.Warnf("%s Validation of supplied annotations failed", tid)
 		return err
@@ -134,10 +138,15 @@ func (s service) Write(contentUUID string, annotationLifecycle string, platformV
 		statements = append(statements, query.Statement)
 		queries = append(queries, query)
 	}
-	log.Infof("%s Updated Annotations for content uuid: %s", tid, contentUUID)
-	log.Debugf("%s For update, ran statements: %+v", tid, statements)
 
-	return s.conn.CypherBatch(queries)
+	log.Debugf("%s For update, running statements: %+v", tid, statements)
+	err := s.conn.CypherBatch(queries)
+	if err != nil {
+		return err
+	}
+
+	log.Infof("%s Updated Annotations for content uuid: %s", tid, contentUUID)
+	return nil
 }
 
 // Check tests neo4j by running a simple cypher query
@@ -184,13 +193,16 @@ func createAnnotationRelationship(relation string) (statement string) {
 	return statement
 }
 
-func getRelationshipFromPredicate(predicate string) (relation string) {
-	if predicate != "" {
-		relation = relations[predicate]
-	} else {
-		relation = relations["mentions"]
+func getRelationshipFromPredicate(predicate string) (string, error) {
+	if predicate == "" {
+		return relations["mentions"], nil
+	}
+
+	r, ok := relations[predicate]
+	if !ok {
+		return "", UnsupportedPredicateErr
 	}
-	return relation
+	return r, nil
 }
 
 func createAnnotationQuery(contentUUID string, ann Annotation, platformVersion string, annotationLifecycle string) (*neoism.CypherQuery, error) {
@@ -232,7 +244,11 @@ func createAnnotationQuery(contentUUID string, ann Annotation, platformVersion s
 		}
 	}
 
-	relation := getRelationshipFromPredicate(ann.Thing.Predicate)
+	relation, err := getRelationshipFromPredicate(ann.Thing.Predicate)
+	if err != nil {
+		return nil, err
+	}
+
 	query.Statement = createAnnotationRelationship(relation)
 	query.Parameters = map[string]interface{}{
 		"contentID":           contentUUID,

diff --git a/annotations/cypher_test.go b/annotations/cypher_test.go
@@ -20,11 +20,13 @@ const (
 	brandUUID                 = "8e21cbd4-e94b-497a-a43b-5b2309badeb3"
 	v2PlatformVersion         = "v2"
 	v1PlatformVersion         = "v1"
+	pacPlatformVersion        = "pac"
 	nextVideoPlatformVersion  = "next-video"
 	brightcovePlatformVersion = "brightcove"
 	contentLifecycle          = "content"
 	v2AnnotationLifecycle     = "annotations-v2"
 	v1AnnotationLifecycle     = "annotations-v1"
+	pacAnnotationLifecycle    = "annotations-pac"
 	tid                       = "transaction_id"
 )
 
@@ -99,6 +101,8 @@ func TestWriteDoesNotRemoveExistingIsClassifiedByBrandRelationshipsWithoutLifecy
 	}
 
 	err := annotationsDriver.conn.CypherBatch([]*neoism.CypherQuery{testSetupQuery})
+	assert.NoError(err)
+
 	annotationsToWrite := exampleConcepts(conceptUUID)
 
 	assert.NoError(annotationsDriver.Write(contentUUID, v2AnnotationLifecycle, v2PlatformVersion, tid, annotationsToWrite), "Failed to write annotation")
@@ -267,6 +271,12 @@ func TestWriteAndReadMultipleAnnotations(t *testing.T) {
 	cleanUp(t, contentUUID, v2AnnotationLifecycle, []string{conceptUUID, secondConceptUUID})
 }
 
+func TestWriteFailsForInvalidPredicate(t *testing.T) {
+	annotationsDriver = getAnnotationsService(t)
+	err := annotationsDriver.Write(contentUUID, v2AnnotationLifecycle, v2PlatformVersion, tid, Annotations{conceptWithInvalidPredicate})
+	assert.EqualError(t, err, "Unsupported predicate")
+}
+
 func TestIfProvenanceGetsWrittenWithEmptyAgentRoleAndTimeValues(t *testing.T) {
 	assert := assert.New(t)
 	annotationsDriver = getAnnotationsService(t)
@@ -351,7 +361,7 @@ func TestNextVideoDeleteCleansAlsoBrightcoveAnnotations(t *testing.T) {
 	assert.NoError(err, "Failed to delete annotation.")
 
 	result := []struct {
-		platformVersion string `json:"r.platformVersion"`
+		PlatformVersion string `json:"r.platformVersion"`
 	}{}
 
 	getContentQuery := &neoism.CypherQuery{
@@ -418,7 +428,9 @@ func TestGetRelationshipFromPredicate(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		actualRelationship := getRelationshipFromPredicate(test.predicate)
+		actualRelationship, err := getRelationshipFromPredicate(test.predicate)
+		assert.NoError(t, err)
+
 		if test.relationship != actualRelationship {
 			t.Errorf("\nExpected: %s\nActual: %s", test.relationship, actualRelationship)
 		}
@@ -431,8 +443,8 @@ func TestCreateAnnotationQueryWithPredicate(t *testing.T) {
 
 	query, err := createAnnotationQuery(contentUUID, annotationToWrite, v2AnnotationLifecycle, v2PlatformVersion)
 	assert.NoError(err, "Cypher query for creating annotations couldn't be created.")
-	assert.Contains(query.Statement, "IS_CLASSIFIED_BY", fmt.Sprintf("\nRelationship name is not inserted!"))
-	assert.NotContains(query.Statement, "MENTIONS", fmt.Sprintf("\nDefault relationship was inserted instead of IS_CLASSIFIED_BY!"))
+	assert.Contains(query.Statement, "IS_CLASSIFIED_BY", "Relationship name is not inserted!")
+	assert.NotContains(query.Statement, "MENTIONS", "IS_CLASSIFIED_BY should be inserted instead of MENTIONS")
 }
 
 func TestCreateAnnotationQueryWithAboutPredicate(t *testing.T) {
@@ -441,8 +453,8 @@ func TestCreateAnnotationQueryWithAboutPredicate(t *testing.T) {
 
 	query, err := createAnnotationQuery(contentUUID, annotationToWrite, v2AnnotationLifecycle, v2PlatformVersion)
 	assert.NoError(err, "Cypher query for creating annotations couldn't be created.")
-	assert.Contains(query.Statement, "ABOUT", fmt.Sprintf("\nRelationship name is not inserted!"))
-	assert.NotContains(query.Statement, "MENTIONS", fmt.Sprintf("\nDefault relationship was inserted instead of ABOUT!"))
+	assert.Contains(query.Statement, "ABOUT", "Relationship name is not inserted!")
+	assert.NotContains(query.Statement, "MENTIONS", "ABOUT should be inserted instead of MENTIONS")
 }
 
 func TestCreateAnnotationQueryWithHasAuthorPredicate(t *testing.T) {
@@ -451,8 +463,28 @@ func TestCreateAnnotationQueryWithHasAuthorPredicate(t *testing.T) {
 
 	query, err := createAnnotationQuery(contentUUID, annotationToWrite, v2AnnotationLifecycle, v2PlatformVersion)
 	assert.NoError(err, "Cypher query for creating annotations couldn't be created.")
-	assert.Contains(query.Statement, "HAS_AUTHOR", fmt.Sprintf("\nRelationship name is not inserted!"))
-	assert.NotContains(query.Statement, "MENTIONS", fmt.Sprintf("\nDefault relationship was inserted instead of HAS_AUTHOR!"))
+	assert.Contains(query.Statement, "HAS_AUTHOR", "Relationship name is not inserted!")
+	assert.NotContains(query.Statement, "MENTIONS", "HAS_AUTHOR should be inserted instead of MENTIONS")
+}
+
+func TestCreateAnnotationQueryWithHasContributorPredicate(t *testing.T) {
+	assert := assert.New(t)
+	annotationToWrite := conceptWithHasContributorPredicate
+
+	query, err := createAnnotationQuery(contentUUID, annotationToWrite, pacAnnotationLifecycle, pacPlatformVersion)
+	assert.NoError(err, "Cypher query for creating annotations couldn't be created.")
+	assert.Contains(query.Statement, "HAS_CONTRIBUTOR", "Relationship name is not inserted!")
+	assert.NotContains(query.Statement, "MENTIONS", "HAS_CONTRIBUTOR should be inserted instead of MENTIONS")
+}
+
+func TestCreateAnnotationQueryWithHasDisplayTagPredicate(t *testing.T) {
+	assert := assert.New(t)
+	annotationToWrite := conceptWithHasDisplayTagPredicate
+
+	query, err := createAnnotationQuery(contentUUID, annotationToWrite, pacAnnotationLifecycle, pacPlatformVersion)
+	assert.NoError(err, "Cypher query for creating annotations couldn't be created.")
+	assert.Contains(query.Statement, "HAS_DISPLAY_TAG", "Relationship name is not inserted!")
+	assert.NotContains(query.Statement, "MENTIONS", "HAS_DISPLAY_TAG should be inserted instead of MENTIONS")
 }
 
 func getAnnotationsService(t *testing.T) service {

diff --git a/annotations/example_data_test.go b/annotations/example_data_test.go
@@ -102,6 +102,70 @@ var (
 			},
 		},
 	}
+	conceptWithHasContributorPredicate = Annotation{
+		Thing: Thing{ID: getURI(oldConceptUUID),
+			PrefLabel: "prefLabel",
+			Types: []string{
+				"http://www.ft.com/ontology/person/Person",
+				"http://www.ft.com/ontology/core/Thing",
+				"http://www.ft.com/ontology/concept/Concept",
+			},
+			Predicate: "hasContributor",
+		},
+		Provenances: []Provenance{
+			{
+				Scores: []Score{
+					Score{ScoringSystem: relevanceScoringSystem, Value: 0.9},
+					Score{ScoringSystem: confidenceScoringSystem, Value: 0.8},
+				},
+				AgentRole: "http://api.ft.com/things/0edd3c31-1fd0-4ef6-9230-8d545be3880a",
+				AtTime:    "2016-01-01T19:43:47.314Z",
+			},
+		},
+	}
+	conceptWithInvalidPredicate = Annotation{
+		Thing: Thing{ID: getURI(oldConceptUUID),
+			PrefLabel: "prefLabel",
+			Types: []string{
+				"http://www.ft.com/ontology/person/Person",
+				"http://www.ft.com/ontology/core/Thing",
+				"http://www.ft.com/ontology/concept/Concept",
+			},
+			Predicate: "hasAFakePredicate",
+		},
+		Provenances: []Provenance{
+			{
+				Scores: []Score{
+					Score{ScoringSystem: relevanceScoringSystem, Value: 0.9},
+					Score{ScoringSystem: confidenceScoringSystem, Value: 0.8},
+				},
+				AgentRole: "http://api.ft.com/things/0edd3c31-1fd0-4ef6-9230-8d545be3880a",
+				AtTime:    "2016-01-01T19:43:47.314Z",
+			},
+		},
+	}
+
+	conceptWithHasDisplayTagPredicate = Annotation{
+		Thing: Thing{ID: getURI(oldConceptUUID),
+			PrefLabel: "prefLabel",
+			Types: []string{
+				"http://www.ft.com/ontology/person/Person",
+				"http://www.ft.com/ontology/core/Thing",
+				"http://www.ft.com/ontology/concept/Concept",
+			},
+			Predicate: "hasDisplayTag",
+		},
+		Provenances: []Provenance{
+			{
+				Scores: []Score{
+					Score{ScoringSystem: relevanceScoringSystem, Value: 0.9},
+					Score{ScoringSystem: confidenceScoringSystem, Value: 0.8},
+				},
+				AgentRole: "http://api.ft.com/things/0edd3c31-1fd0-4ef6-9230-8d545be3880a",
+				AtTime:    "2016-01-01T19:43:47.314Z",
+			},
+		},
+	}
 	multiConceptAnnotations = Annotations{Annotation{
 		Thing: Thing{ID: getURI(conceptUUID),
 			PrefLabel: "prefLabel",

diff --git a/annotations/model.go b/annotations/model.go
@@ -42,4 +42,6 @@ var relations = map[string]string{
 	"isPrimarilyClassifiedBy": "IS_PRIMARILY_CLASSIFIED_BY",
 	"majorMentions":           "MAJOR_MENTIONS",
 	"hasAuthor":               "HAS_AUTHOR",
+	"hasContributor":          "HAS_CONTRIBUTOR",
+	"hasDisplayTag":           "HAS_DISPLAY_TAG",
 }