Skip to content

Commit

Permalink
Merge #492
Browse files Browse the repository at this point in the history 
492: feat!: enhanced enumeration for task types & statuses r=curquiza a=42atomys

## Related issue
Fixes #491 

## What does this PR do?
This PR addresses the lack of clear definitions for `TaskType` in our asynchronous task definition. While `TaskStatus` is well-defined, `TaskType` currently uses generic strings which could lead to confusion and maintenance challenges. By providing a definitive list of types, we enhance clarity, improve code readability, and set the groundwork for easier maintenance in future SDK versions.

## ⚠️ Breaking Change: Enhanced Enumeration for Task Types & Statuses

The Meilisearch Go client has undergone a significant enhancement in how it handles task types and statuses. In prior versions, developers interfaced with tasks using generic strings. This methodology, while functional, lacked clarity and posed maintenance challenges.

`TaskType` and `TaskStatus` have transitioned from generic strings to definitive constants. This change augments code clarity, minimizes potential errors, and paves the way for smoother future updates.

**Quick Exemple**
```go
// Before:
taskType := "documentDeletion"
...

// After:
taskType := meilisearch.TaskTypeDocumentDeletion
```

**Real world example**
```go
// Before:
func Before() {
	resp, _ := meilisearchClient.GetTasks(&meilisearch.TasksQuery{
		Statuses: []string("enqueued", "processing"),
		Types: []string("documentDeletion", "documentAdditionOrUpdate"),
	})

	for _, task := range resp.Results {
		if task.Status == "processing" {
			// ...
		}

		if task.Type == "documentDeletion" {
			// ...
		}
	}
}

// After:
func After() {
	resp, _ := meilisearchClient.GetTasks(&meilisearch.TasksQuery{
		Statuses: []meilisearch.TaskStatus{
			meilisearch.TaskStatusEnqueued,
			meilisearch.TaskStatusProcessing,
		},
		Types: []meilisearch.TaskType{
			meilisearch.TaskTypeDocumentDeletion,
			meilisearch.TaskTypeDocumentAdditionOrUpdate,
		},
	})

	for _, task := range resp.Results {
		if task.Status == meilisearch.TaskStatusProcessing {
			// ...
		}

		if task.Type == meilisearch.TaskTypeDocumentDeletion {
			// ...
		}
	}
}
```


## PR checklist
Please check if your PR fulfills the following requirements:
- [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
- [x] Have you read the contributing guidelines?
- [x] Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!


Co-authored-by: Atomys <[email protected]>
Co-authored-by: 42Atomys <[email protected]>
  • Loading branch information
@meili-bors @42atomys
3 people authored Oct 25, 2023
2 parents fb9cf7b + 502bbab commit 754baad
Show file tree
Hide file tree
Showing 9 changed files with 177 additions and 103 deletions.
19 changes: 15 additions & 4 deletions .code-samples.meilisearch.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,20 +127,31 @@ async_guide_filter_by_date_1: |-
async_guide_multiple_filters_1: |-
client.GetTasks(&meilisearch.TasksQuery{
IndexUIDS: []string{"movie"},
Types: []string{"documentAdditionOrUpdate", "documentDeletion"},
Statuses: []string{"processing"},
Types: []meilisearch.TaskType{
meilisearch.TaskTypeDocumentAdditionOrUpdate,
meilisearch.TaskTypeDocumentDeletion,
},
Statuses: []meilisearch.TaskStatus{
meilisearch.TaskStatusProcessing,
},
})
async_guide_filter_by_ids_1: |-
client.GetTasks(&meilisearch.TasksQuery{
UIDS: []int64{5, 10, 13},
})
async_guide_filter_by_statuses_1: |-
client.GetTasks(&meilisearch.TasksQuery{
Statuses: []string{"failed", "canceled"},
Statuses: []meilisearch.TaskStatus{
meilisearch.TaskStatusFailed,
meilisearch.TaskStatusCanceled,
},
})
async_guide_filter_by_types_1: |-
client.GetTasks(&meilisearch.TasksQuery{
Types: []string{"dumpCreation", "indexSwap"},
Types: []meilisearch.TaskType{
meilisearch.TaskTypeDumpCreation,
meilisearch.TaskTypeIndexSwap,
},
})
async_guide_filter_by_index_uids_1: |-
client.GetTasks(&meilisearch.TasksQuery{
Expand Down
12 changes: 10 additions & 2 deletions client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -510,10 +510,18 @@ func encodeTasksQuery(param *TasksQuery, req *internalRequest) {
req.withQueryParams["from"] = strconv.FormatInt(param.From, 10)
}
if len(param.Statuses) != 0 {
req.withQueryParams["statuses"] = strings.Join(param.Statuses, ",")
var statuses []string
for _, status := range param.Statuses {
statuses = append(statuses, string(status))
}
req.withQueryParams["statuses"] = strings.Join(statuses, ",")
}
if len(param.Types) != 0 {
req.withQueryParams["types"] = strings.Join(param.Types, ",")
var types []string
for _, t := range param.Types {
types = append(types, string(t))
}
req.withQueryParams["types"] = strings.Join(types, ",")
}
if len(param.IndexUIDS) != 0 {
req.withQueryParams["indexUids"] = strings.Join(param.IndexUIDS, ",")
Expand Down
4 changes: 2 additions & 2 deletions client_index_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func TestClient_CreateIndex(t *testing.T) {
err.(*Error).MeilisearchApiError.Code)
} else {
require.NoError(t, err)
require.Equal(t, gotResp.Type, "indexCreation")
require.Equal(t, gotResp.Type, TaskTypeIndexCreation)
require.Equal(t, gotResp.Status, TaskStatusEnqueued)
// Make sure that timestamps are also retrieved
require.NotZero(t, gotResp.EnqueuedAt)
Expand Down Expand Up @@ -224,7 +224,7 @@ func TestClient_DeleteIndex(t *testing.T) {
err.(*Error).MeilisearchApiError.Code)
} else {
require.NoError(t, err)
require.Equal(t, gotResp.Type, "indexDeletion")
require.Equal(t, gotResp.Type, TaskTypeIndexDeletion)
// Make sure that timestamps are also retrieved
require.NotZero(t, gotResp.EnqueuedAt)
}
Expand Down
17 changes: 9 additions & 8 deletions client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -855,7 +855,7 @@ func TestClient_CancelTasks(t *testing.T) {
UID: "indexUID",
client: defaultClient,
query: &CancelTasksQuery{
Statuses: []string{"succeeded"},
Statuses: []TaskStatus{TaskStatusSucceeded},
},
},
want: "?statuses=succeeded",
Expand Down Expand Up @@ -921,13 +921,14 @@ func TestClient_CancelTasks(t *testing.T) {
UID: "indexUID",
client: defaultClient,
query: &CancelTasksQuery{
Statuses: []string{"enqueued"},
Statuses: []TaskStatus{TaskStatusEnqueued},
Types: []TaskType{TaskTypeDocumentAdditionOrUpdate},
IndexUIDS: []string{"indexUID"},
UIDS: []int64{1},
AfterEnqueuedAt: time.Now(),
},
},
want: "?afterEnqueuedAt=" + strings.NewReplacer(":", "%3A").Replace(time.Now().Format("2006-01-02T15:04:05Z")) + "&indexUids=indexUID&statuses=enqueued&uids=1",
want: "?afterEnqueuedAt=" + strings.NewReplacer(":", "%3A").Replace(time.Now().Format("2006-01-02T15:04:05Z")) + "&indexUids=indexUID&statuses=enqueued&types=documentAdditionOrUpdate&uids=1",
},
}
for _, tt := range tests {
Expand All @@ -954,7 +955,7 @@ func TestClient_CancelTasks(t *testing.T) {
require.NotNil(t, gotResp.TaskUID)
require.NotNil(t, gotResp.EnqueuedAt)
require.Equal(t, "", gotResp.IndexUID)
require.Equal(t, "taskCancelation", gotResp.Type)
require.Equal(t, TaskTypeTaskCancelation, gotResp.Type)
require.Equal(t, tt.want, gotTask.Details.OriginalFilter)
}
})
Expand All @@ -978,7 +979,7 @@ func TestClient_DeleteTasks(t *testing.T) {
UID: "indexUID",
client: defaultClient,
query: &DeleteTasksQuery{
Statuses: []string{"enqueued"},
Statuses: []TaskStatus{TaskStatusEnqueued},
},
},
want: "?statuses=enqueued",
Expand Down Expand Up @@ -1055,7 +1056,7 @@ func TestClient_DeleteTasks(t *testing.T) {
UID: "indexUID",
client: defaultClient,
query: &DeleteTasksQuery{
Statuses: []string{"enqueued"},
Statuses: []TaskStatus{TaskStatusEnqueued},
IndexUIDS: []string{"indexUID"},
UIDS: []int64{1},
AfterEnqueuedAt: time.Now(),
Expand Down Expand Up @@ -1083,7 +1084,7 @@ func TestClient_DeleteTasks(t *testing.T) {
require.NotNil(t, gotResp.TaskUID)
require.NotNil(t, gotResp.EnqueuedAt)
require.Equal(t, "", gotResp.IndexUID)
require.Equal(t, "taskDeletion", gotResp.Type)
require.Equal(t, TaskTypeTaskDeletion, gotResp.Type)
require.NotNil(t, gotTask.Details.OriginalFilter)
require.Equal(t, tt.want, gotTask.Details.OriginalFilter)
})
Expand Down Expand Up @@ -1141,7 +1142,7 @@ func TestClient_SwapIndexes(t *testing.T) {
require.NotNil(t, gotResp.TaskUID)
require.NotNil(t, gotResp.EnqueuedAt)
require.Equal(t, "", gotResp.IndexUID)
require.Equal(t, "indexSwap", gotResp.Type)
require.Equal(t, TaskTypeIndexSwap, gotResp.Type)
require.Equal(t, tt.args.query, gotTask.Details.Swaps)
})
}
Expand Down
13 changes: 11 additions & 2 deletions index.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,19 @@ func (i Index) GetTasks(param *TasksQuery) (resp *TaskResult, err error) {
req.withQueryParams["from"] = strconv.FormatInt(param.From, 10)
}
if len(param.Statuses) != 0 {
req.withQueryParams["statuses"] = strings.Join(param.Statuses, ",")
statuses := make([]string, len(param.Statuses))
for i, status := range param.Statuses {
statuses[i] = string(status)
}
req.withQueryParams["statuses"] = strings.Join(statuses, ",")
}

if len(param.Types) != 0 {
req.withQueryParams["types"] = strings.Join(param.Types, ",")
types := make([]string, len(param.Types))
for i, t := range param.Types {
types[i] = string(t)
}
req.withQueryParams["types"] = strings.Join(types, ",")
}
if len(param.IndexUIDS) != 0 {
param.IndexUIDS = append(param.IndexUIDS, i.UID)
Expand Down
Loading

0 comments on commit 754baad

Please sign in to comment.