github-actions[bot] commented on code in PR #17753:
URL: https://github.com/apache/doris/pull/17753#discussion_r1133838318
##########
be/src/http/action/check_tablet_segment_action.h:
##########
@@ -19,18 +19,32 @@
#include <string>
-#include "http/http_handler.h"
+#include "http/http_handler_with_auth.h"
#include "util/easy_json.h"
namespace doris {
-class CheckTabletSegmentAction : public HttpHandler {
+class ExecEnv;
+
+class CheckTabletSegmentAction : public HttpHandlerWithAuth {
public:
- CheckTabletSegmentAction();
+ CheckTabletSegmentAction(ExecEnv* exec_env);
+
+ virtual ~CheckTabletSegmentAction() {}
Review Comment:
warning: use '= default' to define a trivial destructor
[modernize-use-equals-default]
```suggestion
virtual ~CheckTabletSegmentAction() = default;
```
##########
be/src/http/action/check_tablet_segment_action.h:
##########
@@ -19,18 +19,32 @@
#include <string>
-#include "http/http_handler.h"
+#include "http/http_handler_with_auth.h"
#include "util/easy_json.h"
namespace doris {
-class CheckTabletSegmentAction : public HttpHandler {
+class ExecEnv;
+
+class CheckTabletSegmentAction : public HttpHandlerWithAuth {
public:
- CheckTabletSegmentAction();
+ CheckTabletSegmentAction(ExecEnv* exec_env);
+
+ virtual ~CheckTabletSegmentAction() {}
Review Comment:
warning: prefer using 'override' or (rarely) 'final' instead of 'virtual'
[modernize-use-override]
```suggestion
~CheckTabletSegmentAction() override {}
```
##########
be/src/http/action/meta_action.h:
##########
@@ -42,6 +42,14 @@ class MetaAction : public HttpHandler {
private:
META_TYPE _meta_type;
+
+ ExecEnv* _exec_env;
Review Comment:
warning: private field '_exec_env' is not used
[clang-diagnostic-unused-private-field]
```cpp
ExecEnv* _exec_env;
^
```
##########
be/src/http/action/snapshot_action.h:
##########
@@ -20,24 +20,32 @@
#include <cstdint>
#include <string>
-#include "http/http_handler.h"
+#include "http/http_handler_with_auth.h"
namespace doris {
class ExecEnv;
// make snapshot
// be_host:be_http_port/api/snapshot?tablet_id=123&schema_hash=456
-class SnapshotAction : public HttpHandler {
+class SnapshotAction : public HttpHandlerWithAuth {
public:
- explicit SnapshotAction();
+ explicit SnapshotAction(ExecEnv* exec_env);
virtual ~SnapshotAction() {}
void handle(HttpRequest* req) override;
private:
+ ExecEnv* _exec_env;
Review Comment:
warning: private field '_exec_env' is not used
[clang-diagnostic-unused-private-field]
```cpp
ExecEnv* _exec_env;
^
```
##########
be/src/http/action/tablet_migration_action.h:
##########
@@ -21,20 +21,30 @@
#include "common/status.h"
#include "gen_cpp/Status_types.h"
-#include "http/http_handler.h"
+#include "http/http_handler_with_auth.h"
#include "olap/data_dir.h"
#include "olap/tablet.h"
#include "util/threadpool.h"
namespace doris {
+class ExecEnv;
+
// Migrate a tablet from a disk to another.
-class TabletMigrationAction : public HttpHandler {
+class TabletMigrationAction : public HttpHandlerWithAuth {
public:
- TabletMigrationAction();
+ TabletMigrationAction(ExecEnv* exec_env) : HttpHandlerWithAuth(exec_env) {
+ _init_migration_action();
+ }
+
+ virtual ~TabletMigrationAction() {}
Review Comment:
warning: prefer using 'override' or (rarely) 'final' instead of 'virtual'
[modernize-use-override]
```suggestion
~TabletMigrationAction() override {}
```
##########
be/src/http/action/tablets_distribution_action.h:
##########
@@ -19,20 +19,35 @@
#include <string>
-#include "http/http_handler.h"
+#include "http/http_handler_with_auth.h"
#include "util/easy_json.h"
namespace doris {
+class ExecEnv;
+
// Get BE tablets distribution info from http API.
-class TabletsDistributionAction : public HttpHandler {
+class TabletsDistributionAction : public HttpHandlerWithAuth {
public:
- TabletsDistributionAction();
+ TabletsDistributionAction(ExecEnv* exec_env);
+
+ virtual ~TabletsDistributionAction() {}
Review Comment:
warning: prefer using 'override' or (rarely) 'final' instead of 'virtual'
[modernize-use-override]
```suggestion
~TabletsDistributionAction() override {}
```
##########
be/src/http/action/tablets_info_action.h:
##########
@@ -19,20 +19,31 @@
#include <string>
-#include "http/http_handler.h"
+#include "http/http_handler_with_auth.h"
#include "util/easy_json.h"
namespace doris {
+class ExecEnv;
+
// Get BE tablets info from http API.
-class TabletsInfoAction : public HttpHandler {
+class TabletsInfoAction : public HttpHandlerWithAuth {
public:
- TabletsInfoAction();
+ TabletsInfoAction(ExecEnv* exec_env);
+
+ virtual ~TabletsInfoAction() {}
Review Comment:
warning: use '= default' to define a trivial destructor
[modernize-use-equals-default]
```suggestion
virtual ~TabletsInfoAction() = default;
```
##########
be/src/http/action/tablets_info_action.h:
##########
@@ -19,20 +19,31 @@
#include <string>
-#include "http/http_handler.h"
+#include "http/http_handler_with_auth.h"
#include "util/easy_json.h"
namespace doris {
+class ExecEnv;
+
// Get BE tablets info from http API.
-class TabletsInfoAction : public HttpHandler {
+class TabletsInfoAction : public HttpHandlerWithAuth {
public:
- TabletsInfoAction();
+ TabletsInfoAction(ExecEnv* exec_env);
+
+ virtual ~TabletsInfoAction() {}
+
void handle(HttpRequest* req) override;
- EasyJson get_tablets_info(std::string tablet_num_to_return);
- std::string host() { return _host; }
+
+ static EasyJson get_tablets_info(std::string tablet_num_to_return);
private:
- std::string _host;
+ ExecEnv* _exec_env;
Review Comment:
warning: private field '_exec_env' is not used
[clang-diagnostic-unused-private-field]
```cpp
ExecEnv* _exec_env;
^
```
##########
be/test/http/http_auth_test.cpp:
##########
@@ -0,0 +1,90 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <gtest/gtest.h>
+
+#include "common/config.h"
+#include "http/ev_http_server.h"
+#include "http/http_channel.h"
+#include "http/http_handler.h"
+#include "http/http_handler_with_auth.h"
+#include "http/http_request.h"
+
+namespace doris {
+
+class HttpAuthTestHandler : public HttpHandlerWithAuth {
+public:
+ HttpAuthTestHandler(ExecEnv* exec_env) : HttpHandlerWithAuth(exec_env) {}
+
+ virtual ~HttpAuthTestHandler() {}
+
+ void handle(HttpRequest* req) override {}
+
+private:
+ ExecEnv* _exec_env;
+
+ bool set_privilege(const HttpRequest& req, TCheckAuthRequest&
auth_request) override {
Review Comment:
warning: only virtual member functions can be marked 'override'
[clang-diagnostic-error]
```suggestion
bool set_privilege(const HttpRequest& req, TCheckAuthRequest&
auth_request) {
```
##########
be/src/http/action/tablets_distribution_action.h:
##########
@@ -19,20 +19,35 @@
#include <string>
-#include "http/http_handler.h"
+#include "http/http_handler_with_auth.h"
#include "util/easy_json.h"
namespace doris {
+class ExecEnv;
+
// Get BE tablets distribution info from http API.
-class TabletsDistributionAction : public HttpHandler {
+class TabletsDistributionAction : public HttpHandlerWithAuth {
public:
- TabletsDistributionAction();
+ TabletsDistributionAction(ExecEnv* exec_env);
+
+ virtual ~TabletsDistributionAction() {}
+
void handle(HttpRequest* req) override;
+
EasyJson get_tablets_distribution_group_by_partition(uint64_t
partition_id);
+
std::string host() { return _host; }
private:
std::string _host;
+
+ ExecEnv* _exec_env;
Review Comment:
warning: private field '_exec_env' is not used
[clang-diagnostic-unused-private-field]
```cpp
ExecEnv* _exec_env;
^
```
##########
be/src/http/action/tablet_migration_action.h:
##########
@@ -21,20 +21,30 @@
#include "common/status.h"
#include "gen_cpp/Status_types.h"
-#include "http/http_handler.h"
+#include "http/http_handler_with_auth.h"
#include "olap/data_dir.h"
#include "olap/tablet.h"
#include "util/threadpool.h"
namespace doris {
+class ExecEnv;
+
// Migrate a tablet from a disk to another.
-class TabletMigrationAction : public HttpHandler {
+class TabletMigrationAction : public HttpHandlerWithAuth {
public:
- TabletMigrationAction();
+ TabletMigrationAction(ExecEnv* exec_env) : HttpHandlerWithAuth(exec_env) {
+ _init_migration_action();
+ }
+
+ virtual ~TabletMigrationAction() {}
Review Comment:
warning: use '= default' to define a trivial destructor
[modernize-use-equals-default]
```suggestion
virtual ~TabletMigrationAction() = default;
```
##########
be/src/http/action/compaction_action.h:
##########
@@ -61,6 +63,14 @@ class CompactionAction : public HttpHandler {
private:
CompactionActionType _type;
+
+ ExecEnv* _exec_env;
Review Comment:
warning: private field '_exec_env' is not used
[clang-diagnostic-unused-private-field]
```cpp
ExecEnv* _exec_env;
^
```
##########
be/src/http/action/check_tablet_segment_action.h:
##########
@@ -19,18 +19,32 @@
#include <string>
-#include "http/http_handler.h"
+#include "http/http_handler_with_auth.h"
#include "util/easy_json.h"
namespace doris {
-class CheckTabletSegmentAction : public HttpHandler {
+class ExecEnv;
+
+class CheckTabletSegmentAction : public HttpHandlerWithAuth {
public:
- CheckTabletSegmentAction();
+ CheckTabletSegmentAction(ExecEnv* exec_env);
+
+ virtual ~CheckTabletSegmentAction() {}
+
void handle(HttpRequest* req) override;
+
std::string host() { return _host; }
private:
std::string _host;
+
+ ExecEnv* _exec_env;
Review Comment:
warning: private field '_exec_env' is not used
[clang-diagnostic-unused-private-field]
```cpp
ExecEnv* _exec_env;
^
```
##########
be/src/http/action/config_action.h:
##########
@@ -38,9 +40,17 @@ class ConfigAction : public HttpHandler {
private:
ConfigActionType _type;
+ ExecEnv* _exec_env;
Review Comment:
warning: private field '_exec_env' is not used
[clang-diagnostic-unused-private-field]
```cpp
ExecEnv* _exec_env;
^
```
##########
be/src/http/action/checksum_action.h:
##########
@@ -19,21 +19,29 @@
#include <cstdint>
-#include "http/http_handler.h"
+#include "http/http_handler_with_auth.h"
namespace doris {
class ExecEnv;
-class ChecksumAction : public HttpHandler {
+class ChecksumAction : public HttpHandlerWithAuth {
public:
- explicit ChecksumAction();
+ explicit ChecksumAction(ExecEnv* exec_env);
virtual ~ChecksumAction() {}
void handle(HttpRequest* req) override;
private:
+ ExecEnv* _exec_env;
Review Comment:
warning: private field '_exec_env' is not used
[clang-diagnostic-unused-private-field]
```cpp
ExecEnv* _exec_env;
^
```
##########
be/src/http/action/tablet_migration_action.h:
##########
@@ -75,5 +85,14 @@
std::mutex _migration_status_mutex;
std::map<MigrationTask, std::string> _migration_tasks;
std::deque<std::pair<MigrationTask, Status>> _finished_migration_tasks;
+
+private:
+ ExecEnv* _exec_env;
Review Comment:
warning: private field '_exec_env' is not used
[clang-diagnostic-unused-private-field]
```cpp
ExecEnv* _exec_env;
^
```
##########
be/src/http/action/tablet_migration_action.h:
##########
@@ -75,5 +85,14 @@
std::mutex _migration_status_mutex;
std::map<MigrationTask, std::string> _migration_tasks;
std::deque<std::pair<MigrationTask, Status>> _finished_migration_tasks;
+
+private:
Review Comment:
warning: redundant access specifier has the same accessibility as the
previous access specifier [readability-redundant-access-specifiers]
```suggestion
```
**be/src/http/action/tablet_migration_action.h:52:** previously declared here
```cpp
private:
^
```
##########
be/src/http/action/version_action.h:
##########
@@ -18,20 +18,27 @@
#ifndef DORIS_BE_SRC_HTTP_ACTION_VERSION_ACTION_H
#define DORIS_BE_SRC_HTTP_ACTION_VERSION_ACTION_H
-#include "http/http_handler.h"
+#include "http/http_handler_with_auth.h"
namespace doris {
class ExecEnv;
// Get BE version info from http API.
-class VersionAction : public HttpHandler {
+class VersionAction : public HttpHandlerWithAuth {
public:
- VersionAction();
+ VersionAction(ExecEnv* exec_env);
~VersionAction() override = default;
void handle(HttpRequest* req) override;
+
+private:
+ ExecEnv* _exec_env;
Review Comment:
warning: private field '_exec_env' is not used
[clang-diagnostic-unused-private-field]
```cpp
ExecEnv* _exec_env;
^
```
##########
be/src/http/action/tablets_info_action.h:
##########
@@ -19,20 +19,31 @@
#include <string>
-#include "http/http_handler.h"
+#include "http/http_handler_with_auth.h"
#include "util/easy_json.h"
namespace doris {
+class ExecEnv;
+
// Get BE tablets info from http API.
-class TabletsInfoAction : public HttpHandler {
+class TabletsInfoAction : public HttpHandlerWithAuth {
public:
- TabletsInfoAction();
+ TabletsInfoAction(ExecEnv* exec_env);
+
+ virtual ~TabletsInfoAction() {}
Review Comment:
warning: prefer using 'override' or (rarely) 'final' instead of 'virtual'
[modernize-use-override]
```suggestion
~TabletsInfoAction() override {}
```
##########
be/test/http/http_auth_test.cpp:
##########
@@ -0,0 +1,90 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <gtest/gtest.h>
+
+#include "common/config.h"
+#include "http/ev_http_server.h"
+#include "http/http_channel.h"
+#include "http/http_handler.h"
+#include "http/http_handler_with_auth.h"
+#include "http/http_request.h"
+
+namespace doris {
+
+class HttpAuthTestHandler : public HttpHandlerWithAuth {
+public:
+ HttpAuthTestHandler(ExecEnv* exec_env) : HttpHandlerWithAuth(exec_env) {}
+
+ virtual ~HttpAuthTestHandler() {}
+
+ void handle(HttpRequest* req) override {}
+
+private:
+ ExecEnv* _exec_env;
Review Comment:
warning: private field '_exec_env' is not used
[clang-diagnostic-unused-private-field]
```cpp
ExecEnv* _exec_env;
^
```
##########
be/test/http/http_auth_test.cpp:
##########
@@ -0,0 +1,90 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <gtest/gtest.h>
+
+#include "common/config.h"
+#include "http/ev_http_server.h"
+#include "http/http_channel.h"
+#include "http/http_handler.h"
+#include "http/http_handler_with_auth.h"
+#include "http/http_request.h"
+
+namespace doris {
+
+class HttpAuthTestHandler : public HttpHandlerWithAuth {
+public:
+ HttpAuthTestHandler(ExecEnv* exec_env) : HttpHandlerWithAuth(exec_env) {}
+
+ virtual ~HttpAuthTestHandler() {}
+
+ void handle(HttpRequest* req) override {}
+
+private:
+ ExecEnv* _exec_env;
+
+ bool set_privilege(const HttpRequest& req, TCheckAuthRequest&
auth_request) override {
+ return !req.param("table").empty();
+ };
+};
+
+static ExecEnv env;
+static HttpAuthTestHandler s_auth_handler = HttpAuthTestHandler(&env);
Review Comment:
warning: variable type 'doris::HttpAuthTestHandler' is an abstract class
[clang-diagnostic-error]
```cpp
static HttpAuthTestHandler s_auth_handler = HttpAuthTestHandler(&env);
^
```
##########
be/src/http/action/tablets_distribution_action.h:
##########
@@ -19,20 +19,35 @@
#include <string>
-#include "http/http_handler.h"
+#include "http/http_handler_with_auth.h"
#include "util/easy_json.h"
namespace doris {
+class ExecEnv;
+
// Get BE tablets distribution info from http API.
-class TabletsDistributionAction : public HttpHandler {
+class TabletsDistributionAction : public HttpHandlerWithAuth {
public:
- TabletsDistributionAction();
+ TabletsDistributionAction(ExecEnv* exec_env);
+
+ virtual ~TabletsDistributionAction() {}
Review Comment:
warning: use '= default' to define a trivial destructor
[modernize-use-equals-default]
```suggestion
virtual ~TabletsDistributionAction() = default;
```
##########
be/test/http/http_auth_test.cpp:
##########
@@ -0,0 +1,90 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <gtest/gtest.h>
+
+#include "common/config.h"
+#include "http/ev_http_server.h"
+#include "http/http_channel.h"
+#include "http/http_handler.h"
+#include "http/http_handler_with_auth.h"
+#include "http/http_request.h"
+
+namespace doris {
+
+class HttpAuthTestHandler : public HttpHandlerWithAuth {
+public:
+ HttpAuthTestHandler(ExecEnv* exec_env) : HttpHandlerWithAuth(exec_env) {}
+
+ virtual ~HttpAuthTestHandler() {}
Review Comment:
warning: prefer using 'override' or (rarely) 'final' instead of 'virtual'
[modernize-use-override]
```suggestion
~HttpAuthTestHandler() override {}
```
##########
be/src/http/action/version_action.h:
##########
@@ -18,20 +18,27 @@
#ifndef DORIS_BE_SRC_HTTP_ACTION_VERSION_ACTION_H
#define DORIS_BE_SRC_HTTP_ACTION_VERSION_ACTION_H
Review Comment:
warning: macro is not used [clang-diagnostic-unused-macros]
```cpp
#define DORIS_BE_SRC_HTTP_ACTION_VERSION_ACTION_H
^
```
##########
be/test/http/http_auth_test.cpp:
##########
@@ -0,0 +1,90 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <gtest/gtest.h>
+
+#include "common/config.h"
+#include "http/ev_http_server.h"
+#include "http/http_channel.h"
+#include "http/http_handler.h"
+#include "http/http_handler_with_auth.h"
+#include "http/http_request.h"
+
+namespace doris {
+
+class HttpAuthTestHandler : public HttpHandlerWithAuth {
+public:
+ HttpAuthTestHandler(ExecEnv* exec_env) : HttpHandlerWithAuth(exec_env) {}
+
+ virtual ~HttpAuthTestHandler() {}
Review Comment:
warning: use '= default' to define a trivial destructor
[modernize-use-equals-default]
```suggestion
virtual ~HttpAuthTestHandler() = default;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
