feat: Query method for new CouchDB storage implementation #47
Conversation
Codecov Report
@@ Coverage Diff @@
## main #47 +/- ##
==========================================
- Coverage 88.10% 83.36% -4.75%
==========================================
Files 1 8 +7
Lines 227 1088 +861
==========================================
+ Hits 200 907 +707
- Misses 15 100 +85
- Partials 12 81 +69
Continue to review full report at Codecov.
|
DRK3
force-pushed
the
QueryNewCouchDBStorageImplementation
branch
2 times, most recently
from
f4f5e00 to
f2b4c71
Compare
| return nil, fmt.Errorf(failSendRequestToFindEndpoint, err) | ||
| } | ||
|
|
||
| queryWithPageSizeAndBookmarkPlaceholders := `{"selector":{"` + |
There was a problem hiding this comment.
Above you are using Sprintf for generating the query and here you're using concatenation. Should be consistent.
There was a problem hiding this comment.
I had to use concatenation for the "placeholders" once since it itself actually contains formatting directives that I need passed into the iterator. The iterator has to generate new queries dynamically after the initial one for paging.
| case expressionTagNameOnlyLength: | ||
| expressionTagName := expressionSplit[0] | ||
|
|
||
| findQuery := fmt.Sprintf(`{"selector":{"%s":{"$exists":true}},"limit":%d}`, |
There was a problem hiding this comment.
Maybe create a const for the template.
There was a problem hiding this comment.
Done - added consts for both types of queries.
| findQuery := fmt.Sprintf(`{"selector":{"%s":{"$exists":true}},"limit":%d}`, | ||
| expressionTagName, queryOptions.PageSize) | ||
|
|
||
| resultRows, err := s.db.Find(context.Background(), findQuery) |
There was a problem hiding this comment.
One thing that kind of bothers me is the use of context.Background (same with your last PR). I'm wondering if context should actually be passed in the API by the client. Not for this PR though.
There was a problem hiding this comment.
Correct - context should be passed in.
There was a problem hiding this comment.
Created a follow-up issue: #48
Signed-off-by: Derek Trider <Derek.Trider@securekey.com>
DRK3
force-pushed
the
QueryNewCouchDBStorageImplementation
branch
from
f2b4c71 to
a16ac6d
Compare
| option(&queryOptions) | ||
| } | ||
|
|
||
| if queryOptions.PageSize == 0 { |
There was a problem hiding this comment.
Nit: you can set defaults before calling options - that way you don't need if statements.
There was a problem hiding this comment.
Good catch. I've fixed this in #49
Signed-off-by: Derek Trider Derek.Trider@securekey.com