diff --git a/cache/constants.go b/cache/constants.go new file mode 100644 index 0000000..8f6126d --- /dev/null +++ b/cache/constants.go @@ -0,0 +1,6 @@ +package cache + +const ( + // TopicCacheKey is used to cache the topics + TopicCacheKey = "root-topic-cache" +) diff --git a/cache/mock.go b/cache/mock.go index 2b196aa..53a14bc 100644 --- a/cache/mock.go +++ b/cache/mock.go @@ -26,29 +26,27 @@ func getMockTopicCache(ctx context.Context) (*TopicCache, error) { return nil, err } - testTopicCache.Cache.Set("economy", GetMockTopic("6734", "economy", "Economy", "")) - testTopicCache.Cache.Set("environmentalaccounts", GetMockTopic("1834", "environmentalaccounts", "Environmental Accounts", "6734")) - testTopicCache.Cache.Set("governmentpublicsectorandtaxes", GetMockTopic("8268", "governmentpublicsectorandtaxes", "Government Public Sector and Taxes", "6734")) - testTopicCache.Cache.Set("publicsectorfinance", GetMockTopic("3687", "publicsectorfinance", "Public Sector Finance", "8268")) + rootTopicID := testTopicCache.GetTopicCacheKey() + testTopicCache.Set(rootTopicID, GetMockRootTopic(rootTopicID)) return testTopicCache, nil } -// GetMockTopic returns a mocked topic which contains all the information for the mock data topic -func GetMockTopic(id, slug, title, parentID string) *Topic { - mockTopic := &Topic{ - ID: id, - Slug: slug, - LocaliseKeyName: title, - ParentID: parentID, - Query: "1834,1234", +// GetMockRootTopic returns the mocked root topic +func GetMockRootTopic(rootTopicID string) *Topic { + mockRootTopic := &Topic{ + ID: rootTopicID, + Slug: "root", } - mockTopic.List = NewSubTopicsMap() - mockTopic.List.AppendSubtopicID("1234", Subtopic{ID: "1234", LocaliseKeyName: "International Migration", ReleaseDate: timeHelper("2022-10-10T08:30:00Z")}) - mockTopic.List.AppendSubtopicID("1834", Subtopic{ID: "1834", LocaliseKeyName: "Environmental Accounts", ReleaseDate: timeHelper("2022-11-09T09:30:00Z")}) + mockRootTopic.List = NewSubTopicsMap() + mockRootTopic.List.AppendSubtopicID("economy", Subtopic{ID: "6734", Slug: "economy", LocaliseKeyName: "Economy", ReleaseDate: timeHelper("2022-10-10T08:30:00Z"), ParentID: ""}) + mockRootTopic.List.AppendSubtopicID("environmentalaccounts", Subtopic{ID: "1834", Slug: "environmentalaccounts", LocaliseKeyName: "Environmental Accounts", ReleaseDate: timeHelper("2022-10-10T08:30:00Z"), ParentID: "6734"}) + mockRootTopic.List.AppendSubtopicID("governmentpublicsectorandtaxes", Subtopic{ID: "8268", Slug: "governmentpublicsectorandtaxes", LocaliseKeyName: "Government Public Sector and Taxes", ReleaseDate: timeHelper("2022-10-10T08:30:00Z"), ParentID: "6734"}) + mockRootTopic.List.AppendSubtopicID("publicsectorfinance", Subtopic{ID: "3687", Slug: "publicsectorfinance", LocaliseKeyName: "Public Sector Finance", ReleaseDate: timeHelper("2022-10-10T08:30:00Z"), ParentID: "8268"}) + mockRootTopic.List.AppendSubtopicID("internationalmigration", Subtopic{ID: "1234", Slug: "internationalmigration", LocaliseKeyName: "International Migration", ReleaseDate: timeHelper("2022-10-10T08:30:00Z")}) - return mockTopic + return mockRootTopic } // timeHelper is a helper function given a time returns a Time pointer diff --git a/cache/mock_test.go b/cache/mock_test.go index 1c9e90a..bc70a4e 100644 --- a/cache/mock_test.go +++ b/cache/mock_test.go @@ -23,30 +23,30 @@ func TestGetMockCacheList(t *testing.T) { So(cacheList.Topic, ShouldNotBeNil) - topic, err := cacheList.Topic.GetData(ctx, topicSlug) + topic, err := cacheList.Topic.GetTopic(ctx, topicSlug) So(topic, ShouldNotBeNil) So(err, ShouldBeNil) }) }) } -func TestGetMockDataTopic(t *testing.T) { +func TestGetMockRootTopic(t *testing.T) { t.Parallel() - id := "6734" - slug := topicSlug - title := "Economy" - parentID := "" + rootTopicID := "root-topic-cache" + slug := "root" - Convey("When GetMockTopic is called", t, func() { - mockTopic := GetMockTopic(id, slug, title, parentID) + Convey("When GetMockRootTopic is called", t, func() { + mockTopic := GetMockRootTopic(rootTopicID) - Convey("Then the mock topic is returned", func() { + Convey("Then the mocked root topic is returned", func() { So(mockTopic, ShouldNotBeNil) - So(mockTopic.ID, ShouldEqual, id) + So(mockTopic.ID, ShouldEqual, rootTopicID) So(mockTopic.Slug, ShouldEqual, slug) - So(mockTopic.LocaliseKeyName, ShouldEqual, title) - So(mockTopic.ParentID, ShouldNotBeNil) + + subtopic, exists := mockTopic.List.Get("economy") + So(exists, ShouldBeTrue) + So(subtopic, ShouldResemble, Subtopic{ID: "6734", Slug: "economy", LocaliseKeyName: "Economy", ReleaseDate: timeHelper("2022-10-10T08:30:00Z"), ParentID: ""}) }) }) } diff --git a/cache/private/private_topic.go b/cache/private/private_topic.go index 85863c6..27425eb 100644 --- a/cache/private/private_topic.go +++ b/cache/private/private_topic.go @@ -10,14 +10,13 @@ import ( "github.com/ONSdigital/log.go/v2/log" ) -// UpdateTopics is a function to update the topic cache in publishing (private) mode. +// UpdateTopicCache is a function to update the topic cache in publishing (private) mode. // This function talks to the dp-topic-api via its private endpoints to retrieve the root topic and its subtopic ids // The data returned by the dp-topic-api is of type *models.PrivateSubtopics which is then transformed in this function for the controller // If an error has occurred, this is captured in log.Error and then an empty topic is returned -func UpdateTopics(ctx context.Context, serviceAuthToken string, topicClient topicCli.Clienter) func() []*cache.Topic { - return func() []*cache.Topic { - var topics []*cache.Topic - processedTopics := make(map[string]bool) +func UpdateTopicCache(ctx context.Context, serviceAuthToken string, topicClient topicCli.Clienter) func() *cache.Topic { + return func() *cache.Topic { + processedTopics := make(map[string]struct{}) // get root topic from dp-topic-api rootTopic, err := topicClient.GetRootTopicsPrivate(ctx, topicCli.Headers{ServiceAuthToken: "Bearer " + serviceAuthToken}) @@ -26,73 +25,92 @@ func UpdateTopics(ctx context.Context, serviceAuthToken string, topicClient topi "req_headers": topicCli.Headers{}, } log.Error(ctx, "failed to get root topic from topic-api", err, logData) - return []*cache.Topic{cache.GetEmptyTopic()} + return cache.GetEmptyTopic() } - // dereference rootTopic's subtopics(items) to allow ranging through them + // dereference root topics items to allow ranging through them var rootTopicItems []models.TopicResponse if rootTopic.PrivateItems != nil { rootTopicItems = *rootTopic.PrivateItems } else { - err := errors.New("root topic subtopics(items) is nil") - log.Error(ctx, "failed to dereference rootTopic subtopics pointer", err) - return []*cache.Topic{cache.GetEmptyTopic()} + err := errors.New("root topic private items is nil") + log.Error(ctx, "failed to dereference root topic items pointer", err) + return cache.GetEmptyTopic() + } + + // Initialize topicCache + topicCache := &cache.Topic{ + ID: cache.TopicCacheKey, + LocaliseKeyName: "Root", + List: cache.NewSubTopicsMap(), } // recursively process topics and their subtopics for i := range rootTopicItems { - processTopic(ctx, serviceAuthToken, topicClient, rootTopicItems[i].ID, &topics, processedTopics, "") + processTopic(ctx, serviceAuthToken, topicClient, rootTopicItems[i].ID, topicCache, processedTopics, "", 0) } // Check if any topics were found - if len(topics) == 0 { + if len(topicCache.List.GetSubtopics()) == 0 { err := errors.New("root topic found, but no subtopics were returned") log.Error(ctx, "No topics loaded into cache - root topic found, but no subtopics were returned", err) - return []*cache.Topic{cache.GetEmptyTopic()} + return cache.GetEmptyTopic() } - return topics + return topicCache } } -func processTopic(ctx context.Context, serviceAuthToken string, topicClient topicCli.Clienter, topicID string, topics *[]*cache.Topic, processedTopics map[string]bool, parentTopicID string) { - // Check if the topic is already processed - if processedTopics[topicID] { +func processTopic(ctx context.Context, serviceAuthToken string, topicClient topicCli.Clienter, topicID string, topicCache *cache.Topic, processedTopics map[string]struct{}, parentTopicID string, depth int) { + log.Info(ctx, "Processing topic at depth", log.Data{ + "topic_id": topicID, + "depth": depth, + }) + + // Check if the topic has already been processed + if _, exists := processedTopics[topicID]; exists { + err := errors.New("topic already processed") + log.Error(ctx, "Skipping already processed topic", err, log.Data{ + "topic_id": topicID, + "depth": depth, + }) return } // Get the topic details from the topic client topic, err := topicClient.GetTopicPrivate(ctx, topicCli.Headers{ServiceAuthToken: "Bearer " + serviceAuthToken}, topicID) if err != nil { - logData := log.Data{ - topicID: topicID, - } - log.Error(ctx, "failed to get topic details from topic-api", err, logData) + log.Error(ctx, "failed to get topic details from topic-api", err, log.Data{ + "topic_id": topicID, + "depth": depth, + }) return } if topic != nil { - // Append the current topic to the list of topics - *topics = append(*topics, mapTopicModelToCache(*topic.Current, parentTopicID)) + // Initialize subtopic list for the current topic if it doesn't exist + subtopic := mapTopicModelToCache(*topic.Current, parentTopicID) + + // Add the current topic to the topicCache's List + topicCache.List.AppendSubtopicID(topic.Current.Slug, subtopic) + // Mark this topic as processed - processedTopics[topicID] = true + processedTopics[topicID] = struct{}{} // Process each subtopic recursively if topic.Current.SubtopicIds != nil { for _, subTopicID := range *topic.Current.SubtopicIds { - processTopic(ctx, serviceAuthToken, topicClient, subTopicID, topics, processedTopics, topicID) + processTopic(ctx, serviceAuthToken, topicClient, subTopicID, topicCache, processedTopics, topicID, depth+1) } } } } -func mapTopicModelToCache(topic models.Topic, parentID string) *cache.Topic { - rootTopicCache := &cache.Topic{ +func mapTopicModelToCache(topic models.Topic, parentID string) cache.Subtopic { + return cache.Subtopic{ ID: topic.ID, Slug: topic.Slug, LocaliseKeyName: topic.Title, - ParentID: parentID, ReleaseDate: topic.ReleaseDate, - List: cache.NewSubTopicsMap(), + ParentID: parentID, } - return rootTopicCache } diff --git a/cache/private/private_topic_test.go b/cache/private/private_topic_test.go index 0b4d15f..9064f7e 100644 --- a/cache/private/private_topic_test.go +++ b/cache/private/private_topic_test.go @@ -14,43 +14,68 @@ import ( ) var ( - testEconomyRootSubTopicPrivate = models.TopicResponse{ + testEconomyRootTopic = models.Topic{ + ID: "6734", + Title: "Economy", + SubtopicIds: &[]string{"1834"}, + } + + testEconomyRootTopic2 = models.Topic{ + ID: "6734", + Title: "Economy", + SubtopicIds: &[]string{"1834", "1234"}, + } + + testBusinessRootTopic = models.Topic{ + ID: "1234", + Title: "Business", + SubtopicIds: &[]string{}, + } + + testBusinessRootTopic2 = models.Topic{ + ID: "1234", + Title: "Business", + SubtopicIds: &[]string{"6734"}, + } + + testEconomyRootTopicPrivate = models.TopicResponse{ ID: "6734", Next: &testEconomyRootTopic, Current: &testEconomyRootTopic, } - testBusinessRootSubTopicPrivate = models.TopicResponse{ + testEconomyRootTopicPrivate2 = models.TopicResponse{ + ID: "6734", + Next: &testEconomyRootTopic2, + Current: &testEconomyRootTopic2, + } + + testBusinessRootTopicPrivate = models.TopicResponse{ ID: "1234", Next: &testBusinessRootTopic, Current: &testBusinessRootTopic, } - testEconomyRootTopic = models.Topic{ - ID: "6734", - Title: "Economy", - SubtopicIds: &[]string{"1834"}, + testBusinessRootTopicPrivate2 = models.TopicResponse{ + ID: "1234", + Next: &testBusinessRootTopic2, + Current: &testBusinessRootTopic2, } - testBusinessRootTopic = models.Topic{ - ID: "1234", - Title: "Business", - SubtopicIds: &[]string{}, + expectedTopicCache = &cache.Topic{ + ID: cache.TopicCacheKey, + LocaliseKeyName: "Root", + Query: "6734, 1234", } ) -func TestUpdateTopics(t *testing.T) { +func TestUpdateTopicCache(t *testing.T) { ctx := context.Background() serviceAuthToken := "test-token" - expectedTopics := []*cache.Topic{ - {ID: "6734", LocaliseKeyName: "Economy", Query: "6734,1834"}, - {ID: "1834", LocaliseKeyName: "Environmental Accounts"}, - {ID: "1234", LocaliseKeyName: "Business"}, - } - emptyTopic := []*cache.Topic{cache.GetEmptyTopic()} + emptyTopic := cache.GetEmptyTopic() - Convey("Given root topics exist in the topic API and have subtopics", t, func() { + Convey("Given root topic exist and have subtopics with no duplicate topics", t, func() { mockClient := &mockTopic.ClienterMock{ GetRootTopicsPrivateFunc: func(ctx context.Context, reqHeaders sdk.Headers) (*models.PrivateSubtopics, topicCliErr.Error) { return &models.PrivateSubtopics{ @@ -58,32 +83,38 @@ func TestUpdateTopics(t *testing.T) { Offset: 0, Limit: 50, TotalCount: 2, - PrivateItems: &[]models.TopicResponse{testEconomyRootSubTopicPrivate, testBusinessRootSubTopicPrivate}, + PrivateItems: &[]models.TopicResponse{testEconomyRootTopicPrivate, testBusinessRootTopicPrivate}, }, nil }, GetTopicPrivateFunc: func(ctx context.Context, reqHeaders sdk.Headers, topicID string) (*models.TopicResponse, topicCliErr.Error) { switch topicID { case "6734": return &models.TopicResponse{ + ID: topicID, Current: &models.Topic{ ID: "6734", Title: "Economy", + Slug: "economy", SubtopicIds: &[]string{"1834"}, }, }, nil case "1834": return &models.TopicResponse{ + ID: topicID, Current: &models.Topic{ ID: "1834", Title: "Environmental Accounts", + Slug: "environmentalaccounts", SubtopicIds: &[]string{}, }, }, nil case "1234": return &models.TopicResponse{ + ID: topicID, Current: &models.Topic{ ID: "1234", Title: "Business", + Slug: "business", SubtopicIds: &[]string{}, }, }, nil @@ -95,16 +126,63 @@ func TestUpdateTopics(t *testing.T) { }, } - Convey("When UpdateTopicsPrivate is called", func() { - respTopics := UpdateTopics(ctx, serviceAuthToken, mockClient)() + Convey("When UpdateTopicCache (Private) is called", func() { + respTopic := UpdateTopicCache(ctx, serviceAuthToken, mockClient)() Convey("Then the topics cache is returned", func() { - So(respTopics, ShouldNotBeNil) - So(len(respTopics), ShouldEqual, len(expectedTopics)) - for i, expected := range expectedTopics { - So(respTopics[i].ID, ShouldEqual, expected.ID) - So(respTopics[i].LocaliseKeyName, ShouldEqual, expected.LocaliseKeyName) - } + So(respTopic, ShouldNotBeNil) + So(respTopic.ID, ShouldEqual, expectedTopicCache.ID) + So(respTopic.LocaliseKeyName, ShouldEqual, expectedTopicCache.LocaliseKeyName) + + So(len(respTopic.List.GetSubtopics()), ShouldEqual, 3) + }) + }) + + Convey("And given root topics exist and have subtopics with duplicate topics", func() { + mockClient.GetRootTopicsPrivateFunc = func(ctx context.Context, reqHeaders sdk.Headers) (*models.PrivateSubtopics, topicCliErr.Error) { + return &models.PrivateSubtopics{ + Count: 2, + Offset: 0, + Limit: 50, + TotalCount: 2, + PrivateItems: &[]models.TopicResponse{testEconomyRootTopicPrivate, testBusinessRootTopicPrivate, testEconomyRootTopicPrivate, testBusinessRootTopicPrivate}, + }, nil + } + + Convey("When UpdateTopicCache (Private) is called", func() { + respTopic := UpdateTopicCache(ctx, serviceAuthToken, mockClient)() + + Convey("Then the topics cache is returned with expected number of topics excluding duplicates", func() { + So(respTopic, ShouldNotBeNil) + So(respTopic.ID, ShouldEqual, expectedTopicCache.ID) + So(respTopic.LocaliseKeyName, ShouldEqual, expectedTopicCache.LocaliseKeyName) + + So(len(respTopic.List.GetSubtopics()), ShouldEqual, 3) + }) + }) + }) + + Convey("And given root topics exist and private items that have subtopics that can end up in a recursive loop", func() { + mockClient.GetRootTopicsPrivateFunc = func(ctx context.Context, reqHeaders sdk.Headers) (*models.PrivateSubtopics, topicCliErr.Error) { + return &models.PrivateSubtopics{ + Count: 2, + Offset: 0, + Limit: 50, + TotalCount: 2, + PrivateItems: &[]models.TopicResponse{testEconomyRootTopicPrivate, testEconomyRootTopicPrivate2, testBusinessRootTopicPrivate2}, + }, nil + } + + Convey("When UpdateTopicCache (Private) is called", func() { + respTopic := UpdateTopicCache(ctx, serviceAuthToken, mockClient)() + + Convey("Then the topics cache is returned as expected with no duplicates and does not get stuck in a loop", func() { + So(respTopic, ShouldNotBeNil) + So(respTopic.ID, ShouldEqual, expectedTopicCache.ID) + So(respTopic.LocaliseKeyName, ShouldEqual, expectedTopicCache.LocaliseKeyName) + + So(len(respTopic.List.GetSubtopics()), ShouldEqual, 3) + }) }) }) }) @@ -118,11 +196,11 @@ func TestUpdateTopics(t *testing.T) { }, } - Convey("When UpdateTopicsPrivate is called", func() { - respTopics := UpdateTopics(ctx, serviceAuthToken, mockClient)() + Convey("When UpdateTopicCache (Private) is called", func() { + respTopic := UpdateTopicCache(ctx, serviceAuthToken, mockClient)() Convey("Then an empty topic cache should be returned", func() { - So(respTopics, ShouldResemble, emptyTopic) + So(respTopic, ShouldResemble, emptyTopic) }) }) }) @@ -138,13 +216,21 @@ func TestUpdateTopics(t *testing.T) { PrivateItems: nil, }, nil }, + GetTopicPrivateFunc: func(ctx context.Context, reqHeaders sdk.Headers, topicID string) (*models.TopicResponse, topicCliErr.Error) { + return &models.TopicResponse{ + Current: &models.Topic{ + ID: topicID, + SubtopicIds: nil, + }, + }, nil + }, } - Convey("When UpdateTopicsPrivate is called", func() { - respTopics := UpdateTopics(ctx, serviceAuthToken, mockClient)() + Convey("When UpdateTopicCache (Private) is called", func() { + respTopic := UpdateTopicCache(ctx, serviceAuthToken, mockClient)() Convey("Then an empty topic cache should be returned", func() { - So(respTopics, ShouldResemble, emptyTopic) + So(respTopic, ShouldResemble, emptyTopic) }) }) }) @@ -162,8 +248,8 @@ func TestUpdateTopics(t *testing.T) { }, } - Convey("When UpdateTopicsPrivate is called", func() { - respTopics := UpdateTopics(ctx, serviceAuthToken, mockClient)() + Convey("When UpdateTopicCache (Private) is called", func() { + respTopics := UpdateTopicCache(ctx, serviceAuthToken, mockClient)() Convey("Then an empty topic cache should be returned", func() { So(respTopics, ShouldResemble, emptyTopic) diff --git a/cache/subtopic.go b/cache/subtopic.go index c46328b..c31116e 100644 --- a/cache/subtopic.go +++ b/cache/subtopic.go @@ -20,6 +20,8 @@ type Subtopic struct { LocaliseKeyName string Slug string ReleaseDate *time.Time + // This is a reference to the parent topic + ParentID string } // GetNewSubTopicsMap creates a new subtopics id map to store subtopic ids with addition to mutex locking @@ -31,11 +33,12 @@ func NewSubTopicsMap() *Subtopics { } // Get returns subtopic for given key (id) -func (sts *Subtopics) Get(key string) Subtopic { +func (sts *Subtopics) Get(key string) (Subtopic, bool) { sts.mutex.RLock() defer sts.mutex.RUnlock() - return sts.subtopicsMap[key] + subtopic, exists := sts.subtopicsMap[key] + return subtopic, exists } // GetSubtopics returns an array of subtopics diff --git a/cache/subtopic_test.go b/cache/subtopic_test.go index 4638e08..0b036c1 100644 --- a/cache/subtopic_test.go +++ b/cache/subtopic_test.go @@ -3,6 +3,7 @@ package cache import ( "sync" "testing" + "time" . "github.com/smartystreets/goconvey/convey" ) @@ -16,6 +17,10 @@ var ( LocaliseKeyName: "Age", ReleaseDate: timeHelper("2022-11-09T09:30:00Z"), } + subtopic3 = Subtopic{ + LocaliseKeyName: "Health", + ReleaseDate: timeHelper("2023-01-07T09:30:00Z"), + } ) func TestAppendSubtopicID(t *testing.T) { @@ -28,7 +33,9 @@ func TestAppendSubtopicID(t *testing.T) { subtopicIDsStore.AppendSubtopicID("1234", subtopic1) Convey("Then the new subtopic id should be added to the map", func() { - So(subtopicIDsStore.Get("1234"), ShouldResemble, subtopic1) + subtopic, exists := subtopicIDsStore.Get("1234") + So(exists, ShouldBeTrue) + So(subtopic, ShouldResemble, subtopic1) }) }) }) @@ -41,7 +48,9 @@ func TestAppendSubtopicID(t *testing.T) { subtopicIDsStore.AppendSubtopicID("1234", subtopic1) Convey("Then the new subtopic id should be added to the map", func() { - So(subtopicIDsStore.Get("1234"), ShouldResemble, subtopic1) + subtopic, exists := subtopicIDsStore.Get("1234") + So(exists, ShouldBeTrue) + So(subtopic, ShouldResemble, subtopic1) }) }) }) @@ -56,11 +65,37 @@ func TestAppendSubtopicID(t *testing.T) { subtopicIDsStore.AppendSubtopicID("5678", subtopic2) Convey("Then the new subtopic id should be added to the map", func() { - So(subtopicIDsStore.Get("5678"), ShouldResemble, subtopic2) + subtopic, exists := subtopicIDsStore.Get("5678") + So(exists, ShouldBeTrue) + So(subtopic, ShouldResemble, subtopic2) }) Convey("And the existing subtopic `1234` should still exist in map", func() { - So(subtopicIDsStore.Get("1234"), ShouldResemble, subtopic1) + subtopic, exists := subtopicIDsStore.Get("1234") + So(exists, ShouldBeTrue) + So(subtopic, ShouldResemble, subtopic1) + }) + }) + }) + + Convey("Given AppendSubtopicID is called synchronously", t, func() { + subtopicIDsStore := NewSubTopicsMap() + + Convey("When AppendSubtopicID is called", func() { + go subtopicIDsStore.AppendSubtopicID("5678", subtopic2) + go subtopicIDsStore.AppendSubtopicID("9123", subtopic3) + + Convey("Then the new subtopic ids should be added", func() { + // Wait for the goroutines to finish + time.Sleep(300 * time.Millisecond) + + subtopic, exists := subtopicIDsStore.Get("5678") + So(exists, ShouldBeTrue) + So(subtopic, ShouldResemble, subtopic2) + + subtopic, exists = subtopicIDsStore.Get("9123") + So(exists, ShouldBeTrue) + So(subtopic, ShouldResemble, subtopic3) }) }) }) diff --git a/cache/topic.go b/cache/topic.go index 492bbeb..8431c21 100644 --- a/cache/topic.go +++ b/cache/topic.go @@ -25,8 +25,6 @@ type Topic struct { Query string // List is a map[string]Subtopics which contains the topic id and a list of it's subtopics List *Subtopics - // This is a reference to the parent topic - ParentID string } // NewTopicCache create a topic cache object to be used in the service which will update at every updateInterval @@ -84,26 +82,49 @@ func (tc *TopicCache) AddUpdateFunc(title string, updateFunc func() *Topic) { } } -// AddUpdateFuncs adds an update function to the topic cache for a topic with the `title` passed to the function -// This update function will then be triggered once or at every fixed interval as per the prior setup of the TopicCache -func (tc *TopicCache) AddUpdateFuncs(updateFunc func() []*Topic) { - // Invoke the updateFunc to get the slice of *Topic - topics := updateFunc() - - // Iterate over each topic in the returned slice - for _, topic := range topics { - // Define an update function for the current topic - // This update function simply returns the current topic as-is - singleUpdateFunc := func() *Topic { - return topic +// GetTopicCacheKey gets the constant value set for the root topic cache key +func (tc *TopicCache) GetTopicCacheKey() string { + return TopicCacheKey +} + +func (tc *TopicCache) GetTopic(ctx context.Context, slug string) (*Subtopic, error) { + topicCache, err := tc.GetData(ctx, TopicCacheKey) + if err != nil { + logData := log.Data{ + "key": TopicCacheKey, } + log.Error(ctx, "failed to get the topic cache", err, logData) + return nil, err + } - // Add the update function to the TopicCache for the current topic's title - tc.AddUpdateFunc(topic.Slug, singleUpdateFunc) + // Retrieve the subtopic from the list + topicCacheItem, exists := topicCache.List.Get(slug) + if !exists { + err := errors.New("requested topic does not exist in cache") + log.Error(ctx, "failed to get topic from cache", err, log.Data{ + "slug": slug, + }) + return nil, err + } + + return &topicCacheItem, nil +} + +// GetTopicFromSubtopic returns an empty topic cache in the event when updating the cache of the topic fails +func (tc *TopicCache) GetTopicFromSubtopic(subtopic *Subtopic) *Topic { + if subtopic == nil { + return nil + } + + return &Topic{ + ID: subtopic.ID, + Slug: subtopic.Slug, + LocaliseKeyName: subtopic.LocaliseKeyName, + List: NewSubTopicsMap(), } } -// GetEmptyTopic returns an empty topic cache in the event when updating the cache of the topic fails +// GetEmptyTopic returns an empty topic cache e.g in an event where updating the cache of the topic fails func GetEmptyTopic() *Topic { return &Topic{ List: NewSubTopicsMap(), diff --git a/cache/topic_test.go b/cache/topic_test.go index 0bd341d..06a3fc4 100644 --- a/cache/topic_test.go +++ b/cache/topic_test.go @@ -59,7 +59,7 @@ func TestNewTopicCache(t *testing.T) { }) } -func TestGetData(t *testing.T) { +func TestGetTopic(t *testing.T) { t.Parallel() ctx := context.Background() @@ -72,7 +72,7 @@ func TestGetData(t *testing.T) { slug := "economy" Convey("When GetData is called", func() { - testCacheData, err := mockCacheList.Topic.GetData(ctx, slug) + testCacheData, err := mockCacheList.Topic.GetTopic(ctx, slug) Convey("Then a topic cache data should be successfully returned", func() { So(testCacheData, ShouldNotBeNil) diff --git a/service/service.go b/service/service.go index 51a010c..1269b29 100644 --- a/service/service.go +++ b/service/service.go @@ -71,7 +71,7 @@ func (svc *Service) Init(ctx context.Context, cfg *config.Config, buildTime, git } // Load cache with topics on startup - svc.Cache.Topic.AddUpdateFuncs(cachePrivate.UpdateTopics(ctx, svc.Cfg.ServiceAuthToken, svc.TopicCli)) + svc.Cache.Topic.AddUpdateFunc(svc.Cache.Topic.GetTopicCacheKey(), cachePrivate.UpdateTopicCache(ctx, svc.Cfg.ServiceAuthToken, svc.TopicCli)) } err = svc.Consumer.RegisterHandler(ctx, h.Handle) @@ -110,6 +110,11 @@ func (svc *Service) Start(ctx context.Context, svcErrors chan error) error { } } + // Start cache updates + if svc.Cfg.EnableTopicTagging { + go svc.Cache.Topic.StartUpdates(ctx, svcErrors) + } + // Always start healthcheck. // If start/stop on health updates is enabled, // the consumer will start consuming on the first healthy update @@ -142,6 +147,11 @@ func (svc *Service) Close(ctx context.Context) error { log.Info(ctx, "stopped health checker") } + // stop cache updates + if svc.Cfg.EnableTopicTagging { + svc.Cache.Topic.Close() + } + // If kafka consumer exists, stop listening to it. // This will automatically stop the event consumer loops and no more messages will be processed. // The kafka consumer will be closed after the service shuts down.