MF-1357 - Add new endpoint for searching things #1383

blokovi · 2021-03-05T08:08:30Z

Signed-off-by: Ivan Milosevic [email protected]

What does this do?

Add /things/search POST endpoint with filter parameters for name and metadata in json body

Which issue(s) does this PR fix/relate to?

Resolves #1357

List any changes that modify/break current functionality

Have you included tests for your changes?

Yes

Did you document any new/modified functionality?

Notes

nmarcetic · 2021-03-05T08:47:37Z

things/api/things/http/transport.go

@@ -105,6 +105,13 @@ func MakeHandler(tracer opentracing.Tracer, svc things.Service) http.Handler {
 		opts...,
 	))

+	r.Post("/things/list", kithttp.NewServer(


Can we add a search here /things/search? Like search is a resource itself. Then POST makes sense because you are creating a new search resource with a req body (containing search criteria).

nmarcetic · 2021-03-05T08:50:11Z

things/api/things/http/requests.go

@@ -180,6 +180,13 @@ type listResourcesReq struct {
 	pageMetadata things.PageMetadata
 }

+type listResourcesMetaReq struct {
+	token        string


Which token is this?
User token is in req header, thing doesn't have a token, it has key (in domain language we don't call it token).

nmarcetic · 2021-03-05T15:15:42Z

things/openapi.yml

+        '404':
+          description: A non-existent entity request.
+        '422':
+          description: Database can't process request.


I would prefer just Unprocessable Entity here a "standard" 422 status message, if you can't say more (like which field etc...). Without mentioning Database...

things/service_test.go

manuio · 2021-03-09T11:04:29Z

things/redis/streams_test.go

@@ -72,7 +72,7 @@ func TestCreateThings(t *testing.T) {
 			key: token,
 			err: nil,
 			event: map[string]interface{}{
-				"id":        "1",
+				"id":        "001",


We should try to use real IDs

manuio · 2021-03-09T11:07:40Z

things/openapi.yml

+    ThingsReqSchema:
+      type: object
+      properties:
+        name:
+          type: string
+          description: Free-form thing name.
+        metadata:
+          type: object
+          description: Object-encoded thing's data.
+        total:
+          type: integer
+          description: Total number of items.
+        offset:
+          type: integer
+          description: Number of items to skip during retrieval.
+        limit:
+          type: integer
+          description: Maximum number of items to return in one page.
+        order:
+          type: string
+          description: Order type.
+        dir:
+          type: string
+          description: Order direction.


This needs to be described better.
Follow this existent example and adapt it to use params from the body:

Limit: name: limit description: Size of the subset to retrieve. in: query schema: type: integer default: 10 maximum: 100 minimum: 1 required: false Offset: name: offset description: Number of items to skip during retrieval. in: query schema: type: integer default: 0 minimum: 0 required: false Connected: name: connected description: Connection state of the subset to retrieve. in: query schema: type: boolean default: true required: false Name: name: name description: Name filter. Filtering is performed as a case-insensitive partial match. in: query schema: type: string required: false Order: name: order description: Order type. in: query schema: type: string default: id enum: - name - id required: false Direction: name: dir description: Order direction. in: query schema: type: string default: desc enum: - asc - desc required: false Metadata: name: metadata description: Metadata filter. Filtering is performed matching the parameter with metadata on top level. Parameter is json. in: query required: false schema: type: object additionalProperties: {}

things/mocks/things.go

things/mocks/channels.go

things/api/things/http/transport.go

manuio · 2021-03-09T11:10:42Z

pkg/sdk/go/things_test.go

@@ -36,7 +35,7 @@ const (
 var (
 	metadata   = map[string]interface{}{"meta": "data"}
 	metadata2  = map[string]interface{}{"meta": "data2"}
-	thing      = sdk.Thing{ID: "1", Name: "test_device", Metadata: metadata}
+	thing      = sdk.Thing{ID: "001", Name: "test_device", Metadata: metadata}


Try to use UUID

Point is that here we mock id so we can test "order by id" functionalities.
That is the reason why I have change mock IDs to use three chars instead one, because ordering with just one char, like it is now, is 1, 10, 100, 2, 20...
With this change order would be 001, 002, 003... and you can write tests that check order by id functionality.

manuio · 2021-03-09T11:10:53Z

pkg/sdk/go/channels_test.go

@@ -16,7 +15,7 @@ import (
 )

 var (
-	channel      = sdk.Channel{ID: "1", Name: "test"}
+	channel      = sdk.Channel{ID: "001", Name: "test"}


Can we use UUID?

mteodor · 2021-03-10T09:23:50Z

things/api/things/http/endpoint_test.go

@@ -49,6 +49,10 @@ var (
 	invalidName = strings.Repeat("m", maxNameSize+1)
 	notFoundRes = toJSON(errorRes{things.ErrNotFound.Error()})
 	unauthRes   = toJSON(errorRes{things.ErrUnauthorizedAccess.Error()})
+	searchThReq = things.PageMetadata{


searchThingReq

Signed-off-by: Ivan Milosevic <[email protected]>

Use same request as list endpoint Signed-off-by: Ivan Milosevic <[email protected]>

Add swagger file Signed-off-by: Ivan Milosevic <[email protected]>

Signed-off-by: Ivan Milosevic <[email protected]>

fix tests Signed-off-by: Ivan Milosevic <[email protected]>

Signed-off-by: Ivan Milosevic <[email protected]>

fix tracer string change test offset Signed-off-by: Ivan Milosevic <[email protected]>

Signed-off-by: Ivan Milosevic <[email protected]>

manuio

LGTM

blokovi requested a review from a team as a code owner March 5, 2021 08:08

nmarcetic requested changes Mar 5, 2021

View reviewed changes

nmarcetic reviewed Mar 5, 2021

View reviewed changes

blokovi force-pushed the MF-1357 branch from be8ed9f to f46fc09 Compare March 5, 2021 19:10

manuio suggested changes Mar 9, 2021

View reviewed changes

nmarcetic previously approved these changes Mar 9, 2021

View reviewed changes

blokovi dismissed nmarcetic’s stale review via 28e23b6 March 9, 2021 14:26

nmarcetic previously approved these changes Mar 10, 2021

View reviewed changes

mteodor reviewed Mar 10, 2021

View reviewed changes

blokovi dismissed nmarcetic’s stale review via bc18673 March 10, 2021 09:48

blokovi added 13 commits March 10, 2021 13:33

Add new enpoint for thing search

f207638

Signed-off-by: Ivan Milosevic <[email protected]>

Rename endpoint to /search

1f58de4

Use same request as list endpoint Signed-off-by: Ivan Milosevic <[email protected]>

Add optional parameters in body (offset, limit)

745a2b4

Add swagger file Signed-off-by: Ivan Milosevic <[email protected]>

move all parameters into body

3d13624

Signed-off-by: Ivan Milosevic <[email protected]>

fix swagger

b8e692c

Signed-off-by: Ivan Milosevic <[email protected]>

fix error description

05e6d75

Signed-off-by: Ivan Milosevic <[email protected]>

Add tests

20e5f8e

Signed-off-by: Ivan Milosevic <[email protected]>

remove dead code

a6e7f5f

fix tests Signed-off-by: Ivan Milosevic <[email protected]>

remove unused var

0f01811

Signed-off-by: Ivan Milosevic <[email protected]>

fix sdk tests

758fd5a

Signed-off-by: Ivan Milosevic <[email protected]>

add url endpoint for search test

ee1ffb6

Signed-off-by: Ivan Milosevic <[email protected]>

description in swagger

eac2b1f

fix tracer string change test offset Signed-off-by: Ivan Milosevic <[email protected]>

rename in tests searchThReq to searchThingReq

0e85f10

Signed-off-by: Ivan Milosevic <[email protected]>

blokovi force-pushed the MF-1357 branch from bc18673 to 0e85f10 Compare March 10, 2021 12:33

manuio approved these changes Mar 10, 2021

View reviewed changes

mteodor approved these changes Mar 11, 2021

View reviewed changes

manuio merged commit 30ba38c into absmach:master Mar 11, 2021

blokovi deleted the MF-1357 branch March 11, 2021 09:35

blokovi mentioned this pull request Mar 15, 2021

NOISSUE - Add search endpoint in docs absmach/supermq-docs#78

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MF-1357 - Add new endpoint for searching things #1383

MF-1357 - Add new endpoint for searching things #1383

blokovi commented Mar 5, 2021 •

edited

Loading

nmarcetic Mar 5, 2021

nmarcetic Mar 5, 2021

nmarcetic Mar 5, 2021

manuio Mar 9, 2021

manuio Mar 9, 2021

manuio Mar 9, 2021

blokovi Mar 9, 2021

manuio Mar 9, 2021

mteodor Mar 10, 2021

manuio left a comment

MF-1357 - Add new endpoint for searching things #1383

MF-1357 - Add new endpoint for searching things #1383

Conversation

blokovi commented Mar 5, 2021 • edited Loading

What does this do?

Which issue(s) does this PR fix/relate to?

List any changes that modify/break current functionality

Have you included tests for your changes?

Did you document any new/modified functionality?

Notes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manuio left a comment

Choose a reason for hiding this comment

blokovi commented Mar 5, 2021 •

edited

Loading