NOISSUE - Search by metadata #849

mteodor · 2019-09-11T14:28:32Z

What does this do?

Add searching things by metadata

Have you included tests for your changes?

Yes

Did you document any new/modified functionality?

Yes

Notes

drasko

No remaks on the logic. just a minor remarks on a code style.

docs/provisioning.md

things/api/things/http/transport.go

things/postgres/things.go

codecov-io · 2019-09-15T13:49:15Z

Codecov Report

Merging #849 into master will decrease coverage by 0.14%.
The diff coverage is 75%.

@@            Coverage Diff             @@
##           master     #849      +/-   ##
==========================================
- Coverage   85.53%   85.39%   -0.15%     
==========================================
  Files          72       72              
  Lines        4708     4737      +29     
==========================================
+ Hits         4027     4045      +18     
- Misses        445      453       +8     
- Partials      236      239       +3

Impacted Files	Coverage Δ
things/api/things/http/requests.go	`83.87% <ø> (ø)`	⬆️
things/redis/streams.go	`90.47% <100%> (ø)`	⬆️
things/api/things/http/endpoint.go	`98.23% <100%> (ø)`	⬆️
things/service.go	`86.61% <100%> (ø)`	⬆️
things/postgres/init.go	`92.59% <100%> (+1.1%)`	⬆️
things/api/things/http/transport.go	`93.11% <52.63%> (-3.44%)`	⬇️
things/postgres/things.go	`81.25% <84.61%> (-0.35%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a6cf767...5003558. Read the comment docs.

drasko · 2019-09-16T08:43:53Z

@mteodor this starts to look good.

Just one question - does this impact SDK and CLI?

mteodor · 2019-09-16T09:41:12Z

@mteodor this starts to look good.

Just one question - does this impact SDK and CLI?

I dont think so, this affects only list things endpoint which is used in

func (sdk mfSDK) Things(token string, offset, limit uint64, name string) (ThingsPage, error)

but call to that enpoint will remain unchanged and results will be the same

things/api/things/http/requests.go

dborovcanin · 2019-09-16T11:02:22Z

things/postgres/init.go

@@ -57,14 +57,14 @@ func migrateDB(db *sqlx.DB) error {
 						owner    VARCHAR(254),
 						key      VARCHAR(4096) UNIQUE NOT NULL,
 						name     VARCHAR(1024),
-						metadata JSON,
+						metadata JSONB,


Please consider adding a separate migration for type change, rather than changing data type this way.

@dusanb94 I do not see the need to support JSON anymore. Pre 1.0 can stil break APIs and we are not in obligation to drag with us unneeded field.

It would be easier for us because we wouldn't need to change schema manually (no need to drop/change in Docker volumes, nor local DB). I agree we can remove that migration before production because this is <1.0, but adding even temporary migration will help (especially since it's done only once).

nmarcetic · 2019-09-16T11:37:19Z

things/api/things/http/transport.go

+func readMetadataQuery(r *http.Request, key string) (interface{}, error) {
+	vals := bone.GetQuery(r, key)
+	if len(vals) > 1 {
+		return "", errInvalidQueryParams


Why string and not nil like in next lines?

nmarcetic · 2019-09-16T11:39:49Z

things/postgres/init.go

 						PRIMARY KEY (id, owner)
 					)`,
 					`CREATE TABLE IF NOT EXISTS channels (
 						id       UUID,
 						owner    VARCHAR(254),
 						name     VARCHAR(1024),
-						metadata JSON,
+						metadata JSONB,


Please do this as part of Channels metadata PR.

things/api/things/http/requests.go

docs/provisioning.md

things/postgres/things.go

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

anovakovic01 · 2019-09-17T13:01:00Z

things/postgres/init.go

@@ -57,7 +57,7 @@ func migrateDB(db *sqlx.DB) error {
 						owner    VARCHAR(254),
 						key      VARCHAR(4096) UNIQUE NOT NULL,
 						name     VARCHAR(1024),
-						metadata JSON,
+						metadata JSONB,


Please remove this because you have the migration later on.

drasko

LGTM

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

anovakovic01

LGTM!

* add metadata search Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * add metadata search Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * add metadata search Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * add metadata search Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * add metadata search Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * add metadata search Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * add metadata search Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * add space Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * metadata test case Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * add docs and update swagger Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * add test for metadata Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * add test for metadata Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * remove commented out section Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * small change to test Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * remove debug printf Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * small fix for metadata Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * fix tests Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * and => and/or Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * add line Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * fix mixed func params Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * metadata will be added to channels later Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * fix return type Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * fix typings Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * fix typings Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * add migration Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * remove var Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * respecting the order of migrations Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

mteodor added the work-in-progress label Sep 11, 2019

mteodor force-pushed the metadata branch from 3f2b8d5 to 824abd8 Compare September 13, 2019 15:07

drasko requested changes Sep 13, 2019

View reviewed changes

docs/provisioning.md Outdated Show resolved Hide resolved

things/api/things/http/transport.go Show resolved Hide resolved

things/postgres/things.go Show resolved Hide resolved

mteodor removed the work-in-progress label Sep 15, 2019

dborovcanin requested changes Sep 16, 2019

View reviewed changes

nmarcetic reviewed Sep 16, 2019

View reviewed changes

mteodor force-pushed the metadata branch from 5fe806f to c0fe2dd Compare September 16, 2019 13:19

nmarcetic previously approved these changes Sep 16, 2019

View reviewed changes

drasko requested changes Sep 16, 2019

View reviewed changes

docs/provisioning.md Outdated Show resolved Hide resolved

things/postgres/things.go Outdated Show resolved Hide resolved

mteodor dismissed nmarcetic’s stale review via bb3af58 September 17, 2019 09:56

nmarcetic previously approved these changes Sep 17, 2019

View reviewed changes

mteodor added 16 commits September 17, 2019 13:55

add metadata search

1cb292e

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

add metadata search

848faad

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

add metadata search

9ecc543

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

add metadata search

e161ff6

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

add metadata search

43f9252

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

add metadata search

8fe307f

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

add metadata search

89990bd

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

add space

e2db9db

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

metadata test case

8c7a3af

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

add docs and update swagger

059a08d

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

add test for metadata

49bc7f8

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

add test for metadata

a175d17

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

remove commented out section

d300045

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

small change to test

db8167d

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

remove debug printf

853b372

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

small fix for metadata

65f0643

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

mteodor added 10 commits September 17, 2019 13:55

fix tests

31dcb4b

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

and => and/or

15f61a8

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

add line

d1c2454

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

fix mixed func params

3671bfa

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

metadata will be added to channels later

d162134

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

fix return type

f4bf288

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

fix typings

96cbcf6

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

fix typings

401815d

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

add migration

3e0cea9

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

remove var

2666975

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

mteodor dismissed nmarcetic’s stale review via 2666975 September 17, 2019 11:55

mteodor force-pushed the metadata branch from c926302 to 2666975 Compare September 17, 2019 11:55

dborovcanin previously approved these changes Sep 17, 2019

View reviewed changes

nmarcetic previously approved these changes Sep 17, 2019

View reviewed changes

anovakovic01 suggested changes Sep 17, 2019

View reviewed changes

drasko reviewed Sep 17, 2019

View reviewed changes

drasko previously approved these changes Sep 17, 2019

View reviewed changes

respecting the order of migrations

5003558

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

mteodor dismissed stale reviews from drasko, nmarcetic, and dborovcanin via 5003558 September 17, 2019 13:35

anovakovic01 approved these changes Sep 17, 2019

View reviewed changes

nmarcetic approved these changes Sep 17, 2019

View reviewed changes

drasko merged commit d20dfa8 into absmach:master Sep 17, 2019

mteodor mentioned this pull request Oct 8, 2019

Adding search of channels by metadata #859

Closed

mteodor deleted the metadata branch September 22, 2020 10:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NOISSUE - Search by metadata #849

NOISSUE - Search by metadata #849

mteodor commented Sep 11, 2019 •

edited

Loading

drasko left a comment

codecov-io commented Sep 15, 2019 •

edited

Loading

drasko commented Sep 16, 2019 •

edited

Loading

mteodor commented Sep 16, 2019

dborovcanin Sep 16, 2019

drasko Sep 16, 2019

dborovcanin Sep 16, 2019

nmarcetic Sep 16, 2019

nmarcetic Sep 16, 2019

anovakovic01 Sep 17, 2019

drasko left a comment

anovakovic01 left a comment

NOISSUE - Search by metadata #849

NOISSUE - Search by metadata #849

Conversation

mteodor commented Sep 11, 2019 • edited Loading

What does this do?

Have you included tests for your changes?

Did you document any new/modified functionality?

Notes

drasko left a comment

Choose a reason for hiding this comment

codecov-io commented Sep 15, 2019 • edited Loading

Codecov Report

drasko commented Sep 16, 2019 • edited Loading

mteodor commented Sep 16, 2019

dborovcanin Sep 16, 2019

Choose a reason for hiding this comment

drasko Sep 16, 2019

Choose a reason for hiding this comment

dborovcanin Sep 16, 2019

Choose a reason for hiding this comment

nmarcetic Sep 16, 2019

Choose a reason for hiding this comment

nmarcetic Sep 16, 2019

Choose a reason for hiding this comment

anovakovic01 Sep 17, 2019

Choose a reason for hiding this comment

drasko left a comment

Choose a reason for hiding this comment

anovakovic01 left a comment

Choose a reason for hiding this comment

mteodor commented Sep 11, 2019 •

edited

Loading

codecov-io commented Sep 15, 2019 •

edited

Loading

drasko commented Sep 16, 2019 •

edited

Loading