Skip to content

Commit

Permalink
Removed call to internal content API for mainImage. Updated tests and…
Browse files Browse the repository at this point in the history 
… removed dependency on upp-go-sdk.
  • Loading branch information
@vstanchev
vstanchev committed Jan 21, 2022
1 parent a853e74 commit 3c9548c
Show file tree
Hide file tree
Showing 13 changed files with 184 additions and 133 deletions.
29 changes: 0 additions & 29 deletions cmd/content-rw-elasticsearch/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ import (
"github.com/Financial-Times/content-rw-elasticsearch/v2/pkg/message"
"github.com/Financial-Times/go-logger/v2"
"github.com/Financial-Times/kafka-client-go/v2"
"github.com/Financial-Times/upp-go-sdk/pkg/api"
"github.com/Financial-Times/upp-go-sdk/pkg/internalcontent"
cli "github.com/jawher/mow.cli"
)

Expand Down Expand Up @@ -99,27 +97,6 @@ func main() {
EnvVar: "BASE_API_URL",
})

internalContentAPIURL := app.String(cli.StringOpt{
Name: "internal-content-api-url",
Value: "http://internal-content-api:8080",
Desc: "URL of the API uses to retrieve lists data from",
EnvVar: "INTERNAL_CONTENT_API_URL",
})

apiBasicAuthUsername := app.String(cli.StringOpt{
Name: "api-basic-auth-user",
Value: "",
Desc: "API Basic Auth username",
EnvVar: "API_BASIC_USER",
})

apiBasicAuthPassword := app.String(cli.StringOpt{
Name: "api-basic-auth-pass",
Value: "",
Desc: "API Basic Auth password",
EnvVar: "API_BASIC_PASS",
})

log := logger.NewUPPLogger(*appSystemCode, *logLevel)
log.Info("[Startup] Application is starting")

Expand All @@ -141,17 +118,11 @@ func main() {

concordanceAPIService := concept.NewConcordanceAPIService(*publicConcordancesEndpoint, httpClient)

// initialize apiClient
internalAPIConfig := api.NewConfig(*internalContentAPIURL, *apiBasicAuthUsername, *apiBasicAuthPassword)
internalContentAPIClient := api.NewClient(*internalAPIConfig, httpClient)
internalContentClient := internalcontent.NewContentClient(internalContentAPIClient, internalcontent.URLInternalContent)

mapperHandler := mapper.NewMapperHandler(
concordanceAPIService,
*baseAPIUrl,
appConfig,
log,
internalContentClient,
)

consumerConfig := kafka.ConsumerConfig{
Expand Down
1 change: 0 additions & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ require (
github.com/Financial-Times/kafka-client-go/v2 v2.0.0
github.com/Financial-Times/service-status-go v0.0.0-20160323111542-3f5199736a3d
github.com/Financial-Times/transactionid-utils-go v0.2.0
github.com/Financial-Times/upp-go-sdk v0.0.7
github.com/gorilla/mux v1.8.0
github.com/jawher/mow.cli v1.0.4
github.com/pborman/uuid v0.0.0-20170612153648-e790cca94e6c
Expand Down
2 changes: 0 additions & 2 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,6 @@ github.com/Financial-Times/service-status-go v0.0.0-20160323111542-3f5199736a3d
github.com/Financial-Times/service-status-go v0.0.0-20160323111542-3f5199736a3d/go.mod h1:7zULC9rrq6KxFkpB3Y5zNVaEwrf1g2m3dvXJBPDXyvM=
github.com/Financial-Times/transactionid-utils-go v0.2.0 h1:YcET5Hd1fUGWWpQSVszYUlAc15ca8tmjRetUuQKRqEQ=
github.com/Financial-Times/transactionid-utils-go v0.2.0/go.mod h1:tPAcAFs/dR6Q7hBDGNyUyixHRvg/n9NW/JTq8C58oZ0=
github.com/Financial-Times/upp-go-sdk v0.0.7 h1:SzF7gvABbYuHGjNl4Macv4fHF1CSrUBByQSHogiQk9I=
github.com/Financial-Times/upp-go-sdk v0.0.7/go.mod h1:1/Dnsqf8FQ0yOHKG8d/eM3X7rCeqDK4pouCFtp9pdmo=
github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU=
github.com/Shopify/sarama v1.30.0 h1:TOZL6r37xJBDEMLx4yjB77jxbZYXPaDow08TSK6vIL0=
github.com/Shopify/sarama v1.30.0/go.mod h1:zujlQQx1kzHsh4jfV1USnptCQrHAEZ2Hk8fTKCulPVs=
Expand Down
33 changes: 14 additions & 19 deletions pkg/mapper/mapper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,11 @@ import (
"strings"
"time"

"github.com/Financial-Times/go-logger/v2"
"github.com/Financial-Times/upp-go-sdk/pkg/internalcontent"

"github.com/Financial-Times/content-rw-elasticsearch/v2/pkg/concept"
"github.com/Financial-Times/content-rw-elasticsearch/v2/pkg/config"
"github.com/Financial-Times/content-rw-elasticsearch/v2/pkg/html"
"github.com/Financial-Times/content-rw-elasticsearch/v2/pkg/schema"
"github.com/Financial-Times/go-logger/v2"
)

const (
Expand All @@ -32,22 +30,20 @@ const (
)

type Handler struct {
ConceptReader concept.Reader
BaseAPIURL string
Config config.AppConfig
log *logger.UPPLogger
internalClient *internalcontent.ContentClient
ConceptReader concept.Reader
BaseAPIURL string
Config config.AppConfig
log *logger.UPPLogger
}

var errNoAnnotation = errors.New("no annotation to be processed")

func NewMapperHandler(reader concept.Reader, baseAPIURL string, appConfig config.AppConfig, logger *logger.UPPLogger, internalClient *internalcontent.ContentClient) *Handler {
func NewMapperHandler(reader concept.Reader, baseAPIURL string, appConfig config.AppConfig, logger *logger.UPPLogger) *Handler {
return &Handler{
ConceptReader: reader,
BaseAPIURL: baseAPIURL,
Config: appConfig,
log: logger,
internalClient: internalClient,
ConceptReader: reader,
BaseAPIURL: baseAPIURL,
Config: appConfig,
log: logger,
}
}

Expand Down Expand Up @@ -225,16 +221,15 @@ func (h *Handler) populateContentRelatedFields(model *schema.IndexModel, enriche

log := h.log.WithTransactionID(tid).WithUUID(enrichedContent.UUID)

ic, err := h.internalClient.GetContent(enrichedContent.UUID, true)
if err != nil || len(ic.MainImage.Members) == 0 || ic.MainImage.Members[0].APIURL == "" {
log.WithError(err).Warnf("Couldn't get image UUID from %s", internalcontent.URLInternalContent)
if len(enrichedContent.InternalContent.MainImage.Members) == 0 || enrichedContent.InternalContent.MainImage.Members[0].ID == "" {
log.Warnf("Main image is not present or with empty ID.")
} else {
uris := strings.Split(ic.MainImage.Members[0].APIURL, "/")
uris := strings.Split(enrichedContent.InternalContent.MainImage.Members[0].ID, "/")
if len(uris) > 0 {
imageUUID := uris[len(uris)-1]
*model.ThumbnailURL = strings.Replace(imageServiceURL, imagePlaceholder, imageUUID, -1)
} else {
log.WithError(err).Warnf("Couldn't get image UUID from %s", internalcontent.URLInternalContent)
log.Warnf("Couldn't get image UUID from ID %q", enrichedContent.InternalContent.MainImage.Members[0].ID)
}
}
}
Expand Down
62 changes: 2 additions & 60 deletions pkg/mapper/mapper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,18 @@ package mapper

import (
"encoding/json"
"fmt"
"net/http"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"

"github.com/Financial-Times/go-logger/v2"
"github.com/Financial-Times/upp-go-sdk/pkg/api"
"github.com/Financial-Times/upp-go-sdk/pkg/internalcontent"

"github.com/Financial-Times/content-rw-elasticsearch/v2/pkg/concept"
"github.com/Financial-Times/content-rw-elasticsearch/v2/pkg/config"
"github.com/Financial-Times/content-rw-elasticsearch/v2/pkg/schema"
tst "github.com/Financial-Times/content-rw-elasticsearch/v2/test"
"github.com/Financial-Times/go-logger/v2"
)

type concordanceAPIMock struct {
Expand All @@ -30,50 +25,6 @@ func (m *concordanceAPIMock) GetConcepts(tid string, ids []string) (map[string]c
return args.Get(0).(map[string]concept.Model), args.Error(1)
}

var mainImageContent = `{"mainImage": {
"apiUrl": "https://test.api.ft.com/content/ad038207-bfe6-4805-a04c-864af12efef2",
"description": "Traffic on the M4 motorway near Datchet, Berkshire, on Monday",
"id": "https://test.api.ft.com/content/ad038207-bfe6-4805-a04c-864af12efef2",
"lastModified": "2020-04-27T10:50:44.186Z",
"members": [
{
"apiUrl": "https://test.api.ft.com/content/5546cbc4-d4f7-47f9-3f3e-941fb0799c4f",
"binaryUrl": "https://d1e00ek4ebabms.cloudfront.net/production/5546cbc4-d4f7-47f9-3f3e-941fb0799c4f.jpg",
"copyright": {
"notice": "© PA"
},
"description": "Traffic on the M4 motorway near Datchet, Berkshire, on Monday",
"firstPublishedDate": "2020-04-27T10:50:35.897Z",
"id": "https://test.api.ft.com/content/f93fd066-380e-4f68-be63-c27f1fe2fddc",
"identifiers": [
{
"authority": "http://api.ft.com/system/cct",
"identifierValue": "f93fd066-380e-4f68-be63-c27f1fe2fddc"
}
],
"lastModified": "2020-04-27T10:50:44.182Z",
"publishReference": "tid_cct_image_5546cbc4-d4f7-47f9-3f3e-941fb0799c4f_1587984635897",
"publishedDate": "2020-04-27T10:50:35.897Z",
"title": "Admiral announced last week that it would issue customers with a £25 refund per vehicle insured because lockdown restrictions mean fewer people are driving",
"type": "http://www.ft.com/ontology/content/Image"
}
],
"publishReference": "tid_cct_imageSet_ad038207-bfe6-4805-a04c-864af12efef2_1587984635898",
"publishedDate": "2020-04-27T10:50:35.897Z",
"type": "http://www.ft.com/ontology/content/ImageSet"
}}`

type clientMock struct {
sendRequestF func(req *api.Request) (*api.Response, error)
}

func (m *clientMock) SendRequest(req *api.Request) (*api.Response, error) {
if m.sendRequestF != nil {
return m.sendRequestF(req)
}
return nil, fmt.Errorf("not implemented")
}

func TestConvertToESContentModel(t *testing.T) {
expect := assert.New(t)

Expand Down Expand Up @@ -120,17 +71,8 @@ func TestConvertToESContentModel(t *testing.T) {
log.Fatal(err)
}
concordanceAPIMock := new(concordanceAPIMock)
clientAPIMock := &clientMock{
sendRequestF: func(req *api.Request) (*api.Response, error) {
return &api.Response{
StatusCode: http.StatusOK,
Body: mainImageContent,
}, nil
},
}

internalContentAPIClient := internalcontent.NewContentClient(clientAPIMock, "")
mapperHandler := NewMapperHandler(concordanceAPIMock, "http://api.ft.com", appConfig, log, internalContentAPIClient)
mapperHandler := NewMapperHandler(concordanceAPIMock, "http://api.ft.com", appConfig, log)

for _, test := range tests {
if test.inputFileConcordanceModel != "" {
Expand Down
12 changes: 3 additions & 9 deletions pkg/message/message_handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ import (
tst "github.com/Financial-Times/content-rw-elasticsearch/v2/test"
"github.com/Financial-Times/go-logger/v2"
"github.com/Financial-Times/kafka-client-go/v2"
"github.com/Financial-Times/upp-go-sdk/pkg/api"
"github.com/Financial-Times/upp-go-sdk/pkg/internalcontent"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"gopkg.in/olivere/elastic.v2"
Expand Down Expand Up @@ -144,11 +142,7 @@ func mockMessageHandler(esClient ESClient, mocks ...interface{}) (es.AccessConfi
}
}

internalAPIConfig := api.NewConfig("internalContentAPIURL", "apiBasicAuthUsername", "apiBasicAuthPassword")
internalContentAPIClient := api.NewClient(*internalAPIConfig, http.DefaultClient)
internalContentClient := internalcontent.NewContentClient(internalContentAPIClient, internalcontent.URLInternalContent)

mapperHandler := mockMapperHandler(concordanceAPI, uppLogger, internalContentClient)
mapperHandler := mockMapperHandler(concordanceAPI, uppLogger)

var handler *Handler

Expand All @@ -161,9 +155,9 @@ func mockMessageHandler(esClient ESClient, mocks ...interface{}) (es.AccessConfi
return accessConfig, handler
}

func mockMapperHandler(concordanceAPIMock *concordanceAPIMock, log *logger.UPPLogger, internalClient *internalcontent.ContentClient) *mapper.Handler {
func mockMapperHandler(concordanceAPIMock *concordanceAPIMock, log *logger.UPPLogger) *mapper.Handler {
appConfig := initAppConfig()
mapperHandler := mapper.NewMapperHandler(concordanceAPIMock, "http://api.ft.com", appConfig, log, internalClient)
mapperHandler := mapper.NewMapperHandler(concordanceAPIMock, "http://api.ft.com", appConfig, log)
return mapperHandler
}

Expand Down
40 changes: 33 additions & 7 deletions pkg/schema/schema.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,13 @@ type IndexModel struct {
}

type EnrichedContent struct {
UUID string `json:"uuid"`
Content Content `json:"content"`
Metadata Annotations `json:"metadata"`

ContentURI string `json:"contentUri"`
LastModified string `json:"lastModified"`
Deleted bool `json:"deleted"`
UUID string `json:"uuid"`
Content Content `json:"content"`
Metadata Annotations `json:"metadata"`
InternalContent InternalContent `json:"internalContent"`
ContentURI string `json:"contentUri"`
LastModified string `json:"lastModified"`
Deleted bool `json:"deleted"`
}

type Content struct {
Expand Down Expand Up @@ -149,3 +149,29 @@ type ContentType struct {
Format string
Category string
}

type InternalContent struct {
MainImage ImageSet `json:"mainImage"`
Embeds []ImageSet `json:"embeds"`
}

type ImageSet struct {
ID string `json:"id"`
Members []Member `json:"members"`
}

type Member struct {
ID string `json:"id"`
Type string `json:"type"`
Title string `json:"title"`
BinaryURL string `json:"binaryUrl"`
Description string `json:"description"`
CanBeSyndicated string `json:"canBeSyndicated"`
CanBeDistributed string `json:"canBeDistributed"`
RightsGroup string `json:"rightsGroup"`
Copyright struct {
Notice string `json:"notice"`
} `json:"copyright"`
MaxDisplayWidth string `json:"maxDisplayWidth"`
MinDisplayWidth string `json:"minDisplayWidth"`
}
2 changes: 1 addition & 1 deletion test/testdata/exampleElasticModel.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"provider_name": null,
"length_millis": 0,
"short_description": "Agitators are far from perfect, but they can play key role in spurring long-term thinking",
"thumbnail_url": "https://www.ft.com/__origami/service/image/v2/images/raw/http%3A%2F%2Fprod-upp-image-read.ft.com%2F5546cbc4-d4f7-47f9-3f3e-941fb0799c4f?source=search&fit=scale-down&width=167",
"thumbnail_url": "https://www.ft.com/__origami/service/image/v2/images/raw/http%3A%2F%2Fprod-upp-image-read.ft.com%2F4831d979-377a-4f86-b1a2-97665e493f16?source=search&fit=scale-down&width=167",
"section_link": null,
"secondary_image_id": null,
"contributor_rights": null,
Expand Down
44 changes: 43 additions & 1 deletion test/testdata/exampleEnrichedContentModel.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,5 +206,47 @@
],
"contentUri": "http://methode-article-mapper.svc.ft.com/content/aae9611e-f66c-4fe4-a6c6-2e2bdea69060",
"lastModified": "2018-04-04T12:58:00.347Z",
"deleted": false
"deleted": false,
"internalContent": {
"mainImage": {
"apiUrl": "https://api-t.ft.com/content/41f6b287-0460-4df4-96ec-820fc3b1f628",
"brands": [
"http://www.ft.com/thing/dbb0bdae-1f0c-11e4-b0cb-b2227cce2b54"
],
"canBeSyndicated": "verify",
"description": "A man looks desperate on the phone",
"id": "https://api-t.ft.com/content/41f6b287-0460-4df4-96ec-820fc3b1f628",
"lastModified": "2022-01-18T16:01:05.061Z",
"members": [
{
"apiUrl": "https://api-t.ft.com/content/4831d979-377a-4f86-b1a2-97665e493f16",
"binaryUrl": "https://d1e00ek4ebabms.cloudfront.net/production/4831d979-377a-4f86-b1a2-97665e493f16.jpg",
"brands": [
"http://www.ft.com/thing/dbb0bdae-1f0c-11e4-b0cb-b2227cce2b54"
],
"canBeSyndicated": "verify",
"copyright": {
"notice": "© Courtesy of Netflix"
},
"description": "A man looks desperate on the phone",
"firstPublishedDate": "2022-01-18T16:00:59.077Z",
"id": "https://api-t.ft.com/content/4831d979-377a-4f86-b1a2-97665e493f16",
"identifiers": [
{
"authority": "http://api.ft.com/system/cct",
"identifierValue": "4831d979-377a-4f86-b1a2-97665e493f16"
}
],
"lastModified": "2022-01-18T16:01:05.063Z",
"publishReference": "tid_cct_image_4831d979-377a-4f86-b1a2-97665e493f16_1642521659077",
"publishedDate": "2022-01-18T16:00:59.077Z",
"title": "Jason Bateman as the preternaturally calm Marty",
"type": "http://www.ft.com/ontology/content/Image"
}
],
"publishReference": "tid_cct_imageSet_41f6b287-0460-4df4-96ec-820fc3b1f628_1642521659078",
"publishedDate": "2022-01-18T16:00:59.078Z",
"type": "http://www.ft.com/ontology/content/ImageSet"
}
}
}
2 changes: 1 addition & 1 deletion test/testdata/testElasticModel1.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"provider_name": null,
"length_millis": 0,
"short_description": "Founder of news aggregation app promises to respect ‘socialist values’",
"thumbnail_url": "https://www.ft.com/__origami/service/image/v2/images/raw/http%3A%2F%2Fprod-upp-image-read.ft.com%2F5546cbc4-d4f7-47f9-3f3e-941fb0799c4f?source=search&fit=scale-down&width=167",
"thumbnail_url": "https://www.ft.com/__origami/service/image/v2/images/raw/http%3A%2F%2Fprod-upp-image-read.ft.com%2F4831d979-377a-4f86-b1a2-97665e493f16?source=search&fit=scale-down&width=167",
"section_link": null,
"secondary_image_id": null,
"contributor_rights": null,
Expand Down
2 changes: 1 addition & 1 deletion test/testdata/testElasticModel4.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"provider_name": null,
"length_millis": 130388,
"short_description": "Ut enim ad minim veniam, quis nostrud exercitation",
"thumbnail_url": "https://www.ft.com/__origami/service/image/v2/images/raw/http%3A%2F%2Fprod-upp-image-read.ft.com%2F5546cbc4-d4f7-47f9-3f3e-941fb0799c4f?source=search\u0026fit=scale-down\u0026width=167",
"thumbnail_url": "https://www.ft.com/__origami/service/image/v2/images/raw/http%3A%2F%2Fprod-upp-image-read.ft.com%2F4831d979-377a-4f86-b1a2-97665e493f16?source=search\u0026fit=scale-down\u0026width=167",
"section_link": null,
"secondary_image_id": null,
"contributor_rights": null,
Expand Down

0 comments on commit 3c9548c

Please sign in to comment.