Skip to content
This repository has been archived by the owner on Sep 28, 2022. It is now read-only.

Add specification CRUDL routes #135

Merged
merged 4 commits into from
Jun 15, 2017
Merged

Add specification CRUDL routes #135

merged 4 commits into from
Jun 15, 2017

Conversation

samniisan
Copy link
Contributor

@samniisan samniisan commented Jun 12, 2017

fix kuzzleio/kuzzle-sdk#3

Introduces

  • Specifications CRUDL routes (get - search - scroll - update - delete - validate) and related unit tests

@codecov-io
Copy link

codecov-io commented Jun 12, 2017

Codecov Report

Merging #135 into 3.x will increase coverage by 0.67%.
The diff coverage is 95.48%.

Impacted file tree graph

@@            Coverage Diff             @@
##              3.x     #135      +/-   ##
==========================================
+ Coverage   91.13%   91.81%   +0.67%     
==========================================
  Files          33       33              
  Lines        4571     5558     +987     
  Branches      427      578     +151     
==========================================
+ Hits         4166     5103     +937     
- Misses        286      322      +36     
- Partials      119      133      +14
Impacted Files Coverage Δ
src/main/java/io/kuzzle/sdk/core/Collection.java 93.3% <95.48%> (+0.52%) ⬆️
src/main/java/io/kuzzle/sdk/security/Security.java 93.13% <0%> (-0.41%) ⬇️
src/main/java/io/kuzzle/sdk/core/Kuzzle.java 93.66% <0%> (+1.43%) ⬆️

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 9a3769d...17e8343. Read the comment docs.

* @param options Options Optional parameters
* @param listener Response callback
*/
public void scrollSpecifications(@NonNull final String scrollId, final Options options, @NonNull final ResponseListener<JSONObject> listener) {
Copy link
Contributor

Choose a reason for hiding this comment

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

For the sake of consistency, shouldn't this method resolve to a SearchResults object instead of a raw JSONObject?

If so, then the corresponding documentation should be updated accordingly.

* @param options Options Optional parameters
* @param listener Response callback
*/
public void searchSpecifications(final JSONObject filters, final Options options, @NonNull final ResponseListener<JSONObject> listener) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here: other search methods resolve to a SearchObject object, maybe this one should too?

@samniisan
Copy link
Contributor Author

@scottinet Just updated this PR according to your comments

* @param options Options Optional parameters
* @param listener Response callback
*/
public void validateSpecifications(@NonNull JSONObject specifications, Options options, @NonNull final ResponseListener<Boolean> listener) throws JSONException {
Copy link
Contributor

Choose a reason for hiding this comment

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

You should check if listener is null and throw an IllegalArgumentException if so

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 👍

@jenow jenow merged commit 83f5701 into 3.x Jun 15, 2017
@jenow jenow deleted the 3-data-validation branch June 15, 2017 16:01
@scottinet scottinet mentioned this pull request Jun 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants