Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Collection controller #133

Merged
merged 20 commits into from
Mar 21, 2018
Merged

Collection controller #133

merged 20 commits into from
Mar 21, 2018

Conversation

alexandrebouthinon
Copy link
Member

@alexandrebouthinon alexandrebouthinon commented Mar 6, 2018

  • void create(char* index, char* collection)
  • void deleteSpecifications(char* index, char* collection)
  • bool exists(char* index, char* collection)
  • char* getMapping(char* index, char* collection)
  • char* getSpecifications(char* index, char* collection)
  • char* list(char* index, collectionListOptions*) // struct { enum{all, stored, realtime} type, int from, int size } collectionListOptions;
  • // not implemented (replaced by searchSpecifications + searchResult scroll ability): scrollSpecifications
  • *searchResult searchSpecifications(searchOptions* options)
  • void truncate(char* index, char *collection)
  • void updateMapping(char* index, char* collection, char* body)
  • char* updateSpecifications(char* index, char* collection, char* body)
  • bool validateSpecifications(char* body)

@codecov-io
Copy link

codecov-io commented Mar 6, 2018

Codecov Report

Merging #133 into 1.x will decrease coverage by 1.07%.
The diff coverage is 97.54%.

Impacted file tree graph

@@            Coverage Diff             @@
##              1.x     #133      +/-   ##
==========================================
- Coverage   89.04%   87.96%   -1.08%     
==========================================
  Files         210      198      -12     
  Lines        4327     3615     -712     
==========================================
- Hits         3853     3180     -673     
+ Misses        440      409      -31     
+ Partials       34       26       -8
Impacted Files Coverage Δ
collection/truncate.go 100% <100%> (+11.11%) ⬆️
collection/get_mapping.go 100% <100%> (ø) ⬆️
collection/update_specifications.go 100% <100%> (ø)
collection/list.go 100% <100%> (ø)
kuzzle/kuzzle.go 35.29% <100%> (+0.54%) ⬆️
collection/get_specifications.go 100% <100%> (ø)
collection/search_specifications.go 100% <100%> (ø)
collection/create.go 100% <100%> (+11.11%) ⬆️
collection/collection.go 100% <100%> (ø) ⬆️
collection/update_mapping.go 100% <100%> (ø)
... and 11 more

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 0c1b34f...b71f361. Read the comment docs.

@alexandrebouthinon alexandrebouthinon self-assigned this Mar 6, 2018
collection: dc,
type ListOptions struct {
Type string
From int
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from and size should be declared as int*, so that the SDK can tell the difference between a non-setted option and any other value set by a user

Size int
}

func NewListOptions(t string, from int, size int) *ListOptions {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the only constructor these "...Options" structures should allow are default constructors
and in Go this means that we don't need any constructor :)

this method should be removed

err := json.Unmarshal(res.Result, ack)
if err != nil {
return false, types.NewError(fmt.Sprintf("Unable to parse response: %s\n%s", err.Error(), res.Result), 500)
return res.Error
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpicking: this if test is unnecessary as res.Error can be returned directly
(this comment can be applied to all methods returning only an error or nil)

return false, types.NewError(fmt.Sprintf("Unable to parse response: %s\n%s", err.Error(), res.Result), 500)
}

return true, nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

must return the parsed exists value instead of a truthy value, as Kuzzle may return false to tell the user that the requested collection does not exist

"github.com/kuzzleio/sdk-go/types"
)

// GetMapping retrieves the current mapping of the collection.
func (dc *Collection) GetMapping(options types.QueryOptions) (*Mapping, error) {
func (dc *Collection) GetMapping(index string, collection string) (string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

must return a json.rawMessage instead of a string value

res, err := cToGoSearchResult(sr).FetchNext()
return goToCSearchResult(sr.collection, res, err)
}
////export kuzzle_search_result_fetch_next
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file should be deleted instead of containing only commented code


type SearchOptions struct {
Type string
From int
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From and Size must be declared as int* as we need a way to tell whether the option has been set or not

Scroll string
}

func NewSearchOptions(t string, from int, size int, scroll string) *SearchOptions {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this constructor should be removed

return sr
}

//// FetchNext returns a new SearchResult that corresponds to the next result page
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please delete commented code

sr.Options.SetFrom(options.From())
sr.Options.SetSize(options.Size())
} else {
sr.Options.SetFrom(0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from and size should not be defaulted to 0

@alexandrebouthinon
Copy link
Member Author

Depends on #135

@scottinet scottinet merged commit f967f95 into 1.x Mar 21, 2018
@scottinet scottinet deleted the collection_controller branch March 21, 2018 10:34
@jenow jenow mentioned this pull request Sep 10, 2018
@jenow jenow mentioned this pull request Jun 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants