Refactor handler method by mloufra · Pull Request #158 · opensearch-project/opensearch-sdk-java

mloufra · 2022-09-26T23:19:46Z

Description

Create the following five new classes to handle each handler method for refactoring the class in ExtensionsRunner.java.

ExtensionsInitRequestHandler handle method handleExtensionInitRequest
OpensearchRequestHandler handle method handleOpenSearchRequest
ExtensionsIndicesModuleRequestHandler handle method handleIndicesModuleRequest
ExtensionsIndicesModuleNameRequestHandler handle method handleIndicesModuleNameRequest
ExtenionsRestRequestHandler handle method handleRestExecuteOnExtensionRequest

Issues Resolved

#116

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: mloufra <mloufra@amazon.com>

… main

dbwiddis

I'm not sure why so many variables were changed to static, and suspect there's a better way to handle whatever challenge that was meant to address.

src/main/java/org/opensearch/sdk/ExtensionsRunner.java

src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java

src/main/java/org/opensearch/sdk/handlers/OpensearchRequestHandler.java

… main

Signed-off-by: mloufra <mloufra@amazon.com>

codecov-commenter · 2022-09-28T21:54:05Z

Codecov Report

❌ Patch coverage is 57.14286% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.60%. Comparing base (d7b873c) to head (d3388ef).
⚠️ Report is 317 commits behind head on main.

Files with missing lines	Patch %	Lines
...rch/sdk/handlers/ExtensionsRestRequestHandler.java	33.33%	11 Missing and 1 partial ⚠️
...main/java/org/opensearch/sdk/ExtensionsRunner.java	53.33%	7 Missing ⚠️
...ers/ExtensionsIndicesModuleNameRequestHandler.java	40.00%	3 Missing ⚠️
...andlers/ExtensionsIndicesModuleRequestHandler.java	40.00%	3 Missing ⚠️
...nsearch/sdk/handlers/OpensearchRequestHandler.java	60.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #158      +/-   ##
============================================
+ Coverage     63.69%   64.60%   +0.90%     
- Complexity       90       99       +9     
============================================
  Files            20       25       +5     
  Lines           482      500      +18     
  Branches         18       18              
============================================
+ Hits            307      323      +16     
- Misses          167      169       +2     
  Partials          8        8

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…Handler Signed-off-by: mloufra <mloufra@amazon.com>

src/main/java/org/opensearch/sdk/ExtensionRestRequest.java

src/main/java/org/opensearch/sdk/ExtensionsRunner.java

src/main/java/org/opensearch/sdk/handlers/OpensearchRequestHandler.java

owaiskazi19 · 2022-09-29T00:43:23Z

src/main/java/org/opensearch/sdk/handlers/OpensearchRequestHandler.java

+     * @return A response to OpenSearch for the corresponding API
+     * @throws Exception if the corresponding handler for the request is not present
+     */
+    public TransportResponse handleOpenSearchRequest(OpenSearchRequest request) throws Exception {


This should be a class for NamedWriteableRegistry and not OpenSearchRequest. Also, we don't need a switch case here. If/else can do the job.

It used to have a lot more cases before we refactored :-)

I think we can keep this switch case here, it will easier for others adding more case in the future.

src/main/java/org/opensearch/sdk/ExtensionsRunner.java

dbwiddis

Looking good with the documentation comments noted!

src/main/java/org/opensearch/sdk/ExtensionsRunner.java

src/main/java/org/opensearch/sdk/handlers/ExtensionsRestRequestHandler.java

dbwiddis · 2022-09-29T01:49:07Z

src/main/java/org/opensearch/sdk/handlers/OpensearchRequestHandler.java

+     * @return A response to OpenSearch for the corresponding API
+     * @throws Exception if the corresponding handler for the request is not present
+     */
+    public TransportResponse handleOpenSearchRequest(OpenSearchRequest request) throws Exception {


It used to have a lot more cases before we refactored :-)

Signed-off-by: mloufra <mloufra@amazon.com>

dbwiddis

LGTM! Any additional requests please put on the linked issue for the next round, so we can merge this. :)

* issue opensearch-project#28 Signed-off-by: mloufra <mloufra@amazon.com> * Update the lastest coomit Signed-off-by: mloufra <mloufra@amazon.com> * Rename the method and fix the conflict Signed-off-by: mloufra <mloufra@amazon.com> * fix merge conflict Signed-off-by: mloufra <mloufra@amazon.com> * Add code coverage report Signed-off-by: mloufra <mloufra@amazon.com> * Rebase the lastest commit Signed-off-by: mloufra <mloufra@amazon.com> * update the lastest commit Signed-off-by: mloufra <mloufra@amazon.com> * refactor class for handler method in ExtensionsRunner Signed-off-by: mloufra <mloufra@amazon.com> * add documentation for handler Signed-off-by: mloufra <mloufra@amazon.com> * fix merge conflict Signed-off-by: mloufra <mloufra@amazon.com> * delete all the static Signed-off-by: mloufra <mloufra@amazon.com> * fix documentation problem Signed-off-by: mloufra <mloufra@amazon.com> * fix NullPointerException bug Signed-off-by: mloufra <mloufra@amazon.com> * change RestResponse to ExtensionRestResponse in ExtensionsRestRequestHandler Signed-off-by: mloufra <mloufra@amazon.com> * change documentation for ExtensionsRunner Signed-off-by: mloufra <mloufra@amazon.com> Signed-off-by: mloufra <mloufra@amazon.com>

mloufra and others added 12 commits August 26, 2022 16:43

issue #28

7502353

Signed-off-by: mloufra <mloufra@amazon.com>

Update the lastest coomit

dbb47f5

Signed-off-by: mloufra <mloufra@amazon.com>

Rename the method and fix the conflict

bea6873

Signed-off-by: mloufra <mloufra@amazon.com>

fix merge conflict

0e5dd7f

Signed-off-by: mloufra <mloufra@amazon.com>

Add code coverage report

11bd149

Signed-off-by: mloufra <mloufra@amazon.com>

Rebase the lastest commit

bbccd26

Signed-off-by: mloufra <mloufra@amazon.com>

updata the lastest commit

b182b35

Signed-off-by: mloufra <mloufra@amazon.com>

update the lastest commit

1bcb46c

Signed-off-by: mloufra <mloufra@amazon.com>

Merge branch 'opensearch-project:main' into main

50ff5e7

Merge branch 'main' of https://github.com/mloufra/opensearch-sdk into…

2309183

… main

Merge branch 'main' of https://github.com/mloufra/opensearch-sdk into…

05afe09

… main

Merge branch 'main' of https://github.com/mloufra/opensearch-sdk into…

cdac476

… main

mloufra requested a review from a team September 26, 2022 23:19

mloufra mentioned this pull request Sep 26, 2022

[FEATURE] Refactor ExtensionsRunner class #116

Closed

dbwiddis requested changes Sep 27, 2022

View reviewed changes

mloufra added 7 commits September 27, 2022 16:47

Merge branch 'main' of https://github.com/mloufra/opensearch-sdk into…

880001a

… main

refactor class for handler method in ExtensionsRunner

8c57094

Signed-off-by: mloufra <mloufra@amazon.com>

add documentation for handler

5b45902

Signed-off-by: mloufra <mloufra@amazon.com>

fix merge conflict

8ab9c52

Signed-off-by: mloufra <mloufra@amazon.com>

delete all the static

d33bec5

Signed-off-by: mloufra <mloufra@amazon.com>

fix documentation problem

c977ac4

Signed-off-by: mloufra <mloufra@amazon.com>

fix NullPointerException bug

d3bd29c

Signed-off-by: mloufra <mloufra@amazon.com>

change RestResponse to ExtensionRestResponse in ExtensionsRestRequest…

6d690fb

…Handler Signed-off-by: mloufra <mloufra@amazon.com>

mloufra requested a review from dbwiddis September 28, 2022 22:10