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

UPPSF-2526 Remove call to internal content API #54

Merged
merged 3 commits into from
Jan 25, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 0 additions & 29 deletions cmd/content-rw-elasticsearch/main.go
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
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,3 @@ service:
env:
KAFKA_TOPIC: ForcedCombinedPostPublicationEvents
PUBLIC_CONCORDANCES_ENDPOINT: http://public-concordances-api:8080
INTERNAL_CONTENT_API_URL: http://internal-content-api:8080
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,3 @@ service:
env:
KAFKA_TOPIC: CombinedPostPublicationEvents
PUBLIC_CONCORDANCES_ENDPOINT: http://public-concordances-api:8080
INTERNAL_CONTENT_API_URL: http://internal-content-api:8080
2 changes: 0 additions & 2 deletions helm/content-rw-elasticsearch/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,6 @@ spec:
key: aws.secret_access_key
- name: PUBLIC_CONCORDANCES_ENDPOINT
value: "{{ .Values.env.PUBLIC_CONCORDANCES_ENDPOINT }}"
- name: INTERNAL_CONTENT_API_URL
value: "{{ .Values.env.INTERNAL_CONTENT_API_URL }}"
- name: "BASE_API_URL"
valueFrom:
configMapKeyRef:
Expand Down
1 change: 0 additions & 1 deletion helm/content-rw-elasticsearch/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,4 @@ resources:
env:
KAFKA_TOPIC: ""
PUBLIC_CONCORDANCES_ENDPOINT: ""
INTERNAL_CONTENT_API_URL: ""
ELASTICSEARCH_SAPI_INDEX: "ft-v1"
33 changes: 14 additions & 19 deletions pkg/mapper/mapper.go
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].APIURL == "" {
log.Warnf("Main image is not present or with empty apiUrl.")
} else {
uris := strings.Split(ic.MainImage.Members[0].APIURL, "/")
uris := strings.Split(enrichedContent.InternalContent.MainImage.Members[0].APIURL, "/")
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 apiUrl %q", enrichedContent.InternalContent.MainImage.Members[0].APIURL)
}
}
}
Expand Down
62 changes: 2 additions & 60 deletions pkg/mapper/mapper_test.go
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
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
28 changes: 21 additions & 7 deletions pkg/schema/schema.go
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,17 @@ 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 {
APIURL string `json:"apiUrl"`
}
2 changes: 1 addition & 1 deletion test/testdata/exampleElasticModel.json
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
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
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