[ISSUE #4767] Refactor Admin server http handler. #4768

karsonto · 2024-02-06T10:01:17Z

Fixes #4767

Motivation

Explain the content here.
Explain why you want to make the changes and what problem you're trying to solve.

Modifications

Describe the modifications you've done.

Documentation

Does this pull request introduce a new feature? (yes / no)
If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
If a feature is not applicable for documentation, explain why?
If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

codecov · 2024-02-07T21:00:34Z

Codecov Report

Attention: Patch coverage is 0% with 605 lines in your changes are missing coverage. Please review.

Project coverage is 16.20%. Comparing base (172804a) to head (d383da0).

Files	Patch %	Lines
...ventmesh/runtime/admin/handler/MetricsHandler.java	0.00%	49 Missing ⚠️
...esh/runtime/admin/handler/AbstractHttpHandler.java	0.00%	47 Missing ⚠️
...ntmesh/runtime/admin/handler/TCPClientHandler.java	0.00%	42 Missing ⚠️
...tmesh/runtime/admin/handler/GrpcClientHandler.java	0.00%	39 Missing ⚠️
...e/admin/handler/RedirectClientByIpPortHandler.java	0.00%	31 Missing ⚠️
...tmesh/runtime/admin/handler/HTTPClientHandler.java	0.00%	29 Missing ⚠️
...ime/admin/handler/RedirectClientByPathHandler.java	0.00%	29 Missing ⚠️
...dmin/handler/RedirectClientBySubSystemHandler.java	0.00%	27 Missing ⚠️
...ime/admin/handler/RejectClientByIpPortHandler.java	0.00%	27 Missing ⚠️
.../admin/handler/RejectClientBySubSystemHandler.java	0.00%	26 Missing ⚠️
... and 18 more

Additional details and impacted files

@@              Coverage Diff              @@
##             master    #4768       +/-   ##
=============================================
+ Coverage          0   16.20%   +16.20%     
- Complexity        0     1711     +1711     
=============================================
  Files             0      856      +856     
  Lines             0    30898    +30898     
  Branches          0     2684     +2684     
=============================================
+ Hits              0     5007     +5007     
- Misses            0    25424    +25424     
- Partials          0      467      +467

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

xwm1992 · 2024-02-19T02:54:45Z

HttpCommand will be deprecated, we will only use requestURI method for http request, no need to add the requestURI for HttpCommand, you can add the @Deprecated annotation for HttpCommand. @karsonto

karsonto · 2024-02-19T06:59:57Z

In addition to the admin server, HttpCommand is also used in the Processor of the http server. This class should not be deprecated. @xwm1992

Pil0tXia · 2024-02-22T10:25:55Z

So HttpCommand shall be deprecated or not?

xwm1992 · 2024-02-23T02:11:18Z

In addition to the admin server, HttpCommand is also used in the Processor of the http server. This class should not be deprecated. @xwm1992

why not use HttpRequest URL instead of HttpCommand ?

karsonto · 2024-02-23T03:55:12Z

Because in addition to the HttpRequest URL , HttpCommand also wraps other information such as http method header. @xwm1992

Pil0tXia · 2024-03-28T07:19:05Z

Any more problems with the usage of HttpCommand?

Pil0tXia

HttpCommand will be deprecated, we will only use requestURI method for http request, no need to add the requestURI for HttpCommand, you can add the @Deprecated annotation for HttpCommand.

I agree. HttpCommand is not designed to parse web requests, although it is still in use.

@karsonto You may try another way to get requestURI, or introduce another dto class suitable for parsing web requests.

...test/java/org/apache/eventmesh/runtime/admin/handler/QueryRecommendEventMeshHandlerTest.java

eventmesh-common/src/main/java/org/apache/eventmesh/common/protocol/http/HttpCommand.java

codecov-commenter · 2024-04-03T03:53:10Z

Codecov Report

Attention: Patch coverage is 0% with 607 lines in your changes are missing coverage. Please review.

Project coverage is 16.20%. Comparing base (172804a) to head (a62d321).

❗ Current head a62d321 differs from pull request most recent head f90458a. Consider uploading reports for the commit f90458a to get more accurate results

Files	Patch %	Lines
...ventmesh/runtime/admin/handler/MetricsHandler.java	0.00%	49 Missing ⚠️
...esh/runtime/admin/handler/AbstractHttpHandler.java	0.00%	48 Missing ⚠️
...ntmesh/runtime/admin/handler/TCPClientHandler.java	0.00%	42 Missing ⚠️
...tmesh/runtime/admin/handler/GrpcClientHandler.java	0.00%	39 Missing ⚠️
...e/admin/handler/RedirectClientByIpPortHandler.java	0.00%	31 Missing ⚠️
...tmesh/runtime/admin/handler/HTTPClientHandler.java	0.00%	29 Missing ⚠️
...ime/admin/handler/RedirectClientByPathHandler.java	0.00%	29 Missing ⚠️
...ime/admin/handler/RejectClientByIpPortHandler.java	0.00%	27 Missing ⚠️
...dmin/handler/RedirectClientBySubSystemHandler.java	0.00%	26 Missing ⚠️
.../admin/handler/RejectClientBySubSystemHandler.java	0.00%	26 Missing ⚠️
... and 19 more

Additional details and impacted files

@@              Coverage Diff              @@
##             master    #4768       +/-   ##
=============================================
+ Coverage          0   16.20%   +16.20%     
- Complexity        0     1710     +1710     
=============================================
  Files             0      857      +857     
  Lines             0    30883    +30883     
  Branches          0     2685     +2685     
=============================================
+ Hits              0     5005     +5005     
- Misses            0    25410    +25410     
- Partials          0      468      +468

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

karsonto · 2024-04-03T03:55:19Z

@xwm1992 @Pil0tXia I have revised it according to your suggestion.

eventmesh-common/src/main/java/org/apache/eventmesh/common/protocol/http/HttpCommand.java

eventmesh-common/src/main/java/org/apache/eventmesh/common/utils/JsonUtils.java

eventmesh-runtime/src/main/java/org/apache/eventmesh/runtime/boot/AbstractHTTPServer.java

eventmesh-runtime/src/main/java/org/apache/eventmesh/runtime/boot/EventMeshAdminServer.java

karsonto · 2024-04-03T07:40:38Z

@Pil0tXia All the issues mentioned previously have been resolved.

...ime/src/main/java/org/apache/eventmesh/runtime/admin/handler/InsertWebHookConfigHandler.java

...sh-runtime/src/main/java/org/apache/eventmesh/runtime/admin/handler/AbstractHttpHandler.java

eventmesh-runtime/src/main/java/org/apache/eventmesh/runtime/boot/AbstractHTTPServer.java

...sh-runtime/src/main/java/org/apache/eventmesh/runtime/admin/handler/AbstractHttpHandler.java

Pil0tXia

LGTM

Pil0tXia · 2024-04-05T18:41:25Z

.../src/main/java/org/apache/eventmesh/runtime/admin/handler/RedirectClientByIpPortHandler.java

-    /**
-     * Handles requests by redirecting matching clients to a target EventMesh server node.
-     *
-     * @param httpExchange the exchange containing the request from the client and used to send the response
-     * @throws IOException if an I/O error occurs while handling the request
-     */
    @Override
-    public void handle(HttpExchange httpExchange) throws IOException {
+    public void handle(HttpRequest httpRequest, ChannelHandlerContext ctx) throws Exception {
        String result = "";
-        try (OutputStream out = httpExchange.getResponseBody()) {
-            String queryString = httpExchange.getRequestURI().getQuery();


May you please add the short description of JavaDoc back?

rebase master

d383da0

karsonto force-pushed the develop-#4738-2 branch from 5b3dafb to d383da0 Compare April 2, 2024 07:28

Pil0tXia reviewed Apr 2, 2024

View reviewed changes

...test/java/org/apache/eventmesh/runtime/admin/handler/QueryRecommendEventMeshHandlerTest.java Outdated Show resolved Hide resolved

eventmesh-common/src/main/java/org/apache/eventmesh/common/protocol/http/HttpCommand.java Outdated Show resolved Hide resolved

use httpRequest

57d0c04

Pil0tXia reviewed Apr 3, 2024

View reviewed changes

use httpRequest

5e8b938

Pil0tXia reviewed Apr 3, 2024

View reviewed changes

...ime/src/main/java/org/apache/eventmesh/runtime/admin/handler/InsertWebHookConfigHandler.java Outdated Show resolved Hide resolved

...sh-runtime/src/main/java/org/apache/eventmesh/runtime/admin/handler/AbstractHttpHandler.java Outdated Show resolved Hide resolved

some enhancement

24895fa

Pil0tXia reviewed Apr 3, 2024

View reviewed changes

eventmesh-runtime/src/main/java/org/apache/eventmesh/runtime/boot/AbstractHTTPServer.java Outdated Show resolved Hide resolved

some enhancement

f897504

Pil0tXia reviewed Apr 3, 2024

View reviewed changes

...sh-runtime/src/main/java/org/apache/eventmesh/runtime/admin/handler/AbstractHttpHandler.java Outdated Show resolved Hide resolved

...sh-runtime/src/main/java/org/apache/eventmesh/runtime/admin/handler/AbstractHttpHandler.java Outdated Show resolved Hide resolved

karsonto added 2 commits April 3, 2024 16:47

some enhancement

3f2e952

some enhancement

f90458a

Pil0tXia approved these changes Apr 3, 2024

View reviewed changes

Pil0tXia requested a review from xwm1992 April 3, 2024 09:18

xwm1992 approved these changes Apr 3, 2024

View reviewed changes

xwm1992 merged commit 98b1ed1 into apache:master Apr 3, 2024
8 of 9 checks passed

This was referenced Apr 3, 2024

[Enhancement] Integrate Runtime admin endpoints with Netty server #4738

Closed

[ISSUE #4812] Set up Admin Endpoints v2 #4813

Merged

Pil0tXia reviewed Apr 5, 2024

View reviewed changes

Pil0tXia mentioned this pull request Apr 6, 2024

[Unit Test] Update Unit tests for eventmesh-runtime admin endpoints #4815

Open

3 tasks

xwm1992 added this to the 1.11.0 milestone Dec 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ISSUE #4767] Refactor Admin server http handler. #4768

[ISSUE #4767] Refactor Admin server http handler. #4768

karsonto commented Feb 6, 2024

codecov bot commented Feb 7, 2024 •

edited

Loading

xwm1992 commented Feb 19, 2024

karsonto commented Feb 19, 2024

Pil0tXia commented Feb 22, 2024

xwm1992 commented Feb 23, 2024

karsonto commented Feb 23, 2024

Pil0tXia commented Mar 28, 2024

Pil0tXia left a comment •

edited

Loading

codecov-commenter commented Apr 3, 2024 •

edited

Loading

karsonto commented Apr 3, 2024

karsonto commented Apr 3, 2024

Pil0tXia left a comment

Pil0tXia Apr 5, 2024

[ISSUE #4767] Refactor Admin server http handler. #4768

[ISSUE #4767] Refactor Admin server http handler. #4768

Conversation

karsonto commented Feb 6, 2024

Motivation

Modifications

Documentation

codecov bot commented Feb 7, 2024 • edited Loading

Codecov Report

xwm1992 commented Feb 19, 2024

karsonto commented Feb 19, 2024

Pil0tXia commented Feb 22, 2024

xwm1992 commented Feb 23, 2024

karsonto commented Feb 23, 2024

Pil0tXia commented Mar 28, 2024

Pil0tXia left a comment • edited Loading

Choose a reason for hiding this comment

codecov-commenter commented Apr 3, 2024 • edited Loading

Codecov Report

karsonto commented Apr 3, 2024

karsonto commented Apr 3, 2024

Pil0tXia left a comment

Choose a reason for hiding this comment

Pil0tXia Apr 5, 2024

Choose a reason for hiding this comment

codecov bot commented Feb 7, 2024 •

edited

Loading

Pil0tXia left a comment •

edited

Loading

codecov-commenter commented Apr 3, 2024 •

edited

Loading