Skip to content

Commit

Permalink
fix(api): actions with lang not cached properly (#730)
Browse files Browse the repository at this point in the history
* fix(api): actions with lang not cached properly

* fix test
  • Loading branch information
pyshx authored Dec 24, 2024
1 parent 61f8eae commit 7655adb
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 90 deletions.
89 changes: 53 additions & 36 deletions api/internal/app/actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ type SegregatedActions struct {

var (
actionsData ActionsData
once sync.Once
actionsDataMap = make(map[string]ActionsData)
mutex sync.RWMutex
supportedLangs = map[string]bool{
"en": true,
"es": true,
Expand All @@ -95,46 +96,62 @@ func loadActionsData(lang string) error {
return fmt.Errorf("unsupported language: %s", lang)
}

var err error
once.Do(func() {
baseURL := "https://raw.githubusercontent.com/reearth/reearth-flow/main/engine/schema/"
filename := "actions.json"
if lang != "" {
filename = fmt.Sprintf("actions_%s.json", lang)
}
cacheKey := lang

resp, respErr := http.Get(baseURL + filename)
if respErr != nil {
err = respErr
return
}
defer func() {
if closeErr := resp.Body.Close(); closeErr != nil {
if err == nil {
err = closeErr
} else {
err = fmt.Errorf("%w; %v", err, closeErr)
}
}
}()
// Try to get from cache first using read lock
mutex.RLock()
if data, exists := actionsDataMap[cacheKey]; exists {
actionsData = data
mutex.RUnlock()
return nil
}
mutex.RUnlock()

body, readErr := io.ReadAll(resp.Body)
if readErr != nil {
err = readErr
return
}
// If not in cache, acquire write lock
mutex.Lock()
defer mutex.Unlock()

if unmarshalErr := json.Unmarshal(body, &actionsData); unmarshalErr != nil {
err = unmarshalErr
return
}
// Double-check after acquiring write lock
if data, exists := actionsDataMap[cacheKey]; exists {
actionsData = data
return nil
}

baseURL := "https://raw.githubusercontent.com/reearth/reearth-flow/main/engine/schema/"
filename := "actions.json"
if lang != "" {
filename = fmt.Sprintf("actions_%s.json", lang)
}

if validateErr := actionsData.Validate(); validateErr != nil {
err = validateErr
return
resp, err := http.Get(baseURL + filename)
if err != nil {
return err
}
defer func() {
if err := resp.Body.Close(); err != nil {
fmt.Println("Error closing response body:", err)
}
})
return err
}()

body, err := io.ReadAll(resp.Body)
if err != nil {
return err
}

var newData ActionsData
if err := json.Unmarshal(body, &newData); err != nil {
return err
}

if err := newData.Validate(); err != nil {
return err
}

// Store in cache and set current actionsData
actionsDataMap[cacheKey] = newData
actionsData = newData

return nil
}

func listActions(c echo.Context) error {
Expand Down
78 changes: 24 additions & 54 deletions api/internal/app/actions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,35 +4,42 @@ import (
"encoding/json"
"net/http"
"net/http/httptest"
"strings"
"sync"
"testing"

"github.com/labstack/echo/v4"
"github.com/stretchr/testify/assert"
)

func resetTestData() {
actionsData = ActionsData{}
actionsDataMap = make(map[string]ActionsData)
}

func TestLoadActionsData(t *testing.T) {
tests := []struct {
name string
lang string
wantErr bool
}{
{"Default language", "", false},
{"English", "en", false},
{"En1ish", "en", false},
{"Japanese", "ja", false},
{"Invalid language", "invalid", true},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
actionsData = ActionsData{}
once = sync.Once{}
resetTestData()
err := loadActionsData(tt.lang)
if tt.wantErr {
assert.Error(t, err)
} else {
assert.NoError(t, err)
assert.NotEmpty(t, actionsData.Actions)

// Verify cache
assert.NotNil(t, actionsDataMap[tt.lang])
assert.Equal(t, actionsData, actionsDataMap[tt.lang])
}
})
}
Expand All @@ -48,11 +55,13 @@ func TestListActions(t *testing.T) {
}{
{"Default language", "", "", http.StatusOK},
{"With language", "", "en", http.StatusOK},
{"Japanese language", "", "ja", http.StatusOK},
{"Invalid language", "", "invalid", http.StatusBadRequest},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
resetTestData()
req := httptest.NewRequest(http.MethodGet, "/actions?lang="+tt.lang+tt.query, nil)
rec := httptest.NewRecorder()
c := e.NewContext(req, rec)
Expand All @@ -72,9 +81,6 @@ func TestListActions(t *testing.T) {
}

func TestGetSegregatedActions(t *testing.T) {
originalData := actionsData
defer func() { actionsData = originalData }()

testActions := []Action{
{
Name: "FileWriter",
Expand All @@ -89,7 +95,6 @@ func TestGetSegregatedActions(t *testing.T) {
Categories: []string{},
},
}
actionsData = ActionsData{Actions: testActions}

tests := []struct {
name string
Expand All @@ -98,11 +103,16 @@ func TestGetSegregatedActions(t *testing.T) {
}{
{"Default language", "", http.StatusOK},
{"English", "en", http.StatusOK},
{"Japanese", "ja", http.StatusOK},
{"Invalid language", "invalid", http.StatusBadRequest},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
resetTestData()
actionsData = ActionsData{Actions: testActions}
actionsDataMap[tt.lang] = actionsData

e := echo.New()
req := httptest.NewRequest(http.MethodGet, "/actions/segregated?lang="+tt.lang, nil)
rec := httptest.NewRecorder()
Expand All @@ -124,16 +134,12 @@ func TestGetSegregatedActions(t *testing.T) {
}

func TestGetActionDetails(t *testing.T) {
originalData := actionsData
defer func() { actionsData = originalData }()

testAction := Action{
Name: "TestAction",
Type: ActionTypeProcessor,
Description: "Test action description",
Categories: []string{"TestCategory"},
}
actionsData = ActionsData{Actions: []Action{testAction}}

tests := []struct {
name string
Expand All @@ -143,12 +149,17 @@ func TestGetActionDetails(t *testing.T) {
}{
{"Default language", "", testAction.Name, http.StatusOK},
{"English", "en", testAction.Name, http.StatusOK},
{"Japanese", "ja", testAction.Name, http.StatusOK},
{"Invalid language", "invalid", testAction.Name, http.StatusBadRequest},
{"Not found", "en", "NonExistent", http.StatusNotFound},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
resetTestData()
actionsData = ActionsData{Actions: []Action{testAction}}
actionsDataMap[tt.lang] = actionsData

e := echo.New()
req := httptest.NewRequest(http.MethodGet, "/?lang="+tt.lang, nil)
rec := httptest.NewRecorder()
Expand All @@ -171,47 +182,6 @@ func TestGetActionDetails(t *testing.T) {
}
}

func TestListActionsWithSearch(t *testing.T) {
originalData := actionsData
defer func() { actionsData = originalData }()

actionsData = ActionsData{
Actions: []Action{
{
Name: "FileWriter",
Description: "Writes features to a file",
Type: ActionTypeSink,
Categories: []string{"File"},
},
{
Name: "OtherAction",
Description: "Some other action",
Type: ActionTypeProcessor,
},
},
}

e := echo.New()
req := httptest.NewRequest(http.MethodGet, "/actions?q=file", nil)
rec := httptest.NewRecorder()
c := e.NewContext(req, rec)

err := listActions(c)
assert.NoError(t, err)
assert.Equal(t, http.StatusOK, rec.Code)

var response []ActionSummary
err = json.Unmarshal(rec.Body.Bytes(), &response)
assert.NoError(t, err)
assert.NotEmpty(t, response, "Search should return at least one result")

for _, action := range response {
lowercaseContent := strings.ToLower(action.Name + " " + action.Description)
assert.Contains(t, lowercaseContent, "file",
"Each result should contain 'file' in name or description")
}
}

func TestGetActionDetailsNotFound(t *testing.T) {
e := echo.New()
req := httptest.NewRequest(http.MethodGet, "/", nil)
Expand Down

0 comments on commit 7655adb

Please sign in to comment.