Add Index Controller #23

alexandrebouthinon · 2019-07-04T15:26:59Z

What does this PR do ?

Add Index controller to the SDK C#:

Implementation
Unit tests
Kuzzle integration

How should this be manually tested?

Run this oneliner:

$ dotnet build Kuzzle/Kuzzle.csproj && dotnet build Kuzzle.Tests/Kuzzle.Tests.csproj && dotnet test

Other changes

Travis CI / CD improvments

Optimize running and deployment conditions
Build in release mode only when deploying.

* Implementation * Unit tests * Kuzzle integration

codecov · 2019-07-04T15:36:32Z

Codecov Report

Merging #23 into 0-dev will increase coverage by 1.42%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##            0-dev      #23      +/-   ##
==========================================
+ Coverage   81.59%   83.01%   +1.42%     
==========================================
  Files          18       19       +1     
  Lines         880      954      +74     
  Branches      107      116       +9     
==========================================
+ Hits          718      792      +74     
  Misses        155      155              
  Partials        7        7

Impacted Files	Coverage Δ
Kuzzle/API/Controllers/IndexController.cs	`100% <100%> (ø)`
Kuzzle/Kuzzle.cs	`90.56% <100%> (+0.18%)`	⬆️

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 105d0b9...8c01014. Read the comment docs.

* Optimize running and deployment conditions. * Build in release mode only when deploying.

scottinet · 2019-07-05T08:29:31Z

Kuzzle/API/Controllers/IndexController.cs

+        { "index", index },
+      });
+
+			return (JObject)response.Result;


Your files are (very) badly indented.

I forgot to install the VIM C# plugin 😞

scottinet · 2019-07-05T08:30:51Z

Kuzzle.Tests/API/Controllers/IndexControllerTest.cs

+
+    [Fact]
+    public async void ExistsAsyncTest() {
+      _api.SetResult(@"{result: true}");


Unit tests should test all possible outcomes (use a theory instead of a fact). This should be fixed for all comparable tests.

scottinet · 2019-07-05T08:35:01Z

Kuzzle/API/Controllers/IndexController.cs

+        { "action", "refreshInternal" },
+      });
+
+			return (bool)response.Result["acknowledged"];


API routes returning {acknowledge: true} can never return acknowledge: false. These kind of responses are just to inform that the query is well-formed and will be processed. In case of error, the result is null and the response contains an error object.

This method should thus return a Task instead of a Task<bool>

scottinet · 2019-07-05T08:36:13Z

Kuzzle/API/Controllers/IndexController.cs

+    }
+
+    /// <summary>
+    /// Delete an index from the Kuzzle persistence engine.


Suggested change

/// Delete an index from the Kuzzle persistence engine.

/// Deletes an index from the Kuzzle persistence engine.

scottinet · 2019-07-05T08:36:28Z

Kuzzle/API/Controllers/IndexController.cs

+		}
+
+    /// <summary>
+    /// Check if an index exists in the Kuzzle persistence engine.


Suggested change

/// Check if an index exists in the Kuzzle persistence engine.

/// Checks if an index exists in the Kuzzle persistence engine.

scottinet · 2019-07-05T08:36:39Z

Kuzzle/API/Controllers/IndexController.cs

+    }
+
+    /// <summary>
+    /// Get the autoRefresh flag value for the given index.


Suggested change

/// Get the autoRefresh flag value for the given index.

/// Gets the autoRefresh flag value for the given index.

scottinet · 2019-07-05T08:36:49Z

Kuzzle/API/Controllers/IndexController.cs

+    }
+
+    /// <summary>
+    /// List indexes from the Kuzzle persistence engine.


Suggested change

/// List indexes from the Kuzzle persistence engine.

/// Lists indexes from the Kuzzle persistence engine.

.travis.yml

Kuzzle.Tests/API/Controllers/IndexControllerTest.cs

Aschen · 2019-07-08T12:03:08Z

Kuzzle/API/Controllers/IndexController.cs

+        { "index", index },
+      });
+
+      return (JObject)response.Result;


The result is not important here, you could return nothing (like DeleteAsync) instead

This is the case in SDK JS https://docs-v2.kuzzle.io/sdk/js/6/controllers/index/create/#resolves 🙂

Then the js response should be modified in the next release.

@Aschen : Fixed

Aschen · 2019-07-08T12:05:31Z

Kuzzle/API/Controllers/IndexController.cs

+
+      Response response = await api.QueryAsync(request);
+
+      return (bool)response.Result["response"];


The result is not important here, you could return nothing (like DeleteAsync) instead

https://docs-v2.kuzzle.io/sdk/js/6/controllers/index/set-auto-refresh/#resolves

The JS SDK is not an example, we cannot change the return because of the breaking change but IMHO this return is not useful

Aschen · 2019-07-08T12:06:37Z

Kuzzle/Kuzzle.cs

      Auth = new AuthController(this);
      Collection = new CollectionController(this);
      Document = new DocumentController(this);
+      Index = new IndexController(this);


You can add the associated unit test

benoitvidis · 2019-07-10T09:36:04Z

.travis.yml

-    - name: Unit Tests
+    - stage: "Tests"
+      name: "Unit Tests"
+      if: type = pull_request OR (type = push && branch =~ /^master|[0-9]+-(dev|stable)$/) OR type = cron


nitpicking: unneeded parenthesis and mix of || / && with AND / OR

Suggested change

if: type = pull_request OR (type = push && branch =~ /^master|[0-9]+-(dev|stable)$/) OR type = cron

if: type = pull_request OR type = push AND branch =~ /^master|[0-9]+-(dev|stable)$/ OR type = cron

Plus: we don't have any *-stable branch: why include this pattern?

I include this pattern to avoid forgetting it

benoitvidis · 2019-07-10T09:37:30Z

Kuzzle/API/Controllers/IndexController.cs

+        { "index", index },
+      });
+
+      return (JObject)response.Result;


Then the js response should be modified in the next release.

scottinet · 2019-07-15T09:50:59Z

Kuzzle.Tests/API/Controllers/IndexControllerTest.cs

+      ");
+
+      Assert.Equal(
+          new JObject {


(nitpicking) wrong indentation

# [0.2.0](https://github.com/kuzzleio/sdk-csharp/releases/tag/0.2.0) (2019-07-17) #### New features - [ [#22](#22) ] Add Admin controller and unit tests ([Shiranuit](https://github.com/Shiranuit)) - [ [#23](#23) ] Add Index Controller ([alexandrebouthinon](https://github.com/alexandrebouthinon)) #### Enhancements - [ [#14](#14) ] Fix optional parameters design ([scottinet](https://github.com/scottinet)) - [ [#13](#13) ] Add unit tests for the collection controller ([scottinet](https://github.com/scottinet)) - [ [#12](#12) ] Add unit tests for the auth controller ([scottinet](https://github.com/scottinet)) - [ [#11](#11) ] Initialize unit tests project ([scottinet](https://github.com/scottinet)) #### Others - [ [#24](#24) ] Rewrite WebSocket class ([scottinet](https://github.com/scottinet), [alexandrebouthinon](https://github.com/alexandrebouthinon)) - [ [#20](#20) ] Add documentation runner ([Aschen](https://github.com/Aschen)) - [ [#18](#18) ] Add unit test for Realtime Controller ([ThomasF34](https://github.com/ThomasF34)) - [ [#17](#17) ] Unit tests kuzzle class ([Aschen](https://github.com/Aschen)) - [ [#16](#16) ] Add unit tests for the server controller ([ThomasF34](https://github.com/ThomasF34)) - [ [#15](#15) ] Add unit tests for the document controller ([scottinet](https://github.com/scottinet)) ---

Add Index Controller

bf21f24

* Implementation * Unit tests * Kuzzle integration

alexandrebouthinon added the changelog:new-features label Jul 4, 2019

alexandrebouthinon self-assigned this Jul 4, 2019

alexandrebouthinon force-pushed the index-controller branch from 42c6fda to cbd6f3d Compare July 4, 2019 15:31

alexandrebouthinon force-pushed the index-controller branch 2 times, most recently from 50bfef4 to 4655af0 Compare July 4, 2019 15:44

Improve CI / CD

4655af0

* Optimize running and deployment conditions. * Build in release mode only when deploying.

scottinet suggested changes Jul 5, 2019

View reviewed changes

alexandrebouthinon force-pushed the index-controller branch from 14f5b4e to d2f63ca Compare July 5, 2019 12:46

alexandrebouthinon requested a review from scottinet July 5, 2019 12:46

alexandrebouthinon force-pushed the index-controller branch 2 times, most recently from 2862f33 to c1c7301 Compare July 5, 2019 14:32

Apply @scottinet requested changes

c1c7301

ThomasF34 approved these changes Jul 8, 2019

View reviewed changes

.travis.yml Show resolved Hide resolved

.travis.yml Show resolved Hide resolved

Kuzzle.Tests/API/Controllers/IndexControllerTest.cs Show resolved Hide resolved

Kuzzle.Tests/API/Controllers/IndexControllerTest.cs Outdated Show resolved Hide resolved

Aschen suggested changes Jul 8, 2019

View reviewed changes

Apply @ThomasF34 requested changes

9468978

alexandrebouthinon requested a review from Aschen July 8, 2019 13:09

benoitvidis reviewed Jul 10, 2019

View reviewed changes

alexandrebouthinon force-pushed the index-controller branch from 9fa66b7 to b6b4b9d Compare July 10, 2019 09:47

benoitvidis approved these changes Jul 10, 2019

View reviewed changes

alexandrebouthinon force-pushed the index-controller branch from b6b4b9d to e9bf207 Compare July 10, 2019 12:37

alexandrebouthinon added 2 commits July 10, 2019 14:38

Apply @benoitvidis requested changes

b25b496

Add @Aschen requested changes

e9bf207

scottinet approved these changes Jul 15, 2019

View reviewed changes

Fix indentation on IndexControllerTest

8c01014

Aschen approved these changes Jul 15, 2019

View reviewed changes

Aschen merged commit 9c1340a into 0-dev Jul 15, 2019

Aschen deleted the index-controller branch July 15, 2019 16:37

scottinet mentioned this pull request Jul 17, 2019

Release 0.2.0 #25

Merged

	/// Delete an index from the Kuzzle persistence engine.
	/// Deletes an index from the Kuzzle persistence engine.

	/// Check if an index exists in the Kuzzle persistence engine.
	/// Checks if an index exists in the Kuzzle persistence engine.

	/// Get the autoRefresh flag value for the given index.
	/// Gets the autoRefresh flag value for the given index.

	/// List indexes from the Kuzzle persistence engine.
	/// Lists indexes from the Kuzzle persistence engine.


		Response response = await api.QueryAsync(request);

		return (bool)response.Result["response"];

	if: type = pull_request OR (type = push && branch =~ /^master\|[0-9]+-(dev\|stable)$/) OR type = cron
	if: type = pull_request OR type = push AND branch =~ /^master\|[0-9]+-(dev\|stable)$/ OR type = cron

Add Index Controller #23

Add Index Controller #23

Uh oh!

Conversation

alexandrebouthinon commented Jul 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do ?

How should this be manually tested?

Other changes

Uh oh!

codecov bot commented Jul 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

alexandrebouthinon commented Jul 4, 2019 •

edited

Loading

codecov bot commented Jul 4, 2019 •

edited

Loading