From c9a9e629beb858eb8e27beca510d230d0481176c Mon Sep 17 00:00:00 2001 From: bpathak-ons Date: Thu, 7 Apr 2022 13:49:38 +0100 Subject: [PATCH 01/29] rename transformer as legacy transformer updates --- service/service.go | 4 ++-- transformer/transformer.go | 18 +++++++++--------- transformer/transformer_test.go | 2 +- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/service/service.go b/service/service.go index 458b1148..ea932d88 100644 --- a/service/service.go +++ b/service/service.go @@ -29,7 +29,7 @@ type Service struct { router *mux.Router server HTTPServer serviceList *ExternalServiceList - transformer *transformer.Transformer + transformer *transformer.LegacyTransformer } // SetServer sets the http server for a service @@ -53,7 +53,7 @@ func (svc *Service) SetElasticSearchClient(elasticSearchClient elasticsearch.Cli } // SetTransformer sets the transformer for a service -func (svc *Service) SetTransformer(transformerClient *transformer.Transformer) { +func (svc *Service) SetTransformer(transformerClient *transformer.LegacyTransformer) { svc.transformer = transformerClient } diff --git a/transformer/transformer.go b/transformer/transformer.go index eb607367..611a6787 100644 --- a/transformer/transformer.go +++ b/transformer/transformer.go @@ -10,7 +10,7 @@ import ( ) // Transformer represents an instance of the ResponseTransformer interface -type Transformer struct { +type LegacyTransformer struct { higlightReplacer *strings.Replacer } @@ -153,15 +153,15 @@ type ESSearchSuggestOptions struct { } // New returns a new instance of Transformer -func New() *Transformer { +func New() *LegacyTransformer { highlightReplacer := strings.NewReplacer("", "", "", "") - return &Transformer{ + return &LegacyTransformer{ higlightReplacer: highlightReplacer, } } // TransformSearchResponse transforms an elastic search response into a structure that matches the v1 api specification -func (t *Transformer) TransformSearchResponse(ctx context.Context, responseData []byte, query string, highlight bool) ([]byte, error) { +func (t *LegacyTransformer) TransformSearchResponse(ctx context.Context, responseData []byte, query string, highlight bool) ([]byte, error) { var source ESResponse err := json.Unmarshal(responseData, &source) @@ -188,7 +188,7 @@ func (t *Transformer) TransformSearchResponse(ctx context.Context, responseData return transformedData, nil } -func (t *Transformer) transform(source *ESResponse, highlight bool) SearchResponse { +func (t *LegacyTransformer) transform(source *ESResponse, highlight bool) SearchResponse { sr := SearchResponse{ Count: source.Responses[0].Hits.Total, Items: []ContentItem{}, @@ -213,7 +213,7 @@ func (t *Transformer) transform(source *ESResponse, highlight bool) SearchRespon return sr } -func (t *Transformer) buildContentItem(doc ESResponseHit, highlight bool) ContentItem { +func (t *LegacyTransformer) buildContentItem(doc ESResponseHit, highlight bool) ContentItem { ci := ContentItem{ Description: t.buildDescription(doc, highlight), Type: doc.Source.Type, @@ -223,7 +223,7 @@ func (t *Transformer) buildContentItem(doc ESResponseHit, highlight bool) Conten return ci } -func (t *Transformer) buildDescription(doc ESResponseHit, highlight bool) description { +func (t *LegacyTransformer) buildDescription(doc ESResponseHit, highlight bool) description { sd := doc.Source.Description hl := doc.Highlight @@ -262,14 +262,14 @@ func (t *Transformer) buildDescription(doc ESResponseHit, highlight bool) descri return des } -func (t *Transformer) overlaySingleItem(hl *[]string, def string, highlight bool) (overlaid string) { +func (t *LegacyTransformer) overlaySingleItem(hl *[]string, def string, highlight bool) (overlaid string) { if highlight && hl != nil && len(*hl) > 0 { overlaid = (*hl)[0] } return } -func (t *Transformer) overlayItemList(hlList, defaultList *[]string, highlight bool) *[]string { +func (t *LegacyTransformer) overlayItemList(hlList, defaultList *[]string, highlight bool) *[]string { if defaultList == nil || hlList == nil { return nil } diff --git a/transformer/transformer_test.go b/transformer/transformer_test.go index 5acaf5f0..b8bf859f 100644 --- a/transformer/transformer_test.go +++ b/transformer/transformer_test.go @@ -9,7 +9,7 @@ import ( . "github.com/smartystreets/goconvey/convey" ) -func TestTransform(t *testing.T) { +func TestLegacyTransformer(t *testing.T) { Convey("Transforms unmarshalled search responses successfully", t, func() { transformer := New() Convey("Zero suggestions creates empty array", func() { From 1e0990684f729127172eaedc97209ebdd875d18c Mon Sep 17 00:00:00 2001 From: bpathak-ons Date: Thu, 7 Apr 2022 14:09:44 +0100 Subject: [PATCH 02/29] introduce new model folder for data models used --- features/steps/steps.go | 4 +- models/esmodels.go | 139 +++++++++++++++++++++ transformer/releasetransformer_test.go | 3 +- transformer/transformer.go | 163 ++----------------------- transformer/transformer_test.go | 55 +++++---- 5 files changed, 184 insertions(+), 180 deletions(-) create mode 100644 models/esmodels.go diff --git a/features/steps/steps.go b/features/steps/steps.go index 68d9afb7..6d01edb9 100644 --- a/features/steps/steps.go +++ b/features/steps/steps.go @@ -6,7 +6,7 @@ import ( "io" "os" - "github.com/ONSdigital/dp-search-api/transformer" + "github.com/ONSdigital/dp-search-api/models" "github.com/cucumber/godog" "github.com/google/go-cmp/cmp" ) @@ -97,7 +97,7 @@ func (c *Component) failureInternalServerError() error { } func (c *Component) iShouldReceiveTheFollowingSearchResponse(expectedJSONFile string) error { - var searchResponse, expectedSearchResponse transformer.SearchResponse + var searchResponse, expectedSearchResponse models.SearchResponse responseBody, err := io.ReadAll(c.APIFeature.HttpResponse.Body) if err != nil { diff --git a/models/esmodels.go b/models/esmodels.go new file mode 100644 index 00000000..9bca9930 --- /dev/null +++ b/models/esmodels.go @@ -0,0 +1,139 @@ +package models + +// Structs representing the transformed response +type SearchResponse struct { + Count int `json:"count"` + Took int `json:"took"` + ContentTypes []ContentType `json:"content_types"` + Items []ContentItem `json:"items"` + Suggestions []string `json:"suggestions,omitempty"` + AdditionSuggestions []string `json:"additional_suggestions,omitempty"` +} + +type ContentType struct { + Type string `json:"type"` + Count int `json:"count"` +} + +type ContentItem struct { + Description Description `json:"description"` + Type string `json:"type"` + URI string `json:"uri"` +} + +type Description struct { + Contact *contact `json:"contact,omitempty"` + DatasetID string `json:"dataset_id,omitempty"` + Edition string `json:"edition,omitempty"` + Headline1 string `json:"headline1,omitempty"` + Headline2 string `json:"headline2,omitempty"` + Headline3 string `json:"headline3,omitempty"` + Highlight *HighlightObj `json:"highlight,omitempty"` + Keywords *[]string `json:"keywords,omitempty"` + LatestRelease *bool `json:"latest_release,omitempty"` + Language string `json:"language,omitempty"` + MetaDescription string `json:"meta_description,omitempty"` + NationalStatistic *bool `json:"national_statistic,omitempty"` + NextRelease string `json:"next_release,omitempty"` + PreUnit string `json:"pre_unit,omitempty"` + ReleaseDate string `json:"release_date,omitempty"` + Source string `json:"source,omitempty"` + Summary string `json:"summary"` + Title string `json:"title"` + Unit string `json:"unit,omitempty"` +} + +type contact struct { + Name string `json:"name"` + Telephone string `json:"telephone,omitempty"` + Email string `json:"email"` +} + +type HighlightObj struct { + DatasetID string `json:"dataset_id,omitempty"` + Edition string `json:"edition,omitempty"` + Keywords *[]string `json:"keywords,omitempty"` + MetaDescription string `json:"meta_description,omitempty"` + Summary string `json:"summary,omitempty"` + Title string `json:"title,omitempty"` +} + +// Structs representing the raw elastic search response + +type ESResponse struct { + Responses []ESResponseItem `json:"responses"` +} + +type ESResponseItem struct { + Took int `json:"took"` + Hits ESResponseHits `json:"hits"` + Aggregations ESResponseAggregations `json:"aggregations"` + Suggest ESSuggest `json:"suggest"` +} + +type ESResponseHits struct { + Total int + Hits []ESResponseHit `json:"hits"` +} + +type ESResponseHit struct { + Source ESSourceDocument `json:"_source"` + Highlight ESHighlight `json:"highlight"` +} + +type ESSourceDocument struct { + Description struct { + Summary string `json:"summary"` + NextRelease string `json:"nextRelease,omitempty"` + Unit string `json:"unit,omitempty"` + Keywords *[]string `json:"keywords,omitempty"` + ReleaseDate string `json:"releaseDate,omitempty"` + Edition string `json:"edition,omitempty"` + LatestRelease *bool `json:"latestRelease,omitempty"` + Language string `json:"language,omitempty"` + Contact *contact `json:"contact,omitempty"` + DatasetID string `json:"datasetId,omitempty"` + Source string `json:"source,omitempty"` + Title string `json:"title"` + MetaDescription string `json:"metaDescription,omitempty"` + NationalStatistic *bool `json:"nationalStatistic,omitempty"` + PreUnit string `json:"preUnit,omitempty"` + Headline1 string `json:"headline1,omitempty"` + Headline2 string `json:"headline2,omitempty"` + Headline3 string `json:"headline3,omitempty"` + } `json:"description"` + Type string `json:"type"` + URI string `json:"uri"` +} + +type ESHighlight struct { + DescriptionTitle *[]string `json:"description.title"` + DescriptionEdition *[]string `json:"description.edition"` + DescriptionSummary *[]string `json:"description.summary"` + DescriptionMeta *[]string `json:"description.metaDescription"` + DescriptionKeywords *[]string `json:"description.keywords"` + DescriptionDatasetID *[]string `json:"description.datasetId"` +} + +type ESResponseAggregations struct { + DocCounts struct { + Buckets []ESBucket `json:"buckets"` + } `json:"docCounts"` +} + +type ESBucket struct { + Key string `json:"key"` + Count int `json:"doc_count"` +} + +type ESSuggest struct { + SearchSuggest []ESSearchSuggest `json:"search_suggest"` +} + +type ESSearchSuggest struct { + Options []ESSearchSuggestOptions `json:"options"` +} + +type ESSearchSuggestOptions struct { + Text string `json:"text"` +} diff --git a/transformer/releasetransformer_test.go b/transformer/releasetransformer_test.go index 9f0ea7dd..43c03e2f 100644 --- a/transformer/releasetransformer_test.go +++ b/transformer/releasetransformer_test.go @@ -6,6 +6,7 @@ import ( "os" "testing" + "github.com/ONSdigital/dp-search-api/models" . "github.com/smartystreets/goconvey/convey" ) @@ -47,7 +48,7 @@ func TestTransformSearchReleaseResponse(t *testing.T) { actual, err := transformer.TransformSearchResponse(ctx, sampleResponse, "test-query", false) So(err, ShouldBeNil) So(actual, ShouldNotBeEmpty) - var exp, act SearchResponse + var exp, act models.SearchResponse So(json.Unmarshal(expected, &exp), ShouldBeNil) So(json.Unmarshal(actual, &act), ShouldBeNil) So(act, ShouldResemble, exp) diff --git a/transformer/transformer.go b/transformer/transformer.go index 611a6787..53d5792a 100644 --- a/transformer/transformer.go +++ b/transformer/transformer.go @@ -6,6 +6,7 @@ import ( "regexp" "strings" + "github.com/ONSdigital/dp-search-api/models" "github.com/pkg/errors" ) @@ -14,144 +15,6 @@ type LegacyTransformer struct { higlightReplacer *strings.Replacer } -// Structs representing the transformed response -type SearchResponse struct { - Count int `json:"count"` - Took int `json:"took"` - ContentTypes []ContentType `json:"content_types"` - Items []ContentItem `json:"items"` - Suggestions []string `json:"suggestions,omitempty"` - AdditionSuggestions []string `json:"additional_suggestions,omitempty"` -} - -type ContentType struct { - Type string `json:"type"` - Count int `json:"count"` -} - -type ContentItem struct { - Description description `json:"description"` - Type string `json:"type"` - URI string `json:"uri"` -} - -type description struct { - Contact *contact `json:"contact,omitempty"` - DatasetID string `json:"dataset_id,omitempty"` - Edition string `json:"edition,omitempty"` - Headline1 string `json:"headline1,omitempty"` - Headline2 string `json:"headline2,omitempty"` - Headline3 string `json:"headline3,omitempty"` - Highlight *highlightObj `json:"highlight,omitempty"` - Keywords *[]string `json:"keywords,omitempty"` - LatestRelease *bool `json:"latest_release,omitempty"` - Language string `json:"language,omitempty"` - MetaDescription string `json:"meta_description,omitempty"` - NationalStatistic *bool `json:"national_statistic,omitempty"` - NextRelease string `json:"next_release,omitempty"` - PreUnit string `json:"pre_unit,omitempty"` - ReleaseDate string `json:"release_date,omitempty"` - Source string `json:"source,omitempty"` - Summary string `json:"summary"` - Title string `json:"title"` - Unit string `json:"unit,omitempty"` -} - -type contact struct { - Name string `json:"name"` - Telephone string `json:"telephone,omitempty"` - Email string `json:"email"` -} - -type highlightObj struct { - DatasetID string `json:"dataset_id,omitempty"` - Edition string `json:"edition,omitempty"` - Keywords *[]string `json:"keywords,omitempty"` - MetaDescription string `json:"meta_description,omitempty"` - Summary string `json:"summary,omitempty"` - Title string `json:"title,omitempty"` -} - -// Structs representing the raw elastic search response - -type ESResponse struct { - Responses []ESResponseItem `json:"responses"` -} - -type ESResponseItem struct { - Took int `json:"took"` - Hits ESResponseHits `json:"hits"` - Aggregations ESResponseAggregations `json:"aggregations"` - Suggest ESSuggest `json:"suggest"` -} - -type ESResponseHits struct { - Total int - Hits []ESResponseHit `json:"hits"` -} - -type ESResponseHit struct { - Source ESSourceDocument `json:"_source"` - Highlight ESHighlight `json:"highlight"` -} - -type ESSourceDocument struct { - Description struct { - Summary string `json:"summary"` - NextRelease string `json:"nextRelease,omitempty"` - Unit string `json:"unit,omitempty"` - Keywords *[]string `json:"keywords,omitempty"` - ReleaseDate string `json:"releaseDate,omitempty"` - Edition string `json:"edition,omitempty"` - LatestRelease *bool `json:"latestRelease,omitempty"` - Language string `json:"language,omitempty"` - Contact *contact `json:"contact,omitempty"` - DatasetID string `json:"datasetId,omitempty"` - Source string `json:"source,omitempty"` - Title string `json:"title"` - MetaDescription string `json:"metaDescription,omitempty"` - NationalStatistic *bool `json:"nationalStatistic,omitempty"` - PreUnit string `json:"preUnit,omitempty"` - Headline1 string `json:"headline1,omitempty"` - Headline2 string `json:"headline2,omitempty"` - Headline3 string `json:"headline3,omitempty"` - } `json:"description"` - Type string `json:"type"` - URI string `json:"uri"` -} - -type ESHighlight struct { - DescriptionTitle *[]string `json:"description.title"` - DescriptionEdition *[]string `json:"description.edition"` - DescriptionSummary *[]string `json:"description.summary"` - DescriptionMeta *[]string `json:"description.metaDescription"` - DescriptionKeywords *[]string `json:"description.keywords"` - DescriptionDatasetID *[]string `json:"description.datasetId"` -} - -type ESResponseAggregations struct { - DocCounts struct { - Buckets []ESBucket `json:"buckets"` - } `json:"docCounts"` -} - -type ESBucket struct { - Key string `json:"key"` - Count int `json:"doc_count"` -} - -type ESSuggest struct { - SearchSuggest []ESSearchSuggest `json:"search_suggest"` -} - -type ESSearchSuggest struct { - Options []ESSearchSuggestOptions `json:"options"` -} - -type ESSearchSuggestOptions struct { - Text string `json:"text"` -} - // New returns a new instance of Transformer func New() *LegacyTransformer { highlightReplacer := strings.NewReplacer("", "", "", "") @@ -162,7 +25,7 @@ func New() *LegacyTransformer { // TransformSearchResponse transforms an elastic search response into a structure that matches the v1 api specification func (t *LegacyTransformer) TransformSearchResponse(ctx context.Context, responseData []byte, query string, highlight bool) ([]byte, error) { - var source ESResponse + var source models.ESResponse err := json.Unmarshal(responseData, &source) if err != nil { @@ -188,11 +51,11 @@ func (t *LegacyTransformer) TransformSearchResponse(ctx context.Context, respons return transformedData, nil } -func (t *LegacyTransformer) transform(source *ESResponse, highlight bool) SearchResponse { - sr := SearchResponse{ +func (t *LegacyTransformer) transform(source *models.ESResponse, highlight bool) models.SearchResponse { + sr := models.SearchResponse{ Count: source.Responses[0].Hits.Total, - Items: []ContentItem{}, - ContentTypes: []ContentType{}, + Items: []models.ContentItem{}, + ContentTypes: []models.ContentType{}, } var took int for _, response := range source.Responses { @@ -213,8 +76,8 @@ func (t *LegacyTransformer) transform(source *ESResponse, highlight bool) Search return sr } -func (t *LegacyTransformer) buildContentItem(doc ESResponseHit, highlight bool) ContentItem { - ci := ContentItem{ +func (t *LegacyTransformer) buildContentItem(doc models.ESResponseHit, highlight bool) models.ContentItem { + ci := models.ContentItem{ Description: t.buildDescription(doc, highlight), Type: doc.Source.Type, URI: doc.Source.URI, @@ -223,11 +86,11 @@ func (t *LegacyTransformer) buildContentItem(doc ESResponseHit, highlight bool) return ci } -func (t *LegacyTransformer) buildDescription(doc ESResponseHit, highlight bool) description { +func (t *LegacyTransformer) buildDescription(doc models.ESResponseHit, highlight bool) models.Description { sd := doc.Source.Description hl := doc.Highlight - des := description{ + des := models.Description{ Summary: sd.Summary, NextRelease: sd.NextRelease, Unit: sd.Unit, @@ -249,7 +112,7 @@ func (t *LegacyTransformer) buildDescription(doc ESResponseHit, highlight bool) } if highlight { - des.Highlight = &highlightObj{ + des.Highlight = &models.HighlightObj{ DatasetID: t.overlaySingleItem(hl.DescriptionDatasetID, sd.DatasetID, highlight), Edition: t.overlaySingleItem(hl.DescriptionEdition, sd.Edition, highlight), Keywords: t.overlayItemList(hl.DescriptionKeywords, sd.Keywords, highlight), @@ -290,8 +153,8 @@ func (t *LegacyTransformer) overlayItemList(hlList, defaultList *[]string, highl return &overlaid } -func buildContentTypes(bucket ESBucket) ContentType { - return ContentType{ +func buildContentTypes(bucket models.ESBucket) models.ContentType { + return models.ContentType{ Type: bucket.Key, Count: bucket.Count, } diff --git a/transformer/transformer_test.go b/transformer/transformer_test.go index b8bf859f..6c76f281 100644 --- a/transformer/transformer_test.go +++ b/transformer/transformer_test.go @@ -6,6 +6,7 @@ import ( "os" "testing" + "github.com/ONSdigital/dp-search-api/models" . "github.com/smartystreets/goconvey/convey" ) @@ -13,11 +14,11 @@ func TestLegacyTransformer(t *testing.T) { Convey("Transforms unmarshalled search responses successfully", t, func() { transformer := New() Convey("Zero suggestions creates empty array", func() { - es := ESResponse{ - Responses: []ESResponseItem{ESResponseItem{ - Suggest: ESSuggest{ - SearchSuggest: []ESSearchSuggest{ESSearchSuggest{ - Options: []ESSearchSuggestOptions{}, + es := models.ESResponse{ + Responses: []models.ESResponseItem{models.ESResponseItem{ + Suggest: models.ESSuggest{ + SearchSuggest: []models.ESSearchSuggest{models.ESSearchSuggest{ + Options: []models.ESSearchSuggestOptions{}, }}, }, }}, @@ -27,12 +28,12 @@ func TestLegacyTransformer(t *testing.T) { }) Convey("One suggestion creates a populated array", func() { - es := ESResponse{ - Responses: []ESResponseItem{ESResponseItem{ - Suggest: ESSuggest{ - SearchSuggest: []ESSearchSuggest{ESSearchSuggest{ - Options: []ESSearchSuggestOptions{ - ESSearchSuggestOptions{Text: "option1"}, + es := models.ESResponse{ + Responses: []models.ESResponseItem{models.ESResponseItem{ + Suggest: models.ESSuggest{ + SearchSuggest: []models.ESSearchSuggest{models.ESSearchSuggest{ + Options: []models.ESSearchSuggestOptions{ + models.ESSearchSuggestOptions{Text: "option1"}, }, }}, }, @@ -44,23 +45,23 @@ func TestLegacyTransformer(t *testing.T) { So(sr.Suggestions[0], ShouldResemble, "option1") }) Convey("Multiple suggestions creates a populated array incorrect order", func() { - es := ESResponse{ - Responses: []ESResponseItem{ESResponseItem{ - Suggest: ESSuggest{ - SearchSuggest: []ESSearchSuggest{ - ESSearchSuggest{ - Options: []ESSearchSuggestOptions{ - ESSearchSuggestOptions{Text: "option1"}, + es := models.ESResponse{ + Responses: []models.ESResponseItem{models.ESResponseItem{ + Suggest: models.ESSuggest{ + SearchSuggest: []models.ESSearchSuggest{ + models.ESSearchSuggest{ + Options: []models.ESSearchSuggestOptions{ + models.ESSearchSuggestOptions{Text: "option1"}, }, }, - ESSearchSuggest{ - Options: []ESSearchSuggestOptions{ - ESSearchSuggestOptions{Text: "option2"}, + models.ESSearchSuggest{ + Options: []models.ESSearchSuggestOptions{ + models.ESSearchSuggestOptions{Text: "option2"}, }, }, - ESSearchSuggest{ - Options: []ESSearchSuggestOptions{ - ESSearchSuggestOptions{Text: "option3"}, + models.ESSearchSuggest{ + Options: []models.ESSearchSuggestOptions{ + models.ESSearchSuggestOptions{Text: "option3"}, }, }, }, @@ -127,7 +128,7 @@ func TestTransformSearchResponse(t *testing.T) { actual, err := transformer.TransformSearchResponse(ctx, sampleResponse, "test-query", true) So(err, ShouldBeNil) So(actual, ShouldNotBeEmpty) - var exp, act SearchResponse + var exp, act models.SearchResponse So(json.Unmarshal(expected, &exp), ShouldBeNil) So(json.Unmarshal(actual, &act), ShouldBeNil) So(act, ShouldResemble, exp) @@ -142,7 +143,7 @@ func TestTransformSearchResponse(t *testing.T) { actual, err := transformer.TransformSearchResponse(ctx, sampleResponse, "test-query", false) So(err, ShouldBeNil) So(actual, ShouldNotBeEmpty) - var exp, act SearchResponse + var exp, act models.SearchResponse So(json.Unmarshal(expected, &exp), ShouldBeNil) So(json.Unmarshal(actual, &act), ShouldBeNil) So(act, ShouldResemble, exp) @@ -157,7 +158,7 @@ func TestTransformSearchResponse(t *testing.T) { actual, err := transformer.TransformSearchResponse(ctx, sampleResponse, "test query \"with quote marks\"", false) So(err, ShouldBeNil) So(actual, ShouldNotBeEmpty) - var exp, act SearchResponse + var exp, act models.SearchResponse So(json.Unmarshal(expected, &exp), ShouldBeNil) So(json.Unmarshal(actual, &act), ShouldBeNil) So(act, ShouldResemble, exp) From b30ac5e6953365473def15de8efd9778815f7361 Mon Sep 17 00:00:00 2001 From: bpathak-ons Date: Thu, 7 Apr 2022 14:17:21 +0100 Subject: [PATCH 03/29] further refactoring for legacy transformer updates --- service/service.go | 2 +- templates/search/releasecalendar/main.go | 2 +- transformer/transformer.go | 6 +++--- transformer/transformer_test.go | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/service/service.go b/service/service.go index ea932d88..7eadab08 100644 --- a/service/service.go +++ b/service/service.go @@ -62,7 +62,7 @@ func Run(ctx context.Context, cfg *config.Config, serviceList *ExternalServiceLi elasticHTTPClient := dphttp.NewClient() // Initialise transformerClient - transformerClient := transformer.New() + transformerClient := transformer.NewLegacy() // Initialse AWS signer if cfg.SignElasticsearchRequests { diff --git a/templates/search/releasecalendar/main.go b/templates/search/releasecalendar/main.go index b94c8ec4..14749270 100644 --- a/templates/search/releasecalendar/main.go +++ b/templates/search/releasecalendar/main.go @@ -83,7 +83,7 @@ func main() { log.Fatalf("failed to format multi query: %s", err) } esSearch = esClient.MultiSearch - esTransformer = transformer.New() + esTransformer = transformer.NewLegacy() } fmt.Printf("\nformatted query is:\n%s", q) diff --git a/transformer/transformer.go b/transformer/transformer.go index 53d5792a..063fac95 100644 --- a/transformer/transformer.go +++ b/transformer/transformer.go @@ -10,13 +10,13 @@ import ( "github.com/pkg/errors" ) -// Transformer represents an instance of the ResponseTransformer interface +// LegacyTransformer represents an instance of the ResponseTransformer interface type LegacyTransformer struct { higlightReplacer *strings.Replacer } -// New returns a new instance of Transformer -func New() *LegacyTransformer { +// NewLegacy returns a new instance of Transformer +func NewLegacy() *LegacyTransformer { highlightReplacer := strings.NewReplacer("", "", "", "") return &LegacyTransformer{ higlightReplacer: highlightReplacer, diff --git a/transformer/transformer_test.go b/transformer/transformer_test.go index 6c76f281..50098abd 100644 --- a/transformer/transformer_test.go +++ b/transformer/transformer_test.go @@ -12,7 +12,7 @@ import ( func TestLegacyTransformer(t *testing.T) { Convey("Transforms unmarshalled search responses successfully", t, func() { - transformer := New() + transformer := NewLegacy() Convey("Zero suggestions creates empty array", func() { es := models.ESResponse{ Responses: []models.ESResponseItem{models.ESResponseItem{ @@ -102,7 +102,7 @@ func TestBuildAdditionalSuggestionsList(t *testing.T) { func TestTransformSearchResponse(t *testing.T) { Convey("With a transformer initialised", t, func() { ctx := context.Background() - transformer := New() + transformer := NewLegacy() So(t, ShouldNotBeNil) Convey("Throws error on invalid JSON", func() { From 3c80b2bde56717205c85b65d2689b7ff76fea499 Mon Sep 17 00:00:00 2001 From: bpathak-ons Date: Thu, 7 Apr 2022 14:25:46 +0100 Subject: [PATCH 04/29] initial implementation of new transformer for es7x --- api/api.go | 4 +-- config/config.go | 2 +- config/config_test.go | 2 +- models/elasticsearch.go | 60 ++++++++++++++++++++++++++++++++ transformer/transformer.go | 54 +++++++++++++++++++++++++++++ transformer/transformer_test.go | 61 +++++++++++++++++++++++++++++++-- 6 files changed, 177 insertions(+), 6 deletions(-) create mode 100644 models/elasticsearch.go diff --git a/api/api.go b/api/api.go index 18117f4b..0d61795e 100644 --- a/api/api.go +++ b/api/api.go @@ -88,8 +88,8 @@ func NewSearchAPI(router *mux.Router, dpESClient DpElasticSearcher, deprecatedES router.HandleFunc("/search", SearchHandlerFunc(queryBuilder, api.deprecatedESClient, api.Transformer)).Methods("GET") router.HandleFunc("/timeseries/{cdid}", TimeseriesLookupHandlerFunc(api.deprecatedESClient)).Methods("GET") router.HandleFunc("/data", DataLookupHandlerFunc(api.deprecatedESClient)).Methods("GET") - createSearchIndexHandler := permissions.Require(update, api.CreateSearchIndexHandlerFunc) - router.HandleFunc("/search", createSearchIndexHandler).Methods("POST") + // createSearchIndexHandler := permissions.Require(update, api.CreateSearchIndexHandlerFunc) + router.HandleFunc("/search", api.CreateSearchIndexHandlerFunc).Methods("POST") return api, nil } diff --git a/config/config.go b/config/config.go index 3420a845..3d7f7a83 100644 --- a/config/config.go +++ b/config/config.go @@ -39,7 +39,7 @@ func Get() (*Config, error) { cfg = &Config{ BindAddr: ":23900", - ElasticSearchAPIURL: "http://localhost:9200", + ElasticSearchAPIURL: "http://localhost:11200", GracefulShutdownTimeout: 5 * time.Second, SignElasticsearchRequests: false, HealthCheckCriticalTimeout: 90 * time.Second, diff --git a/config/config_test.go b/config/config_test.go index 9a4094a7..a2244d86 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -23,7 +23,7 @@ func TestSpec(t *testing.T) { So(cfg.AWS.Service, ShouldEqual, "es") So(cfg.AWS.TLSInsecureSkipVerify, ShouldEqual, false) So(cfg.BindAddr, ShouldEqual, ":23900") - So(cfg.ElasticSearchAPIURL, ShouldEqual, "http://localhost:9200") + So(cfg.ElasticSearchAPIURL, ShouldEqual, "http://localhost:11200") So(cfg.GracefulShutdownTimeout, ShouldEqual, 5*time.Second) So(cfg.SignElasticsearchRequests, ShouldEqual, false) So(cfg.HealthCheckCriticalTimeout, ShouldEqual, 90*time.Second) diff --git a/models/elasticsearch.go b/models/elasticsearch.go new file mode 100644 index 00000000..c228a308 --- /dev/null +++ b/models/elasticsearch.go @@ -0,0 +1,60 @@ +package models + +// ************************************************************* +// Part 1 : structs representing the raw elastic search response +// ************************************************************* + +// Es7xResponse holds a response slice from ES +type Es7xResponse struct { + Responses []EsResponse7x `json:"responses"` +} + +type EsResponse7x struct { + Took int `json:"took"` + Hits ES7xResponseHits `json:"hits"` +} + +type ES7xResponseHits struct { + Total int + Hits []ES7xResponseHit `json:"hits"` +} + +type ES7xResponseHit struct { + Source ES7xSourceDocument `json:"_source"` + Highlight ES7xHighlight `json:"highlight"` +} + +type ES7xSourceDocument struct { + DataType string `json:"type"` + JobID string `json:"job_id"` + SearchIndex string `json:"search_index"` + CDID string `json:"cdid"` + DatasetID string `json:"dataset_id"` + Keywords []string `json:"keywords"` + MetaDescription string `json:"meta_description"` + ReleaseDate string `json:"release_date,omitempty"` + Summary string `json:"summary"` + Title string `json:"title"` + Topics []string `json:"topics"` +} + +type ES7xHighlight struct { + DescriptionTitle *[]string `json:"description.title"` + DescriptionEdition *[]string `json:"description.edition"` + DescriptionSummary *[]string `json:"description.summary"` + DescriptionMeta *[]string `json:"description.metaDescription"` + DescriptionKeywords *[]string `json:"description.keywords"` + DescriptionDatasetID *[]string `json:"description.datasetId"` +} + +// ******************************************************** +// Part 2 : Structs representing the transformed response +// ******************************************************** + +type Search7xResponse struct { + Count int `json:"count"` + Took int `json:"took"` + Items []ES7xSourceDocument `json:"items"` + Suggestions []string `json:"suggestions,omitempty"` + AdditionSuggestions []string `json:"additional_suggestions,omitempty"` +} diff --git a/transformer/transformer.go b/transformer/transformer.go index 063fac95..5f26c45d 100644 --- a/transformer/transformer.go +++ b/transformer/transformer.go @@ -23,6 +23,19 @@ func NewLegacy() *LegacyTransformer { } } +// Transformer represents an instance of the ResponseTransformer interface for ES7x +type Transformer struct { + higlightReplacer *strings.Replacer +} + +// New7x returns a new instance of Transformer7x +func New() *Transformer { + highlightReplacer := strings.NewReplacer("", "", "", "") + return &Transformer{ + higlightReplacer: highlightReplacer, + } +} + // TransformSearchResponse transforms an elastic search response into a structure that matches the v1 api specification func (t *LegacyTransformer) TransformSearchResponse(ctx context.Context, responseData []byte, query string, highlight bool) ([]byte, error) { var source models.ESResponse @@ -174,3 +187,44 @@ func numberOfSearchTerms(query string) int { st := strings.Fields(query) return len(st) } + +// TransformSearchResponse transforms an elastic search 7.x response +func (t *Transformer) TransformSearchResponse( + ctx context.Context, responseData []byte, + query string, highlight bool) ([]byte, error) { + + var esResponse models.Es7xResponse + + err := json.Unmarshal(responseData, &esResponse) + if err != nil { + return nil, errors.Wrap(err, "Failed to decode elastic search 7x response") + } + + if len(esResponse.Responses) < 1 { + return nil, errors.New("Response to be 7x transformed contained 0 items") + } + + sr := t.transform(&esResponse, highlight) + + needAdditionalSuggestions := numberOfSearchTerms(query) + if needAdditionalSuggestions > 1 { + as := buildAdditionalSuggestionList(query) + sr.AdditionSuggestions = as + } + + transformedData, err := json.Marshal(sr) + if err != nil { + return nil, errors.Wrap(err, "Failed to encode transformed response") + } + return transformedData, nil +} + +// Transform the raw ES to search response +func (t *Transformer) transform(es7xresponse *models.Es7xResponse, highlight bool) models.Search7xResponse { + + var search7xResponse = models.Search7xResponse{ + Took: es7xresponse.Responses[0].Took, + } + return search7xResponse +} + diff --git a/transformer/transformer_test.go b/transformer/transformer_test.go index 50098abd..73924af4 100644 --- a/transformer/transformer_test.go +++ b/transformer/transformer_test.go @@ -78,7 +78,7 @@ func TestLegacyTransformer(t *testing.T) { }) } -func TestBuildAdditionalSuggestionsList(t *testing.T) { +func TestLegacyBuildAdditionalSuggestionsList(t *testing.T) { Convey("buildAdditionalSuggestionList successfully", t, func() { Convey("returns array of strings", func() { query1 := buildAdditionalSuggestionList("test-query") @@ -99,7 +99,7 @@ func TestBuildAdditionalSuggestionsList(t *testing.T) { }) } -func TestTransformSearchResponse(t *testing.T) { +func TestLegacyTransformSearchResponse(t *testing.T) { Convey("With a transformer initialised", t, func() { ctx := context.Background() transformer := NewLegacy() @@ -165,3 +165,60 @@ func TestTransformSearchResponse(t *testing.T) { }) }) } + +func TestTransform(t *testing.T) { + Convey("Given a new instance of Transformer7x with search responses successfully", t, func() { + transformer := New() + esResponse := prepareESMockResponse() + + Convey("When calling a transformer", func() { + transformedResponse := transformer.transform(&esResponse, true) + + Convey("Then transforms unmarshalled search responses successfully", func() { + So(transformedResponse, ShouldNotBeNil) + So(transformedResponse.Took, ShouldEqual, 10) + }) + }) + }) +} + +// Prepare mock ES response +func prepareESMockResponse() models.Es7xResponse { + + esDocument := models.ES7xSourceDocument{ + DataType: "anyDataType2", + JobID: "", + CDID: "", + DatasetID: "", + Keywords: []string{"anykeyword1"}, + MetaDescription: "", + Summary: "", + ReleaseDate: "", + Title: "anyTitle2", + Topics: []string{"anyTopic1"}, + } + + hit7x := models.ES7xResponseHit{ + Source: esDocument, + Highlight: models.ES7xHighlight{}, + } + + esResponse7x1 := models.EsResponse7x{ + Took: 10, + Hits: models.ES7xResponseHits{ + Total: 1, + Hits: []models.ES7xResponseHit{ + hit7x, + }, + }, + } + + // Preparing ES response array + es7xResponse := models.Es7xResponse{ + Responses: []models.EsResponse7x{ + esResponse7x1, + }, + } + + return es7xResponse +} From cd83ff26401df875d66ac62787c625941bd06782 Mon Sep 17 00:00:00 2001 From: rahulmadathumpalliyalil Date: Thu, 7 Apr 2022 14:04:01 +0100 Subject: [PATCH 05/29] Create match_all query builder for ES7.10 --- config/config.go | 2 ++ config/config_test.go | 1 + query/query.go | 10 ++++++++-- query/query_test.go | 10 +++++++++- query/search.go | 14 ++++++++++++++ service/service.go | 14 ++++++-------- templates/search/v710/contentHeader.tmpl | 1 + templates/search/v710/contentQuery.tmpl | 10 ++++++++++ templates/search/v710/search.tmpl | 5 +++++ 9 files changed, 56 insertions(+), 11 deletions(-) create mode 100644 templates/search/v710/contentHeader.tmpl create mode 100644 templates/search/v710/contentQuery.tmpl create mode 100644 templates/search/v710/search.tmpl diff --git a/config/config.go b/config/config.go index 3420a845..22589a42 100644 --- a/config/config.go +++ b/config/config.go @@ -19,6 +19,7 @@ type Config struct { ZebedeeURL string `envconfig:"ZEBEDEE_URL"` DatasetAPIURL string `envconfig:"DATASET_API_URL"` ServiceAuthToken string `envconfig:"SERVICE_AUTH_TOKEN"` + ElasticVersion710 bool `envconfig:"ElASTIC_VERSION_710"` } type AWS struct { @@ -47,6 +48,7 @@ func Get() (*Config, error) { ZebedeeURL: "http://localhost:8082", DatasetAPIURL: "http://localhost:22000", ServiceAuthToken: "", + ElasticVersion710: false, } cfg.AWS = AWS{ diff --git a/config/config_test.go b/config/config_test.go index 9a4094a7..aed1cb3c 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -24,6 +24,7 @@ func TestSpec(t *testing.T) { So(cfg.AWS.TLSInsecureSkipVerify, ShouldEqual, false) So(cfg.BindAddr, ShouldEqual, ":23900") So(cfg.ElasticSearchAPIURL, ShouldEqual, "http://localhost:9200") + So(cfg.ElasticVersion710, ShouldEqual, false) So(cfg.GracefulShutdownTimeout, ShouldEqual, 5*time.Second) So(cfg.SignElasticsearchRequests, ShouldEqual, false) So(cfg.HealthCheckCriticalTimeout, ShouldEqual, 90*time.Second) diff --git a/query/query.go b/query/query.go index d33829e1..4018d632 100644 --- a/query/query.go +++ b/query/query.go @@ -16,8 +16,14 @@ type Builder struct { } // NewQueryBuilder loads the elastic search templates and returns a query builder instance -func NewQueryBuilder(pathToTemplates string) (*Builder, error) { - searchTemplates, err := SetupSearch(pathToTemplates) +func NewQueryBuilder(pathToTemplates string, esVersion710 bool) (*Builder, error) { + var searchTemplates *template.Template + var err error + if esVersion710 { + searchTemplates, err = SetupV710Search(pathToTemplates) + } else { + searchTemplates, err = SetupSearch(pathToTemplates) + } if err != nil { return nil, errors.Wrap(err, "failed to load search templates") } diff --git a/query/query_test.go b/query/query_test.go index 2ea445cf..99e7bf91 100644 --- a/query/query_test.go +++ b/query/query_test.go @@ -19,9 +19,17 @@ func TestSetupSearch(t *testing.T) { func TestNewQueryBuilder(t *testing.T) { Convey("Should return a Builder object with templates", t, func() { - builderObject, err := NewQueryBuilder(testPathToTemplates) + builderObject, err := NewQueryBuilder(testPathToTemplates, false) So(builderObject.searchTemplates, ShouldNotBeNil) So(err, ShouldBeNil) }) + + Convey("Should return a Builder object with elastic v710 templates", t, func() { + builderObject, err := NewQueryBuilder(testPathToTemplates, true) + + So(builderObject.searchTemplates, ShouldNotBeNil) + So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "search.tmpl") + So(err, ShouldBeNil) + }) } diff --git a/query/search.go b/query/search.go index c08fb399..45f65bb4 100644 --- a/query/search.go +++ b/query/search.go @@ -71,6 +71,20 @@ func SetupSearch(pathToTemplates string) (*template.Template, error) { return templates, err } +// SetupSearch loads templates for use by the search handler and should be done only once +func SetupV710Search(pathToTemplates string) (*template.Template, error) { + // Load the templates once, the main entry point for the templates is search.tmpl. The search.tmpl takes + // the SearchRequest struct and uses the Request to build up the multi-query queries that is used to query elastic. + + templates, err := template.ParseFiles( + pathToTemplates+"templates/search/v710/search.tmpl", + pathToTemplates+"templates/search/v710/contentQuery.tmpl", + pathToTemplates+"templates/search/v710/contentHeader.tmpl", + ) + + return templates, err +} + // BuildSearchQuery creates an elastic search query from the provided search parameters func (sb *Builder) BuildSearchQuery(ctx context.Context, q, contentTypes, sort string, topics []string, limit, offset int) ([]byte, error) { reqParams := searchRequest{ diff --git a/service/service.go b/service/service.go index 458b1148..c085918c 100644 --- a/service/service.go +++ b/service/service.go @@ -3,19 +3,17 @@ package service import ( "context" - "github.com/gorilla/mux" - "github.com/pkg/errors" - + legacyESClient "github.com/ONSdigital/dp-elasticsearch/v3/client/elasticsearch/v2" + "github.com/ONSdigital/dp-net/v2/awsauth" + dphttp "github.com/ONSdigital/dp-net/v2/http" "github.com/ONSdigital/dp-search-api/api" "github.com/ONSdigital/dp-search-api/config" "github.com/ONSdigital/dp-search-api/elasticsearch" "github.com/ONSdigital/dp-search-api/query" "github.com/ONSdigital/dp-search-api/transformer" - - legacyESClient "github.com/ONSdigital/dp-elasticsearch/v3/client/elasticsearch/v2" - "github.com/ONSdigital/dp-net/v2/awsauth" - dphttp "github.com/ONSdigital/dp-net/v2/http" "github.com/ONSdigital/log.go/v2/log" + "github.com/gorilla/mux" + "github.com/pkg/errors" ) const pathToTemplates = "" @@ -83,7 +81,7 @@ func Run(ctx context.Context, cfg *config.Config, serviceList *ExternalServiceLi deprecatedESClient := elasticsearch.New(cfg.ElasticSearchAPIURL, elasticHTTPClient, cfg.AWS.Region, cfg.AWS.Service) // Initialise query builder - queryBuilder, err := query.NewQueryBuilder(pathToTemplates) + queryBuilder, err := query.NewQueryBuilder(pathToTemplates, cfg.ElasticVersion710) if err != nil { log.Fatal(ctx, "error initialising query builder", err) return nil, err diff --git a/templates/search/v710/contentHeader.tmpl b/templates/search/v710/contentHeader.tmpl new file mode 100644 index 00000000..f435357c --- /dev/null +++ b/templates/search/v710/contentHeader.tmpl @@ -0,0 +1 @@ +{ /* content query */{{if .Index}} "index":"{{.Index}}"{{if .Types}},{{end}}{{else}} "index":"ons"{{if .Types}},{{end}} {{end}}{{if .Types}} "type" : [{{range $i,$e := .Types}}{{if $i}},{{end}}"{{.}}"{{end}}]{{end}} } \ No newline at end of file diff --git a/templates/search/v710/contentQuery.tmpl b/templates/search/v710/contentQuery.tmpl new file mode 100644 index 00000000..a3aed9ab --- /dev/null +++ b/templates/search/v710/contentQuery.tmpl @@ -0,0 +1,10 @@ +{ +{{/* content query */}} + "query" : { + "bool" : { + "must" : { + "match_all": {} + } + } + } +} \ No newline at end of file diff --git a/templates/search/v710/search.tmpl b/templates/search/v710/search.tmpl new file mode 100644 index 00000000..b31f3cb6 --- /dev/null +++ b/templates/search/v710/search.tmpl @@ -0,0 +1,5 @@ +{{/* $$ indicates newline, each header and query MUST be on its own line - with a blank line at the end */}} +{{- if .HasQuery "search" }} + {{- template "contentHeader.tmpl" .}}$$ + {{- template "contentQuery.tmpl" .}}$$ +{{- end}} \ No newline at end of file From 67ad7876921f2deb1f46c634b266335b9aa6ce19 Mon Sep 17 00:00:00 2001 From: bpathak-ons Date: Thu, 7 Apr 2022 14:32:10 +0100 Subject: [PATCH 06/29] minor refinement updates --- api/api.go | 4 ++-- config/config.go | 2 +- config/config_test.go | 2 +- models/elasticsearch.go | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/api/api.go b/api/api.go index 0d61795e..18117f4b 100644 --- a/api/api.go +++ b/api/api.go @@ -88,8 +88,8 @@ func NewSearchAPI(router *mux.Router, dpESClient DpElasticSearcher, deprecatedES router.HandleFunc("/search", SearchHandlerFunc(queryBuilder, api.deprecatedESClient, api.Transformer)).Methods("GET") router.HandleFunc("/timeseries/{cdid}", TimeseriesLookupHandlerFunc(api.deprecatedESClient)).Methods("GET") router.HandleFunc("/data", DataLookupHandlerFunc(api.deprecatedESClient)).Methods("GET") - // createSearchIndexHandler := permissions.Require(update, api.CreateSearchIndexHandlerFunc) - router.HandleFunc("/search", api.CreateSearchIndexHandlerFunc).Methods("POST") + createSearchIndexHandler := permissions.Require(update, api.CreateSearchIndexHandlerFunc) + router.HandleFunc("/search", createSearchIndexHandler).Methods("POST") return api, nil } diff --git a/config/config.go b/config/config.go index 3d7f7a83..3420a845 100644 --- a/config/config.go +++ b/config/config.go @@ -39,7 +39,7 @@ func Get() (*Config, error) { cfg = &Config{ BindAddr: ":23900", - ElasticSearchAPIURL: "http://localhost:11200", + ElasticSearchAPIURL: "http://localhost:9200", GracefulShutdownTimeout: 5 * time.Second, SignElasticsearchRequests: false, HealthCheckCriticalTimeout: 90 * time.Second, diff --git a/config/config_test.go b/config/config_test.go index a2244d86..9a4094a7 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -23,7 +23,7 @@ func TestSpec(t *testing.T) { So(cfg.AWS.Service, ShouldEqual, "es") So(cfg.AWS.TLSInsecureSkipVerify, ShouldEqual, false) So(cfg.BindAddr, ShouldEqual, ":23900") - So(cfg.ElasticSearchAPIURL, ShouldEqual, "http://localhost:11200") + So(cfg.ElasticSearchAPIURL, ShouldEqual, "http://localhost:9200") So(cfg.GracefulShutdownTimeout, ShouldEqual, 5*time.Second) So(cfg.SignElasticsearchRequests, ShouldEqual, false) So(cfg.HealthCheckCriticalTimeout, ShouldEqual, 90*time.Second) diff --git a/models/elasticsearch.go b/models/elasticsearch.go index c228a308..6fb7679a 100644 --- a/models/elasticsearch.go +++ b/models/elasticsearch.go @@ -1,7 +1,7 @@ package models // ************************************************************* -// Part 1 : structs representing the raw elastic search response +// structs representing the raw elastic search response // ************************************************************* // Es7xResponse holds a response slice from ES @@ -48,7 +48,7 @@ type ES7xHighlight struct { } // ******************************************************** -// Part 2 : Structs representing the transformed response +// Structs representing the transformed response // ******************************************************** type Search7xResponse struct { From 6d88f8fc0e3f3caebdb0767a6d25d63b8467e62d Mon Sep 17 00:00:00 2001 From: bpathak-ons Date: Thu, 7 Apr 2022 14:38:32 +0100 Subject: [PATCH 07/29] fix linter error updates --- transformer/transformer.go | 5 +---- transformer/transformer_test.go | 1 - 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/transformer/transformer.go b/transformer/transformer.go index 5f26c45d..1f8b026d 100644 --- a/transformer/transformer.go +++ b/transformer/transformer.go @@ -188,11 +188,10 @@ func numberOfSearchTerms(query string) int { return len(st) } -// TransformSearchResponse transforms an elastic search 7.x response +// TransformSearchResponse transforms an elastic search 7.x response func (t *Transformer) TransformSearchResponse( ctx context.Context, responseData []byte, query string, highlight bool) ([]byte, error) { - var esResponse models.Es7xResponse err := json.Unmarshal(responseData, &esResponse) @@ -221,10 +220,8 @@ func (t *Transformer) TransformSearchResponse( // Transform the raw ES to search response func (t *Transformer) transform(es7xresponse *models.Es7xResponse, highlight bool) models.Search7xResponse { - var search7xResponse = models.Search7xResponse{ Took: es7xresponse.Responses[0].Took, } return search7xResponse } - diff --git a/transformer/transformer_test.go b/transformer/transformer_test.go index 73924af4..ed52570b 100644 --- a/transformer/transformer_test.go +++ b/transformer/transformer_test.go @@ -184,7 +184,6 @@ func TestTransform(t *testing.T) { // Prepare mock ES response func prepareESMockResponse() models.Es7xResponse { - esDocument := models.ES7xSourceDocument{ DataType: "anyDataType2", JobID: "", From 109bd3d50350bb35bcd4cb169a5e2a1975d10b30 Mon Sep 17 00:00:00 2001 From: PatrickConnollyONS Date: Thu, 7 Apr 2022 17:15:57 +0100 Subject: [PATCH 08/29] Trim the ElasticSearch release results to what is needed --- templates/search/releasecalendar/search.tmpl | 6 +- transformer/releasetransformer.go | 137 ++++++------------- 2 files changed, 45 insertions(+), 98 deletions(-) diff --git a/templates/search/releasecalendar/search.tmpl b/templates/search/releasecalendar/search.tmpl index 56b6b1ec..4f9f81cc 100644 --- a/templates/search/releasecalendar/search.tmpl +++ b/templates/search/releasecalendar/search.tmpl @@ -41,11 +41,7 @@ "fields":{ "description.title":{"fragment_size":0,"number_of_fragments":0}, "description.summary":{"fragment_size":0,"number_of_fragments":0}, - "description.edition":{"fragment_size":0,"number_of_fragments":0}, - "description.metaDescription":{"fragment_size":0,"number_of_fragments":0}, - "description.keywords":{"fragment_size":0,"number_of_fragments":0}, - "description.datasetId":{"fragment_size":0,"number_of_fragments":0}, - "description.cdid":{"fragment_size":0,"number_of_fragments":0} + "description.keywords":{"fragment_size":0,"number_of_fragments":0} } } {{end}} diff --git a/transformer/releasetransformer.go b/transformer/releasetransformer.go index 2cc1b04f..00e357e7 100644 --- a/transformer/releasetransformer.go +++ b/transformer/releasetransformer.go @@ -14,8 +14,6 @@ type ReleaseTransformer struct { type SearchReleaseResponse struct { Took int `json:"took"` - Limit int `json:"limit"` - Offset int `json:"offset"` Breakdown Breakdown `json:"breakdown"` Releases []Release `json:"releases"` } @@ -31,47 +29,35 @@ type Breakdown struct { } type Release struct { - URI string `json:"uri"` - Description ReleaseDescription `json:"description"` - Highlight *highlight `json:"highlight,omitempty"` + URI string `json:"uri"` + DateChanges []ReleaseDateChange `json:"date_changes"` + Description ReleaseDescription `json:"description"` + Highlight *highlight `json:"highlight,omitempty"` } -type ReleaseDescription struct { - Title string `json:"title"` - Summary string `json:"summary"` - ReleaseDate string `json:"release_date"` - Published bool `json:"published"` - Cancelled bool `json:"cancelled"` - Finalised bool `json:"finalised"` - Postponed bool `json:"postponed"` - Census bool `json:"census"` - NationalStatistic bool `json:"national_statistic"` - Keywords []string `json:"keywords,omitempty"` - NextRelease string `json:"next_release,omitempty"` - ProvisionalDate string `json:"provisional_date,omitempty"` - CancellationNotice []string `json:"cancellation_notice,omitempty"` - Edition string `json:"edition,omitempty"` - DatasetID string `json:"dataset_id,omitempty"` - LatestRelease *bool `json:"latest_release,omitempty"` - MetaDescription string `json:"meta_description,omitempty"` - Language string `json:"language,omitempty"` - Source string `json:"source,omitempty"` - Contact *releaseContact `json:"contact,omitempty"` +type ReleaseDateChange struct { + ChangeNotice string `json:"change_notice"` + Date string `json:"previous_date"` } -type releaseContact struct { - Name string `json:"name"` - Telephone string `json:"telephone,omitempty"` - Email string `json:"email"` +type ReleaseDescription struct { + Title string `json:"title"` + Summary string `json:"summary"` + ReleaseDate string `json:"release_date"` + Published bool `json:"published"` + Cancelled bool `json:"cancelled"` + Finalised bool `json:"finalised"` + Postponed bool `json:"postponed"` + Census bool `json:"census"` + Keywords []string `json:"keywords,omitempty"` + ProvisionalDate string `json:"provisional_date,omitempty"` + Language string `json:"language,omitempty"` } type highlight struct { - DatasetID string `json:"dataset_id,omitempty"` - Edition string `json:"edition,omitempty"` - Keywords []string `json:"keywords,omitempty"` - MetaDescription string `json:"meta_description,omitempty"` - Summary string `json:"summary,omitempty"` - Title string `json:"title,omitempty"` + Keywords []string `json:"keywords,omitempty"` + Summary string `json:"summary,omitempty"` + Title string `json:"title,omitempty"` } // Structs representing the raw elastic search response @@ -97,25 +83,15 @@ type ESReleaseSourceDocument struct { DateChanges []dateChange `json:"dateChanges,omitempty"` Description struct { - Title string `json:"title"` - Summary string `json:"summary"` - ReleaseDate string `json:"releaseDate,omitempty"` - Published bool `json:"published"` - Cancelled bool `json:"cancelled"` - Finalised bool `json:"finalised"` - Topics []string `json:"topics"` - NationalStatistic bool `json:"nationalStatistic,omitempty"` - Keywords []string `json:"keywords,omitempty"` - NextRelease string `json:"nextRelease,omitempty"` - CancellationNotice []string `json:"cancellationNotice"` - ProvisionalDate string `json:"provisionalDate"` - Edition string `json:"edition,omitempty"` - DatasetID string `json:"datasetId,omitempty"` - LatestRelease bool `json:"latestRelease"` - MetaDescription string `json:"metaDescription,omitempty"` - Language string `json:"language,omitempty"` - Source string `json:"source,omitempty"` - Contact *releaseContact `json:"contact,omitempty"` + Title string `json:"title"` + Summary string `json:"summary"` + ReleaseDate string `json:"releaseDate,omitempty"` + Published bool `json:"published"` + Cancelled bool `json:"cancelled"` + Finalised bool `json:"finalised"` + Census bool `json:"census"` + Keywords []string `json:"keywords,omitempty"` + Language string `json:"language,omitempty"` } `json:"description"` } @@ -125,12 +101,9 @@ type dateChange struct { } type ESReleaseHighlight struct { - DescriptionTitle []string `json:"description.title"` - DescriptionEdition []string `json:"description.edition"` - DescriptionSummary []string `json:"description.summary"` - DescriptionMeta []string `json:"description.metaDescription"` - DescriptionKeywords []string `json:"description.keywords"` - DescriptionDatasetID []string `json:"description.datasetId"` + DescriptionTitle []string `json:"description.title"` + DescriptionSummary []string `json:"description.summary"` + DescriptionKeywords []string `json:"description.keywords"` } func NewReleaseTransformer() *ReleaseTransformer { @@ -162,8 +135,6 @@ func (t *ReleaseTransformer) TransformSearchResponse(_ context.Context, response func (t *ReleaseTransformer) transform(source *ESReleaseResponse, highlight bool) SearchReleaseResponse { sr := SearchReleaseResponse{ Took: source.Took, - Limit: 10, - Offset: 0, Breakdown: Breakdown{Total: source.Hits.Total}, Releases: []Release{}, } @@ -185,47 +156,27 @@ func (t *ReleaseTransformer) buildRelease(hit ESReleaseResponseHit, highlightOn Title: sd.Title, Summary: sd.Summary, ReleaseDate: sd.ReleaseDate, - // The following 3 need to be added to source document (and indexed) - Published: sd.Published, - Cancelled: sd.Cancelled, - Finalised: sd.Finalised, - Postponed: isPostponed(hit.Source), - Census: isCensus(hit.Source), - NationalStatistic: sd.NationalStatistic, - Keywords: sd.Keywords, - NextRelease: sd.NextRelease, - // The following 3 need to be added to source document - ProvisionalDate: sd.ProvisionalDate, - CancellationNotice: sd.CancellationNotice, - Edition: sd.Edition, - DatasetID: sd.DatasetID, - // The following 1 needs to be added to source document - LatestRelease: &sd.LatestRelease, - MetaDescription: sd.MetaDescription, - Language: sd.Language, - Contact: sd.Contact, - Source: sd.Source, + Published: sd.Published, + Cancelled: sd.Cancelled, + Finalised: sd.Finalised, + Postponed: isPostponed(hit.Source), + Census: sd.Census, + Keywords: sd.Keywords, + Language: sd.Language, }, } if highlightOn { r.Highlight = &highlight{ - DatasetID: t.overlayItem(hl.DescriptionDatasetID, sd.DatasetID, highlightOn), - Edition: t.overlayItem(hl.DescriptionEdition, sd.Edition, highlightOn), - Keywords: t.overlayList(hl.DescriptionKeywords, sd.Keywords, highlightOn), - MetaDescription: t.overlayItem(hl.DescriptionMeta, sd.MetaDescription, highlightOn), - Summary: t.overlayItem(hl.DescriptionSummary, sd.Summary, highlightOn), - Title: t.overlayItem(hl.DescriptionTitle, sd.Title, highlightOn), + Keywords: t.overlayList(hl.DescriptionKeywords, sd.Keywords, highlightOn), + Summary: t.overlayItem(hl.DescriptionSummary, sd.Summary, highlightOn), + Title: t.overlayItem(hl.DescriptionTitle, sd.Title, highlightOn), } } return r } -func isCensus(_ ESReleaseSourceDocument) bool { - return false -} - func isPostponed(release ESReleaseSourceDocument) bool { return release.Description.Finalised && len(release.DateChanges) > 0 } From de2b41db74f7c9f1e6622a900bd6942ff744e3d9 Mon Sep 17 00:00:00 2001 From: PatrickConnollyONS Date: Thu, 7 Apr 2022 17:28:54 +0100 Subject: [PATCH 09/29] Add the ReleaseType type --- features/search-release.feature | 10 ++++---- query/releasesearch.go | 41 +++++++++++++++++++++++++++++++++ query/releasesearch_test.go | 35 ++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 5 deletions(-) diff --git a/features/search-release.feature b/features/search-release.feature index 57b84332..9bbb37ed 100644 --- a/features/search-release.feature +++ b/features/search-release.feature @@ -1,34 +1,34 @@ Feature: search/releases endpoint should return data for various combinations of input parameters Scenario: When Searching for published releases relating to 'Education in Wales' between certain dates I get one result Given elasticsearch returns one item in search/release response - When I GET "/search/releases?q=Education+in+Wales&dateFrom=2020-01-01&dateTo=2020-12-31&published=true" + When I GET "/search/releases?q=Education+in+Wales&dateFrom=2020-01-01&dateTo=2020-12-31&release-type=type-published" Then the HTTP status code should be "200" And the response header "Content-Type" should be "application/json;charset=utf-8" And the response body is the same as the json in "./features/testdata/expected_single_search_release_result.json" Scenario: When Searching for published releases relating to 'Education in Wales' between certain dates with highlighting turned off and I get one result Given elasticsearch returns one item in search/release response - When I GET "/search/releases?q=Education+in+Wales&dateFrom=2020-01-01&dateTo=2020-12-31&published=true&highlight=false" + When I GET "/search/releases?q=Education+in+Wales&dateFrom=2020-01-01&dateTo=2020-12-31&release-type=type-published&highlight=false" Then the HTTP status code should be "200" And the response header "Content-Type" should be "application/json;charset=utf-8" And the response body is the same as the json in "./features/testdata/expected_single_search_release_result_nohighlight.json" Scenario: When Searching for published releases relating to 'Education in Wales' between certain dates I get multiple results Given elasticsearch returns multiple items in search/release response - When I GET "/search/releases?q=Education+in+Wales&dateFrom=2020-01-01&dateTo=2020-12-31&published=true" + When I GET "/search/releases?q=Education+in+Wales&dateFrom=2020-01-01&dateTo=2020-12-31&release-type=type-published" Then the HTTP status code should be "200" And the response header "Content-Type" should be "application/json;charset=utf-8" And the response body is the same as the json in "./features/testdata/expected_multiple_search_release_results.json" Scenario: When Searching for published releases relating to 'Education in Wales' between certain dates I get zero results Given elasticsearch returns zero items in search/release response - When I GET "/search/releases?q=Education+in+Scotland&dateFrom=2020-01-01&dateTo=2020-12-31&published=true" + When I GET "/search/releases?q=Education+in+Scotland&dateFrom=2020-01-01&dateTo=2020-12-31&release-type=type-published" Then the HTTP status code should be "200" And the response header "Content-Type" should be "application/json;charset=utf-8" And the response body is the same as the json in "./features/testdata/expected_zero_search_release_results.json" Scenario: When Searching for published releases relating to 'Education in Wales' between certain dates, I get internal server error Given elasticsearch returns internal server error - When I GET "/search/releases?q=Education+in+Scotland&dateFrom=2020-01-01&dateTo=2020-12-31&published=true" + When I GET "/search/releases?q=Education+in+Scotland&dateFrom=2020-01-01&dateTo=2020-12-31&release-type=type-published" Then the HTTP status code should be "500" And the response header "Content-Type" should be "text/plain; charset=utf-8" diff --git a/query/releasesearch.go b/query/releasesearch.go index 584e13c6..5d374eb6 100644 --- a/query/releasesearch.go +++ b/query/releasesearch.go @@ -65,6 +65,13 @@ func NewReleaseQueryParamValidator() ParamValidator { } return value, nil }, + "release-type": func(param string) (interface{}, error) { + value, err := ParseReleaseType(param) + if err != nil { + return nil, fmt.Errorf("release-type parameter provided is invalid: %w", err) + } + return value, nil + }, } } @@ -152,6 +159,40 @@ func (s Sort) ESString() string { return esSortNames[s] } +type ReleaseType int + +const ( + InvalidReleaseType ReleaseType = iota + Upcoming + Published + Cancelled +) + +var relTypeNames = map[ReleaseType]string{Upcoming: "type-upcoming", Published: "type-published", Cancelled: "type-cancelled", InvalidReleaseType: "Invalid"} + +func ParseReleaseType(s string) (ReleaseType, error) { + for rt, rtn := range relTypeNames { + if strings.EqualFold(s, rtn) { + return rt, nil + } + } + + return InvalidReleaseType, errors.New("invalid release type string") +} + +func MustParseReleaseType(s string) ReleaseType { + rt, err := ParseReleaseType(s) + if err != nil { + panic("invalid release type string: " + s) + } + + return rt +} + +func (rt ReleaseType) String() string { + return relTypeNames[rt] +} + type ReleaseBuilder struct { searchTemplates *template.Template } diff --git a/query/releasesearch_test.go b/query/releasesearch_test.go index 3d70f393..c6e135b7 100644 --- a/query/releasesearch_test.go +++ b/query/releasesearch_test.go @@ -123,6 +123,41 @@ func TestSort(t *testing.T) { }) } +func TestReleaseType(t *testing.T) { + t.Parallel() + Convey("given a release-type validator, and a set of erroneous release-type option strings", t, func() { + validator := validators["release-type"] + badReleaseTypes := []string{"coming-up", "finished", "done"} + + Convey("errors are generated, and zero values returned on validation", func() { + for _, rt := range badReleaseTypes { + v, e := validator(rt) + + So(v, ShouldBeNil) + So(e, ShouldNotBeNil) + } + }) + + Convey("but a good release-type option string is validated without error, and the appropriate ReleaseType returned", func() { + goodReleaseTypes := []struct { + given string + exValue ReleaseType + }{ + {given: "type-upcoming", exValue: Upcoming}, + {given: "type-published", exValue: Published}, + {given: "type-cancelled", exValue: Cancelled}, + } + + for _, grt := range goodReleaseTypes { + v, e := validator(grt.given) + + So(v, ShouldEqual, grt.exValue) + So(e, ShouldBeNil) + } + }) + }) +} + func TestBuildSearchReleaseQuery(t *testing.T) { t.Parallel() Convey("Should return InternalError for invalid template", t, func() { From e408f1cd76b66502447503e6a71dccb0f83ed8ab Mon Sep 17 00:00:00 2001 From: bpathak-ons Date: Thu, 7 Apr 2022 17:32:41 +0100 Subject: [PATCH 10/29] further unit test updates --- models/elasticsearch.go | 23 ++++++++-- transformer/transformer.go | 8 ++-- transformer/transformer_test.go | 74 ++++++++++++++++++++++++++++++--- 3 files changed, 92 insertions(+), 13 deletions(-) diff --git a/models/elasticsearch.go b/models/elasticsearch.go index 6fb7679a..61ed224b 100644 --- a/models/elasticsearch.go +++ b/models/elasticsearch.go @@ -10,8 +10,10 @@ type Es7xResponse struct { } type EsResponse7x struct { - Took int `json:"took"` - Hits ES7xResponseHits `json:"hits"` + Took int `json:"took"` + Hits ES7xResponseHits `json:"hits"` + Aggregations ES7xResponseAggregations `json:"aggregations"` + Suggest []string `json:"suggest"` } type ES7xResponseHits struct { @@ -20,8 +22,21 @@ type ES7xResponseHits struct { } type ES7xResponseHit struct { - Source ES7xSourceDocument `json:"_source"` - Highlight ES7xHighlight `json:"highlight"` + Source []ES7xSourceDocument `json:"_source"` + Highlight ES7xHighlight `json:"highlight"` +} + +type ES7xResponseAggregations struct { + Doccounts ES7xDocCounts `json:"docCounts"` +} + +type ES7xDocCounts struct { + Buckets []ES7xBucket `json:"buckets"` +} + +type ES7xBucket struct { + Key string `json:"key"` + Count int `json:"doc_count"` } type ES7xSourceDocument struct { diff --git a/transformer/transformer.go b/transformer/transformer.go index 1f8b026d..96dd60b8 100644 --- a/transformer/transformer.go +++ b/transformer/transformer.go @@ -49,7 +49,7 @@ func (t *LegacyTransformer) TransformSearchResponse(ctx context.Context, respons return nil, errors.New("Response to be transformed contained 0 items") } - sr := t.transform(&source, highlight) + sr := t.legayTransform(&source, highlight) needAdditionalSuggestions := numberOfSearchTerms(query) if needAdditionalSuggestions > 1 { @@ -64,7 +64,7 @@ func (t *LegacyTransformer) TransformSearchResponse(ctx context.Context, respons return transformedData, nil } -func (t *LegacyTransformer) transform(source *models.ESResponse, highlight bool) models.SearchResponse { +func (t *LegacyTransformer) legayTransform(source *models.ESResponse, highlight bool) models.SearchResponse { sr := models.SearchResponse{ Count: source.Responses[0].Hits.Total, Items: []models.ContentItem{}, @@ -221,7 +221,9 @@ func (t *Transformer) TransformSearchResponse( // Transform the raw ES to search response func (t *Transformer) transform(es7xresponse *models.Es7xResponse, highlight bool) models.Search7xResponse { var search7xResponse = models.Search7xResponse{ - Took: es7xresponse.Responses[0].Took, + Took: es7xresponse.Responses[0].Took, + Items: es7xresponse.Responses[0].Hits.Hits[0].Source, + Suggestions: es7xresponse.Responses[0].Suggest, } return search7xResponse } diff --git a/transformer/transformer_test.go b/transformer/transformer_test.go index ed52570b..4b0e910c 100644 --- a/transformer/transformer_test.go +++ b/transformer/transformer_test.go @@ -23,7 +23,7 @@ func TestLegacyTransformer(t *testing.T) { }, }}, } - sr := transformer.transform(&es, false) + sr := transformer.legayTransform(&es, false) So(sr.Suggestions, ShouldBeEmpty) }) @@ -39,7 +39,7 @@ func TestLegacyTransformer(t *testing.T) { }, }}, } - sr := transformer.transform(&es, true) + sr := transformer.legayTransform(&es, true) So(sr.Suggestions, ShouldNotBeEmpty) So(len(sr.Suggestions), ShouldEqual, 1) So(sr.Suggestions[0], ShouldResemble, "option1") @@ -68,7 +68,7 @@ func TestLegacyTransformer(t *testing.T) { }, }}, } - sr := transformer.transform(&es, true) + sr := transformer.legayTransform(&es, true) So(sr.Suggestions, ShouldNotBeEmpty) So(len(sr.Suggestions), ShouldEqual, 3) So(sr.Suggestions[0], ShouldResemble, "option1") @@ -167,6 +167,31 @@ func TestLegacyTransformSearchResponse(t *testing.T) { } func TestTransform(t *testing.T) { + expectedESDocument1 := models.ES7xSourceDocument{ + DataType: "anyDataType1", + JobID: "", + CDID: "", + DatasetID: "", + Keywords: []string{"anykeyword1"}, + MetaDescription: "", + Summary: "", + ReleaseDate: "", + Title: "anyTitle2", + Topics: []string{"anyTopic1"}, + } + expectedESDocument2 := models.ES7xSourceDocument{ + DataType: "anyDataType2", + JobID: "", + CDID: "", + DatasetID: "", + Keywords: []string{"anykeyword2"}, + MetaDescription: "", + Summary: "", + ReleaseDate: "", + Title: "anyTitle2", + Topics: []string{"anyTopic2"}, + } + Convey("Given a new instance of Transformer7x with search responses successfully", t, func() { transformer := New() esResponse := prepareESMockResponse() @@ -177,6 +202,10 @@ func TestTransform(t *testing.T) { Convey("Then transforms unmarshalled search responses successfully", func() { So(transformedResponse, ShouldNotBeNil) So(transformedResponse.Took, ShouldEqual, 10) + So(len(transformedResponse.Items), ShouldEqual, 2) + So(transformedResponse.Items[0], ShouldResemble, expectedESDocument1) + So(transformedResponse.Items[1], ShouldResemble, expectedESDocument2) + So(transformedResponse.Suggestions[0], ShouldResemble, "testSuggestion") }) }) }) @@ -184,8 +213,8 @@ func TestTransform(t *testing.T) { // Prepare mock ES response func prepareESMockResponse() models.Es7xResponse { - esDocument := models.ES7xSourceDocument{ - DataType: "anyDataType2", + esDocument1 := models.ES7xSourceDocument{ + DataType: "anyDataType1", JobID: "", CDID: "", DatasetID: "", @@ -197,11 +226,40 @@ func prepareESMockResponse() models.Es7xResponse { Topics: []string{"anyTopic1"}, } + esDocument2 := models.ES7xSourceDocument{ + DataType: "anyDataType2", + JobID: "", + CDID: "", + DatasetID: "", + Keywords: []string{"anykeyword2"}, + MetaDescription: "", + Summary: "", + ReleaseDate: "", + Title: "anyTitle2", + Topics: []string{"anyTopic2"}, + } + + esDocuments := []models.ES7xSourceDocument{esDocument1, esDocument2} + hit7x := models.ES7xResponseHit{ - Source: esDocument, + Source: esDocuments, Highlight: models.ES7xHighlight{}, } + bucket1 := models.ES7xBucket{ + Key: "article", + Count: 1, + } + bucket2 := models.ES7xBucket{ + Key: "product_page", + Count: 1, + } + buckets := []models.ES7xBucket{bucket1, bucket2} + + es7xDoccount := models.ES7xDocCounts{ + Buckets: buckets, + } + esResponse7x1 := models.EsResponse7x{ Took: 10, Hits: models.ES7xResponseHits{ @@ -210,6 +268,10 @@ func prepareESMockResponse() models.Es7xResponse { hit7x, }, }, + Aggregations: models.ES7xResponseAggregations{ + Doccounts: es7xDoccount, + }, + Suggest: []string{"testSuggestion"}, } // Preparing ES response array From 29ca327b6f87f7059b86d59914c29f8938674c94 Mon Sep 17 00:00:00 2001 From: PatrickConnollyONS Date: Thu, 7 Apr 2022 17:51:59 +0100 Subject: [PATCH 11/29] Add the new release type to the api --- api/releases.go | 12 +++++++++--- query/releasesearch.go | 18 ++++++++++++++++-- query/releasesearch_test.go | 9 +++------ templates/search/releasecalendar/main.go | 2 +- 4 files changed, 29 insertions(+), 12 deletions(-) diff --git a/api/releases.go b/api/releases.go index 8db44802..81177c70 100644 --- a/api/releases.go +++ b/api/releases.go @@ -69,7 +69,14 @@ func SearchReleasesHandlerFunc(validator QueryParamValidator, builder ReleaseQue http.Error(w, "Invalid date parameters", http.StatusBadRequest) return } - upcoming := paramGetBool(params, "type-upcoming", false) + + relTypeParam := paramGet(params, "release-type", query.Published.String()) + relType, err := validator.Validate(ctx, "release-type", relTypeParam) + if err != nil { + log.Warn(ctx, err.Error(), log.Data{"param": "release-type", "value": relTypeParam}) + http.Error(w, "Invalid release-type parameter", http.StatusBadRequest) + return + } highlight := paramGetBool(params, "highlight", true) formattedQuery, err := builder.BuildSearchQuery(ctx, @@ -80,8 +87,7 @@ func SearchReleasesHandlerFunc(validator QueryParamValidator, builder ReleaseQue SortBy: sort.(query.Sort), ReleasedAfter: fromDate.(query.Date), ReleasedBefore: toDate.(query.Date), - Upcoming: upcoming, - Published: !upcoming, + Type: relType.(query.ReleaseType), Highlight: highlight, Now: query.Date(time.Now()), }) diff --git a/query/releasesearch.go b/query/releasesearch.go index 5d374eb6..51ff2722 100644 --- a/query/releasesearch.go +++ b/query/releasesearch.go @@ -229,8 +229,11 @@ type ReleaseSearchRequest struct { SortBy Sort ReleasedAfter Date ReleasedBefore Date - Upcoming bool - Published bool + Type ReleaseType + Provisional bool + Confirmed bool + Postponed bool + Census bool Highlight bool Now Date } @@ -243,6 +246,17 @@ func (sr *ReleaseSearchRequest) String() string { return string(s) } +func (sr ReleaseSearchRequest) ReleaseType() string { + switch sr.Type { + case Upcoming: + return fmt.Sprintf("%s, %s, %s", `{"term": {"description.published": false}}`, `{"term": {"description.cancelled": false}}`, + fmt.Sprintf(`{"range": {"description.release_date": {"gte": %q}}}`, time.Now().Format(dateFormat))) + case Published: + return fmt.Sprintf("%s, %s", `{"term": {"description.published": true}}`, `{"term": {"description.cancelled": false}}`) + default: + return fmt.Sprintf("%s, %s", `{"term": {"description.published": false}}`, `{"term": {"description.cancelled": true}}`) + } +} func (sr *ReleaseSearchRequest) Set(value string) error { var sr2 ReleaseSearchRequest diff --git a/query/releasesearch_test.go b/query/releasesearch_test.go index c6e135b7..55307e94 100644 --- a/query/releasesearch_test.go +++ b/query/releasesearch_test.go @@ -176,8 +176,7 @@ func TestBuildSearchReleaseQuery(t *testing.T) { "SortBy={{.SortBy.ESString}};" + "ReleasedAfter={{.ReleasedAfter.ESString}};" + "ReleasedBefore={{.ReleasedBefore.ESString}};" + - "Upcoming={{.Upcoming}};" + - "Published={{.Published}};" + + "Type={{.Type.String}};" + "Highlight={{.Highlight}};" + "Now={{.Now}};" + "NowES={{.Now.ESString}}") @@ -189,8 +188,7 @@ func TestBuildSearchReleaseQuery(t *testing.T) { SortBy: TitleAsc, ReleasedAfter: Date{}, ReleasedBefore: MustParseDate("2020-12-31"), - Upcoming: true, - Published: false, + Type: Published, Highlight: true, Now: MustParseDate("2001-01-01"), }) @@ -204,8 +202,7 @@ func TestBuildSearchReleaseQuery(t *testing.T) { So(queryString, ShouldContainSubstring, `SortBy={"description.title": "asc"}`) So(queryString, ShouldContainSubstring, "ReleasedAfter=null") So(queryString, ShouldContainSubstring, `ReleasedBefore="2020-12-31"`) - So(queryString, ShouldContainSubstring, "Upcoming=true") - So(queryString, ShouldContainSubstring, "Published=false") + So(queryString, ShouldContainSubstring, "Type=type-published") So(queryString, ShouldContainSubstring, "Highlight=true") So(queryString, ShouldContainSubstring, `Now=2001-01-01`) So(queryString, ShouldContainSubstring, `NowES="2001-01-01"`) diff --git a/templates/search/releasecalendar/main.go b/templates/search/releasecalendar/main.go index b94c8ec4..bda76899 100644 --- a/templates/search/releasecalendar/main.go +++ b/templates/search/releasecalendar/main.go @@ -26,7 +26,7 @@ func main() { Term: "Education in Wales", ReleasedAfter: query.MustParseDate("2018-01-01"), ReleasedBefore: query.MustParseDate("2018-12-31"), - Published: true, + Type: query.Published, Highlight: false, Now: query.Date(time.Now()), } From 2a382574bd06634ceca36674067c971213e92fa4 Mon Sep 17 00:00:00 2001 From: PatrickConnollyONS Date: Thu, 7 Apr 2022 18:06:03 +0100 Subject: [PATCH 12/29] Remove the published(.tmpl) and upcoming(.tmpl) templates --- query/releasesearch.go | 4 +- .../search/releasecalendar/published.tmpl | 47 ---------------- templates/search/releasecalendar/search.tmpl | 10 +--- .../search/releasecalendar/upcoming.tmpl | 55 ------------------- 4 files changed, 4 insertions(+), 112 deletions(-) delete mode 100644 templates/search/releasecalendar/published.tmpl delete mode 100644 templates/search/releasecalendar/upcoming.tmpl diff --git a/query/releasesearch.go b/query/releasesearch.go index 51ff2722..a52a9f5f 100644 --- a/query/releasesearch.go +++ b/query/releasesearch.go @@ -200,9 +200,7 @@ type ReleaseBuilder struct { func NewReleaseBuilder(pathToTemplates string) (*ReleaseBuilder, error) { searchTemplate, err := template.ParseFiles( pathToTemplates+"templates/search/releasecalendar/search.tmpl", - pathToTemplates+"templates/search/releasecalendar/query.tmpl", - pathToTemplates+"templates/search/releasecalendar/upcoming.tmpl", - pathToTemplates+"templates/search/releasecalendar/published.tmpl") + pathToTemplates+"templates/search/releasecalendar/query.tmpl") if err != nil { return nil, fmt.Errorf("failed to load search template: %w", err) } diff --git a/templates/search/releasecalendar/published.tmpl b/templates/search/releasecalendar/published.tmpl deleted file mode 100644 index ea38ba5e..00000000 --- a/templates/search/releasecalendar/published.tmpl +++ /dev/null @@ -1,47 +0,0 @@ -{ - "bool": { - "should": [ - { - "bool": { - "must": [ - { - "term": { - "description.published": true - } - }, - { - "bool": { - "must_not": { - "term": { - "description.cancelled": true - } - } - } - } - ] - } - }, - { - "bool": { - "must": [ - { - "term": { - "description.cancelled": true - } - }, - { - "range": { - "description.releaseDate": { - "from": null, - "to": {{.Now.ESString}}, - "include_lower": true, - "include_upper": true - } - } - } - ] - } - } - ] - } -} diff --git a/templates/search/releasecalendar/search.tmpl b/templates/search/releasecalendar/search.tmpl index 4f9f81cc..5c0a1d79 100644 --- a/templates/search/releasecalendar/search.tmpl +++ b/templates/search/releasecalendar/search.tmpl @@ -25,13 +25,9 @@ "include_upper": true } } - } - {{if .Upcoming}} - ,{{- template "upcoming.tmpl" . }} - {{else if .Published}} - ,{{- template "published.tmpl" . }} - {{end}} - ] + }, + {{.ReleaseType}} + ] } } {{if .Highlight}} diff --git a/templates/search/releasecalendar/upcoming.tmpl b/templates/search/releasecalendar/upcoming.tmpl deleted file mode 100644 index 8ff537a4..00000000 --- a/templates/search/releasecalendar/upcoming.tmpl +++ /dev/null @@ -1,55 +0,0 @@ -{ - "bool": { - "should": [ - { - "bool": { - "must": [ - { - "bool": { - "must_not": { - "term": { - "description.published": true - } - } - } - }, - { - "bool": { - "must_not": { - "term": { - "description.cancelled": true - } - } - } - } - ] - } - }, - { - "bool": { - "must": [ - { - "term": { - "description.cancelled": true - } - }, - { - "bool": { - "must_not": { - "range": { - "description.releaseDate": { - "from": null, - "to": {{.Now.ESString}}, - "include_lower": true, - "include_upper": true - } - } - } - } - } - ] - } - } - ] - } -} From cbfd026ed05b8211b67930a158785d3c49f6f162 Mon Sep 17 00:00:00 2001 From: PatrickConnollyONS Date: Thu, 7 Apr 2022 18:30:07 +0100 Subject: [PATCH 13/29] Fix bug in from/to date check --- api/releases.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/api/releases.go b/api/releases.go index 81177c70..32805fbd 100644 --- a/api/releases.go +++ b/api/releases.go @@ -64,9 +64,9 @@ func SearchReleasesHandlerFunc(validator QueryParamValidator, builder ReleaseQue return } - if time.Time(fromDate.(query.Date)).After(time.Time(toDate.(query.Date))) { + if fromAfterTo(fromDate.(query.Date), toDate.(query.Date)) { log.Warn(ctx, "fromDate after toDate", log.Data{"fromDate": fromDateParam, "toDate": toDateParam}) - http.Error(w, "Invalid date parameters", http.StatusBadRequest) + http.Error(w, "invalid dates - 'from' after 'to'", http.StatusBadRequest) return } @@ -128,3 +128,11 @@ func SearchReleasesHandlerFunc(validator QueryParamValidator, builder ReleaseQue } } } + +func fromAfterTo(from, to query.Date) bool { + if !time.Time(from).IsZero() && !time.Time(to).IsZero() && time.Time(from).After(time.Time(to)) { + return true + } + + return false +} From 985def203b0d55e53fb33b02bd62a46f17c5a3ab Mon Sep 17 00:00:00 2001 From: PatrickConnollyONS Date: Thu, 7 Apr 2022 18:32:27 +0100 Subject: [PATCH 14/29] Refactor and tidy how the Sort option is handled by the template --- query/releasesearch.go | 21 ++++++++++++++++++-- templates/search/releasecalendar/search.tmpl | 6 +----- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/query/releasesearch.go b/query/releasesearch.go index a52a9f5f..ad61b8f1 100644 --- a/query/releasesearch.go +++ b/query/releasesearch.go @@ -127,10 +127,11 @@ const ( RelDateDesc TitleAsc TitleDesc + Relevance ) -var sortNames = map[Sort]string{RelDateAsc: "release_date_asc", RelDateDesc: "release_date_desc", TitleAsc: "title_asc", TitleDesc: "title_desc", Invalid: "invalid"} -var esSortNames = map[Sort]string{RelDateAsc: `{"description.releaseDate": "asc"}`, RelDateDesc: `{"description.releaseDate": "desc"}`, TitleAsc: `{"description.title": "asc"}`, TitleDesc: `{"description.title": "desc"}`, Invalid: "invalid"} +var sortNames = map[Sort]string{RelDateAsc: "release_date_asc", RelDateDesc: "release_date_desc", TitleAsc: "title_asc", TitleDesc: "title_desc", Relevance: "relevance", Invalid: "invalid"} +var esSortNames = map[Sort]string{RelDateAsc: `{"description.releaseDate": "asc"}`, RelDateDesc: `{"description.releaseDate": "desc"}`, TitleAsc: `{"description.title": "asc"}`, TitleDesc: `{"description.title": "desc"}`, Relevance: `{"_score": "desc"}`, Invalid: "invalid"} func ParseSort(sort string) (Sort, error) { for s, sn := range sortNames { @@ -244,6 +245,22 @@ func (sr *ReleaseSearchRequest) String() string { return string(s) } + +func (sr ReleaseSearchRequest) Sort() string { + if sr.SortBy == Relevance { + switch sr.Type { + case Upcoming: + return fmt.Sprintf("%s, %s", esSortNames[Relevance], esSortNames[RelDateAsc]) + case Published: + return fmt.Sprintf("%s, %s", esSortNames[Relevance], esSortNames[RelDateDesc]) + case Cancelled: + return esSortNames[Relevance] + } + } + + return sr.SortBy.ESString() +} + func (sr ReleaseSearchRequest) ReleaseType() string { switch sr.Type { case Upcoming: diff --git a/templates/search/releasecalendar/search.tmpl b/templates/search/releasecalendar/search.tmpl index 5c0a1d79..7860b472 100644 --- a/templates/search/releasecalendar/search.tmpl +++ b/templates/search/releasecalendar/search.tmpl @@ -2,11 +2,7 @@ { "from": {{.From}}, "size": {{.Size}}, - "sort": [ - {{if .Term}} {"_score": "desc"}, {{- .SortBy.ESString -}} - {{else}} {{- .SortBy.ESString -}} - {{end}} - ], + "sort": [{{.Sort}}], "query": { "bool": { "must": { From 68f3f9198d0cc5289760a5dd8fe20e0926728b3c Mon Sep 17 00:00:00 2001 From: PatrickConnollyONS Date: Thu, 7 Apr 2022 18:36:00 +0100 Subject: [PATCH 15/29] Add the ReleaseSearchRequest as a parameter to the transformer so that the results pertinent to the request can be set correctly --- api/api.go | 6 +- transformer/releasetransformer.go | 62 ++++++++++++------- transformer/releasetransformer_test.go | 8 ++- .../search_release_expected_highlighted.json | 3 +- .../search_release_expected_plain.json | 3 +- 5 files changed, 54 insertions(+), 28 deletions(-) diff --git a/api/api.go b/api/api.go index 18117f4b..9e58efc5 100644 --- a/api/api.go +++ b/api/api.go @@ -64,6 +64,10 @@ type ResponseTransformer interface { TransformSearchResponse(ctx context.Context, responseData []byte, query string, highlight bool) ([]byte, error) } +type ReleaseResponseTransformer interface { + TransformSearchResponse(ctx context.Context, responseData []byte, req query.ReleaseSearchRequest, highlight bool) ([]byte, error) +} + // NewSearchAPI returns a new Search API struct after registering the routes func NewSearchAPI(router *mux.Router, dpESClient DpElasticSearcher, deprecatedESClient ElasticSearcher, queryBuilder QueryBuilder, transformer ResponseTransformer, permissions AuthHandler) (*SearchAPI, error) { errData := SetupData() @@ -93,7 +97,7 @@ func NewSearchAPI(router *mux.Router, dpESClient DpElasticSearcher, deprecatedES return api, nil } -func (a *SearchAPI) AddSearchReleaseAPI(validator QueryParamValidator, builder ReleaseQueryBuilder, searcher ElasticSearcher, transformer ResponseTransformer) *SearchAPI { +func (a *SearchAPI) AddSearchReleaseAPI(validator QueryParamValidator, builder ReleaseQueryBuilder, searcher ElasticSearcher, transformer ReleaseResponseTransformer) *SearchAPI { a.Router.HandleFunc("/search/releases", SearchReleasesHandlerFunc(validator, builder, searcher, transformer)).Methods("GET") return a diff --git a/transformer/releasetransformer.go b/transformer/releasetransformer.go index 00e357e7..88bee73e 100644 --- a/transformer/releasetransformer.go +++ b/transformer/releasetransformer.go @@ -6,6 +6,8 @@ import ( "strings" "github.com/pkg/errors" + + "github.com/ONSdigital/dp-search-api/query" ) type ReleaseTransformer struct { @@ -114,15 +116,29 @@ func NewReleaseTransformer() *ReleaseTransformer { } // TransformSearchResponse transforms an elastic search response into a structure that matches the v1 api specification -func (t *ReleaseTransformer) TransformSearchResponse(_ context.Context, responseData []byte, _ string, highlight bool) ([]byte, error) { - var source ESReleaseResponse +func (t *ReleaseTransformer) TransformSearchResponse(_ context.Context, responseData []byte, req query.ReleaseSearchRequest, highlight bool) ([]byte, error) { + var ( + source ESReleaseResponse + highlighter *strings.Replacer + ) err := json.Unmarshal(responseData, &source) if err != nil { return nil, errors.Wrap(err, "Failed to decode elastic search response") } - sr := t.transform(&source, highlight) + sr := SearchReleaseResponse{ + Took: source.Took, + Breakdown: breakdown(&source, req), + Releases: []Release{}, + } + + if highlight { + highlighter = t.higlightReplacer + } + for i := range source.Hits.Hits { + sr.Releases = append(sr.Releases, buildRelease(source.Hits.Hits[i], highlighter)) + } transformedData, err := json.Marshal(sr) if err != nil { @@ -132,21 +148,23 @@ func (t *ReleaseTransformer) TransformSearchResponse(_ context.Context, response return transformedData, nil } -func (t *ReleaseTransformer) transform(source *ESReleaseResponse, highlight bool) SearchReleaseResponse { - sr := SearchReleaseResponse{ - Took: source.Took, - Breakdown: Breakdown{Total: source.Hits.Total}, - Releases: []Release{}, - } +func breakdown(source *ESReleaseResponse, req query.ReleaseSearchRequest) Breakdown { + b := Breakdown{} - for i := range source.Hits.Hits { - sr.Releases = append(sr.Releases, t.buildRelease(source.Hits.Hits[i], highlight)) + b.Total = source.Hits.Total + switch req.Type { + case query.Upcoming: + b.Confirmed = b.Total + case query.Published: + b.Published = b.Total + case query.Cancelled: + b.Cancelled = b.Total } - return sr + return b } -func (t *ReleaseTransformer) buildRelease(hit ESReleaseResponseHit, highlightOn bool) Release { +func buildRelease(hit ESReleaseResponseHit, highlighter *strings.Replacer) Release { sd := hit.Source.Description hl := hit.Highlight @@ -166,11 +184,11 @@ func (t *ReleaseTransformer) buildRelease(hit ESReleaseResponseHit, highlightOn }, } - if highlightOn { + if highlighter != nil { r.Highlight = &highlight{ - Keywords: t.overlayList(hl.DescriptionKeywords, sd.Keywords, highlightOn), - Summary: t.overlayItem(hl.DescriptionSummary, sd.Summary, highlightOn), - Title: t.overlayItem(hl.DescriptionTitle, sd.Title, highlightOn), + Keywords: overlayList(hl.DescriptionKeywords, sd.Keywords, highlighter), + Summary: overlayItem(hl.DescriptionSummary, sd.Summary, highlighter), + Title: overlayItem(hl.DescriptionTitle, sd.Title, highlighter), } } @@ -181,24 +199,24 @@ func isPostponed(release ESReleaseSourceDocument) bool { return release.Description.Finalised && len(release.DateChanges) > 0 } -func (t *ReleaseTransformer) overlayItem(hl []string, def string, highlight bool) string { - if highlight && len(hl) > 0 { +func overlayItem(hl []string, def string, highlighter *strings.Replacer) string { + if highlighter != nil && len(hl) > 0 { return hl[0] } return def } -func (t *ReleaseTransformer) overlayList(hlList, defaultList []string, highlight bool) []string { +func overlayList(hlList, defaultList []string, highlighter *strings.Replacer) []string { if defaultList == nil || hlList == nil { return nil } overlaid := make([]string, len(defaultList)) copy(overlaid, defaultList) - if highlight { + if highlighter != nil { for _, hl := range hlList { - unformatted := t.higlightReplacer.Replace(hl) + unformatted := highlighter.Replace(hl) for i, defItem := range overlaid { if defItem == unformatted { overlaid[i] = hl diff --git a/transformer/releasetransformer_test.go b/transformer/releasetransformer_test.go index 9f0ea7dd..8728499d 100644 --- a/transformer/releasetransformer_test.go +++ b/transformer/releasetransformer_test.go @@ -7,6 +7,8 @@ import ( "testing" . "github.com/smartystreets/goconvey/convey" + + "github.com/ONSdigital/dp-search-api/query" ) func TestTransformSearchReleaseResponse(t *testing.T) { @@ -18,7 +20,7 @@ func TestTransformSearchReleaseResponse(t *testing.T) { Convey("Throws error on invalid JSON", func() { sampleResponse := []byte(`{"invalid":"json"`) - _, err := transformer.TransformSearchResponse(ctx, sampleResponse, "test-query", true) + _, err := transformer.TransformSearchResponse(ctx, sampleResponse, query.ReleaseSearchRequest{Term: "test-query", Type: query.Published}, true) So(err, ShouldNotBeNil) So(err.Error(), ShouldResemble, "Failed to decode elastic search response: unexpected end of JSON input") }) @@ -29,7 +31,7 @@ func TestTransformSearchReleaseResponse(t *testing.T) { expected, err := os.ReadFile("testdata/search_release_expected_highlighted.json") So(err, ShouldBeNil) - actual, err := transformer.TransformSearchResponse(ctx, sampleResponse, "test-query", true) + actual, err := transformer.TransformSearchResponse(ctx, sampleResponse, query.ReleaseSearchRequest{Term: "test-query", Type: query.Published}, true) So(err, ShouldBeNil) So(actual, ShouldNotBeEmpty) var exp, act SearchReleaseResponse @@ -44,7 +46,7 @@ func TestTransformSearchReleaseResponse(t *testing.T) { expected, err := os.ReadFile("testdata/search_release_expected_plain.json") So(err, ShouldBeNil) - actual, err := transformer.TransformSearchResponse(ctx, sampleResponse, "test-query", false) + actual, err := transformer.TransformSearchResponse(ctx, sampleResponse, query.ReleaseSearchRequest{Term: "test-query", Type: query.Published}, false) So(err, ShouldBeNil) So(actual, ShouldNotBeEmpty) var exp, act SearchResponse diff --git a/transformer/testdata/search_release_expected_highlighted.json b/transformer/testdata/search_release_expected_highlighted.json index 7e2cbcbe..90712cbd 100644 --- a/transformer/testdata/search_release_expected_highlighted.json +++ b/transformer/testdata/search_release_expected_highlighted.json @@ -3,7 +3,8 @@ "limit":10, "offset":0, "breakdown":{ - "total":2 + "total":2, + "published": 2 }, "releases":[ { diff --git a/transformer/testdata/search_release_expected_plain.json b/transformer/testdata/search_release_expected_plain.json index bb0ab87b..71c4fe55 100644 --- a/transformer/testdata/search_release_expected_plain.json +++ b/transformer/testdata/search_release_expected_plain.json @@ -3,7 +3,8 @@ "limit":10, "offset":0, "breakdown":{ - "total":2 + "total":2, + "published": 2 }, "releases":[ { From bea2609001903ee5795705555dd88e41d1cd7f31 Mon Sep 17 00:00:00 2001 From: PatrickConnollyONS Date: Thu, 7 Apr 2022 18:42:48 +0100 Subject: [PATCH 16/29] Refactor the release calendar search handler, and the general ElasticSearch query utility to use the new transformer --- api/releases.go | 33 ++++++++++++------------ templates/search/releasecalendar/main.go | 19 +++++++------- 2 files changed, 27 insertions(+), 25 deletions(-) diff --git a/api/releases.go b/api/releases.go index 32805fbd..61036302 100644 --- a/api/releases.go +++ b/api/releases.go @@ -12,12 +12,12 @@ import ( ) // SearchReleasesHandlerFunc returns a http handler function handling release calendar search api requests. -func SearchReleasesHandlerFunc(validator QueryParamValidator, builder ReleaseQueryBuilder, searcher ElasticSearcher, transformer ResponseTransformer) http.HandlerFunc { +func SearchReleasesHandlerFunc(validator QueryParamValidator, builder ReleaseQueryBuilder, searcher ElasticSearcher, transformer ReleaseResponseTransformer) http.HandlerFunc { return func(w http.ResponseWriter, req *http.Request) { ctx := req.Context() params := req.URL.Query() - q, err := url.QueryUnescape(params.Get("query")) + queryString, err := url.QueryUnescape(params.Get("query")) if err != nil { log.Warn(ctx, err.Error(), log.Data{"param": "query", "value": params.Get("query")}) http.Error(w, "Bad url encoding of the query parameter", http.StatusBadRequest) @@ -79,20 +79,21 @@ func SearchReleasesHandlerFunc(validator QueryParamValidator, builder ReleaseQue } highlight := paramGetBool(params, "highlight", true) - formattedQuery, err := builder.BuildSearchQuery(ctx, - query.ReleaseSearchRequest{ - Term: q, - From: offset.(int), - Size: limit.(int), - SortBy: sort.(query.Sort), - ReleasedAfter: fromDate.(query.Date), - ReleasedBefore: toDate.(query.Date), - Type: relType.(query.ReleaseType), - Highlight: highlight, - Now: query.Date(time.Now()), - }) + searchReq := query.ReleaseSearchRequest{ + Term: queryString, + From: offset.(int), + Size: limit.(int), + SortBy: sort.(query.Sort), + ReleasedAfter: fromDate.(query.Date), + ReleasedBefore: toDate.(query.Date), + Type: relType.(query.ReleaseType), + Highlight: highlight, + Now: query.Date(time.Now()), + } + + formattedQuery, err := builder.BuildSearchQuery(ctx, searchReq) if err != nil { - log.Error(ctx, "creation of search release query failed", err, log.Data{"q": q, "sort": sort, "limit": limit, "offset": offset}) + log.Error(ctx, "creation of search release query failed", err, log.Data{"q": queryString, "sort": sort, "limit": limit, "offset": offset}) http.Error(w, "Failed to create search release query", http.StatusInternalServerError) return } @@ -111,7 +112,7 @@ func SearchReleasesHandlerFunc(validator QueryParamValidator, builder ReleaseQue } if !paramGetBool(params, "raw", false) { - responseData, err = transformer.TransformSearchResponse(ctx, responseData, q, highlight) + responseData, err = transformer.TransformSearchResponse(ctx, responseData, searchReq, highlight) if err != nil { log.Error(ctx, "transformation of response data failed", err) http.Error(w, "Failed to transform search result", http.StatusInternalServerError) diff --git a/templates/search/releasecalendar/main.go b/templates/search/releasecalendar/main.go index bda76899..ec442a5f 100644 --- a/templates/search/releasecalendar/main.go +++ b/templates/search/releasecalendar/main.go @@ -12,7 +12,6 @@ import ( "time" dphttp "github.com/ONSdigital/dp-net/v2/http" - "github.com/ONSdigital/dp-search-api/api" "github.com/ONSdigital/dp-search-api/elasticsearch" "github.com/ONSdigital/dp-search-api/query" "github.com/ONSdigital/dp-search-api/transformer" @@ -33,12 +32,11 @@ func main() { builder *query.ReleaseBuilder q, uq, responseData []byte err error - ctx = context.Background() - multi = flag.Bool("multi", false, "use the multi query format when sending the query to ES") - file = flag.String("file", "", "a file containing the actual query to be sent to ES (in json format)") - esClient = elasticsearch.New("http://localhost:9200", dphttp.NewClient(), "eu-west-1", "es") - esSearch = esClient.Search - esTransformer api.ResponseTransformer = transformer.NewReleaseTransformer() + ctx = context.Background() + multi = flag.Bool("multi", false, "use the multi query format when sending the query to ES") + file = flag.String("file", "", "a file containing the actual query to be sent to ES (in json format)") + esClient = elasticsearch.New("http://localhost:9200", dphttp.NewClient(), "eu-west-1", "es") + esSearch = esClient.Search ) flag.Var(&sr, "sr", "a searchRequest object in json format") @@ -83,7 +81,6 @@ func main() { log.Fatalf("failed to format multi query: %s", err) } esSearch = esClient.MultiSearch - esTransformer = transformer.New() } fmt.Printf("\nformatted query is:\n%s", q) @@ -97,7 +94,11 @@ func main() { } fmt.Printf("\nresponse is:\n%s", responseData) - responseData, err = esTransformer.TransformSearchResponse(ctx, responseData, sr.Term, sr.Highlight) + if *multi { + responseData, err = transformer.New().TransformSearchResponse(ctx, responseData, sr.Term, sr.Highlight) + } else { + responseData, err = transformer.NewReleaseTransformer().TransformSearchResponse(ctx, responseData, sr, sr.Highlight) + } if err != nil { log.Fatalf("transformation of response data failed: %s", err) } From 838631f9b0499521c4e37a342943c8bbddc84360 Mon Sep 17 00:00:00 2001 From: bpathak-ons Date: Fri, 8 Apr 2022 09:42:00 +0100 Subject: [PATCH 17/29] rename legacy models --- features/steps/steps.go | 2 +- models/esmodels.go | 156 ++++++++++++------------- transformer/releasetransformer_test.go | 2 +- transformer/transformer.go | 24 ++-- transformer/transformer_test.go | 54 ++++----- 5 files changed, 119 insertions(+), 119 deletions(-) diff --git a/features/steps/steps.go b/features/steps/steps.go index 6d01edb9..50d10858 100644 --- a/features/steps/steps.go +++ b/features/steps/steps.go @@ -97,7 +97,7 @@ func (c *Component) failureInternalServerError() error { } func (c *Component) iShouldReceiveTheFollowingSearchResponse(expectedJSONFile string) error { - var searchResponse, expectedSearchResponse models.SearchResponse + var searchResponse, expectedSearchResponse models.SearchResponseLegacy responseBody, err := io.ReadAll(c.APIFeature.HttpResponse.Body) if err != nil { diff --git a/models/esmodels.go b/models/esmodels.go index 9bca9930..38cf157b 100644 --- a/models/esmodels.go +++ b/models/esmodels.go @@ -1,55 +1,55 @@ package models // Structs representing the transformed response -type SearchResponse struct { - Count int `json:"count"` - Took int `json:"took"` - ContentTypes []ContentType `json:"content_types"` - Items []ContentItem `json:"items"` - Suggestions []string `json:"suggestions,omitempty"` - AdditionSuggestions []string `json:"additional_suggestions,omitempty"` +type SearchResponseLegacy struct { + Count int `json:"count"` + Took int `json:"took"` + ContentTypes []ContentTypeLegacy `json:"content_types"` + Items []ContentItemLegacy `json:"items"` + Suggestions []string `json:"suggestions,omitempty"` + AdditionSuggestions []string `json:"additional_suggestions,omitempty"` } -type ContentType struct { +type ContentTypeLegacy struct { Type string `json:"type"` Count int `json:"count"` } -type ContentItem struct { - Description Description `json:"description"` - Type string `json:"type"` - URI string `json:"uri"` -} - -type Description struct { - Contact *contact `json:"contact,omitempty"` - DatasetID string `json:"dataset_id,omitempty"` - Edition string `json:"edition,omitempty"` - Headline1 string `json:"headline1,omitempty"` - Headline2 string `json:"headline2,omitempty"` - Headline3 string `json:"headline3,omitempty"` - Highlight *HighlightObj `json:"highlight,omitempty"` - Keywords *[]string `json:"keywords,omitempty"` - LatestRelease *bool `json:"latest_release,omitempty"` - Language string `json:"language,omitempty"` - MetaDescription string `json:"meta_description,omitempty"` - NationalStatistic *bool `json:"national_statistic,omitempty"` - NextRelease string `json:"next_release,omitempty"` - PreUnit string `json:"pre_unit,omitempty"` - ReleaseDate string `json:"release_date,omitempty"` - Source string `json:"source,omitempty"` - Summary string `json:"summary"` - Title string `json:"title"` - Unit string `json:"unit,omitempty"` -} - -type contact struct { +type ContentItemLegacy struct { + Description DescriptionLegacy `json:"description"` + Type string `json:"type"` + URI string `json:"uri"` +} + +type DescriptionLegacy struct { + Contact *contactLegacy `json:"contact,omitempty"` + DatasetID string `json:"dataset_id,omitempty"` + Edition string `json:"edition,omitempty"` + Headline1 string `json:"headline1,omitempty"` + Headline2 string `json:"headline2,omitempty"` + Headline3 string `json:"headline3,omitempty"` + Highlight *HighlightObjLegacy `json:"highlight,omitempty"` + Keywords *[]string `json:"keywords,omitempty"` + LatestRelease *bool `json:"latest_release,omitempty"` + Language string `json:"language,omitempty"` + MetaDescription string `json:"meta_description,omitempty"` + NationalStatistic *bool `json:"national_statistic,omitempty"` + NextRelease string `json:"next_release,omitempty"` + PreUnit string `json:"pre_unit,omitempty"` + ReleaseDate string `json:"release_date,omitempty"` + Source string `json:"source,omitempty"` + Summary string `json:"summary"` + Title string `json:"title"` + Unit string `json:"unit,omitempty"` +} + +type contactLegacy struct { Name string `json:"name"` Telephone string `json:"telephone,omitempty"` Email string `json:"email"` } -type HighlightObj struct { +type HighlightObjLegacy struct { DatasetID string `json:"dataset_id,omitempty"` Edition string `json:"edition,omitempty"` Keywords *[]string `json:"keywords,omitempty"` @@ -60,53 +60,53 @@ type HighlightObj struct { // Structs representing the raw elastic search response -type ESResponse struct { - Responses []ESResponseItem `json:"responses"` +type ESResponseLegacy struct { + Responses []ESResponseItemLegacy `json:"responses"` } -type ESResponseItem struct { - Took int `json:"took"` - Hits ESResponseHits `json:"hits"` - Aggregations ESResponseAggregations `json:"aggregations"` - Suggest ESSuggest `json:"suggest"` +type ESResponseItemLegacy struct { + Took int `json:"took"` + Hits ESResponseHitsLegacy `json:"hits"` + Aggregations ESResponseAggregationsLegacy `json:"aggregations"` + Suggest ESSuggestLegacy `json:"suggest"` } -type ESResponseHits struct { +type ESResponseHitsLegacy struct { Total int - Hits []ESResponseHit `json:"hits"` + Hits []ESResponseHitLegacy `json:"hits"` } -type ESResponseHit struct { - Source ESSourceDocument `json:"_source"` - Highlight ESHighlight `json:"highlight"` +type ESResponseHitLegacy struct { + Source ESSourceDocumentLegacy `json:"_source"` + Highlight ESHighlightLegacy `json:"highlight"` } -type ESSourceDocument struct { +type ESSourceDocumentLegacy struct { Description struct { - Summary string `json:"summary"` - NextRelease string `json:"nextRelease,omitempty"` - Unit string `json:"unit,omitempty"` - Keywords *[]string `json:"keywords,omitempty"` - ReleaseDate string `json:"releaseDate,omitempty"` - Edition string `json:"edition,omitempty"` - LatestRelease *bool `json:"latestRelease,omitempty"` - Language string `json:"language,omitempty"` - Contact *contact `json:"contact,omitempty"` - DatasetID string `json:"datasetId,omitempty"` - Source string `json:"source,omitempty"` - Title string `json:"title"` - MetaDescription string `json:"metaDescription,omitempty"` - NationalStatistic *bool `json:"nationalStatistic,omitempty"` - PreUnit string `json:"preUnit,omitempty"` - Headline1 string `json:"headline1,omitempty"` - Headline2 string `json:"headline2,omitempty"` - Headline3 string `json:"headline3,omitempty"` + Summary string `json:"summary"` + NextRelease string `json:"nextRelease,omitempty"` + Unit string `json:"unit,omitempty"` + Keywords *[]string `json:"keywords,omitempty"` + ReleaseDate string `json:"releaseDate,omitempty"` + Edition string `json:"edition,omitempty"` + LatestRelease *bool `json:"latestRelease,omitempty"` + Language string `json:"language,omitempty"` + Contact *contactLegacy `json:"contact,omitempty"` + DatasetID string `json:"datasetId,omitempty"` + Source string `json:"source,omitempty"` + Title string `json:"title"` + MetaDescription string `json:"metaDescription,omitempty"` + NationalStatistic *bool `json:"nationalStatistic,omitempty"` + PreUnit string `json:"preUnit,omitempty"` + Headline1 string `json:"headline1,omitempty"` + Headline2 string `json:"headline2,omitempty"` + Headline3 string `json:"headline3,omitempty"` } `json:"description"` Type string `json:"type"` URI string `json:"uri"` } -type ESHighlight struct { +type ESHighlightLegacy struct { DescriptionTitle *[]string `json:"description.title"` DescriptionEdition *[]string `json:"description.edition"` DescriptionSummary *[]string `json:"description.summary"` @@ -115,25 +115,25 @@ type ESHighlight struct { DescriptionDatasetID *[]string `json:"description.datasetId"` } -type ESResponseAggregations struct { +type ESResponseAggregationsLegacy struct { DocCounts struct { - Buckets []ESBucket `json:"buckets"` + Buckets []ESBucketLegacy `json:"buckets"` } `json:"docCounts"` } -type ESBucket struct { +type ESBucketLegacy struct { Key string `json:"key"` Count int `json:"doc_count"` } -type ESSuggest struct { - SearchSuggest []ESSearchSuggest `json:"search_suggest"` +type ESSuggestLegacy struct { + SearchSuggest []ESSearchSuggestLegacy `json:"search_suggest"` } -type ESSearchSuggest struct { - Options []ESSearchSuggestOptions `json:"options"` +type ESSearchSuggestLegacy struct { + Options []ESSearchSuggestOptionsLegacy `json:"options"` } -type ESSearchSuggestOptions struct { +type ESSearchSuggestOptionsLegacy struct { Text string `json:"text"` } diff --git a/transformer/releasetransformer_test.go b/transformer/releasetransformer_test.go index 43c03e2f..01c35daa 100644 --- a/transformer/releasetransformer_test.go +++ b/transformer/releasetransformer_test.go @@ -48,7 +48,7 @@ func TestTransformSearchReleaseResponse(t *testing.T) { actual, err := transformer.TransformSearchResponse(ctx, sampleResponse, "test-query", false) So(err, ShouldBeNil) So(actual, ShouldNotBeEmpty) - var exp, act models.SearchResponse + var exp, act models.SearchResponseLegacy So(json.Unmarshal(expected, &exp), ShouldBeNil) So(json.Unmarshal(actual, &act), ShouldBeNil) So(act, ShouldResemble, exp) diff --git a/transformer/transformer.go b/transformer/transformer.go index 96dd60b8..5a5d03a8 100644 --- a/transformer/transformer.go +++ b/transformer/transformer.go @@ -38,7 +38,7 @@ func New() *Transformer { // TransformSearchResponse transforms an elastic search response into a structure that matches the v1 api specification func (t *LegacyTransformer) TransformSearchResponse(ctx context.Context, responseData []byte, query string, highlight bool) ([]byte, error) { - var source models.ESResponse + var source models.ESResponseLegacy err := json.Unmarshal(responseData, &source) if err != nil { @@ -64,11 +64,11 @@ func (t *LegacyTransformer) TransformSearchResponse(ctx context.Context, respons return transformedData, nil } -func (t *LegacyTransformer) legayTransform(source *models.ESResponse, highlight bool) models.SearchResponse { - sr := models.SearchResponse{ +func (t *LegacyTransformer) legayTransform(source *models.ESResponseLegacy, highlight bool) models.SearchResponseLegacy { + sr := models.SearchResponseLegacy{ Count: source.Responses[0].Hits.Total, - Items: []models.ContentItem{}, - ContentTypes: []models.ContentType{}, + Items: []models.ContentItemLegacy{}, + ContentTypes: []models.ContentTypeLegacy{}, } var took int for _, response := range source.Responses { @@ -89,8 +89,8 @@ func (t *LegacyTransformer) legayTransform(source *models.ESResponse, highlight return sr } -func (t *LegacyTransformer) buildContentItem(doc models.ESResponseHit, highlight bool) models.ContentItem { - ci := models.ContentItem{ +func (t *LegacyTransformer) buildContentItem(doc models.ESResponseHitLegacy, highlight bool) models.ContentItemLegacy { + ci := models.ContentItemLegacy{ Description: t.buildDescription(doc, highlight), Type: doc.Source.Type, URI: doc.Source.URI, @@ -99,11 +99,11 @@ func (t *LegacyTransformer) buildContentItem(doc models.ESResponseHit, highlight return ci } -func (t *LegacyTransformer) buildDescription(doc models.ESResponseHit, highlight bool) models.Description { +func (t *LegacyTransformer) buildDescription(doc models.ESResponseHitLegacy, highlight bool) models.DescriptionLegacy { sd := doc.Source.Description hl := doc.Highlight - des := models.Description{ + des := models.DescriptionLegacy{ Summary: sd.Summary, NextRelease: sd.NextRelease, Unit: sd.Unit, @@ -125,7 +125,7 @@ func (t *LegacyTransformer) buildDescription(doc models.ESResponseHit, highlight } if highlight { - des.Highlight = &models.HighlightObj{ + des.Highlight = &models.HighlightObjLegacy{ DatasetID: t.overlaySingleItem(hl.DescriptionDatasetID, sd.DatasetID, highlight), Edition: t.overlaySingleItem(hl.DescriptionEdition, sd.Edition, highlight), Keywords: t.overlayItemList(hl.DescriptionKeywords, sd.Keywords, highlight), @@ -166,8 +166,8 @@ func (t *LegacyTransformer) overlayItemList(hlList, defaultList *[]string, highl return &overlaid } -func buildContentTypes(bucket models.ESBucket) models.ContentType { - return models.ContentType{ +func buildContentTypes(bucket models.ESBucketLegacy) models.ContentTypeLegacy { + return models.ContentTypeLegacy{ Type: bucket.Key, Count: bucket.Count, } diff --git a/transformer/transformer_test.go b/transformer/transformer_test.go index 4b0e910c..02a3bd6c 100644 --- a/transformer/transformer_test.go +++ b/transformer/transformer_test.go @@ -14,11 +14,11 @@ func TestLegacyTransformer(t *testing.T) { Convey("Transforms unmarshalled search responses successfully", t, func() { transformer := NewLegacy() Convey("Zero suggestions creates empty array", func() { - es := models.ESResponse{ - Responses: []models.ESResponseItem{models.ESResponseItem{ - Suggest: models.ESSuggest{ - SearchSuggest: []models.ESSearchSuggest{models.ESSearchSuggest{ - Options: []models.ESSearchSuggestOptions{}, + es := models.ESResponseLegacy{ + Responses: []models.ESResponseItemLegacy{models.ESResponseItemLegacy{ + Suggest: models.ESSuggestLegacy{ + SearchSuggest: []models.ESSearchSuggestLegacy{models.ESSearchSuggestLegacy{ + Options: []models.ESSearchSuggestOptionsLegacy{}, }}, }, }}, @@ -28,12 +28,12 @@ func TestLegacyTransformer(t *testing.T) { }) Convey("One suggestion creates a populated array", func() { - es := models.ESResponse{ - Responses: []models.ESResponseItem{models.ESResponseItem{ - Suggest: models.ESSuggest{ - SearchSuggest: []models.ESSearchSuggest{models.ESSearchSuggest{ - Options: []models.ESSearchSuggestOptions{ - models.ESSearchSuggestOptions{Text: "option1"}, + es := models.ESResponseLegacy{ + Responses: []models.ESResponseItemLegacy{models.ESResponseItemLegacy{ + Suggest: models.ESSuggestLegacy{ + SearchSuggest: []models.ESSearchSuggestLegacy{models.ESSearchSuggestLegacy{ + Options: []models.ESSearchSuggestOptionsLegacy{ + models.ESSearchSuggestOptionsLegacy{Text: "option1"}, }, }}, }, @@ -45,23 +45,23 @@ func TestLegacyTransformer(t *testing.T) { So(sr.Suggestions[0], ShouldResemble, "option1") }) Convey("Multiple suggestions creates a populated array incorrect order", func() { - es := models.ESResponse{ - Responses: []models.ESResponseItem{models.ESResponseItem{ - Suggest: models.ESSuggest{ - SearchSuggest: []models.ESSearchSuggest{ - models.ESSearchSuggest{ - Options: []models.ESSearchSuggestOptions{ - models.ESSearchSuggestOptions{Text: "option1"}, + es := models.ESResponseLegacy{ + Responses: []models.ESResponseItemLegacy{models.ESResponseItemLegacy{ + Suggest: models.ESSuggestLegacy{ + SearchSuggest: []models.ESSearchSuggestLegacy{ + models.ESSearchSuggestLegacy{ + Options: []models.ESSearchSuggestOptionsLegacy{ + models.ESSearchSuggestOptionsLegacy{Text: "option1"}, }, }, - models.ESSearchSuggest{ - Options: []models.ESSearchSuggestOptions{ - models.ESSearchSuggestOptions{Text: "option2"}, + models.ESSearchSuggestLegacy{ + Options: []models.ESSearchSuggestOptionsLegacy{ + models.ESSearchSuggestOptionsLegacy{Text: "option2"}, }, }, - models.ESSearchSuggest{ - Options: []models.ESSearchSuggestOptions{ - models.ESSearchSuggestOptions{Text: "option3"}, + models.ESSearchSuggestLegacy{ + Options: []models.ESSearchSuggestOptionsLegacy{ + models.ESSearchSuggestOptionsLegacy{Text: "option3"}, }, }, }, @@ -128,7 +128,7 @@ func TestLegacyTransformSearchResponse(t *testing.T) { actual, err := transformer.TransformSearchResponse(ctx, sampleResponse, "test-query", true) So(err, ShouldBeNil) So(actual, ShouldNotBeEmpty) - var exp, act models.SearchResponse + var exp, act models.SearchResponseLegacy So(json.Unmarshal(expected, &exp), ShouldBeNil) So(json.Unmarshal(actual, &act), ShouldBeNil) So(act, ShouldResemble, exp) @@ -143,7 +143,7 @@ func TestLegacyTransformSearchResponse(t *testing.T) { actual, err := transformer.TransformSearchResponse(ctx, sampleResponse, "test-query", false) So(err, ShouldBeNil) So(actual, ShouldNotBeEmpty) - var exp, act models.SearchResponse + var exp, act models.SearchResponseLegacy So(json.Unmarshal(expected, &exp), ShouldBeNil) So(json.Unmarshal(actual, &act), ShouldBeNil) So(act, ShouldResemble, exp) @@ -158,7 +158,7 @@ func TestLegacyTransformSearchResponse(t *testing.T) { actual, err := transformer.TransformSearchResponse(ctx, sampleResponse, "test query \"with quote marks\"", false) So(err, ShouldBeNil) So(actual, ShouldNotBeEmpty) - var exp, act models.SearchResponse + var exp, act models.SearchResponseLegacy So(json.Unmarshal(expected, &exp), ShouldBeNil) So(json.Unmarshal(actual, &act), ShouldBeNil) So(act, ShouldResemble, exp) From 9483815826c2565fd623e39ca2e69b3910a1c46c Mon Sep 17 00:00:00 2001 From: bpathak-ons Date: Fri, 8 Apr 2022 11:20:40 +0100 Subject: [PATCH 18/29] rename es7x models update --- models/elasticsearch.go | 52 ++++++++++++++++----------------- transformer/transformer.go | 14 ++++----- transformer/transformer_test.go | 46 ++++++++++++++--------------- 3 files changed, 56 insertions(+), 56 deletions(-) diff --git a/models/elasticsearch.go b/models/elasticsearch.go index 61ed224b..c97dcb40 100644 --- a/models/elasticsearch.go +++ b/models/elasticsearch.go @@ -4,42 +4,42 @@ package models // structs representing the raw elastic search response // ************************************************************* -// Es7xResponse holds a response slice from ES -type Es7xResponse struct { - Responses []EsResponse7x `json:"responses"` +// EsResponse holds a response slice from ES +type EsResponses struct { + Responses []EsResponse `json:"responses"` } -type EsResponse7x struct { - Took int `json:"took"` - Hits ES7xResponseHits `json:"hits"` - Aggregations ES7xResponseAggregations `json:"aggregations"` - Suggest []string `json:"suggest"` +type EsResponse struct { + Took int `json:"took"` + Hits ESResponseHits `json:"hits"` + Aggregations ESResponseAggregations `json:"aggregations"` + Suggest []string `json:"suggest"` } -type ES7xResponseHits struct { +type ESResponseHits struct { Total int - Hits []ES7xResponseHit `json:"hits"` + Hits []ESResponseHit `json:"hits"` } -type ES7xResponseHit struct { - Source []ES7xSourceDocument `json:"_source"` - Highlight ES7xHighlight `json:"highlight"` +type ESResponseHit struct { + Source []ESSourceDocument `json:"_source"` + Highlight ESHighlight `json:"highlight"` } -type ES7xResponseAggregations struct { - Doccounts ES7xDocCounts `json:"docCounts"` +type ESResponseAggregations struct { + Doccounts ESDocCounts `json:"docCounts"` } -type ES7xDocCounts struct { - Buckets []ES7xBucket `json:"buckets"` +type ESDocCounts struct { + Buckets []ESBucket `json:"buckets"` } -type ES7xBucket struct { +type ESBucket struct { Key string `json:"key"` Count int `json:"doc_count"` } -type ES7xSourceDocument struct { +type ESSourceDocument struct { DataType string `json:"type"` JobID string `json:"job_id"` SearchIndex string `json:"search_index"` @@ -53,7 +53,7 @@ type ES7xSourceDocument struct { Topics []string `json:"topics"` } -type ES7xHighlight struct { +type ESHighlight struct { DescriptionTitle *[]string `json:"description.title"` DescriptionEdition *[]string `json:"description.edition"` DescriptionSummary *[]string `json:"description.summary"` @@ -66,10 +66,10 @@ type ES7xHighlight struct { // Structs representing the transformed response // ******************************************************** -type Search7xResponse struct { - Count int `json:"count"` - Took int `json:"took"` - Items []ES7xSourceDocument `json:"items"` - Suggestions []string `json:"suggestions,omitempty"` - AdditionSuggestions []string `json:"additional_suggestions,omitempty"` +type SearchResponse struct { + Count int `json:"count"` + Took int `json:"took"` + Items []ESSourceDocument `json:"items"` + Suggestions []string `json:"suggestions,omitempty"` + AdditionSuggestions []string `json:"additional_suggestions,omitempty"` } diff --git a/transformer/transformer.go b/transformer/transformer.go index 5a5d03a8..184974cd 100644 --- a/transformer/transformer.go +++ b/transformer/transformer.go @@ -28,7 +28,7 @@ type Transformer struct { higlightReplacer *strings.Replacer } -// New7x returns a new instance of Transformer7x +// New returns a new instance of Transformer ES7x func New() *Transformer { highlightReplacer := strings.NewReplacer("", "", "", "") return &Transformer{ @@ -192,7 +192,7 @@ func numberOfSearchTerms(query string) int { func (t *Transformer) TransformSearchResponse( ctx context.Context, responseData []byte, query string, highlight bool) ([]byte, error) { - var esResponse models.Es7xResponse + var esResponse models.EsResponses err := json.Unmarshal(responseData, &esResponse) if err != nil { @@ -219,11 +219,11 @@ func (t *Transformer) TransformSearchResponse( } // Transform the raw ES to search response -func (t *Transformer) transform(es7xresponse *models.Es7xResponse, highlight bool) models.Search7xResponse { - var search7xResponse = models.Search7xResponse{ - Took: es7xresponse.Responses[0].Took, - Items: es7xresponse.Responses[0].Hits.Hits[0].Source, - Suggestions: es7xresponse.Responses[0].Suggest, +func (t *Transformer) transform(esresponse *models.EsResponses, highlight bool) models.SearchResponse { + var search7xResponse = models.SearchResponse{ + Took: esresponse.Responses[0].Took, + Items: esresponse.Responses[0].Hits.Hits[0].Source, + Suggestions: esresponse.Responses[0].Suggest, } return search7xResponse } diff --git a/transformer/transformer_test.go b/transformer/transformer_test.go index 02a3bd6c..04043245 100644 --- a/transformer/transformer_test.go +++ b/transformer/transformer_test.go @@ -167,7 +167,7 @@ func TestLegacyTransformSearchResponse(t *testing.T) { } func TestTransform(t *testing.T) { - expectedESDocument1 := models.ES7xSourceDocument{ + expectedESDocument1 := models.ESSourceDocument{ DataType: "anyDataType1", JobID: "", CDID: "", @@ -179,7 +179,7 @@ func TestTransform(t *testing.T) { Title: "anyTitle2", Topics: []string{"anyTopic1"}, } - expectedESDocument2 := models.ES7xSourceDocument{ + expectedESDocument2 := models.ESSourceDocument{ DataType: "anyDataType2", JobID: "", CDID: "", @@ -192,7 +192,7 @@ func TestTransform(t *testing.T) { Topics: []string{"anyTopic2"}, } - Convey("Given a new instance of Transformer7x with search responses successfully", t, func() { + Convey("Given a new instance of Transformer for ES7x with search responses successfully", t, func() { transformer := New() esResponse := prepareESMockResponse() @@ -212,8 +212,8 @@ func TestTransform(t *testing.T) { } // Prepare mock ES response -func prepareESMockResponse() models.Es7xResponse { - esDocument1 := models.ES7xSourceDocument{ +func prepareESMockResponse() models.EsResponses { + esDocument1 := models.ESSourceDocument{ DataType: "anyDataType1", JobID: "", CDID: "", @@ -226,7 +226,7 @@ func prepareESMockResponse() models.Es7xResponse { Topics: []string{"anyTopic1"}, } - esDocument2 := models.ES7xSourceDocument{ + esDocument2 := models.ESSourceDocument{ DataType: "anyDataType2", JobID: "", CDID: "", @@ -239,47 +239,47 @@ func prepareESMockResponse() models.Es7xResponse { Topics: []string{"anyTopic2"}, } - esDocuments := []models.ES7xSourceDocument{esDocument1, esDocument2} + esDocuments := []models.ESSourceDocument{esDocument1, esDocument2} - hit7x := models.ES7xResponseHit{ + hit := models.ESResponseHit{ Source: esDocuments, - Highlight: models.ES7xHighlight{}, + Highlight: models.ESHighlight{}, } - bucket1 := models.ES7xBucket{ + bucket1 := models.ESBucket{ Key: "article", Count: 1, } - bucket2 := models.ES7xBucket{ + bucket2 := models.ESBucket{ Key: "product_page", Count: 1, } - buckets := []models.ES7xBucket{bucket1, bucket2} + buckets := []models.ESBucket{bucket1, bucket2} - es7xDoccount := models.ES7xDocCounts{ + esDoccount := models.ESDocCounts{ Buckets: buckets, } - esResponse7x1 := models.EsResponse7x{ + esResponse1 := models.EsResponse{ Took: 10, - Hits: models.ES7xResponseHits{ + Hits: models.ESResponseHits{ Total: 1, - Hits: []models.ES7xResponseHit{ - hit7x, + Hits: []models.ESResponseHit{ + hit, }, }, - Aggregations: models.ES7xResponseAggregations{ - Doccounts: es7xDoccount, + Aggregations: models.ESResponseAggregations{ + Doccounts: esDoccount, }, Suggest: []string{"testSuggestion"}, } // Preparing ES response array - es7xResponse := models.Es7xResponse{ - Responses: []models.EsResponse7x{ - esResponse7x1, + esResponse := models.EsResponses{ + Responses: []models.EsResponse{ + esResponse1, }, } - return es7xResponse + return esResponse } From 5fecb1bcf9d39db04ad89b0d621a7c95d7a9af36 Mon Sep 17 00:00:00 2001 From: PatrickConnollyONS Date: Fri, 8 Apr 2022 11:56:40 +0100 Subject: [PATCH 19/29] Refactor the error handling on derived parameter types to conform with review feedback --- query/releasesearch.go | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/query/releasesearch.go b/query/releasesearch.go index ad61b8f1..20bac782 100644 --- a/query/releasesearch.go +++ b/query/releasesearch.go @@ -10,6 +10,8 @@ import ( "strings" "text/template" "time" + + "github.com/ONSdigital/log.go/v2/log" ) type ParamValidator map[paramName]validator @@ -79,21 +81,29 @@ type Date time.Time const dateFormat = "2006-01-02" +type InvalidDateString struct { + value, err string +} + +func (ids InvalidDateString) Error() string { + return fmt.Sprintf("invalid date string (%q): %s", ids.value, ids.err) +} + func ParseDate(date string) (Date, error) { if date == "" { return Date{}, nil } d, err := time.Parse(dateFormat, date) if err != nil { - return Date{}, err + return Date{}, InvalidDateString{date, err.Error()} } if d.Before(time.Date(1800, 1, 1, 0, 0, 0, 0, time.UTC)) { - return Date{}, errors.New("date too far in past") + return Date{}, InvalidDateString{value: date, err: "date too far in past"} } if d.After(time.Date(2200, 1, 1, 0, 0, 0, 0, time.UTC)) { - return Date{}, errors.New("date too far in future") + return Date{}, InvalidDateString{value: date, err: "date too far in future"} } return Date(d), nil @@ -102,7 +112,7 @@ func ParseDate(date string) (Date, error) { func MustParseDate(date string) Date { d, err := ParseDate(date) if err != nil { - panic("invalid date string: " + date) + log.Fatal(context.Background(), "MustParseDate", InvalidDateString{value: date}) } return d @@ -133,6 +143,12 @@ const ( var sortNames = map[Sort]string{RelDateAsc: "release_date_asc", RelDateDesc: "release_date_desc", TitleAsc: "title_asc", TitleDesc: "title_desc", Relevance: "relevance", Invalid: "invalid"} var esSortNames = map[Sort]string{RelDateAsc: `{"description.releaseDate": "asc"}`, RelDateDesc: `{"description.releaseDate": "desc"}`, TitleAsc: `{"description.title": "asc"}`, TitleDesc: `{"description.title": "desc"}`, Relevance: `{"_score": "desc"}`, Invalid: "invalid"} +type InvalidSortString string + +func (iss InvalidSortString) Error() string { + return fmt.Sprintf("invalid sort string: %q", string(iss)) +} + func ParseSort(sort string) (Sort, error) { for s, sn := range sortNames { if strings.EqualFold(sort, sn) { @@ -140,13 +156,13 @@ func ParseSort(sort string) (Sort, error) { } } - return Invalid, errors.New("invalid sort option string") + return Invalid, InvalidSortString(sort) } func MustParseSort(sort string) Sort { s, err := ParseSort(sort) if err != nil { - panic("invalid sort string: " + sort) + log.Fatal(context.Background(), "MustParseSort", InvalidSortString(sort)) } return s @@ -171,6 +187,12 @@ const ( var relTypeNames = map[ReleaseType]string{Upcoming: "type-upcoming", Published: "type-published", Cancelled: "type-cancelled", InvalidReleaseType: "Invalid"} +type InvalidReleaseTypeString string + +func (irts InvalidReleaseTypeString) Error() string { + return fmt.Sprintf("invalid ReleaseType string: %q", string(irts)) +} + func ParseReleaseType(s string) (ReleaseType, error) { for rt, rtn := range relTypeNames { if strings.EqualFold(s, rtn) { @@ -178,13 +200,13 @@ func ParseReleaseType(s string) (ReleaseType, error) { } } - return InvalidReleaseType, errors.New("invalid release type string") + return InvalidReleaseType, InvalidReleaseTypeString(s) } func MustParseReleaseType(s string) ReleaseType { rt, err := ParseReleaseType(s) if err != nil { - panic("invalid release type string: " + s) + log.Fatal(context.Background(), "MustParseReleaseType", InvalidReleaseTypeString(s)) } return rt From 1cf945df571c61e566c18bbc214627d40235f708 Mon Sep 17 00:00:00 2001 From: rahulmadathumpalliyalil Date: Mon, 11 Apr 2022 16:33:32 +0100 Subject: [PATCH 20/29] Fix comments --- query/query_test.go | 2 ++ query/search.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/query/query_test.go b/query/query_test.go index 99e7bf91..6ad17acb 100644 --- a/query/query_test.go +++ b/query/query_test.go @@ -30,6 +30,8 @@ func TestNewQueryBuilder(t *testing.T) { So(builderObject.searchTemplates, ShouldNotBeNil) So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "search.tmpl") + So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "contentQuery.tmpl") + So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "contentHeader.tmpl") So(err, ShouldBeNil) }) } diff --git a/query/search.go b/query/search.go index 45f65bb4..baf46871 100644 --- a/query/search.go +++ b/query/search.go @@ -71,7 +71,7 @@ func SetupSearch(pathToTemplates string) (*template.Template, error) { return templates, err } -// SetupSearch loads templates for use by the search handler and should be done only once +// SetupV710Search loads v710 templates for use by the search handler and should be done only once func SetupV710Search(pathToTemplates string) (*template.Template, error) { // Load the templates once, the main entry point for the templates is search.tmpl. The search.tmpl takes // the SearchRequest struct and uses the Request to build up the multi-query queries that is used to query elastic. From 1b8788f8ac04065acad0d73613be04c51a8bfe37 Mon Sep 17 00:00:00 2001 From: PatrickConnollyONS Date: Tue, 12 Apr 2022 16:17:53 +0100 Subject: [PATCH 21/29] Refactor ElasticSearch queries to provide the extra aggregation data required, and switch to using the ElasticSearch multi-search query api --- api/releases.go | 3 +- query/esclauses.go | 77 +++++++++++++++ query/releasesearch.go | 58 ++++++++++-- query/releasesearch_test.go | 10 +- templates/search/releasecalendar/search.tmpl | 99 +++++++++++++++++--- 5 files changed, 219 insertions(+), 28 deletions(-) create mode 100644 query/esclauses.go diff --git a/api/releases.go b/api/releases.go index 61036302..e89de12e 100644 --- a/api/releases.go +++ b/api/releases.go @@ -88,7 +88,6 @@ func SearchReleasesHandlerFunc(validator QueryParamValidator, builder ReleaseQue ReleasedBefore: toDate.(query.Date), Type: relType.(query.ReleaseType), Highlight: highlight, - Now: query.Date(time.Now()), } formattedQuery, err := builder.BuildSearchQuery(ctx, searchReq) @@ -98,7 +97,7 @@ func SearchReleasesHandlerFunc(validator QueryParamValidator, builder ReleaseQue return } - responseData, err := searcher.Search(ctx, "ons", "", formattedQuery) + responseData, err := searcher.MultiSearch(ctx, "ons", "", formattedQuery) if err != nil { log.Error(ctx, "elasticsearch release query failed", err) http.Error(w, "Failed to run search release query", http.StatusInternalServerError) diff --git a/query/esclauses.go b/query/esclauses.go new file mode 100644 index 00000000..54e934f2 --- /dev/null +++ b/query/esclauses.go @@ -0,0 +1,77 @@ +package query + +import ( + "fmt" + "time" +) + +const ( + Separator = "," + + NoProvisionalNoConfirmedPostponed = ` +{"term":{"description.finalised":true}}, {"exists":{"field":"dateChanges"}} +` + + NoProvisionalConfirmedNoPostponed = ` +{"term":{"description.finalised":true}}, {"bool":{"must_not":{"exists":{"field":"dateChanges"}}}} +` + + NoProvisionalConfirmedPostponed = ` +{"term":{"description.finalised":true}} +` + + ProvisionalNoConfirmedNoPostponed = ` +{"term":{"description.finalised":false}} +` + + ProvisionalNoConfirmedPostponed = ` +{"bool":{ + "should":[ + {"term":{"description.finalised":false}}, + {"bool":{ + "must":[ + {"term":{"description.finalised":true}}, + {"exists":{"field":"dateChanges"}} + ]} + } + ]} +}` + + ProvisionalConfirmedNoPostponed = ` +{"bool":{ + "should":[ + {"term":{"description.finalised":false}}, + {"bool":{ + "must":[ + {"term":{"description.finalised":true}}, + {"bool":{"must_not":{"exists":{"field":"dateChanges"}}}} + ]} + } + ]} +}` +) + +func mainUpcomingClause(now time.Time) string { + return fmt.Sprintf("%s, %s, %s", `{"term": {"description.published": false}}`, `{"term": {"description.cancelled": false}}`, + fmt.Sprintf(`{"range": {"description.release_date": {"gte": %q}}}`, now.Format(dateFormat))) +} + +//gocyclo:ignore +func supplementaryUpcomingClause(sr ReleaseSearchRequest) string { + switch { + case !sr.Provisional && !sr.Confirmed && sr.Postponed: + return NoProvisionalNoConfirmedPostponed + case !sr.Provisional && sr.Confirmed && !sr.Postponed: + return NoProvisionalConfirmedNoPostponed + case !sr.Provisional && sr.Confirmed && sr.Postponed: + return NoProvisionalConfirmedPostponed + case sr.Provisional && !sr.Confirmed && !sr.Postponed: + return ProvisionalNoConfirmedNoPostponed + case sr.Provisional && !sr.Confirmed && sr.Postponed: + return ProvisionalNoConfirmedPostponed + case sr.Provisional && sr.Confirmed && !sr.Postponed: + return ProvisionalConfirmedNoPostponed + } + + return "" +} diff --git a/query/releasesearch.go b/query/releasesearch.go index 20bac782..886eec47 100644 --- a/query/releasesearch.go +++ b/query/releasesearch.go @@ -240,7 +240,12 @@ func (sb *ReleaseBuilder) BuildSearchQuery(_ context.Context, sr ReleaseSearchRe return nil, fmt.Errorf("creation of search from template failed: %w", err) } - return doc.Bytes(), nil + formattedQuery, err := FormatMultiQuery(doc.Bytes()) + if err != nil { + return nil, fmt.Errorf("formating of query for elasticsearch failed: %w", err) + } + + return formattedQuery, nil } type ReleaseSearchRequest struct { @@ -256,7 +261,6 @@ type ReleaseSearchRequest struct { Postponed bool Census bool Highlight bool - Now Date } func (sr *ReleaseSearchRequest) String() string { @@ -268,7 +272,11 @@ func (sr *ReleaseSearchRequest) String() string { return string(s) } -func (sr ReleaseSearchRequest) Sort() string { +func (sr ReleaseSearchRequest) Now() string { + return fmt.Sprintf("%q", time.Now().Format(dateFormat)) +} + +func (sr ReleaseSearchRequest) SortClause() string { if sr.SortBy == Relevance { switch sr.Type { case Upcoming: @@ -283,18 +291,52 @@ func (sr ReleaseSearchRequest) Sort() string { return sr.SortBy.ESString() } -func (sr ReleaseSearchRequest) ReleaseType() string { +// ReleaseTypeClause returns the query clause to select the type of release +// Note that it is possible for a Release to have both its Published and Cancelled flags true (yes indeed!) +// In this case it is deemed cancelled +func (sr ReleaseSearchRequest) ReleaseTypeClause() string { switch sr.Type { case Upcoming: - return fmt.Sprintf("%s, %s, %s", `{"term": {"description.published": false}}`, `{"term": {"description.cancelled": false}}`, - fmt.Sprintf(`{"range": {"description.release_date": {"gte": %q}}}`, time.Now().Format(dateFormat))) + var buf bytes.Buffer + buf.WriteString(mainUpcomingClause(time.Now())) + if secondary := supplementaryUpcomingClause(sr); secondary != "" { + buf.WriteString(Separator + secondary) + } + return buf.String() case Published: - return fmt.Sprintf("%s, %s", `{"term": {"description.published": true}}`, `{"term": {"description.cancelled": false}}`) + return fmt.Sprintf("%s, %s", + `{"term": {"description.published": true}}`, `{"term": {"description.cancelled": false}}`) default: - return fmt.Sprintf("%s, %s", `{"term": {"description.published": false}}`, `{"term": {"description.cancelled": true}}`) + return `{"term": {"description.cancelled": true}}` } } +func (sr ReleaseSearchRequest) CensusClause() string { + if sr.Census { + return `{"term": {"census": true}}` + } + + return `{}` +} + +func (sr ReleaseSearchRequest) HighlightClause() string { + if sr.Census { + return ` + "highlight":{ + "pre_tags":[""], + "post_tags":[""], + "fields":{ + "description.title":{"fragment_size":0,"number_of_fragments":0}, + "description.summary":{"fragment_size":0,"number_of_fragments":0}, + "description.keywords":{"fragment_size":0,"number_of_fragments":0} + } + } +` + } + + return `"highlight":{}` +} + func (sr *ReleaseSearchRequest) Set(value string) error { var sr2 ReleaseSearchRequest err := json.Unmarshal([]byte(value), &sr2) diff --git a/query/releasesearch_test.go b/query/releasesearch_test.go index 55307e94..c4d4b310 100644 --- a/query/releasesearch_test.go +++ b/query/releasesearch_test.go @@ -6,6 +6,7 @@ import ( "fmt" "testing" "text/template" + "time" . "github.com/smartystreets/goconvey/convey" ) @@ -178,8 +179,7 @@ func TestBuildSearchReleaseQuery(t *testing.T) { "ReleasedBefore={{.ReleasedBefore.ESString}};" + "Type={{.Type.String}};" + "Highlight={{.Highlight}};" + - "Now={{.Now}};" + - "NowES={{.Now.ESString}}") + "Now={{.Now}}") query, err := qb.BuildSearchQuery(context.Background(), ReleaseSearchRequest{ Term: "query+term", @@ -190,7 +190,6 @@ func TestBuildSearchReleaseQuery(t *testing.T) { ReleasedBefore: MustParseDate("2020-12-31"), Type: Published, Highlight: true, - Now: MustParseDate("2001-01-01"), }) So(err, ShouldBeNil) @@ -199,13 +198,12 @@ func TestBuildSearchReleaseQuery(t *testing.T) { So(queryString, ShouldContainSubstring, "Term=query+term") So(queryString, ShouldContainSubstring, "From=0") So(queryString, ShouldContainSubstring, "Size=25") - So(queryString, ShouldContainSubstring, `SortBy={"description.title": "asc"}`) + So(queryString, ShouldContainSubstring, `SortBy={"description.title":"asc"}`) So(queryString, ShouldContainSubstring, "ReleasedAfter=null") So(queryString, ShouldContainSubstring, `ReleasedBefore="2020-12-31"`) So(queryString, ShouldContainSubstring, "Type=type-published") So(queryString, ShouldContainSubstring, "Highlight=true") - So(queryString, ShouldContainSubstring, `Now=2001-01-01`) - So(queryString, ShouldContainSubstring, `NowES="2001-01-01"`) + So(queryString, ShouldContainSubstring, fmt.Sprintf(`Now=%q`, time.Now().Format(dateFormat))) }) } diff --git a/templates/search/releasecalendar/search.tmpl b/templates/search/releasecalendar/search.tmpl index 7860b472..e3a4a383 100644 --- a/templates/search/releasecalendar/search.tmpl +++ b/templates/search/releasecalendar/search.tmpl @@ -1,8 +1,9 @@ {{- /*gotype:github.com/ONSdigital/dp-search-api/query.ReleaseSearchRequest*/ -}} +{"index" : "ons", "type": ["release"], "search_type": "dfs_query_then_fetch"} { "from": {{.From}}, "size": {{.Size}}, - "sort": [{{.Sort}}], + "sort": [{{.SortClause}}], "query": { "bool": { "must": { @@ -22,19 +23,93 @@ } } }, - {{.ReleaseType}} + {{.ReleaseTypeClause}}, + {{.CensusClause}} ] } - } - {{if .Highlight}} - ,"highlight":{ - "pre_tags":[""], - "post_tags":[""], - "fields":{ - "description.title":{"fragment_size":0,"number_of_fragments":0}, - "description.summary":{"fragment_size":0,"number_of_fragments":0}, - "description.keywords":{"fragment_size":0,"number_of_fragments":0} + }, + {{.HighlightClause}}, + "aggs":{ + "breakdown":{ + "filters":{ + "filters":{ + "provisional":{"term":{"description.finalised":false}}, + "confirmed":{"term":{"description.finalised":true}}, + "postponed":{ + "bool":{ + "must":[ + {"term":{"description.finalised":true}}, + {"exists":{"field":"dateChanges"}} + ] + } + } + } + } + }, + "census" : { + "filters":{ + "filters":{ + "census" : { "term": { "census": "true" } } + } } } - {{end}} + } } +{"index" : "ons", "type": ["release"], "search_type": "dfs_query_then_fetch"} +{ + "size":0, + "query":{"match_all":{}}, + "aggs":{ + "release_types":{ + "filters":{ + "filters":{ + "upcoming":{ + "bool":{ + "must":[ + {"term":{"description.published":false}}, + {"term":{"description.cancelled":false}}, + {"range":{"description.releaseDate":{"gte":{{.Now}}}}} + ] + } + }, + "outdated":{ + "bool":{ + "must":[ + {"term":{"description.published":false}}, + {"term":{"description.cancelled":false}}, + {"range":{"description.releaseDate":{"lt":{{.Now}}}}} + ] + } + }, + "published":{ + "bool":{ + "must":[ + {"term":{"description.published":true}}, + {"term":{"description.cancelled":false}} + ] + } + }, + "cancelled":{"term":{"description.cancelled":true}} + } + }, + "aggs":{ + "breakdown":{ + "filters":{ + "other_bucket_key":"confirmed", + "filters":{ + "provisional":{"term":{"description.finalised":false}}, + "postponed":{ + "bool":{ + "must":[ + {"term":{"description.finalised":true}}, + {"exists":{"field":"dateChanges"}} + ] + } + } + } + } + } + } + } + } +}$$ From 4e5cc2986663d4e1797d9862a69e3de5134478cf Mon Sep 17 00:00:00 2001 From: PatrickConnollyONS Date: Tue, 12 Apr 2022 16:22:08 +0100 Subject: [PATCH 22/29] Refactor the ReleaseTransformer to handle the multi-query responses and aggregation data transformation --- transformer/releasetransformer.go | 71 ++++- transformer/releasetransformer_test.go | 6 +- .../testdata/search_release_es_response.json | 297 ++++++++++++------ .../search_release_expected_highlighted.json | 20 +- .../search_release_expected_plain.json | 20 +- 5 files changed, 285 insertions(+), 129 deletions(-) diff --git a/transformer/releasetransformer.go b/transformer/releasetransformer.go index 88bee73e..e6702084 100644 --- a/transformer/releasetransformer.go +++ b/transformer/releasetransformer.go @@ -65,9 +65,14 @@ type highlight struct { // Structs representing the raw elastic search response type ESReleaseResponse struct { - Took int `json:"took"` - TimedOut bool `json:"timed_out"` - Hits ESReleaseResponseSummary `json:"hits"` + Responses []ESReleaseResponseItem `json:"responses"` +} + +type ESReleaseResponseItem struct { + Took int `json:"took"` + TimedOut bool `json:"timed_out"` + Hits ESReleaseResponseSummary `json:"hits"` + Aggregations ESReleaseResponseAggregations `json:"aggregations"` } type ESReleaseResponseSummary struct { @@ -108,6 +113,19 @@ type ESReleaseHighlight struct { DescriptionKeywords []string `json:"description.keywords"` } +type aggName string +type ESReleaseResponseAggregations map[aggName]aggregation + +type bucketName string +type aggregation struct { + Buckets map[bucketName]bucketContents `json:"buckets"` +} + +type bucketContents struct { + Count int `json:"doc_count"` + Breakdown aggregation `json:"breakdown"` +} + func NewReleaseTransformer() *ReleaseTransformer { highlightReplacer := strings.NewReplacer("", "", "", "") return &ReleaseTransformer{ @@ -115,7 +133,7 @@ func NewReleaseTransformer() *ReleaseTransformer { } } -// TransformSearchResponse transforms an elastic search response into a structure that matches the v1 api specification +// TransformSearchResponse transforms an elastic search response to a release query into a serialised ReleaseResponse func (t *ReleaseTransformer) TransformSearchResponse(_ context.Context, responseData []byte, req query.ReleaseSearchRequest, highlight bool) ([]byte, error) { var ( source ESReleaseResponse @@ -127,17 +145,20 @@ func (t *ReleaseTransformer) TransformSearchResponse(_ context.Context, response return nil, errors.Wrap(err, "Failed to decode elastic search response") } + if len(source.Responses) != 2 { + return nil, errors.New("invalid number of responses from ElasticSearch query") + } + sr := SearchReleaseResponse{ - Took: source.Took, - Breakdown: breakdown(&source, req), - Releases: []Release{}, + Took: source.Responses[0].Took + source.Responses[1].Took, + Breakdown: breakdown(source, req), } if highlight { highlighter = t.higlightReplacer } - for i := range source.Hits.Hits { - sr.Releases = append(sr.Releases, buildRelease(source.Hits.Hits[i], highlighter)) + for i := 0; i < len(source.Responses[0].Hits.Hits); i++ { + sr.Releases = append(sr.Releases, buildRelease(source.Responses[0].Hits.Hits[i], highlighter)) } transformedData, err := json.Marshal(sr) @@ -148,19 +169,35 @@ func (t *ReleaseTransformer) TransformSearchResponse(_ context.Context, response return transformedData, nil } -func breakdown(source *ESReleaseResponse, req query.ReleaseSearchRequest) Breakdown { - b := Breakdown{} +func breakdown(source ESReleaseResponse, req query.ReleaseSearchRequest) Breakdown { + b := Breakdown{Total: source.Responses[0].Hits.Total} - b.Total = source.Hits.Total switch req.Type { case query.Upcoming: - b.Confirmed = b.Total + b.Provisional = source.Responses[0].Aggregations["breakdown"].Buckets["provisional"].Count + b.Confirmed = source.Responses[0].Aggregations["breakdown"].Buckets["confirmed"].Count + b.Postponed = source.Responses[0].Aggregations["breakdown"].Buckets["postponed"].Count + + b.Published = source.Responses[1].Aggregations["release_types"].Buckets["published"].Count + b.Cancelled = source.Responses[1].Aggregations["release_types"].Buckets["cancelled"].Count case query.Published: - b.Published = b.Total + b.Published = source.Responses[0].Hits.Total + b.Cancelled = source.Responses[1].Aggregations["release_types"].Buckets["cancelled"].Count + + b.Provisional = source.Responses[1].Aggregations["release_types"].Buckets["upcoming"].Breakdown.Buckets["provisional"].Count + b.Confirmed = source.Responses[1].Aggregations["release_types"].Buckets["upcoming"].Breakdown.Buckets["confirmed"].Count + b.Postponed = source.Responses[1].Aggregations["release_types"].Buckets["upcoming"].Breakdown.Buckets["postponed"].Count case query.Cancelled: - b.Cancelled = b.Total + b.Cancelled = source.Responses[0].Hits.Total + b.Published = source.Responses[1].Aggregations["release_types"].Buckets["published"].Count + + b.Provisional = source.Responses[1].Aggregations["release_types"].Buckets["upcoming"].Breakdown.Buckets["provisional"].Count + b.Confirmed = source.Responses[1].Aggregations["release_types"].Buckets["upcoming"].Breakdown.Buckets["confirmed"].Count + b.Postponed = source.Responses[1].Aggregations["release_types"].Buckets["upcoming"].Breakdown.Buckets["postponed"].Count } + b.Census = source.Responses[0].Aggregations["census"].Buckets["census"].Count + return b } @@ -184,6 +221,10 @@ func buildRelease(hit ESReleaseResponseHit, highlighter *strings.Replacer) Relea }, } + for _, dc := range hit.Source.DateChanges { + r.DateChanges = append(r.DateChanges, ReleaseDateChange{Date: dc.PreviousDate, ChangeNotice: dc.ChangeNotice}) + } + if highlighter != nil { r.Highlight = &highlight{ Keywords: overlayList(hl.DescriptionKeywords, sd.Keywords, highlighter), diff --git a/transformer/releasetransformer_test.go b/transformer/releasetransformer_test.go index 8728499d..410029fb 100644 --- a/transformer/releasetransformer_test.go +++ b/transformer/releasetransformer_test.go @@ -20,7 +20,7 @@ func TestTransformSearchReleaseResponse(t *testing.T) { Convey("Throws error on invalid JSON", func() { sampleResponse := []byte(`{"invalid":"json"`) - _, err := transformer.TransformSearchResponse(ctx, sampleResponse, query.ReleaseSearchRequest{Term: "test-query", Type: query.Published}, true) + _, err := transformer.TransformSearchResponse(ctx, sampleResponse, query.ReleaseSearchRequest{Term: "Education in Wales", Type: query.Upcoming, Size: 2, Provisional: true, Postponed: true}, true) So(err, ShouldNotBeNil) So(err.Error(), ShouldResemble, "Failed to decode elastic search response: unexpected end of JSON input") }) @@ -31,7 +31,7 @@ func TestTransformSearchReleaseResponse(t *testing.T) { expected, err := os.ReadFile("testdata/search_release_expected_highlighted.json") So(err, ShouldBeNil) - actual, err := transformer.TransformSearchResponse(ctx, sampleResponse, query.ReleaseSearchRequest{Term: "test-query", Type: query.Published}, true) + actual, err := transformer.TransformSearchResponse(ctx, sampleResponse, query.ReleaseSearchRequest{Term: "Education in Wales", Type: query.Upcoming, Size: 2, Provisional: true, Postponed: true}, true) So(err, ShouldBeNil) So(actual, ShouldNotBeEmpty) var exp, act SearchReleaseResponse @@ -46,7 +46,7 @@ func TestTransformSearchReleaseResponse(t *testing.T) { expected, err := os.ReadFile("testdata/search_release_expected_plain.json") So(err, ShouldBeNil) - actual, err := transformer.TransformSearchResponse(ctx, sampleResponse, query.ReleaseSearchRequest{Term: "test-query", Type: query.Published}, false) + actual, err := transformer.TransformSearchResponse(ctx, sampleResponse, query.ReleaseSearchRequest{Term: "Education in Wales", Type: query.Upcoming, Size: 2, Provisional: true, Postponed: true}, false) So(err, ShouldBeNil) So(actual, ShouldNotBeEmpty) var exp, act SearchResponse diff --git a/transformer/testdata/search_release_es_response.json b/transformer/testdata/search_release_es_response.json index a2d1edb3..0f7a02d6 100644 --- a/transformer/testdata/search_release_es_response.json +++ b/transformer/testdata/search_release_es_response.json @@ -1,101 +1,212 @@ { - "took":8, - "timed_out":false, - "_shards":{ - "total":3, - "successful":3, - "failed":0 - }, - "hits":{ - "total":2, - "max_score":null, - "hits":[ - { - "_index":"ons1646652430467", - "_type":"release", - "_id":"/releases/estimatingsuicideamonghighereducationstudentsenglandandwales", - "_score":0.2374177, - "_source":{ - "uri":"/releases/estimatingsuicideamonghighereducationstudentsenglandandwales", - "type":"release", - "description":{ - "finalised":true, - "title":"Estimating suicide among higher education students, England and Wales: Experimental Statistics", - "summary":"Estimates of suicides among higher education students by sex, age and ethnicity. Analysis based on mortality records linked to Higher Education Statistics Agency (HESA) Student records. ", - "nationalStatistic":false, - "contact":{ - "email":"mortality@ons.gov.uk", - "name":"Sarah Caul", - "telephone":"+44 (0)1633 456490" + "responses":[ + { + "took":8, + "timed_out":false, + "_shards":{ + "total":3, + "successful":3, + "failed":0 + }, + "hits":{ + "total":16, + "max_score":1.0, + "hits":[ + { + "_index":"ons1646652430467", + "_type":"release", + "_id":"/releases/estimatingsuicideamonghighereducationstudentsenglandandwales", + "_score":0.2374177, + "_source":{ + "uri":"/releases/estimatingsuicideamonghighereducationstudentsenglandandwales", + "dateChanges": [{"previousDate": "2018-06-01", "changeNotice": "changed it"}], + "type":"release", + "description":{ + "finalised":true, + "title":"Estimating suicide among higher education students, England and Wales: Experimental Statistics", + "summary":"Estimates of suicides among higher education students by sex, age and ethnicity. Analysis based on mortality records linked to Higher Education Statistics Agency (HESA) Student records. ", + "nationalStatistic":false, + "contact":{ + "email":"mortality@ons.gov.uk", + "name":"Sarah Caul", + "telephone":"+44 (0)1633 456490" + }, + "releaseDate":"2018-06-25T08:30:00.000Z", + "nextRelease":"", + "unit":"", + "preUnit":"", + "source":"", + "cancelled":false, + "cancellationNotice":[], + "published":false, + "provisionalDate":"" + }, + "searchBoost":[] + }, + "highlight":{ + "description.summary":[ + "Estimates of suicides among higher education students by sex, age and ethnicity. Analysis based on mortality records linked to Higher Education Statistics Agency (HESA) Student records. " + ], + "description.title":[ + "Estimating suicide among higher education students, England and Wales: Experimental Statistics" + ] }, - "releaseDate":"2018-06-25T08:30:00.000Z", - "nextRelease":"", - "unit":"", - "preUnit":"", - "source":"", - "cancelled":false, - "cancellationNotice":[], - "published":true, - "provisionalDate":"" + "sort":[ + 0.2374177, + 1529915400000 + ] }, - "searchBoost":[] - }, - "highlight":{ - "description.summary":[ - "Estimates of suicides among higher education students by sex, age and ethnicity. Analysis based on mortality records linked to Higher Education Statistics Agency (HESA) Student records. " - ], - "description.title":[ - "Estimating suicide among higher education students, England and Wales: Experimental Statistics" - ] - }, - "sort":[ - 0.2374177, - 1529915400000 + { + "_index":"ons1646652430467", + "_type":"release", + "_id":"/releases/youngpeoplenotineducationemploymentortrainingneetukaugust2018", + "_score":0.04704748, + "_source":{ + "uri":"/releases/youngpeoplenotineducationemploymentortrainingneetukaugust2018", + "type":"release", + "description":{ + "finalised":false, + "title":"Young people not in education, employment or training (NEET), UK: August 2018", + "summary":"Estimates of young people (aged 16 to 24) who are not in education, employment or training, by age and sex.", + "nationalStatistic":true, + "contact":{ + "email":"yanitsa.petkova@ons.gov.uk", + "name":"Yanitsa Petkova ", + "telephone":"+44 (0)1633 651599 " + }, + "releaseDate":"2018-08-23T08:30:00.000Z", + "nextRelease":"22 November 2018", + "unit":"", + "preUnit":"", + "source":"", + "cancelled":false, + "cancellationNotice":[], + "published":false, + "provisionalDate":"2018-08-23" + }, + "searchBoost":[] + }, + "highlight":{ + "description.summary":[ + "Estimates of young people (aged 16 to 24) who are not in education, employment or training, by age and sex." + ], + "description.title":[ + "Young people not in education, employment or training (NEET), UK: August 2018" + ] + }, + "sort":[ + 0.04704748, + 1535013000000 + ] + } ] }, - { - "_index":"ons1646652430467", - "_type":"release", - "_id":"/releases/youngpeoplenotineducationemploymentortrainingneetukaugust2018", - "_score":0.04704748, - "_source":{ - "uri":"/releases/youngpeoplenotineducationemploymentortrainingneetukaugust2018", - "type":"release", - "description":{ - "finalised":true, - "title":"Young people not in education, employment or training (NEET), UK: August 2018", - "summary":"Estimates of young people (aged 16 to 24) who are not in education, employment or training, by age and sex.", - "nationalStatistic":true, - "contact":{ - "email":"yanitsa.petkova@ons.gov.uk", - "name":"Yanitsa Petkova ", - "telephone":"+44 (0)1633 651599 " + "aggregations":{ + "breakdown":{ + "buckets":{ + "provisional":{ + "doc_count":15 }, - "releaseDate":"2018-08-23T08:30:00.000Z", - "nextRelease":"22 November 2018", - "unit":"", - "preUnit":"", - "source":"", - "cancelled":false, - "cancellationNotice":[], - "published":true, - "provisionalDate":"" - }, - "searchBoost":[] - }, - "highlight":{ - "description.summary":[ - "Estimates of young people (aged 16 to 24) who are not in education, employment or training, by age and sex." - ], - "description.title":[ - "Young people not in education, employment or training (NEET), UK: August 2018" - ] + "confirmed":{ + "doc_count":0 + }, + "postponed":{ + "doc_count":1 + } + } }, - "sort":[ - 0.04704748, - 1535013000000 - ] + "census":{ + "buckets":{ + "census":{ + "doc_count":0 + } + } + } + } + }, + { + "took":3, + "timed_out":false, + "_shards":{ + "total":3, + "successful":3, + "failed":0 + }, + "hits":{ + "total":2979, + "max_score":0.0, + "hits":[] + }, + "aggregations":{ + "release_types":{ + "buckets":{ + "upcoming":{ + "doc_count":25, + "breakdown":{ + "buckets":{ + "provisional":{ + "doc_count":15 + }, + "postponed":{ + "doc_count":1 + }, + "confirmed":{ + "doc_count":9 + } + } + } + }, + "outdated":{ + "doc_count":473, + "breakdown":{ + "buckets":{ + "provisional":{ + "doc_count":213 + }, + "postponed":{ + "doc_count":0 + }, + "confirmed":{ + "doc_count":260 + } + } + } + }, + "published":{ + "doc_count":2466, + "breakdown":{ + "buckets":{ + "provisional":{ + "doc_count":5 + }, + "postponed":{ + "doc_count":0 + }, + "confirmed":{ + "doc_count":2461 + } + } + } + }, + "cancelled":{ + "doc_count":45, + "breakdown":{ + "buckets":{ + "provisional":{ + "doc_count":18 + }, + "postponed":{ + "doc_count":0 + }, + "confirmed":{ + "doc_count":27 + } + } + } + } + } + } } - ] - } -} \ No newline at end of file + } + ] +} diff --git a/transformer/testdata/search_release_expected_highlighted.json b/transformer/testdata/search_release_expected_highlighted.json index 90712cbd..03f713c8 100644 --- a/transformer/testdata/search_release_expected_highlighted.json +++ b/transformer/testdata/search_release_expected_highlighted.json @@ -1,22 +1,24 @@ { - "took":8, - "limit":10, - "offset":0, + "took":11, "breakdown":{ - "total":2, - "published": 2 + "total":16, + "provisional": 15, + "postponed": 1, + "published": 2466, + "cancelled": 45 }, "releases":[ { "uri":"/releases/estimatingsuicideamonghighereducationstudentsenglandandwales", + "date_changes": [{"previous_date": "2018-06-01", "change_notice": "changed it"}], "description":{ "title":"Estimating suicide among higher education students, England and Wales: Experimental Statistics", "summary":"Estimates of suicides among higher education students by sex, age and ethnicity. Analysis based on mortality records linked to Higher Education Statistics Agency (HESA) Student records. ", "release_date":"2018-06-25T08:30:00.000Z", - "published":true, + "published":false, "cancelled":false, "finalised":true, - "postponed":false, + "postponed":true, "census":false, "national_statistic":false, "latest_release":false, @@ -37,9 +39,9 @@ "title":"Young people not in education, employment or training (NEET), UK: August 2018", "summary":"Estimates of young people (aged 16 to 24) who are not in education, employment or training, by age and sex.", "release_date":"2018-08-23T08:30:00.000Z", - "published":true, + "published":false, "cancelled":false, - "finalised":true, + "finalised":false, "postponed":false, "census":false, "national_statistic":true, diff --git a/transformer/testdata/search_release_expected_plain.json b/transformer/testdata/search_release_expected_plain.json index 71c4fe55..d43a6c71 100644 --- a/transformer/testdata/search_release_expected_plain.json +++ b/transformer/testdata/search_release_expected_plain.json @@ -1,22 +1,24 @@ { - "took":8, - "limit":10, - "offset":0, + "took":11, "breakdown":{ - "total":2, - "published": 2 + "total":16, + "provisional": 15, + "postponed": 1, + "published": 2466, + "cancelled": 45 }, "releases":[ { "uri":"/releases/estimatingsuicideamonghighereducationstudentsenglandandwales", + "date_changes": [{"previous_date": "2018-06-01", "change_notice": "changed it"}], "description":{ "title":"Estimating suicide among higher education students, England and Wales: Experimental Statistics", "summary":"Estimates of suicides among higher education students by sex, age and ethnicity. Analysis based on mortality records linked to Higher Education Statistics Agency (HESA) Student records. ", "release_date":"2018-06-25T08:30:00.000Z", - "published":true, + "published":false, "cancelled":false, "finalised":true, - "postponed":false, + "postponed":true, "census":false, "national_statistic":false, "latest_release":false, @@ -33,9 +35,9 @@ "title":"Young people not in education, employment or training (NEET), UK: August 2018", "summary":"Estimates of young people (aged 16 to 24) who are not in education, employment or training, by age and sex.", "release_date":"2018-08-23T08:30:00.000Z", - "published":true, + "published":false, "cancelled":false, - "finalised":true, + "finalised":false, "postponed":false, "census":false, "national_statistic":true, From 973d59f3a53f5a759942f8c102e7cd78afbccbc2 Mon Sep 17 00:00:00 2001 From: PatrickConnollyONS Date: Tue, 12 Apr 2022 16:32:04 +0100 Subject: [PATCH 23/29] Refactor the component tests and the ElasticSearch query utility --- features/steps/steps.go | 6 +- .../es_mulitple_search_release_results.json | 297 ++++++++++++------ .../es_single_search_release_result.json | 219 +++++++++---- .../es_zero_search_release_results.json | 134 +++++++- ...ected_multiple_search_release_results.json | 19 +- ...expected_single_search_release_result.json | 14 +- ...gle_search_release_result_nohighlight.json | 14 +- .../expected_zero_search_release_results.json | 4 +- templates/search/releasecalendar/main.go | 53 +--- 9 files changed, 535 insertions(+), 225 deletions(-) diff --git a/features/steps/steps.go b/features/steps/steps.go index 68d9afb7..0e43d93f 100644 --- a/features/steps/steps.go +++ b/features/steps/steps.go @@ -41,7 +41,7 @@ func (c *Component) successfullyReturnMultipleSearchReleaseResults() error { return err } - c.FakeElasticSearchAPI.setJSONResponseForPost("/elasticsearch/ons/_search", 200, body) + c.FakeElasticSearchAPI.setJSONResponseForPost("/elasticsearch/ons/_msearch", 200, body) return nil } @@ -63,7 +63,7 @@ func (c *Component) successfullyReturnSingleSearchReleaseResult() error { return err } - c.FakeElasticSearchAPI.setJSONResponseForPost("/elasticsearch/ons/_search", 200, body) + c.FakeElasticSearchAPI.setJSONResponseForPost("/elasticsearch/ons/_msearch", 200, body) return nil } @@ -85,7 +85,7 @@ func (c *Component) successfullyReturnNoSearchReleaseResults() error { return err } - c.FakeElasticSearchAPI.setJSONResponseForPost("/elasticsearch/ons/_search", 200, body) + c.FakeElasticSearchAPI.setJSONResponseForPost("/elasticsearch/ons/_msearch", 200, body) return nil } diff --git a/features/testdata/es_mulitple_search_release_results.json b/features/testdata/es_mulitple_search_release_results.json index a2d1edb3..c3e09b95 100644 --- a/features/testdata/es_mulitple_search_release_results.json +++ b/features/testdata/es_mulitple_search_release_results.json @@ -1,101 +1,212 @@ { - "took":8, - "timed_out":false, - "_shards":{ - "total":3, - "successful":3, - "failed":0 - }, - "hits":{ - "total":2, - "max_score":null, - "hits":[ - { - "_index":"ons1646652430467", - "_type":"release", - "_id":"/releases/estimatingsuicideamonghighereducationstudentsenglandandwales", - "_score":0.2374177, - "_source":{ - "uri":"/releases/estimatingsuicideamonghighereducationstudentsenglandandwales", - "type":"release", - "description":{ - "finalised":true, - "title":"Estimating suicide among higher education students, England and Wales: Experimental Statistics", - "summary":"Estimates of suicides among higher education students by sex, age and ethnicity. Analysis based on mortality records linked to Higher Education Statistics Agency (HESA) Student records. ", - "nationalStatistic":false, - "contact":{ - "email":"mortality@ons.gov.uk", - "name":"Sarah Caul", - "telephone":"+44 (0)1633 456490" + "responses":[ + { + "took":8, + "timed_out":false, + "_shards":{ + "total":3, + "successful":3, + "failed":0 + }, + "hits":{ + "total":2, + "max_score":1.0, + "hits":[ + { + "_index":"ons1646652430467", + "_type":"release", + "_id":"/releases/estimatingsuicideamonghighereducationstudentsenglandandwales", + "_score":0.2374177, + "_source":{ + "uri":"/releases/estimatingsuicideamonghighereducationstudentsenglandandwales", + "dateChanges": [{"previousDate": "2018-06-01", "changeNotice": "changed it"}], + "type":"release", + "description":{ + "finalised":true, + "title":"Estimating suicide among higher education students, England and Wales: Experimental Statistics", + "summary":"Estimates of suicides among higher education students by sex, age and ethnicity. Analysis based on mortality records linked to Higher Education Statistics Agency (HESA) Student records. ", + "nationalStatistic":false, + "contact":{ + "email":"mortality@ons.gov.uk", + "name":"Sarah Caul", + "telephone":"+44 (0)1633 456490" + }, + "releaseDate":"2018-06-25T08:30:00.000Z", + "nextRelease":"", + "unit":"", + "preUnit":"", + "source":"", + "cancelled":false, + "cancellationNotice":[], + "published":false, + "provisionalDate":"" + }, + "searchBoost":[] + }, + "highlight":{ + "description.summary":[ + "Estimates of suicides among higher education students by sex, age and ethnicity. Analysis based on mortality records linked to Higher Education Statistics Agency (HESA) Student records. " + ], + "description.title":[ + "Estimating suicide among higher education students, England and Wales: Experimental Statistics" + ] }, - "releaseDate":"2018-06-25T08:30:00.000Z", - "nextRelease":"", - "unit":"", - "preUnit":"", - "source":"", - "cancelled":false, - "cancellationNotice":[], - "published":true, - "provisionalDate":"" + "sort":[ + 0.2374177, + 1529915400000 + ] }, - "searchBoost":[] - }, - "highlight":{ - "description.summary":[ - "Estimates of suicides among higher education students by sex, age and ethnicity. Analysis based on mortality records linked to Higher Education Statistics Agency (HESA) Student records. " - ], - "description.title":[ - "Estimating suicide among higher education students, England and Wales: Experimental Statistics" - ] - }, - "sort":[ - 0.2374177, - 1529915400000 + { + "_index":"ons1646652430467", + "_type":"release", + "_id":"/releases/youngpeoplenotineducationemploymentortrainingneetukaugust2018", + "_score":0.04704748, + "_source":{ + "uri":"/releases/youngpeoplenotineducationemploymentortrainingneetukaugust2018", + "type":"release", + "description":{ + "finalised":false, + "title":"Young people not in education, employment or training (NEET), UK: August 2018", + "summary":"Estimates of young people (aged 16 to 24) who are not in education, employment or training, by age and sex.", + "nationalStatistic":true, + "contact":{ + "email":"yanitsa.petkova@ons.gov.uk", + "name":"Yanitsa Petkova ", + "telephone":"+44 (0)1633 651599 " + }, + "releaseDate":"2018-08-23T08:30:00.000Z", + "nextRelease":"22 November 2018", + "unit":"", + "preUnit":"", + "source":"", + "cancelled":false, + "cancellationNotice":[], + "published":false, + "provisionalDate":"2018-08-23" + }, + "searchBoost":[] + }, + "highlight":{ + "description.summary":[ + "Estimates of young people (aged 16 to 24) who are not in education, employment or training, by age and sex." + ], + "description.title":[ + "Young people not in education, employment or training (NEET), UK: August 2018" + ] + }, + "sort":[ + 0.04704748, + 1535013000000 + ] + } ] }, - { - "_index":"ons1646652430467", - "_type":"release", - "_id":"/releases/youngpeoplenotineducationemploymentortrainingneetukaugust2018", - "_score":0.04704748, - "_source":{ - "uri":"/releases/youngpeoplenotineducationemploymentortrainingneetukaugust2018", - "type":"release", - "description":{ - "finalised":true, - "title":"Young people not in education, employment or training (NEET), UK: August 2018", - "summary":"Estimates of young people (aged 16 to 24) who are not in education, employment or training, by age and sex.", - "nationalStatistic":true, - "contact":{ - "email":"yanitsa.petkova@ons.gov.uk", - "name":"Yanitsa Petkova ", - "telephone":"+44 (0)1633 651599 " + "aggregations":{ + "breakdown":{ + "buckets":{ + "provisional":{ + "doc_count":1 }, - "releaseDate":"2018-08-23T08:30:00.000Z", - "nextRelease":"22 November 2018", - "unit":"", - "preUnit":"", - "source":"", - "cancelled":false, - "cancellationNotice":[], - "published":true, - "provisionalDate":"" - }, - "searchBoost":[] - }, - "highlight":{ - "description.summary":[ - "Estimates of young people (aged 16 to 24) who are not in education, employment or training, by age and sex." - ], - "description.title":[ - "Young people not in education, employment or training (NEET), UK: August 2018" - ] + "confirmed":{ + "doc_count":0 + }, + "postponed":{ + "doc_count":1 + } + } }, - "sort":[ - 0.04704748, - 1535013000000 - ] + "census":{ + "buckets":{ + "census":{ + "doc_count":0 + } + } + } + } + }, + { + "took":3, + "timed_out":false, + "_shards":{ + "total":3, + "successful":3, + "failed":0 + }, + "hits":{ + "total":2979, + "max_score":0.0, + "hits":[] + }, + "aggregations":{ + "release_types":{ + "buckets":{ + "upcoming":{ + "doc_count":25, + "breakdown":{ + "buckets":{ + "provisional":{ + "doc_count":15 + }, + "postponed":{ + "doc_count":1 + }, + "confirmed":{ + "doc_count":9 + } + } + } + }, + "outdated":{ + "doc_count":473, + "breakdown":{ + "buckets":{ + "provisional":{ + "doc_count":213 + }, + "postponed":{ + "doc_count":0 + }, + "confirmed":{ + "doc_count":260 + } + } + } + }, + "published":{ + "doc_count":2466, + "breakdown":{ + "buckets":{ + "provisional":{ + "doc_count":5 + }, + "postponed":{ + "doc_count":0 + }, + "confirmed":{ + "doc_count":2461 + } + } + } + }, + "cancelled":{ + "doc_count":45, + "breakdown":{ + "buckets":{ + "provisional":{ + "doc_count":18 + }, + "postponed":{ + "doc_count":0 + }, + "confirmed":{ + "doc_count":27 + } + } + } + } + } + } } - ] - } -} \ No newline at end of file + } + ] +} diff --git a/features/testdata/es_single_search_release_result.json b/features/testdata/es_single_search_release_result.json index 1c54da70..f6927380 100644 --- a/features/testdata/es_single_search_release_result.json +++ b/features/testdata/es_single_search_release_result.json @@ -1,58 +1,169 @@ { - "took":8, - "timed_out":false, - "_shards":{ - "total":3, - "successful":3, - "failed":0 - }, - "hits":{ - "total":1, - "max_score":null, - "hits":[ - { - "_index":"ons1646652430467", - "_type":"release", - "_id":"/releases/estimatingsuicideamonghighereducationstudentsenglandandwales", - "_score":0.2374177, - "_source":{ - "uri":"/releases/estimatingsuicideamonghighereducationstudentsenglandandwales", - "type":"release", - "description":{ - "finalised":true, - "title":"Estimating suicide among higher education students, England and Wales: Experimental Statistics", - "summary":"Estimates of suicides among higher education students by sex, age and ethnicity. Analysis based on mortality records linked to Higher Education Statistics Agency (HESA) Student records. ", - "nationalStatistic":false, - "contact":{ - "email":"mortality@ons.gov.uk", - "name":"Sarah Caul", - "telephone":"+44 (0)1633 456490" + "responses":[ + { + "took":8, + "timed_out":false, + "_shards":{ + "total":3, + "successful":3, + "failed":0 + }, + "hits":{ + "total":1, + "max_score":1.0, + "hits":[ + { + "_index":"ons1646652430467", + "_type":"release", + "_id":"/releases/estimatingsuicideamonghighereducationstudentsenglandandwales", + "_score":0.2374177, + "_source":{ + "uri":"/releases/estimatingsuicideamonghighereducationstudentsenglandandwales", + "dateChanges": [{"previousDate": "2018-06-01", "changeNotice": "changed it"}], + "type":"release", + "description":{ + "finalised":true, + "title":"Estimating suicide among higher education students, England and Wales: Experimental Statistics", + "summary":"Estimates of suicides among higher education students by sex, age and ethnicity. Analysis based on mortality records linked to Higher Education Statistics Agency (HESA) Student records. ", + "nationalStatistic":false, + "contact":{ + "email":"mortality@ons.gov.uk", + "name":"Sarah Caul", + "telephone":"+44 (0)1633 456490" + }, + "releaseDate":"2018-06-25T08:30:00.000Z", + "nextRelease":"", + "unit":"", + "preUnit":"", + "source":"", + "cancelled":false, + "cancellationNotice":[], + "published":false, + "provisionalDate":"" + }, + "searchBoost":[] }, - "releaseDate":"2018-06-25T08:30:00.000Z", - "nextRelease":"", - "unit":"", - "preUnit":"", - "source":"", - "cancelled":false, - "cancellationNotice":[], - "published":true, - "provisionalDate":"" - }, - "searchBoost":[] - }, - "highlight":{ - "description.summary":[ - "Estimates of suicides among higher education students by sex, age and ethnicity. Analysis based on mortality records linked to Higher Education Statistics Agency (HESA) Student records. " - ], - "description.title":[ - "Estimating suicide among higher education students, England and Wales: Experimental Statistics" - ] - }, - "sort":[ - 0.2374177, - 1529915400000 + "highlight":{ + "description.summary":[ + "Estimates of suicides among higher education students by sex, age and ethnicity. Analysis based on mortality records linked to Higher Education Statistics Agency (HESA) Student records. " + ], + "description.title":[ + "Estimating suicide among higher education students, England and Wales: Experimental Statistics" + ] + }, + "sort":[ + 0.2374177, + 1529915400000 + ] + } ] + }, + "aggregations":{ + "breakdown":{ + "buckets":{ + "provisional":{ + "doc_count":0 + }, + "confirmed":{ + "doc_count":0 + }, + "postponed":{ + "doc_count":1 + } + } + }, + "census":{ + "buckets":{ + "census":{ + "doc_count":0 + } + } + } + } + }, + { + "took":3, + "timed_out":false, + "_shards":{ + "total":3, + "successful":3, + "failed":0 + }, + "hits":{ + "total":2979, + "max_score":0.0, + "hits":[] + }, + "aggregations":{ + "release_types":{ + "buckets":{ + "upcoming":{ + "doc_count":25, + "breakdown":{ + "buckets":{ + "provisional":{ + "doc_count":15 + }, + "postponed":{ + "doc_count":1 + }, + "confirmed":{ + "doc_count":9 + } + } + } + }, + "outdated":{ + "doc_count":473, + "breakdown":{ + "buckets":{ + "provisional":{ + "doc_count":213 + }, + "postponed":{ + "doc_count":0 + }, + "confirmed":{ + "doc_count":260 + } + } + } + }, + "published":{ + "doc_count":2466, + "breakdown":{ + "buckets":{ + "provisional":{ + "doc_count":5 + }, + "postponed":{ + "doc_count":0 + }, + "confirmed":{ + "doc_count":2461 + } + } + } + }, + "cancelled":{ + "doc_count":45, + "breakdown":{ + "buckets":{ + "provisional":{ + "doc_count":1 + }, + "postponed":{ + "doc_count":0 + }, + "confirmed":{ + "doc_count":0 + } + } + } + } + } + } } - ] - } -} \ No newline at end of file + } + ] +} diff --git a/features/testdata/es_zero_search_release_results.json b/features/testdata/es_zero_search_release_results.json index c32054e9..2f75b9db 100644 --- a/features/testdata/es_zero_search_release_results.json +++ b/features/testdata/es_zero_search_release_results.json @@ -1,14 +1,122 @@ { - "took":8, - "timed_out":false, - "_shards":{ - "total":3, - "successful":3, - "failed":0 - }, - "hits":{ - "total":0, - "max_score":null, - "hits":[] - } -} \ No newline at end of file + "responses":[ + { + "took":8, + "timed_out":false, + "_shards":{ + "total":3, + "successful":3, + "failed":0 + }, + "hits":{ + "total":0 + }, + "aggregations":{ + "breakdown":{ + "buckets":{ + "provisional":{ + "doc_count":0 + }, + "confirmed":{ + "doc_count":0 + }, + "postponed":{ + "doc_count":0 + } + } + }, + "census":{ + "buckets":{ + "census":{ + "doc_count":0 + } + } + } + } + }, + { + "took":3, + "timed_out":false, + "_shards":{ + "total":3, + "successful":3, + "failed":0 + }, + "hits":{ + "total":2979, + "max_score":0.0, + "hits":[] + }, + "aggregations":{ + "release_types":{ + "buckets":{ + "upcoming":{ + "doc_count":25, + "breakdown":{ + "buckets":{ + "provisional":{ + "doc_count":15 + }, + "postponed":{ + "doc_count":1 + }, + "confirmed":{ + "doc_count":9 + } + } + } + }, + "outdated":{ + "doc_count":473, + "breakdown":{ + "buckets":{ + "provisional":{ + "doc_count":213 + }, + "postponed":{ + "doc_count":0 + }, + "confirmed":{ + "doc_count":260 + } + } + } + }, + "published":{ + "doc_count":2466, + "breakdown":{ + "buckets":{ + "provisional":{ + "doc_count":5 + }, + "postponed":{ + "doc_count":0 + }, + "confirmed":{ + "doc_count":2461 + } + } + } + }, + "cancelled":{ + "doc_count":45, + "breakdown":{ + "buckets":{ + "provisional":{ + "doc_count":18 + }, + "postponed":{ + "doc_count":0 + }, + "confirmed":{ + "doc_count":27 + } + } + } + } + } + } + } + } + ] +} diff --git a/features/testdata/expected_multiple_search_release_results.json b/features/testdata/expected_multiple_search_release_results.json index 7e2cbcbe..9f0bf0a6 100644 --- a/features/testdata/expected_multiple_search_release_results.json +++ b/features/testdata/expected_multiple_search_release_results.json @@ -1,21 +1,24 @@ { - "took":8, - "limit":10, - "offset":0, + "took":11, "breakdown":{ - "total":2 + "total":2, + "provisional": 1, + "postponed": 1, + "published": 2466, + "cancelled": 45 }, "releases":[ { "uri":"/releases/estimatingsuicideamonghighereducationstudentsenglandandwales", + "date_changes": [{"previous_date": "2018-06-01", "change_notice": "changed it"}], "description":{ "title":"Estimating suicide among higher education students, England and Wales: Experimental Statistics", "summary":"Estimates of suicides among higher education students by sex, age and ethnicity. Analysis based on mortality records linked to Higher Education Statistics Agency (HESA) Student records. ", "release_date":"2018-06-25T08:30:00.000Z", - "published":true, + "published":false, "cancelled":false, "finalised":true, - "postponed":false, + "postponed":true, "census":false, "national_statistic":false, "latest_release":false, @@ -36,9 +39,9 @@ "title":"Young people not in education, employment or training (NEET), UK: August 2018", "summary":"Estimates of young people (aged 16 to 24) who are not in education, employment or training, by age and sex.", "release_date":"2018-08-23T08:30:00.000Z", - "published":true, + "published":false, "cancelled":false, - "finalised":true, + "finalised":false, "postponed":false, "census":false, "national_statistic":true, diff --git a/features/testdata/expected_single_search_release_result.json b/features/testdata/expected_single_search_release_result.json index 67bf5c89..d1b01238 100644 --- a/features/testdata/expected_single_search_release_result.json +++ b/features/testdata/expected_single_search_release_result.json @@ -1,21 +1,23 @@ { - "took":8, - "limit":10, - "offset":0, + "took":11, "breakdown":{ - "total":1 + "total":1, + "postponed": 1, + "published": 2466, + "cancelled": 45 }, "releases":[ { "uri":"/releases/estimatingsuicideamonghighereducationstudentsenglandandwales", + "date_changes": [{"previous_date": "2018-06-01", "change_notice": "changed it"}], "description":{ "title":"Estimating suicide among higher education students, England and Wales: Experimental Statistics", "summary":"Estimates of suicides among higher education students by sex, age and ethnicity. Analysis based on mortality records linked to Higher Education Statistics Agency (HESA) Student records. ", "release_date":"2018-06-25T08:30:00.000Z", - "published":true, + "published":false, "cancelled":false, "finalised":true, - "postponed":false, + "postponed":true, "census":false, "national_statistic":false, "latest_release":false, diff --git a/features/testdata/expected_single_search_release_result_nohighlight.json b/features/testdata/expected_single_search_release_result_nohighlight.json index 40f9031d..39202c19 100644 --- a/features/testdata/expected_single_search_release_result_nohighlight.json +++ b/features/testdata/expected_single_search_release_result_nohighlight.json @@ -1,21 +1,23 @@ { - "took":8, - "limit":10, - "offset":0, + "took":11, "breakdown":{ - "total":1 + "total":1, + "postponed": 1, + "published": 2466, + "cancelled": 45 }, "releases":[ { "uri":"/releases/estimatingsuicideamonghighereducationstudentsenglandandwales", + "date_changes": [{"previous_date": "2018-06-01", "change_notice": "changed it"}], "description":{ "title":"Estimating suicide among higher education students, England and Wales: Experimental Statistics", "summary":"Estimates of suicides among higher education students by sex, age and ethnicity. Analysis based on mortality records linked to Higher Education Statistics Agency (HESA) Student records. ", "release_date":"2018-06-25T08:30:00.000Z", - "published":true, + "published":false, "cancelled":false, "finalised":true, - "postponed":false, + "postponed":true, "census":false, "national_statistic":false, "latest_release":false, diff --git a/features/testdata/expected_zero_search_release_results.json b/features/testdata/expected_zero_search_release_results.json index efc01d84..c8b20a77 100644 --- a/features/testdata/expected_zero_search_release_results.json +++ b/features/testdata/expected_zero_search_release_results.json @@ -1,7 +1,5 @@ { - "took":8, - "limit":10, - "offset":0, + "took":11, "breakdown":{ "total":0 }, diff --git a/templates/search/releasecalendar/main.go b/templates/search/releasecalendar/main.go index ec442a5f..480d9f24 100644 --- a/templates/search/releasecalendar/main.go +++ b/templates/search/releasecalendar/main.go @@ -1,7 +1,6 @@ package main import ( - "bytes" "context" "encoding/json" "flag" @@ -9,7 +8,6 @@ import ( "io" "log" "os" - "time" dphttp "github.com/ONSdigital/dp-net/v2/http" "github.com/ONSdigital/dp-search-api/elasticsearch" @@ -21,22 +19,22 @@ func main() { var ( sr = query.ReleaseSearchRequest{ Size: 10, - SortBy: query.RelDateDesc, + SortBy: query.Relevance, Term: "Education in Wales", ReleasedAfter: query.MustParseDate("2018-01-01"), ReleasedBefore: query.MustParseDate("2018-12-31"), Type: query.Published, - Highlight: false, - Now: query.Date(time.Now()), + Provisional: true, + Confirmed: true, + Postponed: true, + Highlight: true, } - builder *query.ReleaseBuilder - q, uq, responseData []byte - err error - ctx = context.Background() - multi = flag.Bool("multi", false, "use the multi query format when sending the query to ES") - file = flag.String("file", "", "a file containing the actual query to be sent to ES (in json format)") - esClient = elasticsearch.New("http://localhost:9200", dphttp.NewClient(), "eu-west-1", "es") - esSearch = esClient.Search + builder *query.ReleaseBuilder + q, responseData []byte + err error + ctx = context.Background() + file = flag.String("file", "", "a file containing the actual multi-query to be sent to ES (in json format)") + esClient = elasticsearch.New("http://localhost:9200", dphttp.NewClient(), "eu-west-1", "es") ) flag.Var(&sr, "sr", "a searchRequest object in json format") @@ -58,33 +56,14 @@ func main() { log.Fatalf("failed to create builder: %s", err) } - uq, err = builder.BuildSearchQuery(ctx, sr) + q, err = builder.BuildSearchQuery(ctx, sr) if err != nil { log.Fatalf("failed to build query: %s", err) } - - var b bytes.Buffer - err = json.Compact(&b, uq) - if err != nil { - log.Fatalf("failed to compact query: %s", err) - } - q = b.Bytes() - } - - if *multi { - var buf bytes.Buffer - buf.Write([]byte(`{"index" : "ons", "type": ["release"], "search_type": "dfs_query_then_fetch"}$$`)) - buf.Write(q) - buf.Write([]byte(`$$`)) - q, err = query.FormatMultiQuery(buf.Bytes()) - if err != nil { - log.Fatalf("failed to format multi query: %s", err) - } - esSearch = esClient.MultiSearch } fmt.Printf("\nformatted query is:\n%s", q) - responseData, err = esSearch(ctx, "ons", "release", q) + responseData, err = esClient.MultiSearch(ctx, "ons", "release", q) if err != nil { log.Fatalf("elasticsearch query failed: %s", err) } @@ -94,11 +73,7 @@ func main() { } fmt.Printf("\nresponse is:\n%s", responseData) - if *multi { - responseData, err = transformer.New().TransformSearchResponse(ctx, responseData, sr.Term, sr.Highlight) - } else { - responseData, err = transformer.NewReleaseTransformer().TransformSearchResponse(ctx, responseData, sr, sr.Highlight) - } + responseData, err = transformer.NewReleaseTransformer().TransformSearchResponse(ctx, responseData, sr, sr.Highlight) if err != nil { log.Fatalf("transformation of response data failed: %s", err) } From e33ee481e313f10f3dab5bdb0eba82b6d485710b Mon Sep 17 00:00:00 2001 From: rahulmadathumpalliyalil Date: Tue, 12 Apr 2022 15:33:21 +0100 Subject: [PATCH 24/29] Create templates for ES 7.10 --- query/query_test.go | 23 ++++ query/search.go | 20 ++++ .../v710/contentFilterOnFirstLetter.tmpl | 8 ++ .../search/v710/contentFilterOnLatest.tmpl | 8 ++ .../v710/contentFilterOnReleaseDate.tmpl | 15 +++ .../search/v710/contentFilterOnTopic.tmpl | 12 +++ .../v710/contentFilterOnTopicWildcard.tmpl | 8 ++ .../search/v710/contentFilterOnURIPrefix.tmpl | 7 ++ .../search/v710/contentFilterPublished.tmpl | 36 +++++++ .../search/v710/contentFilterUpcoming.tmpl | 29 +++++ templates/search/v710/contentFilters.tmpl | 30 ++++++ templates/search/v710/contentQuery.tmpl | 49 ++++++++- templates/search/v710/coreQuery.tmpl | 102 ++++++++++++++++++ templates/search/v710/countFilterLatest.tmpl | 8 ++ templates/search/v710/countHeader.tmpl | 8 ++ templates/search/v710/countQuery.tmpl | 32 ++++++ templates/search/v710/matchAll.tmpl | 1 + templates/search/v710/search.tmpl | 4 + templates/search/v710/sortByFirstLetter.tmpl | 19 ++++ templates/search/v710/sortByReleaseDate.tmpl | 14 +++ .../search/v710/sortByReleaseDateAsc.tmpl | 15 +++ templates/search/v710/sortByRelevance.tmpl | 10 ++ templates/search/v710/sortByTitle.tmpl | 14 +++ templates/search/v710/weightedQuery.tmpl | 48 +++++++++ 24 files changed, 518 insertions(+), 2 deletions(-) create mode 100644 templates/search/v710/contentFilterOnFirstLetter.tmpl create mode 100644 templates/search/v710/contentFilterOnLatest.tmpl create mode 100644 templates/search/v710/contentFilterOnReleaseDate.tmpl create mode 100644 templates/search/v710/contentFilterOnTopic.tmpl create mode 100644 templates/search/v710/contentFilterOnTopicWildcard.tmpl create mode 100644 templates/search/v710/contentFilterOnURIPrefix.tmpl create mode 100644 templates/search/v710/contentFilterPublished.tmpl create mode 100644 templates/search/v710/contentFilterUpcoming.tmpl create mode 100644 templates/search/v710/contentFilters.tmpl create mode 100644 templates/search/v710/coreQuery.tmpl create mode 100644 templates/search/v710/countFilterLatest.tmpl create mode 100644 templates/search/v710/countHeader.tmpl create mode 100644 templates/search/v710/countQuery.tmpl create mode 100644 templates/search/v710/matchAll.tmpl create mode 100644 templates/search/v710/sortByFirstLetter.tmpl create mode 100644 templates/search/v710/sortByReleaseDate.tmpl create mode 100644 templates/search/v710/sortByReleaseDateAsc.tmpl create mode 100644 templates/search/v710/sortByRelevance.tmpl create mode 100644 templates/search/v710/sortByTitle.tmpl create mode 100644 templates/search/v710/weightedQuery.tmpl diff --git a/query/query_test.go b/query/query_test.go index 6ad17acb..12d5fcf5 100644 --- a/query/query_test.go +++ b/query/query_test.go @@ -32,6 +32,29 @@ func TestNewQueryBuilder(t *testing.T) { So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "search.tmpl") So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "contentQuery.tmpl") So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "contentHeader.tmpl") + So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "matchAll.tmpl") + So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "contentQuery.tmpl") + So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "contentHeader.tmpl") + So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "countHeader.tmpl") + So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "countQuery.tmpl") + So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "coreQuery.tmpl") + So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "weightedQuery.tmpl") + So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "countFilterLatest.tmpl") + So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "contentFilters.tmpl") + So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "contentFilterUpcoming.tmpl") + So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "contentFilterPublished.tmpl") + So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "contentFilterOnLatest.tmpl") + So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "contentFilterOnFirstLetter.tmpl") + So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "contentFilterOnReleaseDate.tmpl") + So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "contentFilterOnURIPrefix.tmpl") + So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "contentFilterOnTopic.tmpl") + So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "contentFilterOnTopicWildcard.tmpl") + So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "sortByTitle.tmpl") + So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "sortByRelevance.tmpl") + So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "sortByReleaseDate.tmpl") + So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "sortByReleaseDateAsc.tmpl") + So(builderObject.searchTemplates.DefinedTemplates(), ShouldContainSubstring, "sortByFirstLetter.tmpl") + So(err, ShouldBeNil) }) } diff --git a/query/search.go b/query/search.go index baf46871..881c47ec 100644 --- a/query/search.go +++ b/query/search.go @@ -79,7 +79,27 @@ func SetupV710Search(pathToTemplates string) (*template.Template, error) { templates, err := template.ParseFiles( pathToTemplates+"templates/search/v710/search.tmpl", pathToTemplates+"templates/search/v710/contentQuery.tmpl", + pathToTemplates+"templates/search/v710/matchAll.tmpl", pathToTemplates+"templates/search/v710/contentHeader.tmpl", + pathToTemplates+"templates/search/v710/countHeader.tmpl", + pathToTemplates+"templates/search/v710/countQuery.tmpl", + pathToTemplates+"templates/search/v710/coreQuery.tmpl", + pathToTemplates+"templates/search/v710/weightedQuery.tmpl", + pathToTemplates+"templates/search/v710/countFilterLatest.tmpl", + pathToTemplates+"templates/search/v710/contentFilters.tmpl", + pathToTemplates+"templates/search/v710/contentFilterUpcoming.tmpl", + pathToTemplates+"templates/search/v710/contentFilterPublished.tmpl", + pathToTemplates+"templates/search/v710/contentFilterOnLatest.tmpl", + pathToTemplates+"templates/search/v710/contentFilterOnFirstLetter.tmpl", + pathToTemplates+"templates/search/v710/contentFilterOnReleaseDate.tmpl", + pathToTemplates+"templates/search/v710/contentFilterOnURIPrefix.tmpl", + pathToTemplates+"templates/search/v710/contentFilterOnTopic.tmpl", + pathToTemplates+"templates/search/v710/contentFilterOnTopicWildcard.tmpl", + pathToTemplates+"templates/search/v710/sortByTitle.tmpl", + pathToTemplates+"templates/search/v710/sortByRelevance.tmpl", + pathToTemplates+"templates/search/v710/sortByReleaseDate.tmpl", + pathToTemplates+"templates/search/v710/sortByReleaseDateAsc.tmpl", + pathToTemplates+"templates/search/v710/sortByFirstLetter.tmpl", ) return templates, err diff --git a/templates/search/v710/contentFilterOnFirstLetter.tmpl b/templates/search/v710/contentFilterOnFirstLetter.tmpl new file mode 100644 index 00000000..896af21f --- /dev/null +++ b/templates/search/v710/contentFilterOnFirstLetter.tmpl @@ -0,0 +1,8 @@ +{{if .FilterOnFirstLetter }} + { + "term": { + "description.title.title_first_letter": "{{.FilterOnFirstLetter}}" + } + } + {{if or .ReleasedBefore .ReleasedAfter .URIPrefix .Topic .TopicWildcard .Upcoming .Published}},{{end}} + {{end}} diff --git a/templates/search/v710/contentFilterOnLatest.tmpl b/templates/search/v710/contentFilterOnLatest.tmpl new file mode 100644 index 00000000..d15b3622 --- /dev/null +++ b/templates/search/v710/contentFilterOnLatest.tmpl @@ -0,0 +1,8 @@ +{{ if .FilterOnLatest }} + { + "term" : { + "description.latestRelease" : true + } + } + {{if or .FilterOnFirstLetter .ReleasedBefore .ReleasedAfter .URIPrefix .Topic .TopicWildcard .Upcoming .Published}},{{end}} +{{end}} \ No newline at end of file diff --git a/templates/search/v710/contentFilterOnReleaseDate.tmpl b/templates/search/v710/contentFilterOnReleaseDate.tmpl new file mode 100644 index 00000000..a0c37d8c --- /dev/null +++ b/templates/search/v710/contentFilterOnReleaseDate.tmpl @@ -0,0 +1,15 @@ +{{- if or .ReleasedBefore .ReleasedAfter }} + { + "range": { + "description.releaseDate": { + {{- if .ReleasedAfter}} + "from": "{{.ReleasedAfter}}" + {{- end}} + {{- if and .ReleasedBefore .ReleasedAfter }},{{- end}} + {{- if .ReleasedBefore}} + "to": "{{.ReleasedBefore}}" + {{- end}} + } + } + } + {{- end}} \ No newline at end of file diff --git a/templates/search/v710/contentFilterOnTopic.tmpl b/templates/search/v710/contentFilterOnTopic.tmpl new file mode 100644 index 00000000..192c6d9d --- /dev/null +++ b/templates/search/v710/contentFilterOnTopic.tmpl @@ -0,0 +1,12 @@ +{{if .Topic}} + { "terms" : + { "topic" : [ + {{range $i,$e := .Topic}} + {{if $i}},{{end}} + "{{.}}" + {{end}} + ] + } + } + {{if .TopicWildcard}},{{end}} +{{end}} \ No newline at end of file diff --git a/templates/search/v710/contentFilterOnTopicWildcard.tmpl b/templates/search/v710/contentFilterOnTopicWildcard.tmpl new file mode 100644 index 00000000..58440c3b --- /dev/null +++ b/templates/search/v710/contentFilterOnTopicWildcard.tmpl @@ -0,0 +1,8 @@ +{{ if .TopicWildcard }} + {{range $i,$e := .TopicWildcard}} + {{if $i}},{{end}} + { "wildcard" : + { "topic" : "{{.}}" } + } + {{end}} +{{end}} \ No newline at end of file diff --git a/templates/search/v710/contentFilterOnURIPrefix.tmpl b/templates/search/v710/contentFilterOnURIPrefix.tmpl new file mode 100644 index 00000000..b7724c22 --- /dev/null +++ b/templates/search/v710/contentFilterOnURIPrefix.tmpl @@ -0,0 +1,7 @@ + + {{if .URIPrefix}} + { "prefix" : + { "uri" : "{{.URIPrefix}}" } + } + {{if or .Topic .TopicWildcard}},{{end}} + {{end}} \ No newline at end of file diff --git a/templates/search/v710/contentFilterPublished.tmpl b/templates/search/v710/contentFilterPublished.tmpl new file mode 100644 index 00000000..6a6526f3 --- /dev/null +++ b/templates/search/v710/contentFilterPublished.tmpl @@ -0,0 +1,36 @@ +{ + "bool": { + "should":[ + { + "bool": { + "must_not": { + "term": { + "description.cancelled": true + } + }, + "must": { + "range": { + "description.releaseDate": { + "lte": "{{.Now}}" + } + } + } + } + }, + { + "bool": { + "must": { + "term": { + "description.published": true + } + }, + "must_not": { + "term": { + "description.cancelled": true + } + } + } + } + ] + } +} diff --git a/templates/search/v710/contentFilterUpcoming.tmpl b/templates/search/v710/contentFilterUpcoming.tmpl new file mode 100644 index 00000000..ac8f40ec --- /dev/null +++ b/templates/search/v710/contentFilterUpcoming.tmpl @@ -0,0 +1,29 @@ +{ + "bool": { + "should": [ + { + "bool": { + "must_not": [ + { + "term": { + "description.published": true + } + }, + { + "term": { + "description.cancelled": true + } + }, + { + "range": { + "description.releaseDate": { + "lte": "{{.Now}}" + } + } + } + ] + } + } + ] + } +} diff --git a/templates/search/v710/contentFilters.tmpl b/templates/search/v710/contentFilters.tmpl new file mode 100644 index 00000000..4984b1ed --- /dev/null +++ b/templates/search/v710/contentFilters.tmpl @@ -0,0 +1,30 @@ +{{ if or .FilterOnLatest .FilterOnFirstLetter .ReleasedBefore .ReleasedAfter .URIPrefix .Topic .TopicWildcard .Upcoming .Published}} +, +"filter" : [ +{{ template "contentFilterOnLatest.tmpl" . }} +{{ template "contentFilterOnFirstLetter.tmpl" . }} +{{if or .URIPrefix .Topic .TopicWildcard}} + { "bool": { "should": [ + + {{ template "contentFilterOnURIPrefix.tmpl" . }} + {{ template "contentFilterOnTopic.tmpl" . }} + {{ template "contentFilterOnTopicWildcard.tmpl" . }} + + ], + "must":[ + {{ template "contentFilterOnReleaseDate.tmpl" .}} + ]} + } + {{if or .Upcoming .Published}} + , + {{end}} + + {{end}} + + {{if .Upcoming}} + {{- template "contentFilterUpcoming.tmpl" . }} + {{else if .Published}} + {{- template "contentFilterPublished.tmpl" . }} + {{end}} +] +{{end}} \ No newline at end of file diff --git a/templates/search/v710/contentQuery.tmpl b/templates/search/v710/contentQuery.tmpl index a3aed9ab..08a37775 100644 --- a/templates/search/v710/contentQuery.tmpl +++ b/templates/search/v710/contentQuery.tmpl @@ -1,10 +1,55 @@ { {{/* content query */}} + {{- if .From -}} + "from" : {{- .From}}, + {{- end}} + "size" : {{.Size}}, "query" : { "bool" : { "must" : { - "match_all": {} + {{- if .Term}} + {{- template "weightedQuery.tmpl" .}} + {{- else}} + {{- template "matchAll.tmpl" .}} + {{- end}} } + {{template "contentFilters.tmpl" .}} } - } + }, + "suggest":{ + "search_suggest":{ + "text":"{{.Term}}", + "phrase":{"field":"description.title.title_no_synonym_no_stem"}} + }, + "_source":{ + "includes":[], + "excludes":["downloads.content","downloads*","pageData"]}, + {{if .Highlight}} + "highlight":{ + "pre_tags":[""], + "post_tags":[""], + "fields":{"terms":{"fragment_size":0,"number_of_fragments":0}, + "description.title":{"fragment_size":0,"number_of_fragments":0}, + "description.edition":{"fragment_size":0,"number_of_fragments":0}, + "description.summary":{"fragment_size":0,"number_of_fragments":0}, + "description.metaDescription":{"fragment_size":0,"number_of_fragments":0}, + "description.keywords":{"fragment_size":0,"number_of_fragments":0}, + "description.cdid":{"fragment_size":0,"number_of_fragments":0}, + "description.datasetId":{"fragment_size":0,"number_of_fragments":0}, + "downloads.content":{"fragment_size": 45,"number_of_fragments": 5}, + "pageData":{"fragment_size": 45,"number_of_fragments": 5} + } + }, + {{end}} + {{ if eq .SortBy "release_date" }} + {{ template "sortByReleaseDate.tmpl" }} + {{ else if eq .SortBy "release_date_asc" }} + {{template "sortByReleaseDateAsc.tmpl" }} + {{ else if eq .SortBy "title" }} + {{ template "sortByTitle.tmpl" }} + {{ else if eq .SortBy "first_letter" }} + {{ template "sortByFirstLetter.tmpl" }} + {{ else }} + {{template "sortByRelevance.tmpl" }} + {{ end }} } \ No newline at end of file diff --git a/templates/search/v710/coreQuery.tmpl b/templates/search/v710/coreQuery.tmpl new file mode 100644 index 00000000..b8ae8e92 --- /dev/null +++ b/templates/search/v710/coreQuery.tmpl @@ -0,0 +1,102 @@ +"dis_max": { + "queries": [ + { + "bool": { + "should": [ + { + "match": { + "description.title.title_no_dates": { + "query": "{{.Term}}", + "type": "boolean", + "boost": 10.0, + "minimum_should_match": "1<-2 3<80% 5<60%" + } + } + }, + { + "match": { + "description.title.title_no_stem": { + "query": "{{.Term}}", + "type": "boolean", + "boost": 10.0, + "minimum_should_match": "1<-2 3<80% 5<60%" + } + } + }, + { + "multi_match": { + "query": "{{.Term}}", + "fields": [ + "description.title^10", + "description.edition", + "downloads.content^1" + ], + "type": "cross_fields", + "minimum_should_match": "3<80% 5<60%" + } + }, + { + "multi_match": { + "query": "{{.Term}}", + "fields": [ + "description.title^10", + "description.summary", + "description.metaDescription", + "description.edition", + "downloads.content^1", + "pageData^1", + "description.keywords" + ], + "type": "phrase", + "boost": 10.0, + "slop": 2 + } + } + ] + } + }, + { + "multi_match": { + "query": "{{.Term}}", + "fields": [ + "description.summary", + "description.metaDescription", + "downloads.content^1", + "pageData^1", + "description.keywords" + ], + "type": "best_fields", + "minimum_should_match": "75%" + } + }, + { + "match": { + "description.keywords": { + "query": "{{.Term}}", + "type": "boolean", + "operator": "AND", + "boost": 10.0 + } + } + }, + { + "multi_match": { + "query": "{{.Term}}", + "fields": [ + "description.cdid", + "description.datasetId" + ] + } + }, + { + "match": { + "searchBoost": { + "query": "{{.Term}}", + "type": "boolean", + "operator": "AND", + "boost": 100.0 + } + } + } + ] + } \ No newline at end of file diff --git a/templates/search/v710/countFilterLatest.tmpl b/templates/search/v710/countFilterLatest.tmpl new file mode 100644 index 00000000..75e3a090 --- /dev/null +++ b/templates/search/v710/countFilterLatest.tmpl @@ -0,0 +1,8 @@ +{{- if .FilterOnLatest -}} +, +"filter" : { + "term" : { + "description.latestRelease" : true + } +} +{{ end}} \ No newline at end of file diff --git a/templates/search/v710/countHeader.tmpl b/templates/search/v710/countHeader.tmpl new file mode 100644 index 00000000..0169b4ca --- /dev/null +++ b/templates/search/v710/countHeader.tmpl @@ -0,0 +1,8 @@ +{ {{/* count query */}} + {{if .Index}} + "index":"{{.Index}}" + {{else}} + "index":"ons" + {{end}} + {{if .Types}}, "type" : [{{range $i,$e := .Types}}{{if $i}},{{end}}"{{.}}"{{end}}]{{end}} +} \ No newline at end of file diff --git a/templates/search/v710/countQuery.tmpl b/templates/search/v710/countQuery.tmpl new file mode 100644 index 00000000..7928c86f --- /dev/null +++ b/templates/search/v710/countQuery.tmpl @@ -0,0 +1,32 @@ +{ {{/* count query */}} +"query" : { + "bool" : { + "must" : { + {{- if .Term}} + {{- template "coreQuery.tmpl" .}} + {{- else}} + {{- template "matchAll.tmpl" .}} + {{- end}} + } + {{- template "countFilterLatest.tmpl" .}} + {{- if .URIPrefix }} + ,"filter": [{ + "prefix": { + "uri": "{{.URIPrefix}}" + }} + {{- if or .ReleasedBefore .ReleasedAfter}}, {{- end}} + {{ template "contentFilterOnReleaseDate.tmpl" .}} + ] + {{end}} + } + }, + "size": 0, + "aggregations": { + "docCounts": { + "terms": { + "size": 1000, + "field":"{{.AggregationField}}" + } + } + } +} \ No newline at end of file diff --git a/templates/search/v710/matchAll.tmpl b/templates/search/v710/matchAll.tmpl new file mode 100644 index 00000000..821c31d4 --- /dev/null +++ b/templates/search/v710/matchAll.tmpl @@ -0,0 +1 @@ +"match_all": {} \ No newline at end of file diff --git a/templates/search/v710/search.tmpl b/templates/search/v710/search.tmpl index b31f3cb6..8c22a7d6 100644 --- a/templates/search/v710/search.tmpl +++ b/templates/search/v710/search.tmpl @@ -2,4 +2,8 @@ {{- if .HasQuery "search" }} {{- template "contentHeader.tmpl" .}}$$ {{- template "contentQuery.tmpl" .}}$$ +{{- end}} +{{- if .HasQuery "counts" }} + {{- template "countHeader.tmpl" .}}$$ + {{- template "countQuery.tmpl" .}}$$ {{- end}} \ No newline at end of file diff --git a/templates/search/v710/sortByFirstLetter.tmpl b/templates/search/v710/sortByFirstLetter.tmpl new file mode 100644 index 00000000..998b2084 --- /dev/null +++ b/templates/search/v710/sortByFirstLetter.tmpl @@ -0,0 +1,19 @@ +{{/* //First letter skips non-letter character in the beginning. that's why sorting by first letter and then title + first_letter( + get(Field.title_first_letter, SortOrder.ASC), + get(Field.title_raw, SortOrder.ASC), + get(Field.releaseDate, SortOrder.ASC) + ),*/}} +"sort" : [{ + "description.title.title_first_letter" : { + "order" : "asc" + } + }, { + "description.title.title_raw" : { + "order" : "asc" + } + }, { + "description.releaseDate" : { + "order" : "asc" + } + }] \ No newline at end of file diff --git a/templates/search/v710/sortByReleaseDate.tmpl b/templates/search/v710/sortByReleaseDate.tmpl new file mode 100644 index 00000000..c44b8390 --- /dev/null +++ b/templates/search/v710/sortByReleaseDate.tmpl @@ -0,0 +1,14 @@ +{{/* release_date( + get(Field.releaseDate, SortOrder.DESC), + get(Field._score, SortOrder.DESC) + ) +*/}} +"sort" : [{ + "description.releaseDate" : { + "order" : "desc" + } + }, { + "_score" : { + "order" : "desc" + } + }] \ No newline at end of file diff --git a/templates/search/v710/sortByReleaseDateAsc.tmpl b/templates/search/v710/sortByReleaseDateAsc.tmpl new file mode 100644 index 00000000..dbfd6a40 --- /dev/null +++ b/templates/search/v710/sortByReleaseDateAsc.tmpl @@ -0,0 +1,15 @@ +{{/* +release_date_asc( + get(Field.releaseDate, SortOrder.ASC), + get(Field._score, SortOrder.DESC) + ) +*/}} +"sort" : [{ + "description.releaseDate" : { + "order" : "asc" + } + }, { + "_score" : { + "order" : "desc" + } + }] \ No newline at end of file diff --git a/templates/search/v710/sortByRelevance.tmpl b/templates/search/v710/sortByRelevance.tmpl new file mode 100644 index 00000000..cc0fad66 --- /dev/null +++ b/templates/search/v710/sortByRelevance.tmpl @@ -0,0 +1,10 @@ +"sort" : [ + { + "_score" : { + "order" : "desc" + } + }, { + "description.releaseDate" : { + "order" : "desc" + } + }] diff --git a/templates/search/v710/sortByTitle.tmpl b/templates/search/v710/sortByTitle.tmpl new file mode 100644 index 00000000..3288ae0e --- /dev/null +++ b/templates/search/v710/sortByTitle.tmpl @@ -0,0 +1,14 @@ +{{/* title( + get(Field.title_raw, SortOrder.ASC), + get(Field.releaseDate, SortOrder.DESC) + )*/}} +"sort" : [ + { + "description.title.title_raw" : { + "order" : "asc" + } + }, { + "description.releaseDate" : { + "order" : "desc" + } + }] \ No newline at end of file diff --git a/templates/search/v710/weightedQuery.tmpl b/templates/search/v710/weightedQuery.tmpl new file mode 100644 index 00000000..c8b8a5a2 --- /dev/null +++ b/templates/search/v710/weightedQuery.tmpl @@ -0,0 +1,48 @@ +"function_score": { +"query": { {{template "coreQuery.tmpl" .}} }, + + "functions": [ + { + "filter": { + "term": { + "_type": "bulletin" + } + }, + "weight": 100 + }, + { + "filter": { + "term": { + "_type": "dataset_landing_page" + } + }, + "weight": 70 + }, + { + "filter": { + "terms": { + "_type": ["article", + "compendium_landing_page", + "article_download"] + } + }, + "weight": 50 + }, + { + "filter": { + "term": { + "_type": "static_adhoc" + } + }, + "weight": 30 + }, + { + "filter": { + "term": { + "_type": "timeseries" + } + }, + "weight": 10 + } + ] + } \ No newline at end of file From 9ea17f9f8de02ab068bba92638c67a13f40d0dac Mon Sep 17 00:00:00 2001 From: rahulmadathumpalliyalil Date: Thu, 14 Apr 2022 16:40:39 +0100 Subject: [PATCH 25/29] Remove description attribute from templates --- .../v710/contentFilterOnFirstLetter.tmpl | 2 +- .../search/v710/contentFilterOnLatest.tmpl | 2 +- .../v710/contentFilterOnReleaseDate.tmpl | 2 +- .../search/v710/contentFilterUpcoming.tmpl | 6 ++-- templates/search/v710/contentQuery.tmpl | 16 +++++----- templates/search/v710/coreQuery.tmpl | 30 +++++++++---------- templates/search/v710/countFilterLatest.tmpl | 2 +- templates/search/v710/sortByFirstLetter.tmpl | 6 ++-- templates/search/v710/sortByReleaseDate.tmpl | 2 +- .../search/v710/sortByReleaseDateAsc.tmpl | 2 +- templates/search/v710/sortByRelevance.tmpl | 2 +- templates/search/v710/sortByTitle.tmpl | 4 +-- 12 files changed, 38 insertions(+), 38 deletions(-) diff --git a/templates/search/v710/contentFilterOnFirstLetter.tmpl b/templates/search/v710/contentFilterOnFirstLetter.tmpl index 896af21f..48e869ed 100644 --- a/templates/search/v710/contentFilterOnFirstLetter.tmpl +++ b/templates/search/v710/contentFilterOnFirstLetter.tmpl @@ -1,7 +1,7 @@ {{if .FilterOnFirstLetter }} { "term": { - "description.title.title_first_letter": "{{.FilterOnFirstLetter}}" + "title.title_first_letter": "{{.FilterOnFirstLetter}}" } } {{if or .ReleasedBefore .ReleasedAfter .URIPrefix .Topic .TopicWildcard .Upcoming .Published}},{{end}} diff --git a/templates/search/v710/contentFilterOnLatest.tmpl b/templates/search/v710/contentFilterOnLatest.tmpl index d15b3622..c7e96229 100644 --- a/templates/search/v710/contentFilterOnLatest.tmpl +++ b/templates/search/v710/contentFilterOnLatest.tmpl @@ -1,7 +1,7 @@ {{ if .FilterOnLatest }} { "term" : { - "description.latestRelease" : true + "latestRelease" : true } } {{if or .FilterOnFirstLetter .ReleasedBefore .ReleasedAfter .URIPrefix .Topic .TopicWildcard .Upcoming .Published}},{{end}} diff --git a/templates/search/v710/contentFilterOnReleaseDate.tmpl b/templates/search/v710/contentFilterOnReleaseDate.tmpl index a0c37d8c..58ea8468 100644 --- a/templates/search/v710/contentFilterOnReleaseDate.tmpl +++ b/templates/search/v710/contentFilterOnReleaseDate.tmpl @@ -1,7 +1,7 @@ {{- if or .ReleasedBefore .ReleasedAfter }} { "range": { - "description.releaseDate": { + "releaseDate": { {{- if .ReleasedAfter}} "from": "{{.ReleasedAfter}}" {{- end}} diff --git a/templates/search/v710/contentFilterUpcoming.tmpl b/templates/search/v710/contentFilterUpcoming.tmpl index ac8f40ec..69469db0 100644 --- a/templates/search/v710/contentFilterUpcoming.tmpl +++ b/templates/search/v710/contentFilterUpcoming.tmpl @@ -6,17 +6,17 @@ "must_not": [ { "term": { - "description.published": true + "published": true } }, { "term": { - "description.cancelled": true + "cancelled": true } }, { "range": { - "description.releaseDate": { + "releaseDate": { "lte": "{{.Now}}" } } diff --git a/templates/search/v710/contentQuery.tmpl b/templates/search/v710/contentQuery.tmpl index 08a37775..788a5bbc 100644 --- a/templates/search/v710/contentQuery.tmpl +++ b/templates/search/v710/contentQuery.tmpl @@ -19,7 +19,7 @@ "suggest":{ "search_suggest":{ "text":"{{.Term}}", - "phrase":{"field":"description.title.title_no_synonym_no_stem"}} + "phrase":{"field":"title.title_no_synonym_no_stem"}} }, "_source":{ "includes":[], @@ -29,13 +29,13 @@ "pre_tags":[""], "post_tags":[""], "fields":{"terms":{"fragment_size":0,"number_of_fragments":0}, - "description.title":{"fragment_size":0,"number_of_fragments":0}, - "description.edition":{"fragment_size":0,"number_of_fragments":0}, - "description.summary":{"fragment_size":0,"number_of_fragments":0}, - "description.metaDescription":{"fragment_size":0,"number_of_fragments":0}, - "description.keywords":{"fragment_size":0,"number_of_fragments":0}, - "description.cdid":{"fragment_size":0,"number_of_fragments":0}, - "description.datasetId":{"fragment_size":0,"number_of_fragments":0}, + "title":{"fragment_size":0,"number_of_fragments":0}, + "edition":{"fragment_size":0,"number_of_fragments":0}, + "summary":{"fragment_size":0,"number_of_fragments":0}, + "metaDescription":{"fragment_size":0,"number_of_fragments":0}, + "keywords":{"fragment_size":0,"number_of_fragments":0}, + "cdid":{"fragment_size":0,"number_of_fragments":0}, + "datasetId":{"fragment_size":0,"number_of_fragments":0}, "downloads.content":{"fragment_size": 45,"number_of_fragments": 5}, "pageData":{"fragment_size": 45,"number_of_fragments": 5} } diff --git a/templates/search/v710/coreQuery.tmpl b/templates/search/v710/coreQuery.tmpl index b8ae8e92..b399fc05 100644 --- a/templates/search/v710/coreQuery.tmpl +++ b/templates/search/v710/coreQuery.tmpl @@ -5,7 +5,7 @@ "should": [ { "match": { - "description.title.title_no_dates": { + "title.title_no_dates": { "query": "{{.Term}}", "type": "boolean", "boost": 10.0, @@ -15,7 +15,7 @@ }, { "match": { - "description.title.title_no_stem": { + "title.title_no_stem": { "query": "{{.Term}}", "type": "boolean", "boost": 10.0, @@ -27,8 +27,8 @@ "multi_match": { "query": "{{.Term}}", "fields": [ - "description.title^10", - "description.edition", + "title^10", + "edition", "downloads.content^1" ], "type": "cross_fields", @@ -39,13 +39,13 @@ "multi_match": { "query": "{{.Term}}", "fields": [ - "description.title^10", - "description.summary", - "description.metaDescription", - "description.edition", + "title^10", + "summary", + "metaDescription", + "edition", "downloads.content^1", "pageData^1", - "description.keywords" + "keywords" ], "type": "phrase", "boost": 10.0, @@ -59,11 +59,11 @@ "multi_match": { "query": "{{.Term}}", "fields": [ - "description.summary", - "description.metaDescription", + "summary", + "metaDescription", "downloads.content^1", "pageData^1", - "description.keywords" + "keywords" ], "type": "best_fields", "minimum_should_match": "75%" @@ -71,7 +71,7 @@ }, { "match": { - "description.keywords": { + "keywords": { "query": "{{.Term}}", "type": "boolean", "operator": "AND", @@ -83,8 +83,8 @@ "multi_match": { "query": "{{.Term}}", "fields": [ - "description.cdid", - "description.datasetId" + "cdid", + "datasetId" ] } }, diff --git a/templates/search/v710/countFilterLatest.tmpl b/templates/search/v710/countFilterLatest.tmpl index 75e3a090..7f20d8f3 100644 --- a/templates/search/v710/countFilterLatest.tmpl +++ b/templates/search/v710/countFilterLatest.tmpl @@ -2,7 +2,7 @@ , "filter" : { "term" : { - "description.latestRelease" : true + "latestRelease" : true } } {{ end}} \ No newline at end of file diff --git a/templates/search/v710/sortByFirstLetter.tmpl b/templates/search/v710/sortByFirstLetter.tmpl index 998b2084..d9f96ca2 100644 --- a/templates/search/v710/sortByFirstLetter.tmpl +++ b/templates/search/v710/sortByFirstLetter.tmpl @@ -5,15 +5,15 @@ get(Field.releaseDate, SortOrder.ASC) ),*/}} "sort" : [{ - "description.title.title_first_letter" : { + "title.title_first_letter" : { "order" : "asc" } }, { - "description.title.title_raw" : { + "title.title_raw" : { "order" : "asc" } }, { - "description.releaseDate" : { + "releaseDate" : { "order" : "asc" } }] \ No newline at end of file diff --git a/templates/search/v710/sortByReleaseDate.tmpl b/templates/search/v710/sortByReleaseDate.tmpl index c44b8390..5f2cce06 100644 --- a/templates/search/v710/sortByReleaseDate.tmpl +++ b/templates/search/v710/sortByReleaseDate.tmpl @@ -4,7 +4,7 @@ ) */}} "sort" : [{ - "description.releaseDate" : { + "releaseDate" : { "order" : "desc" } }, { diff --git a/templates/search/v710/sortByReleaseDateAsc.tmpl b/templates/search/v710/sortByReleaseDateAsc.tmpl index dbfd6a40..a3fc7bdc 100644 --- a/templates/search/v710/sortByReleaseDateAsc.tmpl +++ b/templates/search/v710/sortByReleaseDateAsc.tmpl @@ -5,7 +5,7 @@ release_date_asc( ) */}} "sort" : [{ - "description.releaseDate" : { + "releaseDate" : { "order" : "asc" } }, { diff --git a/templates/search/v710/sortByRelevance.tmpl b/templates/search/v710/sortByRelevance.tmpl index cc0fad66..9be8f88a 100644 --- a/templates/search/v710/sortByRelevance.tmpl +++ b/templates/search/v710/sortByRelevance.tmpl @@ -4,7 +4,7 @@ "order" : "desc" } }, { - "description.releaseDate" : { + "releaseDate" : { "order" : "desc" } }] diff --git a/templates/search/v710/sortByTitle.tmpl b/templates/search/v710/sortByTitle.tmpl index 3288ae0e..02500ecb 100644 --- a/templates/search/v710/sortByTitle.tmpl +++ b/templates/search/v710/sortByTitle.tmpl @@ -4,11 +4,11 @@ )*/}} "sort" : [ { - "description.title.title_raw" : { + "title.title_raw" : { "order" : "asc" } }, { - "description.releaseDate" : { + "releaseDate" : { "order" : "desc" } }] \ No newline at end of file From 7d0e2cc01fa71d6ee3887664bdb0022dc5b5b0a7 Mon Sep 17 00:00:00 2001 From: rahulmadathumpalliyalil Date: Thu, 14 Apr 2022 17:33:33 +0100 Subject: [PATCH 26/29] fixed comments --- templates/search/v710/contentFilterPublished.tmpl | 8 ++++---- templates/search/v710/weightedQuery.tmpl | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/templates/search/v710/contentFilterPublished.tmpl b/templates/search/v710/contentFilterPublished.tmpl index 6a6526f3..4331ef42 100644 --- a/templates/search/v710/contentFilterPublished.tmpl +++ b/templates/search/v710/contentFilterPublished.tmpl @@ -5,12 +5,12 @@ "bool": { "must_not": { "term": { - "description.cancelled": true + "cancelled": true } }, "must": { "range": { - "description.releaseDate": { + "releaseDate": { "lte": "{{.Now}}" } } @@ -21,12 +21,12 @@ "bool": { "must": { "term": { - "description.published": true + "published": true } }, "must_not": { "term": { - "description.cancelled": true + "cancelled": true } } } diff --git a/templates/search/v710/weightedQuery.tmpl b/templates/search/v710/weightedQuery.tmpl index c8b8a5a2..ee7d311d 100644 --- a/templates/search/v710/weightedQuery.tmpl +++ b/templates/search/v710/weightedQuery.tmpl @@ -5,7 +5,7 @@ { "filter": { "term": { - "_type": "bulletin" + "type": "bulletin" } }, "weight": 100 @@ -13,7 +13,7 @@ { "filter": { "term": { - "_type": "dataset_landing_page" + "type": "dataset_landing_page" } }, "weight": 70 @@ -21,7 +21,7 @@ { "filter": { "terms": { - "_type": ["article", + "type": ["article", "compendium_landing_page", "article_download"] } @@ -31,7 +31,7 @@ { "filter": { "term": { - "_type": "static_adhoc" + "type": "static_adhoc" } }, "weight": 30 @@ -39,7 +39,7 @@ { "filter": { "term": { - "_type": "timeseries" + "type": "timeseries" } }, "weight": 10 From efdb1cfff44f757e02dd63e79f0aca22f80fe67f Mon Sep 17 00:00:00 2001 From: PatrickConnollyONS Date: Tue, 19 Apr 2022 12:01:08 +0100 Subject: [PATCH 27/29] review changes and updating to latest version of `golangci-lint` --- .golangci.yml | 4 ++-- Makefile | 2 +- query/esclauses.go | 1 - transformer/releasetransformer.go | 4 +++- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 4f7a50e7..07897275 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -12,7 +12,7 @@ linters-settings: golint: min-confidence: 0 gocyclo: - min-complexity: 15 + min-complexity: 20 gocognit: min-complexity: 40 maligned: @@ -69,7 +69,7 @@ linters: - staticcheck - structcheck - stylecheck - - typecheck +# - typecheck - unconvert - unused - varcheck diff --git a/Makefile b/Makefile index d5e38485..bb981034 100644 --- a/Makefile +++ b/Makefile @@ -22,7 +22,7 @@ audit: .PHONY: lint lint: - go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.43.0 + go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.45.2 golangci-lint run ./... .PHONY: fmt diff --git a/query/esclauses.go b/query/esclauses.go index 54e934f2..3ceb40cd 100644 --- a/query/esclauses.go +++ b/query/esclauses.go @@ -56,7 +56,6 @@ func mainUpcomingClause(now time.Time) string { fmt.Sprintf(`{"range": {"description.release_date": {"gte": %q}}}`, now.Format(dateFormat))) } -//gocyclo:ignore func supplementaryUpcomingClause(sr ReleaseSearchRequest) string { switch { case !sr.Provisional && !sr.Confirmed && sr.Postponed: diff --git a/transformer/releasetransformer.go b/transformer/releasetransformer.go index e6702084..3e328b5d 100644 --- a/transformer/releasetransformer.go +++ b/transformer/releasetransformer.go @@ -133,6 +133,8 @@ func NewReleaseTransformer() *ReleaseTransformer { } } +const numberOfReleaseQueries = 2 + // TransformSearchResponse transforms an elastic search response to a release query into a serialised ReleaseResponse func (t *ReleaseTransformer) TransformSearchResponse(_ context.Context, responseData []byte, req query.ReleaseSearchRequest, highlight bool) ([]byte, error) { var ( @@ -145,7 +147,7 @@ func (t *ReleaseTransformer) TransformSearchResponse(_ context.Context, response return nil, errors.Wrap(err, "Failed to decode elastic search response") } - if len(source.Responses) != 2 { + if len(source.Responses) != numberOfReleaseQueries { return nil, errors.New("invalid number of responses from ElasticSearch query") } From 8398c48e6a4a135ffe2094714569d56fbafb152f Mon Sep 17 00:00:00 2001 From: Jessica Jenkins Date: Tue, 19 Apr 2022 15:37:35 +0100 Subject: [PATCH 28/29] Remove timeseries and data endpoints --- api/api.go | 16 +--- api/data.go | 82 ----------------- api/data_test.go | 132 ---------------------------- api/timeseries.go | 62 ------------- api/timeseries_test.go | 110 ----------------------- swagger.yaml | 146 ------------------------------- templates/data/queryByUri.tmpl | 14 --- templates/timeseries/lookup.tmpl | 26 ------ 8 files changed, 1 insertion(+), 587 deletions(-) delete mode 100644 api/data.go delete mode 100644 api/data_test.go delete mode 100644 api/timeseries.go delete mode 100644 api/timeseries_test.go delete mode 100644 templates/data/queryByUri.tmpl delete mode 100644 templates/timeseries/lookup.tmpl diff --git a/api/api.go b/api/api.go index 9e58efc5..cad35c52 100644 --- a/api/api.go +++ b/api/api.go @@ -6,11 +6,9 @@ import ( "context" "net/http" - "github.com/gorilla/mux" - "github.com/pkg/errors" - "github.com/ONSdigital/dp-authorisation/auth" "github.com/ONSdigital/dp-search-api/query" + "github.com/gorilla/mux" ) var ( @@ -70,16 +68,6 @@ type ReleaseResponseTransformer interface { // NewSearchAPI returns a new Search API struct after registering the routes func NewSearchAPI(router *mux.Router, dpESClient DpElasticSearcher, deprecatedESClient ElasticSearcher, queryBuilder QueryBuilder, transformer ResponseTransformer, permissions AuthHandler) (*SearchAPI, error) { - errData := SetupData() - if errData != nil { - return nil, errors.Wrap(errData, "Failed to setup data templates") - } - - errTimeseries := SetupTimeseries() - if errTimeseries != nil { - return nil, errors.Wrap(errTimeseries, "Failed to setup timeseries templates") - } - api := &SearchAPI{ Router: router, QueryBuilder: queryBuilder, @@ -90,8 +78,6 @@ func NewSearchAPI(router *mux.Router, dpESClient DpElasticSearcher, deprecatedES } router.HandleFunc("/search", SearchHandlerFunc(queryBuilder, api.deprecatedESClient, api.Transformer)).Methods("GET") - router.HandleFunc("/timeseries/{cdid}", TimeseriesLookupHandlerFunc(api.deprecatedESClient)).Methods("GET") - router.HandleFunc("/data", DataLookupHandlerFunc(api.deprecatedESClient)).Methods("GET") createSearchIndexHandler := permissions.Require(update, api.CreateSearchIndexHandlerFunc) router.HandleFunc("/search", createSearchIndexHandler).Methods("POST") return api, nil diff --git a/api/data.go b/api/data.go deleted file mode 100644 index 8a7b0e15..00000000 --- a/api/data.go +++ /dev/null @@ -1,82 +0,0 @@ -package api - -import ( - "bytes" - "encoding/json" - "html/template" - "net/http" - - "github.com/ONSdigital/dp-search-api/query" - "github.com/ONSdigital/log.go/v2/log" -) - -type dataLookupRequest struct { - Uris []string - Types []string -} - -type dataLookupResponse struct { - Responses []interface{} `json:"responses"` -} - -var dataTemplates *template.Template - -// SetupData loads templates for use by the timeseries lookup handler and should be done only once -func SetupData() error { - templates, err := template.ParseFiles("templates/data/queryByUri.tmpl") - dataTemplates = templates - return err -} - -// DataLookupHandlerFunc returns a http handler function handling search api requests. -func DataLookupHandlerFunc(elasticSearchClient ElasticSearcher) http.HandlerFunc { - return func(w http.ResponseWriter, req *http.Request) { - ctx := req.Context() - params := req.URL.Query() - reqParams := dataLookupRequest{ - Uris: params["uris"], - Types: params["types"], - } - var doc bytes.Buffer - - err := dataTemplates.Execute(&doc, reqParams) - if err != nil { - log.Error(ctx, "creation of search from template failed", err, log.Data{"Params": reqParams}) - http.Error(w, "Failed to create query", http.StatusInternalServerError) - return - } - - formattedQuery, err := query.FormatMultiQuery(doc.Bytes()) - if err != nil { - log.Error(ctx, "formating of data query for elasticsearch failed", err) - http.Error(w, "Failed to create query", http.StatusInternalServerError) - return - } - - responseString, err := elasticSearchClient.Search(ctx, "", "", formattedQuery) - if err != nil { - log.Error(ctx, "elasticsearch data query failed", err) - http.Error(w, "Failed to run data query", http.StatusInternalServerError) - return - } - - responseData := dataLookupResponse{Responses: make([]interface{}, 1)} - if unMarshalErr := json.Unmarshal(responseString, &responseData.Responses[0]); unMarshalErr != nil { - log.Error(ctx, "failed to unmarshal response from elasticsearch for data query", unMarshalErr) - http.Error(w, "Failed to process data query", http.StatusInternalServerError) - return - } - - dataWithResponse, err := json.Marshal(responseData) - if err != nil { - log.Error(ctx, "Failed to marshal response data for data query", err) - http.Error(w, "Failed to encode data query response", http.StatusInternalServerError) - return - } - - w.Header().Set("Content-Type", "application/json;charset=utf-8") - if _, err := w.Write(dataWithResponse); err != nil { - log.Error(ctx, "error occured while writing response data", err) - } - } -} diff --git a/api/data_test.go b/api/data_test.go deleted file mode 100644 index 70ef120d..00000000 --- a/api/data_test.go +++ /dev/null @@ -1,132 +0,0 @@ -package api - -import ( - "context" - "html/template" - "net/http" - "net/http/httptest" - "testing" - - "github.com/pkg/errors" - . "github.com/smartystreets/goconvey/convey" -) - -func TestDataLookupHandlerFunc(t *testing.T) { - Convey("Should return InternalError for invalid template", t, func() { - setupDataTestTemplates("dummy{{.Moo}}") - esMock := &ElasticSearcherMock{ - SearchFunc: func(ctx context.Context, index string, docType string, request []byte) ([]byte, error) { - return nil, nil - }, - } - - dataHandler := DataLookupHandlerFunc(esMock) - - req := httptest.NewRequest("GET", "http://localhost:8080/data", nil) - resp := httptest.NewRecorder() - - dataHandler.ServeHTTP(resp, req) - - So(resp.Code, ShouldEqual, http.StatusInternalServerError) - So(resp.Body.String(), ShouldContainSubstring, "Failed to create query") - So(esMock.MultiSearchCalls(), ShouldHaveLength, 0) - }) - - Convey("Should return InternalError for errors returned from elastic search", t, func() { - setupDataTestTemplates("Uris={{.Uris}};Types={{.Types}};") - esMock := &ElasticSearcherMock{ - SearchFunc: func(ctx context.Context, index string, docType string, request []byte) ([]byte, error) { - return nil, errors.New("Something") - }, - } - - dataHandler := DataLookupHandlerFunc(esMock) - - req := httptest.NewRequest("GET", "http://localhost:8080/data?uris=u&types=t", nil) - resp := httptest.NewRecorder() - - dataHandler.ServeHTTP(resp, req) - - So(resp.Code, ShouldEqual, http.StatusInternalServerError) - So(resp.Body.String(), ShouldContainSubstring, "Failed to run data query") - So(esMock.SearchCalls(), ShouldHaveLength, 1) - actualRequest := string(esMock.SearchCalls()[0].Request) - So(actualRequest, ShouldContainSubstring, "Uris=[u]") - So(actualRequest, ShouldContainSubstring, "Types=[t]") - }) - - Convey("Should return InternalError for invalid json from elastic search", t, func() { - setupDataTestTemplates("Uris={{.Uris}};Types={{.Types}};") - esMock := &ElasticSearcherMock{ - SearchFunc: func(ctx context.Context, index string, docType string, request []byte) ([]byte, error) { - return []byte(`{"dummy":"response"`), nil - }, - } - - dataHandler := DataLookupHandlerFunc(esMock) - - req := httptest.NewRequest("GET", "http://localhost:8080/data?uris=u&types=t", nil) - resp := httptest.NewRecorder() - - dataHandler.ServeHTTP(resp, req) - - So(resp.Code, ShouldEqual, http.StatusInternalServerError) - So(resp.Body.String(), ShouldContainSubstring, "Failed to process data query") - So(esMock.SearchCalls(), ShouldHaveLength, 1) - actualRequest := string(esMock.SearchCalls()[0].Request) - So(actualRequest, ShouldContainSubstring, "Uris=[u]") - So(actualRequest, ShouldContainSubstring, "Types=[t]") - }) - - Convey("Should return OK for valid search result", t, func() { - setupDataTestTemplates("Uris={{.Uris}};Types={{.Types}};") - esMock := &ElasticSearcherMock{ - SearchFunc: func(ctx context.Context, index string, docType string, request []byte) ([]byte, error) { - return []byte(`{"dummy":"response"}`), nil - }, - } - - dataHandler := DataLookupHandlerFunc(esMock) - - req := httptest.NewRequest("GET", "http://localhost:8080/data?uris=u&types=t", nil) - resp := httptest.NewRecorder() - - dataHandler.ServeHTTP(resp, req) - - So(resp.Code, ShouldEqual, http.StatusOK) - So(resp.Body.String(), ShouldResemble, `{"responses":[{"dummy":"response"}]}`) - So(esMock.SearchCalls(), ShouldHaveLength, 1) - actualRequest := string(esMock.SearchCalls()[0].Request) - So(actualRequest, ShouldContainSubstring, "Uris=[u]") - So(actualRequest, ShouldContainSubstring, "Types=[t]") - }) - - Convey("Should pass multiple terms to elastic search", t, func() { - setupDataTestTemplates("Uris={{.Uris}};Types={{.Types}};") - esMock := &ElasticSearcherMock{ - SearchFunc: func(ctx context.Context, index string, docType string, request []byte) ([]byte, error) { - return []byte(`{"dummy":"response"}`), nil - }, - } - - dataHandler := DataLookupHandlerFunc(esMock) - - req := httptest.NewRequest("GET", "http://localhost:8080/data?uris=ua&uris=ub&types=t1&types=t2", nil) - resp := httptest.NewRecorder() - - dataHandler.ServeHTTP(resp, req) - - So(resp.Code, ShouldEqual, http.StatusOK) - So(resp.Body.String(), ShouldResemble, `{"responses":[{"dummy":"response"}]}`) - So(esMock.SearchCalls(), ShouldHaveLength, 1) - actualRequest := string(esMock.SearchCalls()[0].Request) - So(actualRequest, ShouldContainSubstring, "Uris=[ua ub]") - So(actualRequest, ShouldContainSubstring, "Types=[t1 t2]") - }) -} - -func setupDataTestTemplates(rawtemplate string) { - temp, err := template.New("queryByUri.tmpl").Parse(rawtemplate) - So(err, ShouldBeNil) - dataTemplates = temp -} diff --git a/api/timeseries.go b/api/timeseries.go deleted file mode 100644 index 34a8d882..00000000 --- a/api/timeseries.go +++ /dev/null @@ -1,62 +0,0 @@ -package api - -import ( - "bytes" - "encoding/json" - "net/http" - "strings" - "text/template" - - "github.com/ONSdigital/log.go/v2/log" - "github.com/gorilla/mux" - "github.com/pkg/errors" -) - -type timeseriesLookupRequest struct { - Cdid string -} - -var timeseriesTemplate *template.Template - -// SetupTimeseries loads templates for use by the timeseries lookup handler and should be done only once -func SetupTimeseries() error { - templates, err := template.ParseFiles("templates/timeseries/lookup.tmpl") - timeseriesTemplate = templates - return err -} - -// TimeseriesLookupHandlerFunc returns a http handler function handling search api requests. -func TimeseriesLookupHandlerFunc(elasticSearchClient ElasticSearcher) http.HandlerFunc { - return func(w http.ResponseWriter, req *http.Request) { - ctx := req.Context() - - vars := mux.Vars(req) - reqParams := timeseriesLookupRequest{Cdid: strings.ToLower(vars["cdid"])} - - var doc bytes.Buffer - err := timeseriesTemplate.Execute(&doc, reqParams) - if err != nil { - log.Error(ctx, "creation of timeseries query from template failed", err, log.Data{"Params": reqParams}) - http.Error(w, "Failed to create query", http.StatusInternalServerError) - return - } - - responseData, err := elasticSearchClient.Search(ctx, "ons", "timeseries", doc.Bytes()) - if err != nil { - log.Error(ctx, "elasticsearch query failed", err) - http.Error(w, "Failed to run timeseries query", http.StatusInternalServerError) - return - } - - if !json.Valid(responseData) { - log.Error(ctx, "elastic search returned invalid JSON for timeseries query", errors.New("elastic search returned invalid JSON for timeseries query")) - http.Error(w, "Failed to process timeseries query", http.StatusInternalServerError) - return - } - - w.Header().Set("Content-Type", "application/json;charset=utf-8") - if _, err := w.Write(responseData); err != nil { - log.Error(ctx, "error occured while writing response data", err) - } - } -} diff --git a/api/timeseries_test.go b/api/timeseries_test.go deleted file mode 100644 index 1c2c73c0..00000000 --- a/api/timeseries_test.go +++ /dev/null @@ -1,110 +0,0 @@ -package api - -import ( - "context" - "net/http" - "net/http/httptest" - "testing" - "text/template" - - "github.com/gorilla/mux" - "github.com/pkg/errors" - . "github.com/smartystreets/goconvey/convey" -) - -func TestTimeseriesLookupHandlerFunc(t *testing.T) { - Convey("Should return InternalError for invalid template", t, func() { - setupTimeseriesTestTemplates("dummy{{.Moo}}") - esMock := &ElasticSearcherMock{ - SearchFunc: func(ctx context.Context, index string, docType string, request []byte) ([]byte, error) { - return nil, nil - }, - } - - timeSeriesHandler := TimeseriesLookupHandlerFunc(esMock) - - req := httptest.NewRequest("GET", "http://localhost:8080/timeseries/a", nil) - resp := httptest.NewRecorder() - - timeSeriesHandler.ServeHTTP(resp, req) - - So(resp.Code, ShouldEqual, http.StatusInternalServerError) - So(resp.Body.String(), ShouldContainSubstring, "Failed to create query") - So(esMock.MultiSearchCalls(), ShouldHaveLength, 0) - }) - - Convey("Should return InternalError for errors returned from elastic search", t, func() { - setupTimeseriesTestTemplates("Cdid={{.Cdid}}") - esMock := &ElasticSearcherMock{ - SearchFunc: func(ctx context.Context, index string, docType string, request []byte) ([]byte, error) { - return nil, errors.New("Something") - }, - } - - timeSeriesHandler := TimeseriesLookupHandlerFunc(esMock) - - req := httptest.NewRequest("GET", "http://localhost:8080/timeseries/a", nil) - req = mux.SetURLVars(req, map[string]string{"cdid": "a"}) - resp := httptest.NewRecorder() - - timeSeriesHandler.ServeHTTP(resp, req) - - So(resp.Code, ShouldEqual, http.StatusInternalServerError) - So(resp.Body.String(), ShouldContainSubstring, "Failed to run timeseries query") - So(esMock.SearchCalls(), ShouldHaveLength, 1) - actualRequest := string(esMock.SearchCalls()[0].Request) - So(actualRequest, ShouldContainSubstring, "Cdid=a") - }) - - Convey("Should return InternalError for invalid json from elastic search", t, func() { - setupTimeseriesTestTemplates("Cdid={{.Cdid}}") - esMock := &ElasticSearcherMock{ - SearchFunc: func(ctx context.Context, index string, docType string, request []byte) ([]byte, error) { - return []byte(`{"dummy":"response"`), nil - }, - } - - timeSeriesHandler := TimeseriesLookupHandlerFunc(esMock) - - req := httptest.NewRequest("GET", "http://localhost:8080/timeseries/a", nil) - req = mux.SetURLVars(req, map[string]string{"cdid": "a"}) - resp := httptest.NewRecorder() - - timeSeriesHandler.ServeHTTP(resp, req) - - So(resp.Code, ShouldEqual, http.StatusInternalServerError) - So(resp.Body.String(), ShouldContainSubstring, "Failed to process timeseries query") - So(esMock.SearchCalls(), ShouldHaveLength, 1) - actualRequest := string(esMock.SearchCalls()[0].Request) - So(actualRequest, ShouldContainSubstring, "Cdid=a") - }) - - Convey("Should return OK for valid search result", t, func() { - setupTimeseriesTestTemplates("Cdid={{.Cdid}}") - esMock := &ElasticSearcherMock{ - SearchFunc: func(ctx context.Context, index string, docType string, request []byte) ([]byte, error) { - return []byte(`{"dummy":"response"}`), nil - }, - } - - timeSeriesHandler := TimeseriesLookupHandlerFunc(esMock) - - req := httptest.NewRequest("GET", "http://localhost:8080/timeseries/a", nil) - req = mux.SetURLVars(req, map[string]string{"cdid": "a"}) - resp := httptest.NewRecorder() - - timeSeriesHandler.ServeHTTP(resp, req) - - So(resp.Code, ShouldEqual, http.StatusOK) - So(resp.Body.String(), ShouldResemble, `{"dummy":"response"}`) - So(esMock.SearchCalls(), ShouldHaveLength, 1) - actualRequest := string(esMock.SearchCalls()[0].Request) - So(actualRequest, ShouldContainSubstring, "Cdid=a") - }) -} - -func setupTimeseriesTestTemplates(rawtemplate string) { - temp, err := template.New("lookup.tmpl").Parse(rawtemplate) - So(err, ShouldBeNil) - timeseriesTemplate = temp -} diff --git a/swagger.yaml b/swagger.yaml index 2f3c16b6..bbb75f8d 100644 --- a/swagger.yaml +++ b/swagger.yaml @@ -13,30 +13,6 @@ tags: - name: "public" - name: "private" paths: - /data: - get: - tags: - - public - summary: "Data query API" - description: "Data query API which returns matching links to department" - parameters: - - in: query - name: uris - type: string - required: true - - in: query - name: types - description: "Type of data returned" - type: string - required: true - responses: - 200: - description: OK - schema: - $ref: "#/definitions/DataResponse" - 500: - description: Internal server error - /departments/search: get: tags: @@ -219,47 +195,8 @@ paths: 500: description: Internal server error - /timeseries: - get: - tags: - - public - summary: "Query for timeseries data" - description: "API to query the app for timeseries by id" - parameters: - - in: query - name: cdid - description: "The timeseries id." - type: string - required: true - responses: - 200: - description: OK - schema: - $ref: "#/definitions/TimeseriesResponse" - 500: - description: Internal server error - definitions: - DataResponse: - type: object - properties: - responses: - type: array - description: "A json list containing data associated with a particular dataset" - items: - type: object - properties: - _shards: - $ref: "#/definitions/ShardsItem" - hits: - $ref: "#/definitions/HitsObject" - timed_out: - type: boolean - took: - description: "Time taken to execute query in milliseconds" - type: integer - GetSearchResponse: type: object properties: @@ -631,86 +568,3 @@ definitions: dp_fasttext: type: string example: available - - ShardsItem: - type: object - properties: - failed: - type: integer - successful: - type: integer - total: - type: integer - - HitsObject: - type: object - properties: - hits: - type: array - items: - $ref: "#/definitions/HitsItem" - max_score: - type: integer - total: - type: integer - - HitsItem: - type: object - properties: - _id: - type: string - _index: - type: string - _score: - type: integer - _source: - type: object - properties: - code: - type: string - name: - type: string - terms: - type: array - items: - type: string - url: - type: string - _type: - type: string - - TimeseriesResponse: - type: object - properties: - took: - type: integer - description: "Time taken to execute query in milliseconds" - example: 530 - timed_out: - type: boolean - _shards: - items: - $ref: "#/definitions/ShardsItem" - hits: - type: array - items: - type: object - properties: - _index: - type: string - _type: - type: string - example: "timeseries" - _id: - type: string - _score: - type: string - _source: - type: object - properties: - uri: - type: string - sort: - type: array - items: - type: integer diff --git a/templates/data/queryByUri.tmpl b/templates/data/queryByUri.tmpl deleted file mode 100644 index 124174e7..00000000 --- a/templates/data/queryByUri.tmpl +++ /dev/null @@ -1,14 +0,0 @@ -{{- if and .Types .Uris }} -{ - "query": { - "ids" : { - "type" : [ {{- range $i,$e := .Types}} - {{if $i}},{{end}}"{{.}}" - {{end}}], - "values" : [{{- range $i,$e := .Uris}} - {{if $i}},{{end}}"{{.}}" - {{end}}] - } - } -} -{{- end}} \ No newline at end of file diff --git a/templates/timeseries/lookup.tmpl b/templates/timeseries/lookup.tmpl deleted file mode 100644 index c55431c6..00000000 --- a/templates/timeseries/lookup.tmpl +++ /dev/null @@ -1,26 +0,0 @@ -{ - "from" : 0, - "size" : 1, - "query" : { - "bool" : { - "filter" : { - "term" : { - "description.cdid" : "{{.Cdid}}" - } - } - } - }, - "_source" : { - "includes" : [ "uri" ], - "excludes" : [ "downloads.content", "downloads*", "pageData" ] - }, - "sort" : [ { - "description.releaseDate" : { - "order" : "desc" - } - }, { - "_score" : { - "order" : "desc" - } - } ] -} \ No newline at end of file From af0ef4aca31ab328a20d3386de858c8ad353ea22 Mon Sep 17 00:00:00 2001 From: bpathak-ons Date: Thu, 21 Apr 2022 13:40:59 +0100 Subject: [PATCH 29/29] peer review updates --- models/elasticsearch.go | 12 ++++++------ models/esmodels.go | 18 +++++++++--------- transformer/transformer.go | 20 ++++++++++---------- transformer/transformer_test.go | 4 ++++ 4 files changed, 29 insertions(+), 25 deletions(-) diff --git a/models/elasticsearch.go b/models/elasticsearch.go index c97dcb40..697679fd 100644 --- a/models/elasticsearch.go +++ b/models/elasticsearch.go @@ -54,12 +54,12 @@ type ESSourceDocument struct { } type ESHighlight struct { - DescriptionTitle *[]string `json:"description.title"` - DescriptionEdition *[]string `json:"description.edition"` - DescriptionSummary *[]string `json:"description.summary"` - DescriptionMeta *[]string `json:"description.metaDescription"` - DescriptionKeywords *[]string `json:"description.keywords"` - DescriptionDatasetID *[]string `json:"description.datasetId"` + DescriptionTitle []*string `json:"description.title"` + DescriptionEdition []*string `json:"description.edition"` + DescriptionSummary []*string `json:"description.summary"` + DescriptionMeta []*string `json:"description.metaDescription"` + DescriptionKeywords []*string `json:"description.keywords"` + DescriptionDatasetID []*string `json:"description.datasetId"` } // ******************************************************** diff --git a/models/esmodels.go b/models/esmodels.go index 38cf157b..d91043bd 100644 --- a/models/esmodels.go +++ b/models/esmodels.go @@ -29,7 +29,7 @@ type DescriptionLegacy struct { Headline2 string `json:"headline2,omitempty"` Headline3 string `json:"headline3,omitempty"` Highlight *HighlightObjLegacy `json:"highlight,omitempty"` - Keywords *[]string `json:"keywords,omitempty"` + Keywords []*string `json:"keywords,omitempty"` LatestRelease *bool `json:"latest_release,omitempty"` Language string `json:"language,omitempty"` MetaDescription string `json:"meta_description,omitempty"` @@ -52,7 +52,7 @@ type contactLegacy struct { type HighlightObjLegacy struct { DatasetID string `json:"dataset_id,omitempty"` Edition string `json:"edition,omitempty"` - Keywords *[]string `json:"keywords,omitempty"` + Keywords []*string `json:"keywords,omitempty"` MetaDescription string `json:"meta_description,omitempty"` Summary string `json:"summary,omitempty"` Title string `json:"title,omitempty"` @@ -86,7 +86,7 @@ type ESSourceDocumentLegacy struct { Summary string `json:"summary"` NextRelease string `json:"nextRelease,omitempty"` Unit string `json:"unit,omitempty"` - Keywords *[]string `json:"keywords,omitempty"` + Keywords []*string `json:"keywords,omitempty"` ReleaseDate string `json:"releaseDate,omitempty"` Edition string `json:"edition,omitempty"` LatestRelease *bool `json:"latestRelease,omitempty"` @@ -107,12 +107,12 @@ type ESSourceDocumentLegacy struct { } type ESHighlightLegacy struct { - DescriptionTitle *[]string `json:"description.title"` - DescriptionEdition *[]string `json:"description.edition"` - DescriptionSummary *[]string `json:"description.summary"` - DescriptionMeta *[]string `json:"description.metaDescription"` - DescriptionKeywords *[]string `json:"description.keywords"` - DescriptionDatasetID *[]string `json:"description.datasetId"` + DescriptionTitle []*string `json:"description.title"` + DescriptionEdition []*string `json:"description.edition"` + DescriptionSummary []*string `json:"description.summary"` + DescriptionMeta []*string `json:"description.metaDescription"` + DescriptionKeywords []*string `json:"description.keywords"` + DescriptionDatasetID []*string `json:"description.datasetId"` } type ESResponseAggregationsLegacy struct { diff --git a/transformer/transformer.go b/transformer/transformer.go index 184974cd..a817fd80 100644 --- a/transformer/transformer.go +++ b/transformer/transformer.go @@ -138,32 +138,32 @@ func (t *LegacyTransformer) buildDescription(doc models.ESResponseHitLegacy, hig return des } -func (t *LegacyTransformer) overlaySingleItem(hl *[]string, def string, highlight bool) (overlaid string) { - if highlight && hl != nil && len(*hl) > 0 { - overlaid = (*hl)[0] +func (t *LegacyTransformer) overlaySingleItem(hl []*string, def string, highlight bool) (overlaid string) { + if highlight && hl != nil && len(hl) > 0 { + overlaid = *(hl)[0] } return } -func (t *LegacyTransformer) overlayItemList(hlList, defaultList *[]string, highlight bool) *[]string { +func (t *LegacyTransformer) overlayItemList(hlList, defaultList []*string, highlight bool) []*string { if defaultList == nil || hlList == nil { return nil } - overlaid := make([]string, len(*defaultList)) - copy(overlaid, *defaultList) + overlaid := make([]*string, len(defaultList)) + copy(overlaid, defaultList) if highlight { - for _, hl := range *hlList { - unformatted := t.higlightReplacer.Replace(hl) + for _, hl := range hlList { + unformatted := t.higlightReplacer.Replace(*hl) for i, defItem := range overlaid { - if defItem == unformatted { + if *defItem == unformatted { overlaid[i] = hl } } } } - return &overlaid + return overlaid } func buildContentTypes(bucket models.ESBucketLegacy) models.ContentTypeLegacy { diff --git a/transformer/transformer_test.go b/transformer/transformer_test.go index 04043245..d5b7e391 100644 --- a/transformer/transformer_test.go +++ b/transformer/transformer_test.go @@ -11,6 +11,7 @@ import ( ) func TestLegacyTransformer(t *testing.T) { + t.Parallel() Convey("Transforms unmarshalled search responses successfully", t, func() { transformer := NewLegacy() Convey("Zero suggestions creates empty array", func() { @@ -79,6 +80,7 @@ func TestLegacyTransformer(t *testing.T) { } func TestLegacyBuildAdditionalSuggestionsList(t *testing.T) { + t.Parallel() Convey("buildAdditionalSuggestionList successfully", t, func() { Convey("returns array of strings", func() { query1 := buildAdditionalSuggestionList("test-query") @@ -100,6 +102,7 @@ func TestLegacyBuildAdditionalSuggestionsList(t *testing.T) { } func TestLegacyTransformSearchResponse(t *testing.T) { + t.Parallel() Convey("With a transformer initialised", t, func() { ctx := context.Background() transformer := NewLegacy() @@ -167,6 +170,7 @@ func TestLegacyTransformSearchResponse(t *testing.T) { } func TestTransform(t *testing.T) { + t.Parallel() expectedESDocument1 := models.ESSourceDocument{ DataType: "anyDataType1", JobID: "",