You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@streampark.apache.org by GitBox <gi...@apache.org> on 2022/10/13 11:51:25 UTC
[GitHub] [incubator-streampark] macksonmu opened a new pull request, #1831: [Feature] Add basic functionality of variable modules such as create,…
macksonmu opened a new pull request, #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831
… modify, delete, query and search #1779
<!--
Thank you for contributing to StreamPark! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
## Contribution Checklist
- If this is your first time, please read our contributor guidelines: [Submit Code](https://streampark.apache.org/community/submit_guide/submit_code).
- Make sure that the pull request corresponds to a [GITHUB issue](https://github.com/apache/streampark/issues).
- Name the pull request in the form "[Feature] Title of the pull request", where *Feature* can be replaced by `Hotfix`, `Bug`, etc.
- Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review.
- If the PR is unfinished, add `[WIP]` in your PR title, e.g., `[WIP][Feature] Title of the pull request`.
-->
## What changes were proposed in this pull request
Issue Number: close #1779 <!-- REMOVE this line if no issue to close -->
Related issues #1477
<!--(For example: This pull request proposed to add checkstyle plugin).-->
## Brief change log
<!--*(for example:)*
- *Add maven-checkstyle-plugin to root pom.xml*
-->
## Verifying this change
<!--*(Please pick either of the following options)*-->
This change is a trivial rework / code cleanup without any test coverage.
*(or)*
This change is already covered by existing tests, such as *(please describe tests)*.
*(or)*
This change added tests and can be verified as follows:
<!--*(example:)*
- *Added integration tests for end-to-end.*
- *Added *Test to verify the change.*
- *Manually verified the change by testing locally.* -->
## Does this pull request potentially affect one of the following parts
- Dependencies (does it add or upgrade a dependency): (yes / no)
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r994791170
##########
streampark-console/streampark-console-service/src/assembly/script/schema/mysql-schema.sql:
##########
@@ -319,6 +319,24 @@ create table `t_team` (
unique key `team_name_idx` (`team_name`) using btree
) engine = innodb default charset = utf8mb4 collate = utf8mb4_general_ci;
+-- ----------------------------
+-- Table of t_variable
+-- ----------------------------
+drop table if exists `t_variable`;
+create table `t_variable` (
+ `id` bigint not null auto_increment,
+ `variable_code` varchar(100) collate utf8mb4_general_ci not null comment 'variable code',
+ `variable_value` varchar(1024) collate utf8mb4_general_ci not null comment 'variable value',
+ `variable_name` varchar(100) collate utf8mb4_general_ci not null comment 'variable name',
Review Comment:
"kafka_cluster" is variable_code, it will act as a placeholder
"kafka集群" is variable_name
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r994909581
##########
streampark-console/streampark-console-service/src/assembly/script/schema/mysql-schema.sql:
##########
@@ -319,6 +319,24 @@ create table `t_team` (
unique key `team_name_idx` (`team_name`) using btree
) engine = innodb default charset = utf8mb4 collate = utf8mb4_general_ci;
+-- ----------------------------
+-- Table of t_variable
+-- ----------------------------
+drop table if exists `t_variable`;
+create table `t_variable` (
+ `id` bigint not null auto_increment,
+ `variable_code` varchar(100) collate utf8mb4_general_ci not null comment 'variable code',
Review Comment:
Ok, I'll describe it in more detail, like it's used as a parameter name or variable placeholder
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996294560
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/entity/Variable.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.entity;
+
+import com.baomidou.mybatisplus.annotation.IdType;
+import com.baomidou.mybatisplus.annotation.TableId;
+import com.baomidou.mybatisplus.annotation.TableName;
+import lombok.Data;
+
+import javax.validation.constraints.NotBlank;
+import javax.validation.constraints.NotNull;
+import javax.validation.constraints.Size;
+
+import java.io.Serializable;
+import java.util.Date;
+
+@Data
+@TableName("t_variable")
+public class Variable implements Serializable {
+
+ private static final long serialVersionUID = -7720746591258904369L;
+
+ @TableId(type = IdType.AUTO)
+ private Long id;
+
+ @NotBlank(message = "{required}")
+ private String variableCode;
+
+ @NotBlank(message = "{required}")
+ @Size(max = 50, message = "{noMoreThan}")
+ private String variableValue;
+
+ @Size(max = 100, message = "{noMoreThan}")
+ private String description;
+
+ private Long creator;
+
+ private transient String userName;
+
+ @NotNull(message = "{required}")
+ private Long teamId;
+
+ private transient String teamName;
Review Comment:
Good suggestion, I'll deal with it.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996293081
##########
streampark-console/streampark-console-service/src/assembly/script/upgrade/mysql/1.2.4.sql:
##########
@@ -201,5 +205,21 @@ alter table `t_user`
modify `username` varchar(255) collate utf8mb4_general_ci not null comment 'user name',
add unique key `un_username` (`username`) using btree;
+drop table if exists `t_variable`;
+create table `t_variable` (
+ `id` bigint not null auto_increment,
+ `variable_code` varchar(100) collate utf8mb4_general_ci not null comment 'Variable code is used for parameter names passed to the program or as placeholders',
+ `variable_value` text collate utf8mb4_general_ci not null comment 'The specific value corresponding to the variable',
+ `variable_name` varchar(100) collate utf8mb4_general_ci not null comment 'The name of the variable is used for simple display and for searching',
Review Comment:
Sorry, I missed that, I'll deal with it right away
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
wolfboys commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996305233
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/VariableController.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.controller;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.domain.RestResponse;
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shiro.authz.annotation.RequiresPermissions;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.DeleteMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotBlank;
+
+import java.util.List;
+
+@Slf4j
+@Validated
+@RestController
+@RequestMapping("variable")
+public class VariableController {
+
+ @Autowired
+ private CommonService commonService;
+
+ @Autowired
+ private VariableService variableService;
+
+ @PostMapping("list")
+ @RequiresPermissions("variable:view")
+ public RestResponse variableList(RestRequest restRequest, Variable variable) {
+ IPage<Variable> variableList = variableService.findVariables(variable, restRequest);
+ return RestResponse.success(variableList);
+ }
+
+ @PostMapping("post")
+ @RequiresPermissions("variable:add")
+ public RestResponse addVariable(@Valid Variable variable) throws Exception {
+ boolean isExists = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode()) != null;
+ if (isExists) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ variable.setCreator(commonService.getCurrentUser().getUserId());
+ this.variableService.createVariable(variable);
+ return RestResponse.success();
+ }
+
+ @PutMapping("update")
+ @RequiresPermissions("variable:update")
+ public RestResponse updateVariable(@Valid Variable variable) throws Exception {
+ Variable findVariable = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
Review Comment:
> I considered possible api calls, so I judged more, whether the id will be wrong when calling the api
can you give an example?
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996293537
##########
streampark-console/streampark-console-service/src/assembly/script/data/mysql-data.sql:
##########
@@ -104,7 +104,10 @@ insert into `t_menu` values (100050, 100048, 'update', null, null, 'member:updat
insert into `t_menu` values (100051, 100048, 'delete', null, null, 'member:delete', null, '1', 1, null, now(), now());
insert into `t_menu` values (100052, 100048, 'role view', null, null, 'role:view', null, '1', 1, null, now(), now());
insert into `t_menu` values (100053, 100001, 'types', null, null, 'user:types', null, '1', 1, null, now(), now());
-
+insert into `t_menu` VALUES (100054, 100000, 'Variable Management', '/system/variable', 'system/variable/View', 'variable:view', 'code', '0', 1, 3, now(), now());
+insert into `t_menu` VALUES (100055, 100054, 'add', NULL, NULL, 'variable:add', NULL, '1', 1, NULL, now(), now());
+insert into `t_menu` VALUES (100056, 100054, 'update', NULL, NULL, 'variable:update', NULL, '1', 1, NULL, now(), now());
+insert into `t_menu` VALUES (100057, 100054, 'delete', NULL, NULL, 'variable:delete', NULL, '1', 1, NULL, now(), now());
Review Comment:
I don't think developers can add, modify and delete variables, only the admin role has permissions, what do you think?
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#issuecomment-1277494145
@wolfboys @1996fanrui
Add basic functionality of variable modules such as create, modify, delete, query and search;
Later, I will submit 2 to 3 pull requests to complete all the functions, so that it is convenient to submit a pull request a day, which can be fully tested and easily reviewed
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] 1996fanrui commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
1996fanrui commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r994846644
##########
streampark-console/streampark-console-service/src/assembly/script/schema/mysql-schema.sql:
##########
@@ -319,6 +319,24 @@ create table `t_team` (
unique key `team_name_idx` (`team_name`) using btree
) engine = innodb default charset = utf8mb4 collate = utf8mb4_general_ci;
+-- ----------------------------
+-- Table of t_variable
+-- ----------------------------
+drop table if exists `t_variable`;
+create table `t_variable` (
+ `id` bigint not null auto_increment,
+ `variable_code` varchar(100) collate utf8mb4_general_ci not null comment 'variable code',
+ `variable_value` varchar(1024) collate utf8mb4_general_ci not null comment 'variable value',
+ `variable_name` varchar(100) collate utf8mb4_general_ci not null comment 'variable name',
Review Comment:
> variable_code: collect_kafka_cluster(When creating an application, you can add variables by searching for this code) variable_value: 192.169.1.10:9092,192.169.1.11:9092 variable_name: 数据采集kafka(When creating an application, you can add variables by searching for this name, you may not be able to remember the code, but you can remember the name.) description: 50 nodes, data retention is 14 days
Hi @macksonmu @wolfboys
I'm sorry. I don't think so. why need the variable_name. From your example, I think 数据采集kafka is the description.
Do you think so?
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
wolfboys commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996332776
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/entity/Variable.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.entity;
+
+import com.baomidou.mybatisplus.annotation.IdType;
+import com.baomidou.mybatisplus.annotation.TableId;
+import com.baomidou.mybatisplus.annotation.TableName;
+import lombok.Data;
+
+import javax.validation.constraints.NotBlank;
+import javax.validation.constraints.NotNull;
+import javax.validation.constraints.Size;
+
+import java.io.Serializable;
+import java.util.Date;
+
+@Data
+@TableName("t_variable")
+public class Variable implements Serializable {
+
+ private static final long serialVersionUID = -7720746591258904369L;
+
+ @TableId(type = IdType.AUTO)
+ private Long id;
+
+ @NotBlank(message = "{required}")
+ private String variableCode;
+
+ @NotBlank(message = "{required}")
+ @Size(max = 50, message = "{noMoreThan}")
+ private String variableValue;
+
+ @Size(max = 100, message = "{noMoreThan}")
+ private String description;
+
+ private Long creator;
Review Comment:
It needs to be consistent with other tables, change field to `userId`, and the table column change to `user_id` ,make sure the comment clearly.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] 1996fanrui commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
1996fanrui commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996381931
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/entity/Variable.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.entity;
+
+import com.baomidou.mybatisplus.annotation.IdType;
+import com.baomidou.mybatisplus.annotation.TableId;
+import com.baomidou.mybatisplus.annotation.TableName;
+import lombok.Data;
+
+import javax.validation.constraints.NotBlank;
+import javax.validation.constraints.NotNull;
+import javax.validation.constraints.Size;
+
+import java.io.Serializable;
+import java.util.Date;
+
+@Data
+@TableName("t_variable")
+public class Variable implements Serializable {
+
+ private static final long serialVersionUID = -7720746591258904369L;
+
+ @TableId(type = IdType.AUTO)
+ private Long id;
+
+ @NotBlank(message = "{required}")
+ private String variableCode;
+
+ @NotBlank(message = "{required}")
+ @Size(max = 50, message = "{noMoreThan}")
+ private String variableValue;
+
+ @Size(max = 100, message = "{noMoreThan}")
+ private String description;
+
+ private Long creator;
Review Comment:
How about creatorId?
In other places, roleId is clear. My concern is why these field should be same in the different table. It isn't necessary.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] 1996fanrui commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
1996fanrui commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996382266
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/entity/Variable.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.entity;
+
+import com.baomidou.mybatisplus.annotation.IdType;
+import com.baomidou.mybatisplus.annotation.TableId;
+import com.baomidou.mybatisplus.annotation.TableName;
+import lombok.Data;
+
+import javax.validation.constraints.NotBlank;
+import javax.validation.constraints.NotNull;
+import javax.validation.constraints.Size;
+
+import java.io.Serializable;
+import java.util.Date;
+
+@Data
+@TableName("t_variable")
+public class Variable implements Serializable {
+
+ private static final long serialVersionUID = -7720746591258904369L;
+
+ @TableId(type = IdType.AUTO)
+ private Long id;
+
+ @NotBlank(message = "{required}")
+ private String variableCode;
+
+ @NotBlank(message = "{required}")
+ @Size(max = 50, message = "{noMoreThan}")
+ private String variableValue;
+
+ @Size(max = 100, message = "{noMoreThan}")
+ private String description;
+
+ private Long creator;
Review Comment:
In other word, in some places, userId is clear. But at here, It isn't clear.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996313165
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/impl/VariableServiceImpl.java:
##########
@@ -0,0 +1,120 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.service.impl;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.ApplicationService;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.mapper.VariableMapper;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.conditions.query.LambdaQueryWrapper;
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
+import com.baomidou.mybatisplus.extension.service.impl.ServiceImpl;
+import lombok.SneakyThrows;
+import lombok.extern.slf4j.Slf4j;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Propagation;
+import org.springframework.transaction.annotation.Transactional;
+
+import java.util.List;
+
+@Slf4j
+@Service
+@Transactional(propagation = Propagation.SUPPORTS, readOnly = true, rollbackFor = Exception.class)
+public class VariableServiceImpl extends ServiceImpl<VariableMapper, Variable> implements VariableService {
+
+ @Autowired
+ private ApplicationService applicationService;
+
+ @Autowired
+ private CommonService commonService;
+
+ @Override
+ @Transactional(rollbackFor = Exception.class)
+ public void createVariable(Variable variable) throws Exception {
+ if (isExists(variable)) {
Review Comment:
I deleted isExists method.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
wolfboys commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996334309
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/VariableController.java:
##########
@@ -0,0 +1,87 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.controller;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.domain.RestResponse;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shiro.authz.annotation.RequiresPermissions;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.DeleteMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotBlank;
+
+@Slf4j
+@Validated
+@RestController
+@RequestMapping("variable")
+public class VariableController {
+
+ @Autowired
+ private VariableService variableService;
+
+ @PostMapping("list")
+ @RequiresPermissions("variable:view")
+ public RestResponse variableList(RestRequest restRequest, Variable variable) {
+ IPage<Variable> variableList = variableService.findVariables(variable, restRequest);
+ return RestResponse.success(variableList);
+ }
+
+ @PostMapping("post")
+ @RequiresPermissions("variable:add")
+ public RestResponse addVariable(@Valid Variable variable) throws Exception {
+ this.variableService.createVariable(variable);
+ return RestResponse.success();
+ }
+
+ @PutMapping("update")
+ @RequiresPermissions("variable:update")
+ public RestResponse updateVariable(@Valid Variable variable) throws Exception {
+ this.variableService.updateById(variable);
Review Comment:
oh, before do update, We need to ensure:
1. id cannot be null
2 variable_core cannot already exist (except itself)
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
wolfboys commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r994860222
##########
streampark-console/streampark-console-service/src/assembly/script/schema/mysql-schema.sql:
##########
@@ -319,6 +319,24 @@ create table `t_team` (
unique key `team_name_idx` (`team_name`) using btree
) engine = innodb default charset = utf8mb4 collate = utf8mb4_general_ci;
+-- ----------------------------
+-- Table of t_variable
+-- ----------------------------
+drop table if exists `t_variable`;
+create table `t_variable` (
+ `id` bigint not null auto_increment,
+ `variable_code` varchar(100) collate utf8mb4_general_ci not null comment 'variable code',
+ `variable_value` varchar(1024) collate utf8mb4_general_ci not null comment 'variable value',
+ `variable_name` varchar(100) collate utf8mb4_general_ci not null comment 'variable name',
Review Comment:
> > variable_code: collect_kafka_cluster(When creating an application, you can add variables by searching for this code) variable_value: 192.169.1.10:9092,192.169.1.11:9092 variable_name: 数据采集kafka(When creating an application, you can add variables by searching for this name, you may not be able to remember the code, but you can remember the name.) description: 50 nodes, data retention is 14 days
>
> Hi @macksonmu @wolfboys I'm sorry. I don't think so. why need the variable_name. From your example, I think 数据采集kafka is the description.
>
> Do you think so?
variable_name is the title of the variable. Just like jobName userName, For front-end table display.. As the name implies, I don't know how to explain it
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
wolfboys commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996384109
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/entity/Variable.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.entity;
+
+import com.baomidou.mybatisplus.annotation.IdType;
+import com.baomidou.mybatisplus.annotation.TableId;
+import com.baomidou.mybatisplus.annotation.TableName;
+import lombok.Data;
+
+import javax.validation.constraints.NotBlank;
+import javax.validation.constraints.NotNull;
+import javax.validation.constraints.Size;
+
+import java.io.Serializable;
+import java.util.Date;
+
+@Data
+@TableName("t_variable")
+public class Variable implements Serializable {
+
+ private static final long serialVersionUID = -7720746591258904369L;
+
+ @TableId(type = IdType.AUTO)
+ private Long id;
+
+ @NotBlank(message = "{required}")
+ private String variableCode;
+
+ @NotBlank(message = "{required}")
+ @Size(max = 50, message = "{noMoreThan}")
+ private String variableValue;
+
+ @Size(max = 100, message = "{noMoreThan}")
+ private String description;
+
+ private Long creator;
Review Comment:
> In other word, in some places, userId is clear. But at here, It isn't clear.
>
> Especially teamId in the user table. It is very ambiguous. For other developers, they will think the user belong to the teamId.
>
> We import some ambiguous code to reduce some code. It doesn't make sense.
>
> Or could you ask some decelopers in our WeChat group? If most of them can understand what's the teamId mean in the user table. I will agree your idea.
>
> Code that most contributors can understand is good code. Not only developers and reviewers can understand.
>
> Actually, I said this problem before.
>
> [#1792 (comment)](https://github.com/apache/incubator-streampark/pull/1792#discussion_r991347657)
After this pr is merged, we will start code cleaning and deal with such problems uniformly
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r994909935
##########
streampark-console/streampark-console-service/src/assembly/script/upgrade/mysql/1.2.4.sql:
##########
@@ -201,5 +205,21 @@ alter table `t_user`
modify `username` varchar(255) collate utf8mb4_general_ci not null comment 'user name',
add unique key `un_username` (`username`) using btree;
+drop table if exists `t_variable`;
+create table `t_variable` (
+ `id` bigint not null auto_increment,
+ `variable_code` varchar(100) collate utf8mb4_general_ci not null comment 'variable code',
+ `variable_value` varchar(1024) collate utf8mb4_general_ci not null comment 'variable value',
Review Comment:
I agree
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r994819093
##########
streampark-console/streampark-console-service/src/assembly/script/schema/mysql-schema.sql:
##########
@@ -319,6 +319,24 @@ create table `t_team` (
unique key `team_name_idx` (`team_name`) using btree
) engine = innodb default charset = utf8mb4 collate = utf8mb4_general_ci;
+-- ----------------------------
+-- Table of t_variable
+-- ----------------------------
+drop table if exists `t_variable`;
+create table `t_variable` (
+ `id` bigint not null auto_increment,
+ `variable_code` varchar(100) collate utf8mb4_general_ci not null comment 'variable code',
+ `variable_value` varchar(1024) collate utf8mb4_general_ci not null comment 'variable value',
+ `variable_name` varchar(100) collate utf8mb4_general_ci not null comment 'variable name',
Review Comment:
variable_code: collect_kafka_cluster(When creating an application, you can add variables by searching for this code)
variable_value: 192.169.1.10:9092,192.169.1.11:9092
variable_name: 数据采集kafka(When creating an application, you can add variables by searching for this name, you may not be able to remember the code, but you can remember the name.)
description: 50 nodes, data retention is 14 days
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r994796756
##########
streampark-console/streampark-console-service/src/assembly/script/schema/mysql-schema.sql:
##########
@@ -319,6 +319,24 @@ create table `t_team` (
unique key `team_name_idx` (`team_name`) using btree
) engine = innodb default charset = utf8mb4 collate = utf8mb4_general_ci;
+-- ----------------------------
+-- Table of t_variable
+-- ----------------------------
+drop table if exists `t_variable`;
+create table `t_variable` (
+ `id` bigint not null auto_increment,
+ `variable_code` varchar(100) collate utf8mb4_general_ci not null comment 'variable code',
+ `variable_value` varchar(1024) collate utf8mb4_general_ci not null comment 'variable value',
+ `variable_name` varchar(100) collate utf8mb4_general_ci not null comment 'variable name',
+ `description` varchar(100) collate utf8mb4_general_ci default null comment 'description',
+ `user_id` bigint collate utf8mb4_general_ci not null comment 'user id',
Review Comment:
Are you referring to comment or fields?
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
wolfboys commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r994835741
##########
streampark-console/streampark-console-service/src/assembly/script/schema/mysql-schema.sql:
##########
@@ -319,6 +319,24 @@ create table `t_team` (
unique key `team_name_idx` (`team_name`) using btree
) engine = innodb default charset = utf8mb4 collate = utf8mb4_general_ci;
+-- ----------------------------
+-- Table of t_variable
+-- ----------------------------
+drop table if exists `t_variable`;
+create table `t_variable` (
+ `id` bigint not null auto_increment,
+ `variable_code` varchar(100) collate utf8mb4_general_ci not null comment 'variable code',
+ `variable_value` varchar(1024) collate utf8mb4_general_ci not null comment 'variable value',
+ `variable_name` varchar(100) collate utf8mb4_general_ci not null comment 'variable name',
Review Comment:
> variable_code: collect_kafka_cluster(When creating an application, you can add variables by searching for this code) variable_value: 192.169.1.10:9092,192.169.1.11:9092 variable_name: 数据采集kafka(When creating an application, you can add variables by searching for this name, you may not be able to remember the code, but you can remember the name.) description: 50 nodes, data retention is 14 days
looks good.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] 1996fanrui commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
1996fanrui commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996374019
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/entity/Variable.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.entity;
+
+import com.baomidou.mybatisplus.annotation.IdType;
+import com.baomidou.mybatisplus.annotation.TableId;
+import com.baomidou.mybatisplus.annotation.TableName;
+import lombok.Data;
+
+import javax.validation.constraints.NotBlank;
+import javax.validation.constraints.NotNull;
+import javax.validation.constraints.Size;
+
+import java.io.Serializable;
+import java.util.Date;
+
+@Data
+@TableName("t_variable")
+public class Variable implements Serializable {
+
+ private static final long serialVersionUID = -7720746591258904369L;
+
+ @TableId(type = IdType.AUTO)
+ private Long id;
+
+ @NotBlank(message = "{required}")
+ private String variableCode;
+
+ @NotBlank(message = "{required}")
+ @Size(max = 50, message = "{noMoreThan}")
+ private String variableValue;
+
+ @Size(max = 100, message = "{noMoreThan}")
+ private String description;
+
+ private Long creator;
Review Comment:
> It needs to be consistent with other tables
Sorry, I cannot get any advantage. The creator is the really business mean at here. User id is just the value.
If the field name is userId, it's not clear. Other develops don't know what is the relationship between user id and variables?
Creator is more clear.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996295167
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/entity/Variable.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.entity;
+
+import com.baomidou.mybatisplus.annotation.IdType;
+import com.baomidou.mybatisplus.annotation.TableId;
+import com.baomidou.mybatisplus.annotation.TableName;
+import lombok.Data;
+
+import javax.validation.constraints.NotBlank;
+import javax.validation.constraints.NotNull;
+import javax.validation.constraints.Size;
+
+import java.io.Serializable;
+import java.util.Date;
+
+@Data
+@TableName("t_variable")
+public class Variable implements Serializable {
+
+ private static final long serialVersionUID = -7720746591258904369L;
+
+ @TableId(type = IdType.AUTO)
+ private Long id;
+
+ @NotBlank(message = "{required}")
+ private String variableCode;
+
+ @NotBlank(message = "{required}")
+ @Size(max = 50, message = "{noMoreThan}")
+ private String variableValue;
+
+ @Size(max = 100, message = "{noMoreThan}")
+ private String description;
+
+ private Long creator;
+
+ private transient String userName;
Review Comment:
ok i agree
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996306942
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/VariableController.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.controller;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.domain.RestResponse;
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shiro.authz.annotation.RequiresPermissions;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.DeleteMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotBlank;
+
+import java.util.List;
+
+@Slf4j
+@Validated
+@RestController
+@RequestMapping("variable")
+public class VariableController {
+
+ @Autowired
+ private CommonService commonService;
+
+ @Autowired
+ private VariableService variableService;
+
+ @PostMapping("list")
+ @RequiresPermissions("variable:view")
+ public RestResponse variableList(RestRequest restRequest, Variable variable) {
+ IPage<Variable> variableList = variableService.findVariables(variable, restRequest);
+ return RestResponse.success(variableList);
+ }
+
+ @PostMapping("post")
+ @RequiresPermissions("variable:add")
+ public RestResponse addVariable(@Valid Variable variable) throws Exception {
+ boolean isExists = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode()) != null;
+ if (isExists) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ variable.setCreator(commonService.getCurrentUser().getUserId());
+ this.variableService.createVariable(variable);
+ return RestResponse.success();
+ }
+
+ @PutMapping("update")
+ @RequiresPermissions("variable:update")
+ public RestResponse updateVariable(@Valid Variable variable) throws Exception {
+ Variable findVariable = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ throw new ApiAlertException("Sorry, the variable does not exist.");
+ }
+ if (findVariable.getId().longValue() != variable.getId().longValue()) {
+ throw new ApiAlertException("Sorry, the variable id is inconsistent.");
+ }
+ this.variableService.updateVariable(variable);
+ return RestResponse.success();
+ }
+
+ @DeleteMapping("delete")
+ @RequiresPermissions("variable:delete")
+ public RestResponse deleteVariables(@Valid Variable variable) {
+ Variable findVariable = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
Review Comment:
Maybe I think too much, there may not be interface calls in the future, I use id to update and delete instead.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996304988
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/VariableController.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.controller;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.domain.RestResponse;
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shiro.authz.annotation.RequiresPermissions;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.DeleteMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotBlank;
+
+import java.util.List;
+
+@Slf4j
+@Validated
+@RestController
+@RequestMapping("variable")
+public class VariableController {
+
+ @Autowired
+ private CommonService commonService;
+
+ @Autowired
+ private VariableService variableService;
+
+ @PostMapping("list")
+ @RequiresPermissions("variable:view")
+ public RestResponse variableList(RestRequest restRequest, Variable variable) {
+ IPage<Variable> variableList = variableService.findVariables(variable, restRequest);
+ return RestResponse.success(variableList);
+ }
+
+ @PostMapping("post")
+ @RequiresPermissions("variable:add")
+ public RestResponse addVariable(@Valid Variable variable) throws Exception {
+ boolean isExists = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode()) != null;
+ if (isExists) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ variable.setCreator(commonService.getCurrentUser().getUserId());
+ this.variableService.createVariable(variable);
+ return RestResponse.success();
+ }
+
+ @PutMapping("update")
+ @RequiresPermissions("variable:update")
+ public RestResponse updateVariable(@Valid Variable variable) throws Exception {
+ Variable findVariable = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
Review Comment:
I considered possible api calls, so I judged more, whether the id will be wrong when calling the api, do you think this is necessary?
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r994905811
##########
streampark-console/streampark-console-service/src/assembly/script/schema/mysql-schema.sql:
##########
@@ -319,6 +319,24 @@ create table `t_team` (
unique key `team_name_idx` (`team_name`) using btree
) engine = innodb default charset = utf8mb4 collate = utf8mb4_general_ci;
+-- ----------------------------
+-- Table of t_variable
+-- ----------------------------
+drop table if exists `t_variable`;
+create table `t_variable` (
+ `id` bigint not null auto_increment,
+ `variable_code` varchar(100) collate utf8mb4_general_ci not null comment 'variable code',
+ `variable_value` varchar(1024) collate utf8mb4_general_ci not null comment 'variable value',
+ `variable_name` varchar(100) collate utf8mb4_general_ci not null comment 'variable name',
Review Comment:
@1996fanrui For example, the variable code is "external.kafka.collect.brokers", and the variable name is "外部采集整合kafka". When we created the application, I forgot what the variable code was, but I knew it was called "采集整合", so I can search Enter the "采集整合" keywords in the box, and of course you can also search for kafka keywords.
At the same time, I want to know more detailed information about this kafka, such as who the operator is, what is the data retention period, and how big is the cluster size, but I can't use the description to search, so I can only search or variable code or variable name, and the variable name is also for a simple display, is this feasible?
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996295729
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/VariableController.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.controller;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.domain.RestResponse;
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shiro.authz.annotation.RequiresPermissions;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.DeleteMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotBlank;
+
+import java.util.List;
+
+@Slf4j
+@Validated
+@RestController
+@RequestMapping("variable")
+public class VariableController {
+
+ @Autowired
+ private CommonService commonService;
+
+ @Autowired
+ private VariableService variableService;
+
+ @PostMapping("list")
+ @RequiresPermissions("variable:view")
+ public RestResponse variableList(RestRequest restRequest, Variable variable) {
+ IPage<Variable> variableList = variableService.findVariables(variable, restRequest);
+ return RestResponse.success(variableList);
+ }
+
+ @PostMapping("post")
+ @RequiresPermissions("variable:add")
+ public RestResponse addVariable(@Valid Variable variable) throws Exception {
+ boolean isExists = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode()) != null;
+ if (isExists) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ variable.setCreator(commonService.getCurrentUser().getUserId());
+ this.variableService.createVariable(variable);
+ return RestResponse.success();
+ }
+
+ @PutMapping("update")
+ @RequiresPermissions("variable:update")
+ public RestResponse updateVariable(@Valid Variable variable) throws Exception {
+ Variable findVariable = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ throw new ApiAlertException("Sorry, the variable does not exist.");
+ }
+ if (findVariable.getId().longValue() != variable.getId().longValue()) {
+ throw new ApiAlertException("Sorry, the variable id is inconsistent.");
+ }
+ this.variableService.updateVariable(variable);
Review Comment:
> This logic is wired, in general, we always findById, then check `findVariable.teamId== variable.teamId && findVariable.getVariableCode() == variable.getVariableCode()`.
>
> Also, I'm not sure these checks should in Controller or Service. @wolfboys Do we have any related specifications before?
@1996fanrui I will replace these checks with variableService.checkParam(variable)
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996311760
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/VariableService.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.service;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.system.entity.Variable;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import com.baomidou.mybatisplus.extension.service.IService;
+
+import java.util.List;
+
+public interface VariableService extends IService<Variable> {
+
+ /**
+ * find variable
+ *
+ * @param variable variable
+ * @param restRequest queryRequest
+ * @return IPage
+ */
+ IPage<Variable> findVariables(Variable variable, RestRequest restRequest);
+
+ /**
+ * get variables through team
+ * @param teamId
+ * @return
+ */
+ List<Variable> findByTeamId(Long teamId);
+
+ /**
+ * create variable
+ *
+ * @param variable variable
+ */
+ void createVariable(Variable variable) throws Exception;
+
+ /**
+ * delete variable
+ *
+ * @param variable variable
+ */
+ void deleteVariable(Variable variable) throws Exception;
+
+ /**
+ * update variable
+ *
+ * @param variable variable
+ */
+ void updateVariable(Variable variable) throws Exception;
+
+ Variable findByVariableCode(Long teamId, String variableCode);
+
+ boolean isExists(Variable variable);
+
+ List<Application> findDependByCode(Variable variable);
Review Comment:
I deleted this judgment, I will judge in the next PR, because it is indeed related to the next PR.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#issuecomment-1279759400
Maybe there are still some small problems in the front end, I will test it again later, and submit PR if there are any problems.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
wolfboys commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996383676
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/impl/VariableServiceImpl.java:
##########
@@ -0,0 +1,79 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.service.impl;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.base.mybatis.pager.MybatisPager;
+import org.apache.streampark.console.core.service.ApplicationService;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.mapper.VariableMapper;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.conditions.query.LambdaQueryWrapper;
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
+import com.baomidou.mybatisplus.extension.service.impl.ServiceImpl;
+import lombok.extern.slf4j.Slf4j;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Propagation;
+import org.springframework.transaction.annotation.Transactional;
+
+import java.util.List;
+
+@Slf4j
+@Service
+@Transactional(propagation = Propagation.SUPPORTS, readOnly = true, rollbackFor = Exception.class)
+public class VariableServiceImpl extends ServiceImpl<VariableMapper, Variable> implements VariableService {
+
+ @Autowired
+ private ApplicationService applicationService;
+
+ @Autowired
+ private CommonService commonService;
+
+ @Override
+ @Transactional(rollbackFor = Exception.class)
+ public void createVariable(Variable variable) throws Exception {
+ if (this.findByVariableCode(variable.getTeamId(), variable.getVariableCode()) != null) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ variable.setUserId(commonService.getCurrentUser().getUserId());
Review Comment:
can be improved to:
`variable.setUserId(commonService.getUserId());`
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
wolfboys commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996303833
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/VariableController.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.controller;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.domain.RestResponse;
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shiro.authz.annotation.RequiresPermissions;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.DeleteMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotBlank;
+
+import java.util.List;
+
+@Slf4j
+@Validated
+@RestController
+@RequestMapping("variable")
+public class VariableController {
+
+ @Autowired
+ private CommonService commonService;
+
+ @Autowired
+ private VariableService variableService;
+
+ @PostMapping("list")
+ @RequiresPermissions("variable:view")
+ public RestResponse variableList(RestRequest restRequest, Variable variable) {
+ IPage<Variable> variableList = variableService.findVariables(variable, restRequest);
+ return RestResponse.success(variableList);
+ }
+
+ @PostMapping("post")
+ @RequiresPermissions("variable:add")
+ public RestResponse addVariable(@Valid Variable variable) throws Exception {
+ boolean isExists = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode()) != null;
+ if (isExists) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ variable.setCreator(commonService.getCurrentUser().getUserId());
+ this.variableService.createVariable(variable);
+ return RestResponse.success();
+ }
+
+ @PutMapping("update")
+ @RequiresPermissions("variable:update")
+ public RestResponse updateVariable(@Valid Variable variable) throws Exception {
+ Variable findVariable = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ throw new ApiAlertException("Sorry, the variable does not exist.");
+ }
+ if (findVariable.getId().longValue() != variable.getId().longValue()) {
+ throw new ApiAlertException("Sorry, the variable id is inconsistent.");
+ }
+ this.variableService.updateVariable(variable);
+ return RestResponse.success();
+ }
+
+ @DeleteMapping("delete")
+ @RequiresPermissions("variable:delete")
+ public RestResponse deleteVariables(@Valid Variable variable) {
+ Variable findVariable = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
Review Comment:
Why not use id to modify and delete?
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/VariableController.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.controller;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.domain.RestResponse;
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shiro.authz.annotation.RequiresPermissions;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.DeleteMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotBlank;
+
+import java.util.List;
+
+@Slf4j
+@Validated
+@RestController
+@RequestMapping("variable")
+public class VariableController {
+
+ @Autowired
+ private CommonService commonService;
+
+ @Autowired
+ private VariableService variableService;
+
+ @PostMapping("list")
+ @RequiresPermissions("variable:view")
+ public RestResponse variableList(RestRequest restRequest, Variable variable) {
+ IPage<Variable> variableList = variableService.findVariables(variable, restRequest);
+ return RestResponse.success(variableList);
+ }
+
+ @PostMapping("post")
+ @RequiresPermissions("variable:add")
+ public RestResponse addVariable(@Valid Variable variable) throws Exception {
+ boolean isExists = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode()) != null;
+ if (isExists) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ variable.setCreator(commonService.getCurrentUser().getUserId());
+ this.variableService.createVariable(variable);
+ return RestResponse.success();
+ }
+
+ @PutMapping("update")
+ @RequiresPermissions("variable:update")
+ public RestResponse updateVariable(@Valid Variable variable) throws Exception {
+ Variable findVariable = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
Review Comment:
It is not recommended to use variable_code for this query, use id is a better choice, id is the primary key of the table.
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/VariableController.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.controller;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.domain.RestResponse;
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shiro.authz.annotation.RequiresPermissions;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.DeleteMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotBlank;
+
+import java.util.List;
+
+@Slf4j
+@Validated
+@RestController
+@RequestMapping("variable")
+public class VariableController {
+
+ @Autowired
+ private CommonService commonService;
+
+ @Autowired
+ private VariableService variableService;
+
+ @PostMapping("list")
+ @RequiresPermissions("variable:view")
+ public RestResponse variableList(RestRequest restRequest, Variable variable) {
+ IPage<Variable> variableList = variableService.findVariables(variable, restRequest);
+ return RestResponse.success(variableList);
+ }
+
+ @PostMapping("post")
+ @RequiresPermissions("variable:add")
+ public RestResponse addVariable(@Valid Variable variable) throws Exception {
+ boolean isExists = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode()) != null;
+ if (isExists) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ variable.setCreator(commonService.getCurrentUser().getUserId());
+ this.variableService.createVariable(variable);
+ return RestResponse.success();
+ }
+
+ @PutMapping("update")
+ @RequiresPermissions("variable:update")
+ public RestResponse updateVariable(@Valid Variable variable) throws Exception {
+ Variable findVariable = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ throw new ApiAlertException("Sorry, the variable does not exist.");
+ }
+ if (findVariable.getId().longValue() != variable.getId().longValue()) {
Review Comment:
I don't think it's necessary to add the logic here.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996312987
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/impl/VariableServiceImpl.java:
##########
@@ -0,0 +1,120 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.service.impl;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.ApplicationService;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.mapper.VariableMapper;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.conditions.query.LambdaQueryWrapper;
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
+import com.baomidou.mybatisplus.extension.service.impl.ServiceImpl;
+import lombok.SneakyThrows;
+import lombok.extern.slf4j.Slf4j;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Propagation;
+import org.springframework.transaction.annotation.Transactional;
+
+import java.util.List;
+
+@Slf4j
+@Service
+@Transactional(propagation = Propagation.SUPPORTS, readOnly = true, rollbackFor = Exception.class)
+public class VariableServiceImpl extends ServiceImpl<VariableMapper, Variable> implements VariableService {
+
+ @Autowired
+ private ApplicationService applicationService;
+
+ @Autowired
+ private CommonService commonService;
+
+ @Override
+ @Transactional(rollbackFor = Exception.class)
+ public void createVariable(Variable variable) throws Exception {
+ if (isExists(variable)) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ variable.setCreator(commonService.getCurrentUser().getUserId());
+ this.save(variable);
+ }
+
+ @Override
+ public IPage<Variable> findVariables(Variable variable, RestRequest request) {
+ Page<Variable> page = new Page<>();
+ page.setCurrent(request.getPageNum());
+ page.setSize(request.getPageSize());
+ return this.baseMapper.findVariable(page, variable);
+ }
+
+ @Override
+ public void updateVariable(Variable variable) throws Exception {
+ Variable findVariable = this.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ if (findVariable.getId().longValue() != variable.getId().longValue()) {
+ throw new ApiAlertException("Sorry, the variable id is inconsistent.");
+ }
+ updateById(variable);
+ }
+
+ @Override
+ public void deleteVariable(Variable variable) throws Exception {
+ Variable findVariable = this.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ if (findVariable.getId().longValue() != variable.getId().longValue()) {
+ throw new ApiAlertException("Sorry, the variable id is inconsistent.");
+ }
+ List<Application> dependApplications = this.findDependByCode(variable);
+ if (!(dependApplications == null || dependApplications.isEmpty())) {
+ throw new ApiAlertException(String.format("Sorry, this variable is being used by [%s] applications.", dependApplications.size()));
+ }
+ this.removeById(findVariable.getId());
+ }
+
+ @Override
+ public Variable findByVariableCode(Long teamId, String variableCode) {
+ return baseMapper.selectOne(new LambdaQueryWrapper<Variable>()
+ .eq(Variable::getVariableCode, variableCode)
+ .eq(Variable::getTeamId, teamId));
+ }
+
+ @Override
+ public List<Variable> findByTeamId(Long teamId) {
+ return baseMapper.selectByTeamId(teamId);
+ }
+
+ @Override
+ public boolean isExists(Variable variable) {
+ return this.findByVariableCode(variable.getTeamId(), variable.getVariableCode()) != null;
+ }
Review Comment:
I deleted it.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996312531
##########
streampark-console/streampark-console-webapp/src/api/index.js:
##########
@@ -209,6 +209,15 @@ export default {
UPDATE: '/menu/update',
ROUTER: '/menu/router'
},
+ Variable: {
+ LIST: '/variable/list',
+ UPDATE: '/variable/update',
+ GET: '/variable/get',
Review Comment:
you are so serious，i deleted it. ^_^
##########
streampark-console/streampark-console-webapp/src/api/variable.js:
##########
@@ -0,0 +1,51 @@
+/*
+ * 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.
+ */
+
+import api from './index'
+import http from '@/utils/request'
+
+export function list (queryParam) {
+ return http.post(api.Variable.LIST, queryParam)
+}
+
+export function update (queryParam) {
+ return http.put(api.Variable.UPDATE, queryParam)
+}
+
+export function get (queryParam) {
+ return http.get(api.Variable.GET, queryParam)
Review Comment:
you are so serious，i deleted it. ^_^
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996293095
##########
streampark-console/streampark-console-service/src/assembly/script/upgrade/mysql/1.2.4.sql:
##########
@@ -201,5 +205,21 @@ alter table `t_user`
modify `username` varchar(255) collate utf8mb4_general_ci not null comment 'user name',
add unique key `un_username` (`username`) using btree;
+drop table if exists `t_variable`;
+create table `t_variable` (
+ `id` bigint not null auto_increment,
+ `variable_code` varchar(100) collate utf8mb4_general_ci not null comment 'Variable code is used for parameter names passed to the program or as placeholders',
+ `variable_value` text collate utf8mb4_general_ci not null comment 'The specific value corresponding to the variable',
+ `variable_name` varchar(100) collate utf8mb4_general_ci not null comment 'The name of the variable is used for simple display and for searching',
+ `description` text collate utf8mb4_general_ci default null comment 'More detailed description of variables, only for display, not involved in program logic',
+ `creator` bigint collate utf8mb4_general_ci not null comment 'user_id of creator',
+ `team_id` bigint collate utf8mb4_general_ci not null comment 'team id',
+ `create_time` datetime not null default current_timestamp comment 'create time',
+ `modify_time` datetime not null default current_timestamp on update current_timestamp comment 'modify time',
+ primary key (`id`) using btree,
+ unique key `un_team_vcode_inx` (`team_id`,`variable_code`) using btree,
+ unique key `un_team_vname_inx` (`team_id`,`variable_name`) using btree
Review Comment:
Sorry, I missed that, I'll deal with it right away
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996294321
##########
streampark-console/streampark-console-webapp/src/views/system/variable/View.vue:
##########
@@ -0,0 +1,347 @@
+<!--
+
+ 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
+
+ https://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.
+
+-->
+
+<template>
+ <a-card :bordered="false">
+ <div
+ class="table-page-search-wrapper">
+ <a-form
+ layout="inline">
+ <a-row
+ :gutter="48">
+ <div
+ class="fold">
+ <a-col
+ :md="8"
+ :sm="24">
+ <a-form-item
+ label="Variable Code"
+ :label-col="{span: 4}"
+ :wrapper-col="{span: 18, offset: 2}">
+ <a-input
+ v-model="queryParams.variableCode" />
+ </a-form-item>
+ </a-col>
+ <a-col
+ :md="8"
+ :sm="24">
+ <a-form-item
+ label="Description"
+ :label-col="{span: 4}"
+ :wrapper-col="{span: 18, offset: 2}">
+ <a-input
+ v-model="queryParams.description" />
+ </a-form-item>
+ </a-col>
+ </div>
+ <a-col
+ :md="8"
+ :sm="24">
+ <span
+ class="table-page-search-bar">
+ <a-button
+ type="primary"
+ shape="circle"
+ icon="search"
+ @click="search" />
+ <a-button
+ type="primary"
+ shape="circle"
+ icon="rest"
+ @click="reset" />
+ <a-button
+ type="primary"
+ shape="circle"
+ icon="plus"
+ v-permit="'variable:add'"
+ @click="handleAdd" />
+ </span>
+ </a-col>
+ </a-row>
+ </a-form>
+ </div>
+
+ <!-- 表格区域 -->
+ <a-table
+ ref="TableInfo"
+ :columns="columns"
+ :data-source="dataSource"
+ :pagination="pagination"
+ :loading="loading"
+ :scroll="{ x: 900 }"
+ @change="handleTableChange">
+ <template
+ slot="email"
+ slot-scope="text">
+ <a-popover
+ placement="topLeft">
+ <template
+ slot="content">
+ <div>
+ {{ text }}
+ </div>
+ </template>
+ <p
+ style="width: 150px;margin-bottom: 0">
+ {{ text }}
+ </p>
+ </a-popover>
+ </template>
+ <template
+ slot="operation"
+ slot-scope="text, record">
+ <svg-icon
+ v-permit="'variable:update'"
+ v-if="(record.username !== 'admin' || userName === 'admin')"
Review Comment:
Your review is very detailed, thank you very much, I will change it immediately
##########
streampark-console/streampark-console-webapp/src/views/system/variable/View.vue:
##########
@@ -0,0 +1,347 @@
+<!--
+
+ 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
+
+ https://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.
+
+-->
+
+<template>
+ <a-card :bordered="false">
+ <div
+ class="table-page-search-wrapper">
+ <a-form
+ layout="inline">
+ <a-row
+ :gutter="48">
+ <div
+ class="fold">
+ <a-col
+ :md="8"
+ :sm="24">
+ <a-form-item
+ label="Variable Code"
+ :label-col="{span: 4}"
+ :wrapper-col="{span: 18, offset: 2}">
+ <a-input
+ v-model="queryParams.variableCode" />
+ </a-form-item>
+ </a-col>
+ <a-col
+ :md="8"
+ :sm="24">
+ <a-form-item
+ label="Description"
+ :label-col="{span: 4}"
+ :wrapper-col="{span: 18, offset: 2}">
+ <a-input
+ v-model="queryParams.description" />
+ </a-form-item>
+ </a-col>
+ </div>
+ <a-col
+ :md="8"
+ :sm="24">
+ <span
+ class="table-page-search-bar">
+ <a-button
+ type="primary"
+ shape="circle"
+ icon="search"
+ @click="search" />
+ <a-button
+ type="primary"
+ shape="circle"
+ icon="rest"
+ @click="reset" />
+ <a-button
+ type="primary"
+ shape="circle"
+ icon="plus"
+ v-permit="'variable:add'"
+ @click="handleAdd" />
+ </span>
+ </a-col>
+ </a-row>
+ </a-form>
+ </div>
+
+ <!-- 表格区域 -->
+ <a-table
+ ref="TableInfo"
+ :columns="columns"
+ :data-source="dataSource"
+ :pagination="pagination"
+ :loading="loading"
+ :scroll="{ x: 900 }"
+ @change="handleTableChange">
+ <template
+ slot="email"
+ slot-scope="text">
+ <a-popover
+ placement="topLeft">
+ <template
+ slot="content">
+ <div>
+ {{ text }}
+ </div>
+ </template>
+ <p
+ style="width: 150px;margin-bottom: 0">
+ {{ text }}
+ </p>
+ </a-popover>
+ </template>
+ <template
+ slot="operation"
+ slot-scope="text, record">
+ <svg-icon
+ v-permit="'variable:update'"
+ v-if="(record.username !== 'admin' || userName === 'admin')"
+ name="edit"
+ border
+ @click.native="handleEdit(record)"
+ title="modify" />
+ <svg-icon
+ name="see"
+ border
+ @click.native="handleView(record)"
+ title="view" />
+ <a-popconfirm
+ v-permit="'variable:delete'"
+ v-if="record.username !== 'admin'"
Review Comment:
Your review is very detailed, thank you very much, I will change it immediately
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] 1996fanrui commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
1996fanrui commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996293979
##########
streampark-console/streampark-console-service/src/assembly/script/data/mysql-data.sql:
##########
@@ -104,7 +104,10 @@ insert into `t_menu` values (100050, 100048, 'update', null, null, 'member:updat
insert into `t_menu` values (100051, 100048, 'delete', null, null, 'member:delete', null, '1', 1, null, now(), now());
insert into `t_menu` values (100052, 100048, 'role view', null, null, 'role:view', null, '1', 1, null, now(), now());
insert into `t_menu` values (100053, 100001, 'types', null, null, 'user:types', null, '1', 1, null, now(), now());
-
+insert into `t_menu` VALUES (100054, 100000, 'Variable Management', '/system/variable', 'system/variable/View', 'variable:view', 'code', '0', 1, 3, now(), now());
+insert into `t_menu` VALUES (100055, 100054, 'add', NULL, NULL, 'variable:add', NULL, '1', 1, NULL, now(), now());
+insert into `t_menu` VALUES (100056, 100054, 'update', NULL, NULL, 'variable:update', NULL, '1', 1, NULL, now(), now());
+insert into `t_menu` VALUES (100057, 100054, 'delete', NULL, NULL, 'variable:delete', NULL, '1', 1, NULL, now(), now());
Review Comment:
I think it's OK, and I'm adding the `team_admin` in #1843 .
After it is merged, `team_admin` also have the permission, do you think so too?
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996316353
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/VariableController.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.controller;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.domain.RestResponse;
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shiro.authz.annotation.RequiresPermissions;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.DeleteMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotBlank;
+
+import java.util.List;
+
+@Slf4j
+@Validated
+@RestController
+@RequestMapping("variable")
+public class VariableController {
+
+ @Autowired
+ private CommonService commonService;
+
+ @Autowired
+ private VariableService variableService;
+
+ @PostMapping("list")
+ @RequiresPermissions("variable:view")
+ public RestResponse variableList(RestRequest restRequest, Variable variable) {
+ IPage<Variable> variableList = variableService.findVariables(variable, restRequest);
+ return RestResponse.success(variableList);
+ }
+
+ @PostMapping("post")
+ @RequiresPermissions("variable:add")
+ public RestResponse addVariable(@Valid Variable variable) throws Exception {
+ boolean isExists = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode()) != null;
+ if (isExists) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ variable.setCreator(commonService.getCurrentUser().getUserId());
+ this.variableService.createVariable(variable);
+ return RestResponse.success();
+ }
+
+ @PutMapping("update")
+ @RequiresPermissions("variable:update")
+ public RestResponse updateVariable(@Valid Variable variable) throws Exception {
+ Variable findVariable = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ throw new ApiAlertException("Sorry, the variable does not exist.");
+ }
+ if (findVariable.getId().longValue() != variable.getId().longValue()) {
Review Comment:
> If id is wrong, back-end should throw exception.
>
> We should getById, and check whether immutable field is changed. If yes, throw exception.If no, it's ok. BTW, we should use the old Object, and just change some fields that can be changed.
>
> You can take a look `TeamServiceImpl.updateTeam`. From my side, this is more reasonable.
>
> ```
> @Override
> public void updateTeam(Team team) {
> Team oldTeam = Optional.ofNullable(this.getById(team))
> .orElseThrow(() -> new IllegalArgumentException(String.format("Team id [id=%s] not found", team.getId())));
> AssertUtils.isTrue(oldTeam.getTeamName().equals(team.getTeamName()), "Team name cannot be changed.");
> oldTeam.setDescription(team.getDescription());
> oldTeam.setModifyTime(new Date());
> updateById(oldTeam);
> }
> ```
Just communicated with Huajie, he said not to think so much. ^_^
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] 1996fanrui commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
1996fanrui commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996314711
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/VariableService.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.service;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.system.entity.Variable;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import com.baomidou.mybatisplus.extension.service.IService;
+
+import java.util.List;
+
+public interface VariableService extends IService<Variable> {
+
+ /**
+ * find variable
+ *
+ * @param variable variable
+ * @param restRequest queryRequest
+ * @return IPage
+ */
+ IPage<Variable> findVariables(Variable variable, RestRequest restRequest);
+
+ /**
+ * get variables through team
+ * @param teamId
+ * @return
+ */
+ List<Variable> findByTeamId(Long teamId);
+
+ /**
+ * create variable
+ *
+ * @param variable variable
+ */
+ void createVariable(Variable variable) throws Exception;
+
+ /**
+ * delete variable
+ *
+ * @param variable variable
+ */
+ void deleteVariable(Variable variable) throws Exception;
+
+ /**
+ * update variable
+ *
+ * @param variable variable
+ */
+ void updateVariable(Variable variable) throws Exception;
+
+ Variable findByVariableCode(Long teamId, String variableCode);
Review Comment:
Actually, the method is `findByTeamAndUserName` when I develop the team management. But @wolfboys change it from `findByTeamAndUserName` to `findByUserName` in #1792 . I don't know why.
Hi @wolfboys , could you help explain why? Is it a mistake, or is there another reason?
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
wolfboys commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996334309
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/VariableController.java:
##########
@@ -0,0 +1,87 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.controller;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.domain.RestResponse;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shiro.authz.annotation.RequiresPermissions;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.DeleteMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotBlank;
+
+@Slf4j
+@Validated
+@RestController
+@RequestMapping("variable")
+public class VariableController {
+
+ @Autowired
+ private VariableService variableService;
+
+ @PostMapping("list")
+ @RequiresPermissions("variable:view")
+ public RestResponse variableList(RestRequest restRequest, Variable variable) {
+ IPage<Variable> variableList = variableService.findVariables(variable, restRequest);
+ return RestResponse.success(variableList);
+ }
+
+ @PostMapping("post")
+ @RequiresPermissions("variable:add")
+ public RestResponse addVariable(@Valid Variable variable) throws Exception {
+ this.variableService.createVariable(variable);
+ return RestResponse.success();
+ }
+
+ @PutMapping("update")
+ @RequiresPermissions("variable:update")
+ public RestResponse updateVariable(@Valid Variable variable) throws Exception {
+ this.variableService.updateById(variable);
Review Comment:
oh, before do update, We need to ensure:
1. id cannot be null
2. variable_core cannot already exist (except itself)
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r994805957
##########
streampark-console/streampark-console-service/src/assembly/script/schema/mysql-schema.sql:
##########
@@ -319,6 +319,24 @@ create table `t_team` (
unique key `team_name_idx` (`team_name`) using btree
) engine = innodb default charset = utf8mb4 collate = utf8mb4_general_ci;
+-- ----------------------------
+-- Table of t_variable
+-- ----------------------------
+drop table if exists `t_variable`;
+create table `t_variable` (
+ `id` bigint not null auto_increment,
+ `variable_code` varchar(100) collate utf8mb4_general_ci not null comment 'variable code',
+ `variable_value` varchar(1024) collate utf8mb4_general_ci not null comment 'variable value',
+ `variable_name` varchar(100) collate utf8mb4_general_ci not null comment 'variable name',
+ `description` varchar(100) collate utf8mb4_general_ci default null comment 'description',
+ `user_id` bigint collate utf8mb4_general_ci not null comment 'user id',
Review Comment:
Thank you for your suggestion, I think it is ok, I will change to creator
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
wolfboys commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r995724000
##########
streampark-console/streampark-console-webapp/src/views/system/variable/View.vue:
##########
@@ -0,0 +1,348 @@
+<!--
+
+ 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
+
+ https://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.
+
+-->
+
+<template>
+ <a-card :bordered="false">
+ <div
+ class="table-page-search-wrapper">
+ <a-form
+ layout="inline">
+ <a-row
+ :gutter="48">
+ <div
+ class="fold">
+ <a-col
+ :md="8"
+ :sm="24">
+ <a-form-item
+ label="Variable Code"
+ :label-col="{span: 4}"
+ :wrapper-col="{span: 18, offset: 2}">
+ <a-input
+ v-model="queryParams.variableCode" />
+ </a-form-item>
+ </a-col>
+ <a-col
+ :md="8"
+ :sm="24">
+ <a-form-item
+ label="Variable Name"
+ :label-col="{span: 4}"
+ :wrapper-col="{span: 18, offset: 2}">
+ <a-input
+ v-model="queryParams.variableName" />
+ </a-form-item>
+ </a-col>
+ </div>
+ <a-col
+ :md="8"
+ :sm="24">
+ <span
+ class="table-page-search-bar">
+ <a-button
+ type="primary"
+ shape="circle"
+ icon="search"
+ @click="search" />
+ <a-button
+ type="primary"
+ shape="circle"
+ icon="rest"
+ @click="reset" />
+ <a-button
+ type="primary"
+ shape="circle"
+ icon="plus"
+ v-permit="'variable:add'"
+ @click="handleAdd" />
+ </span>
+ </a-col>
+ </a-row>
+ </a-form>
+ </div>
+
+ <!-- 表格区域 -->
+ <a-table
+ ref="TableInfo"
+ :columns="columns"
+ :data-source="dataSource"
+ :pagination="pagination"
+ :loading="loading"
+ :scroll="{ x: 900 }"
+ @change="handleTableChange">
+ <template
+ slot="email"
+ slot-scope="text">
+ <a-popover
+ placement="topLeft">
+ <template
+ slot="content">
+ <div>
+ {{ text }}
+ </div>
+ </template>
+ <p
+ style="width: 150px;margin-bottom: 0">
+ {{ text }}
+ </p>
+ </a-popover>
+ </template>
+ <template
+ slot="operation"
+ slot-scope="text, record">
+ <svg-icon
+ v-permit="'variable:update'"
+ v-if="(record.username !== 'admin' || userName === 'admin')"
+ name="edit"
+ border
+ @click.native="handleEdit(record)"
+ title="modify" />
+ <svg-icon
+ name="see"
+ border
+ @click.native="handleView(record)"
+ title="view" />
+ <a-popconfirm
+ v-permit="'variable:delete'"
+ v-if="record.username !== 'admin'"
+ title="Are you sure delete this variable ?"
+ cancel-text="No"
+ ok-text="Yes"
+ @confirm="handleDelete(record)">
+ <svg-icon name="remove" border/>
+ </a-popconfirm>
+ </template>
+ </a-table>
+
+ <!-- view variable -->
+ <variable-info
+ :data="variableInfo.data"
+ :visible="variableInfo.visible"
+ @close="handleVariableInfoClose" />
+ <!-- add variable -->
+ <variable-add
+ @close="handleVariableAddClose"
+ @success="handleVariableAddSuccess"
+ :visible="variableAdd.visible" />
+ <!-- edit variable -->
+ <variable-edit
+ ref="variableEdit"
+ @close="handleVariableEditClose"
+ @success="handleVariableEditSuccess"
+ :visible="variableEdit.visible" />
+ </a-card>
+</template>
+
+<script>
+import VariableInfo from './Detail'
+import VariableAdd from './Add'
+import VariableEdit from './Edit'
+import SvgIcon from '@/components/SvgIcon'
+import { list, deleteVariable} from '@/api/variable'
+
+export default {
+ name: 'Variable',
+ components: { VariableInfo, VariableAdd, VariableEdit, SvgIcon },
+ data () {
+ return {
+ variableInfo: {
+ visible: false,
+ data: {}
+ },
+ variableAdd: {
+ visible: false
+ },
+ variableEdit: {
+ visible: false
+ },
+ queryParams: {},
+ filteredInfo: null,
+ sortedInfo: null,
+ paginationInfo: null,
+ dataSource: [],
+ loading: false,
+ pagination: {
+ pageSizeOptions: ['10', '20', '30', '40', '100'],
+ defaultCurrent: 1,
+ defaultPageSize: 10,
+ showQuickJumper: true,
+ showSizeChanger: true,
+ showTotal: (total, range) => `display ${range[0]} ~ ${range[1]} records，total ${total}`
+ }
+ }
+ },
+ computed: {
+ columns () {
+ let { sortedInfo, filteredInfo } = this // eslint-disable-line no-unused-vars
+ sortedInfo = sortedInfo || {}
+ filteredInfo = filteredInfo || {}
+ return [{
+ title: 'Variable Code',
+ dataIndex: 'variableCode',
+ sorter: true,
+ sortOrder: sortedInfo.columnKey === 'variableCode' && sortedInfo.order
+ }, {
+ title: 'Variable Value',
+ dataIndex: 'variableValue'
+ }, {
+ title: 'Variable Name',
Review Comment:
Suggest `Variable Name` in the first column of the table
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996306923
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/VariableController.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.controller;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.domain.RestResponse;
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shiro.authz.annotation.RequiresPermissions;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.DeleteMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotBlank;
+
+import java.util.List;
+
+@Slf4j
+@Validated
+@RestController
+@RequestMapping("variable")
+public class VariableController {
+
+ @Autowired
+ private CommonService commonService;
+
+ @Autowired
+ private VariableService variableService;
+
+ @PostMapping("list")
+ @RequiresPermissions("variable:view")
+ public RestResponse variableList(RestRequest restRequest, Variable variable) {
+ IPage<Variable> variableList = variableService.findVariables(variable, restRequest);
+ return RestResponse.success(variableList);
+ }
+
+ @PostMapping("post")
+ @RequiresPermissions("variable:add")
+ public RestResponse addVariable(@Valid Variable variable) throws Exception {
+ boolean isExists = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode()) != null;
+ if (isExists) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ variable.setCreator(commonService.getCurrentUser().getUserId());
+ this.variableService.createVariable(variable);
+ return RestResponse.success();
+ }
+
+ @PutMapping("update")
+ @RequiresPermissions("variable:update")
+ public RestResponse updateVariable(@Valid Variable variable) throws Exception {
+ Variable findVariable = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
Review Comment:
Maybe I think too much, there may not be interface calls in the future, I use id to update and delete instead.
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/VariableController.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.controller;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.domain.RestResponse;
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shiro.authz.annotation.RequiresPermissions;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.DeleteMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotBlank;
+
+import java.util.List;
+
+@Slf4j
+@Validated
+@RestController
+@RequestMapping("variable")
+public class VariableController {
+
+ @Autowired
+ private CommonService commonService;
+
+ @Autowired
+ private VariableService variableService;
+
+ @PostMapping("list")
+ @RequiresPermissions("variable:view")
+ public RestResponse variableList(RestRequest restRequest, Variable variable) {
+ IPage<Variable> variableList = variableService.findVariables(variable, restRequest);
+ return RestResponse.success(variableList);
+ }
+
+ @PostMapping("post")
+ @RequiresPermissions("variable:add")
+ public RestResponse addVariable(@Valid Variable variable) throws Exception {
+ boolean isExists = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode()) != null;
+ if (isExists) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ variable.setCreator(commonService.getCurrentUser().getUserId());
+ this.variableService.createVariable(variable);
+ return RestResponse.success();
+ }
+
+ @PutMapping("update")
+ @RequiresPermissions("variable:update")
+ public RestResponse updateVariable(@Valid Variable variable) throws Exception {
+ Variable findVariable = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ throw new ApiAlertException("Sorry, the variable does not exist.");
+ }
+ if (findVariable.getId().longValue() != variable.getId().longValue()) {
Review Comment:
Maybe I think too much, there may not be interface calls in the future, I use id to update and delete instead.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] 1996fanrui commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
1996fanrui commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996306704
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/VariableController.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.controller;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.domain.RestResponse;
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shiro.authz.annotation.RequiresPermissions;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.DeleteMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotBlank;
+
+import java.util.List;
+
+@Slf4j
+@Validated
+@RestController
+@RequestMapping("variable")
+public class VariableController {
+
+ @Autowired
+ private CommonService commonService;
+
+ @Autowired
+ private VariableService variableService;
+
+ @PostMapping("list")
+ @RequiresPermissions("variable:view")
+ public RestResponse variableList(RestRequest restRequest, Variable variable) {
+ IPage<Variable> variableList = variableService.findVariables(variable, restRequest);
+ return RestResponse.success(variableList);
+ }
+
+ @PostMapping("post")
+ @RequiresPermissions("variable:add")
+ public RestResponse addVariable(@Valid Variable variable) throws Exception {
+ boolean isExists = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode()) != null;
+ if (isExists) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ variable.setCreator(commonService.getCurrentUser().getUserId());
+ this.variableService.createVariable(variable);
+ return RestResponse.success();
+ }
+
+ @PutMapping("update")
+ @RequiresPermissions("variable:update")
+ public RestResponse updateVariable(@Valid Variable variable) throws Exception {
+ Variable findVariable = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ throw new ApiAlertException("Sorry, the variable does not exist.");
+ }
+ if (findVariable.getId().longValue() != variable.getId().longValue()) {
Review Comment:
If id is wrong, back-end should throw exception.
We should getById, and check whether immutable field is changed. If yes, throw exception.If no, it's ok. BTW, we should use the old Object, and just change some fields that can be changed.
You can take a look `TeamServiceImpl.updateTeam`. From my side, this is more reasonable.
```
@Override
public void updateTeam(Team team) {
Team oldTeam = Optional.ofNullable(this.getById(team))
.orElseThrow(() -> new IllegalArgumentException(String.format("Team id [id=%s] not found", team.getId())));
AssertUtils.isTrue(oldTeam.getTeamName().equals(team.getTeamName()), "Team name cannot be changed.");
oldTeam.setDescription(team.getDescription());
oldTeam.setModifyTime(new Date());
updateById(oldTeam);
}
```
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996298984
##########
streampark-console/streampark-console-service/src/assembly/script/data/mysql-data.sql:
##########
@@ -104,7 +104,10 @@ insert into `t_menu` values (100050, 100048, 'update', null, null, 'member:updat
insert into `t_menu` values (100051, 100048, 'delete', null, null, 'member:delete', null, '1', 1, null, now(), now());
insert into `t_menu` values (100052, 100048, 'role view', null, null, 'role:view', null, '1', 1, null, now(), now());
insert into `t_menu` values (100053, 100001, 'types', null, null, 'user:types', null, '1', 1, null, now(), now());
-
+insert into `t_menu` VALUES (100054, 100000, 'Variable Management', '/system/variable', 'system/variable/View', 'variable:view', 'code', '0', 1, 3, now(), now());
+insert into `t_menu` VALUES (100055, 100054, 'add', NULL, NULL, 'variable:add', NULL, '1', 1, NULL, now(), now());
+insert into `t_menu` VALUES (100056, 100054, 'update', NULL, NULL, 'variable:update', NULL, '1', 1, NULL, now(), now());
+insert into `t_menu` VALUES (100057, 100054, 'delete', NULL, NULL, 'variable:delete', NULL, '1', 1, NULL, now(), now());
Review Comment:
I also think so.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] 1996fanrui commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
1996fanrui commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996382266
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/entity/Variable.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.entity;
+
+import com.baomidou.mybatisplus.annotation.IdType;
+import com.baomidou.mybatisplus.annotation.TableId;
+import com.baomidou.mybatisplus.annotation.TableName;
+import lombok.Data;
+
+import javax.validation.constraints.NotBlank;
+import javax.validation.constraints.NotNull;
+import javax.validation.constraints.Size;
+
+import java.io.Serializable;
+import java.util.Date;
+
+@Data
+@TableName("t_variable")
+public class Variable implements Serializable {
+
+ private static final long serialVersionUID = -7720746591258904369L;
+
+ @TableId(type = IdType.AUTO)
+ private Long id;
+
+ @NotBlank(message = "{required}")
+ private String variableCode;
+
+ @NotBlank(message = "{required}")
+ @Size(max = 50, message = "{noMoreThan}")
+ private String variableValue;
+
+ @Size(max = 100, message = "{noMoreThan}")
+ private String description;
+
+ private Long creator;
Review Comment:
In other word, in some places, userId is clear. But at here, It isn't clear.
Especially teamId in the user table. It is very ambiguous. For other developers, they will think the user belong to the teamId.
We import some ambiguous code to reduce some code. It doesn't make sense.
Or could you ask some decelopers in our WeChat group? If most of them can understand what's the teamId mean in the user table. I will agree your idea.
Code that most contributors can understand is good code. Not only developers and reviewers can understand.
Actually, I said this problem before.
https://github.com/apache/incubator-streampark/pull/1792#discussion_r991347657
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996297539
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/VariableController.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.controller;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.domain.RestResponse;
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shiro.authz.annotation.RequiresPermissions;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.DeleteMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotBlank;
+
+import java.util.List;
+
+@Slf4j
+@Validated
+@RestController
+@RequestMapping("variable")
+public class VariableController {
+
+ @Autowired
+ private CommonService commonService;
+
+ @Autowired
+ private VariableService variableService;
+
+ @PostMapping("list")
+ @RequiresPermissions("variable:view")
+ public RestResponse variableList(RestRequest restRequest, Variable variable) {
+ IPage<Variable> variableList = variableService.findVariables(variable, restRequest);
+ return RestResponse.success(variableList);
+ }
+
+ @PostMapping("post")
+ @RequiresPermissions("variable:add")
+ public RestResponse addVariable(@Valid Variable variable) throws Exception {
+ boolean isExists = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode()) != null;
+ if (isExists) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ variable.setCreator(commonService.getCurrentUser().getUserId());
+ this.variableService.createVariable(variable);
+ return RestResponse.success();
+ }
+
+ @PutMapping("update")
+ @RequiresPermissions("variable:update")
+ public RestResponse updateVariable(@Valid Variable variable) throws Exception {
+ Variable findVariable = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ throw new ApiAlertException("Sorry, the variable does not exist.");
+ }
+ if (findVariable.getId().longValue() != variable.getId().longValue()) {
+ throw new ApiAlertException("Sorry, the variable id is inconsistent.");
+ }
+ this.variableService.updateVariable(variable);
Review Comment:
@1996fanrui I will put them in the service.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#issuecomment-1279758265
@1996fanrui @wolfboys Thanks for the huge review work, I have revised all your suggestions.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] 1996fanrui commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
1996fanrui commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r994766010
##########
streampark-console/streampark-console-service/src/assembly/script/schema/mysql-schema.sql:
##########
@@ -319,6 +319,24 @@ create table `t_team` (
unique key `team_name_idx` (`team_name`) using btree
) engine = innodb default charset = utf8mb4 collate = utf8mb4_general_ci;
+-- ----------------------------
+-- Table of t_variable
+-- ----------------------------
+drop table if exists `t_variable`;
+create table `t_variable` (
+ `id` bigint not null auto_increment,
+ `variable_code` varchar(100) collate utf8mb4_general_ci not null comment 'variable code',
+ `variable_value` varchar(1024) collate utf8mb4_general_ci not null comment 'variable value',
+ `variable_name` varchar(100) collate utf8mb4_general_ci not null comment 'variable name',
+ `description` varchar(100) collate utf8mb4_general_ci default null comment 'description',
+ `user_id` bigint collate utf8mb4_general_ci not null comment 'user id',
Review Comment:
The `user_id` is the creator, right?
If yes, could you rename it to creator? It is more clear.
##########
streampark-console/streampark-console-service/src/assembly/script/schema/mysql-schema.sql:
##########
@@ -319,6 +319,24 @@ create table `t_team` (
unique key `team_name_idx` (`team_name`) using btree
) engine = innodb default charset = utf8mb4 collate = utf8mb4_general_ci;
+-- ----------------------------
+-- Table of t_variable
+-- ----------------------------
+drop table if exists `t_variable`;
+create table `t_variable` (
+ `id` bigint not null auto_increment,
+ `variable_code` varchar(100) collate utf8mb4_general_ci not null comment 'variable code',
+ `variable_value` varchar(1024) collate utf8mb4_general_ci not null comment 'variable value',
+ `variable_name` varchar(100) collate utf8mb4_general_ci not null comment 'variable name',
Review Comment:
`${kafka_cluster}`
As I understand, the `kafka_cluster` is the variable_name, right?
I don't understand what the variable_code mean?
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] 1996fanrui commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
1996fanrui commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996319799
##########
streampark-console/streampark-console-service/src/assembly/script/data/mysql-data.sql:
##########
@@ -104,7 +104,10 @@ insert into `t_menu` values (100050, 100048, 'update', null, null, 'member:updat
insert into `t_menu` values (100051, 100048, 'delete', null, null, 'member:delete', null, '1', 1, null, now(), now());
insert into `t_menu` values (100052, 100048, 'role view', null, null, 'role:view', null, '1', 1, null, now(), now());
insert into `t_menu` values (100053, 100001, 'types', null, null, 'user:types', null, '1', 1, null, now(), now());
-
+insert into `t_menu` VALUES (100054, 100000, 'Variable Management', '/system/variable', 'system/variable/View', 'variable:view', 'code', '0', 1, 3, now(), now());
+insert into `t_menu` VALUES (100055, 100054, 'add', NULL, NULL, 'variable:add', NULL, '1', 1, NULL, now(), now());
+insert into `t_menu` VALUES (100056, 100054, 'update', NULL, NULL, 'variable:update', NULL, '1', 1, NULL, now(), now());
+insert into `t_menu` VALUES (100057, 100054, 'delete', NULL, NULL, 'variable:delete', NULL, '1', 1, NULL, now(), now());
Review Comment:
Please rebase the latest dev branch, #1834 has been merged. Please add the variable management permission for `team admin`.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996378738
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/VariableController.java:
##########
@@ -0,0 +1,87 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.controller;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.domain.RestResponse;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shiro.authz.annotation.RequiresPermissions;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.DeleteMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotBlank;
+
+@Slf4j
+@Validated
+@RestController
+@RequestMapping("variable")
+public class VariableController {
+
+ @Autowired
+ private VariableService variableService;
+
+ @PostMapping("list")
+ @RequiresPermissions("variable:view")
+ public RestResponse variableList(RestRequest restRequest, Variable variable) {
+ IPage<Variable> variableList = variableService.findVariables(variable, restRequest);
+ return RestResponse.success(variableList);
+ }
+
+ @PostMapping("post")
+ @RequiresPermissions("variable:add")
+ public RestResponse addVariable(@Valid Variable variable) throws Exception {
+ this.variableService.createVariable(variable);
+ return RestResponse.success();
+ }
+
+ @PutMapping("update")
+ @RequiresPermissions("variable:update")
+ public RestResponse updateVariable(@Valid Variable variable) throws Exception {
+ this.variableService.updateById(variable);
Review Comment:
@wolfboys Is the following code logic possible?
@PutMapping("update")
@RequiresPermissions("variable:update")
public RestResponse updateVariable(@Valid Variable variable) throws Exception {
if (variable.getId() == null) {
throw new ApiAlertException("Sorry, the variable id cannot be null.");
}
Variable findVariable = this.variableService.getById(variable.getId());
if (findVariable == null) {
throw new ApiAlertException("Sorry, the variable does not exist.");
}
if (!findVariable.getVariableCode().equals(variable.getVariableCode())) {
throw new ApiAlertException("Sorry, the variable code cannot be updated.");
}
this.variableService.updateById(variable);
return RestResponse.success();
}
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] 1996fanrui commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
1996fanrui commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996374643
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/entity/Variable.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.entity;
+
+import com.baomidou.mybatisplus.annotation.IdType;
+import com.baomidou.mybatisplus.annotation.TableId;
+import com.baomidou.mybatisplus.annotation.TableName;
+import lombok.Data;
+
+import javax.validation.constraints.NotBlank;
+import javax.validation.constraints.NotNull;
+import javax.validation.constraints.Size;
+
+import java.io.Serializable;
+import java.util.Date;
+
+@Data
+@TableName("t_variable")
+public class Variable implements Serializable {
+
+ private static final long serialVersionUID = -7720746591258904369L;
+
+ @TableId(type = IdType.AUTO)
+ private Long id;
+
+ @NotBlank(message = "{required}")
+ private String variableCode;
+
+ @NotBlank(message = "{required}")
+ @Size(max = 50, message = "{noMoreThan}")
+ private String variableValue;
+
+ @Size(max = 100, message = "{noMoreThan}")
+ private String description;
+
+ private Long creator;
Review Comment:
一个好的名字应该描述意图，而非细节。
摘选自极客时间专栏《代码之丑》
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#issuecomment-1278400717
@wolfboys @1996fanrui Thanks for all the suggestions, I think the ones that can be modified have been modified, please help to review again.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r994907115
##########
streampark-console/streampark-console-service/src/assembly/script/schema/mysql-schema.sql:
##########
@@ -319,6 +319,24 @@ create table `t_team` (
unique key `team_name_idx` (`team_name`) using btree
) engine = innodb default charset = utf8mb4 collate = utf8mb4_general_ci;
+-- ----------------------------
+-- Table of t_variable
+-- ----------------------------
+drop table if exists `t_variable`;
+create table `t_variable` (
+ `id` bigint not null auto_increment,
+ `variable_code` varchar(100) collate utf8mb4_general_ci not null comment 'variable code',
+ `variable_value` varchar(1024) collate utf8mb4_general_ci not null comment 'variable value',
Review Comment:
Ok
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r994791674
##########
streampark-console/streampark-console-service/src/assembly/script/schema/mysql-schema.sql:
##########
@@ -319,6 +319,24 @@ create table `t_team` (
unique key `team_name_idx` (`team_name`) using btree
) engine = innodb default charset = utf8mb4 collate = utf8mb4_general_ci;
+-- ----------------------------
+-- Table of t_variable
+-- ----------------------------
+drop table if exists `t_variable`;
+create table `t_variable` (
+ `id` bigint not null auto_increment,
+ `variable_code` varchar(100) collate utf8mb4_general_ci not null comment 'variable code',
+ `variable_value` varchar(1024) collate utf8mb4_general_ci not null comment 'variable value',
+ `variable_name` varchar(100) collate utf8mb4_general_ci not null comment 'variable name',
Review Comment:
Do you think it is normative?
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996312932
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/VariableService.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.service;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.system.entity.Variable;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import com.baomidou.mybatisplus.extension.service.IService;
+
+import java.util.List;
+
+public interface VariableService extends IService<Variable> {
+
+ /**
+ * find variable
+ *
+ * @param variable variable
+ * @param restRequest queryRequest
+ * @return IPage
+ */
+ IPage<Variable> findVariables(Variable variable, RestRequest restRequest);
+
+ /**
+ * get variables through team
+ * @param teamId
+ * @return
+ */
+ List<Variable> findByTeamId(Long teamId);
+
+ /**
+ * create variable
+ *
+ * @param variable variable
+ */
+ void createVariable(Variable variable) throws Exception;
+
+ /**
+ * delete variable
+ *
+ * @param variable variable
+ */
+ void deleteVariable(Variable variable) throws Exception;
+
+ /**
+ * update variable
+ *
+ * @param variable variable
+ */
+ void updateVariable(Variable variable) throws Exception;
+
+ Variable findByVariableCode(Long teamId, String variableCode);
Review Comment:
I have really thought about this naming, and also referred to the writing method of team. Considering that team is public in variable management, there is no name to add teamId to it. Is it OK to keep it unchanged?
MemberService：
Member findByUserName(Long teamId, String userName);
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] 1996fanrui commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
1996fanrui commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996319516
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/VariableService.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.service;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.system.entity.Variable;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import com.baomidou.mybatisplus.extension.service.IService;
+
+import java.util.List;
+
+public interface VariableService extends IService<Variable> {
+
+ /**
+ * find variable
+ *
+ * @param variable variable
+ * @param restRequest queryRequest
+ * @return IPage
+ */
+ IPage<Variable> findVariables(Variable variable, RestRequest restRequest);
+
+ /**
+ * get variables through team
+ * @param teamId
+ * @return
+ */
+ List<Variable> findByTeamId(Long teamId);
+
+ /**
+ * create variable
+ *
+ * @param variable variable
+ */
+ void createVariable(Variable variable) throws Exception;
+
+ /**
+ * delete variable
+ *
+ * @param variable variable
+ */
+ void deleteVariable(Variable variable) throws Exception;
+
+ /**
+ * update variable
+ *
+ * @param variable variable
+ */
+ void updateVariable(Variable variable) throws Exception;
+
+ Variable findByVariableCode(Long teamId, String variableCode);
Review Comment:
Thanks for your clarification. LGTM
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
wolfboys commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996333706
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/impl/VariableServiceImpl.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.service.impl;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.core.service.ApplicationService;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.mapper.VariableMapper;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.conditions.query.LambdaQueryWrapper;
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
+import com.baomidou.mybatisplus.extension.service.impl.ServiceImpl;
+import lombok.extern.slf4j.Slf4j;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Propagation;
+import org.springframework.transaction.annotation.Transactional;
+
+import java.util.List;
+
+@Slf4j
+@Service
+@Transactional(propagation = Propagation.SUPPORTS, readOnly = true, rollbackFor = Exception.class)
+public class VariableServiceImpl extends ServiceImpl<VariableMapper, Variable> implements VariableService {
+
+ @Autowired
+ private ApplicationService applicationService;
+
+ @Autowired
+ private CommonService commonService;
+
+ @Override
+ @Transactional(rollbackFor = Exception.class)
+ public void createVariable(Variable variable) throws Exception {
+ if (this.findByVariableCode(variable.getTeamId(), variable.getVariableCode()) != null) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ variable.setCreator(commonService.getCurrentUser().getUserId());
+ this.save(variable);
+ }
+
+ @Override
+ public IPage<Variable> findVariables(Variable variable, RestRequest request) {
+ Page<Variable> page = new Page<>();
+ page.setCurrent(request.getPageNum());
+ page.setSize(request.getPageSize());
+ return this.baseMapper.findVariable(page, variable);
Review Comment:
code can be improved:
`Page<Variable > page = new MybatisPager<Variable>().getDefaultPage(request);`
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] 1996fanrui commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
1996fanrui commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996289895
##########
streampark-console/streampark-console-service/src/assembly/script/upgrade/mysql/1.2.4.sql:
##########
@@ -201,5 +205,21 @@ alter table `t_user`
modify `username` varchar(255) collate utf8mb4_general_ci not null comment 'user name',
add unique key `un_username` (`username`) using btree;
+drop table if exists `t_variable`;
+create table `t_variable` (
+ `id` bigint not null auto_increment,
+ `variable_code` varchar(100) collate utf8mb4_general_ci not null comment 'Variable code is used for parameter names passed to the program or as placeholders',
+ `variable_value` text collate utf8mb4_general_ci not null comment 'The specific value corresponding to the variable',
+ `variable_name` varchar(100) collate utf8mb4_general_ci not null comment 'The name of the variable is used for simple display and for searching',
Review Comment:
This line should be deleted, right?
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/entity/Variable.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.entity;
+
+import com.baomidou.mybatisplus.annotation.IdType;
+import com.baomidou.mybatisplus.annotation.TableId;
+import com.baomidou.mybatisplus.annotation.TableName;
+import lombok.Data;
+
+import javax.validation.constraints.NotBlank;
+import javax.validation.constraints.NotNull;
+import javax.validation.constraints.Size;
+
+import java.io.Serializable;
+import java.util.Date;
+
+@Data
+@TableName("t_variable")
+public class Variable implements Serializable {
+
+ private static final long serialVersionUID = -7720746591258904369L;
+
+ @TableId(type = IdType.AUTO)
+ private Long id;
+
+ @NotBlank(message = "{required}")
+ private String variableCode;
+
+ @NotBlank(message = "{required}")
+ @Size(max = 50, message = "{noMoreThan}")
+ private String variableValue;
+
+ @Size(max = 100, message = "{noMoreThan}")
+ private String description;
+
+ private Long creator;
+
+ private transient String userName;
Review Comment:
`creatorName` is better?
##########
streampark-console/streampark-console-webapp/src/views/system/variable/View.vue:
##########
@@ -0,0 +1,347 @@
+<!--
+
+ 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
+
+ https://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.
+
+-->
+
+<template>
+ <a-card :bordered="false">
+ <div
+ class="table-page-search-wrapper">
+ <a-form
+ layout="inline">
+ <a-row
+ :gutter="48">
+ <div
+ class="fold">
+ <a-col
+ :md="8"
+ :sm="24">
+ <a-form-item
+ label="Variable Code"
+ :label-col="{span: 4}"
+ :wrapper-col="{span: 18, offset: 2}">
+ <a-input
+ v-model="queryParams.variableCode" />
+ </a-form-item>
+ </a-col>
+ <a-col
+ :md="8"
+ :sm="24">
+ <a-form-item
+ label="Description"
+ :label-col="{span: 4}"
+ :wrapper-col="{span: 18, offset: 2}">
+ <a-input
+ v-model="queryParams.description" />
+ </a-form-item>
+ </a-col>
+ </div>
+ <a-col
+ :md="8"
+ :sm="24">
+ <span
+ class="table-page-search-bar">
+ <a-button
+ type="primary"
+ shape="circle"
+ icon="search"
+ @click="search" />
+ <a-button
+ type="primary"
+ shape="circle"
+ icon="rest"
+ @click="reset" />
+ <a-button
+ type="primary"
+ shape="circle"
+ icon="plus"
+ v-permit="'variable:add'"
+ @click="handleAdd" />
+ </span>
+ </a-col>
+ </a-row>
+ </a-form>
+ </div>
+
+ <!-- 表格区域 -->
+ <a-table
+ ref="TableInfo"
+ :columns="columns"
+ :data-source="dataSource"
+ :pagination="pagination"
+ :loading="loading"
+ :scroll="{ x: 900 }"
+ @change="handleTableChange">
+ <template
+ slot="email"
+ slot-scope="text">
+ <a-popover
+ placement="topLeft">
+ <template
+ slot="content">
+ <div>
+ {{ text }}
+ </div>
+ </template>
+ <p
+ style="width: 150px;margin-bottom: 0">
+ {{ text }}
+ </p>
+ </a-popover>
+ </template>
+ <template
+ slot="operation"
+ slot-scope="text, record">
+ <svg-icon
+ v-permit="'variable:update'"
+ v-if="(record.username !== 'admin' || userName === 'admin')"
Review Comment:
This condition is wired. I guess you copy from user management, right?
Admin user cannot allow to change, but variable should can be update even if it is created by admin.
##########
streampark-console/streampark-console-service/src/assembly/script/data/mysql-data.sql:
##########
@@ -104,7 +104,10 @@ insert into `t_menu` values (100050, 100048, 'update', null, null, 'member:updat
insert into `t_menu` values (100051, 100048, 'delete', null, null, 'member:delete', null, '1', 1, null, now(), now());
insert into `t_menu` values (100052, 100048, 'role view', null, null, 'role:view', null, '1', 1, null, now(), now());
insert into `t_menu` values (100053, 100001, 'types', null, null, 'user:types', null, '1', 1, null, now(), now());
-
+insert into `t_menu` VALUES (100054, 100000, 'Variable Management', '/system/variable', 'system/variable/View', 'variable:view', 'code', '0', 1, 3, now(), now());
+insert into `t_menu` VALUES (100055, 100054, 'add', NULL, NULL, 'variable:add', NULL, '1', 1, NULL, now(), now());
+insert into `t_menu` VALUES (100056, 100054, 'update', NULL, NULL, 'variable:update', NULL, '1', 1, NULL, now(), now());
+insert into `t_menu` VALUES (100057, 100054, 'delete', NULL, NULL, 'variable:delete', NULL, '1', 1, NULL, now(), now());
Review Comment:
Can developer create or update variable?
##########
streampark-console/streampark-console-webapp/src/views/system/variable/Detail.vue:
##########
@@ -0,0 +1,105 @@
+<!--
+
+ 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
+
+ https://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.
+
+-->
+
+<template>
+ <a-modal
+ v-model="show"
+ :centered="true"
+ :keyboard="false"
+ :footer="null"
+ :width="600"
+ @cancel="handleCancelClick">
+ <template slot="title">
+ <a-icon type="code" />
+ Variable Info
+ </template>
+ <a-layout class="variable-info">
+ <a-layout-content class="variable-content">
+ <p>
+ <a-icon type="code" />Variable Code：{{ data.variableCode }}
+ </p>
+ <p>
+ <a-icon type="down-circle" />Variable Value：<br>{{ data.variableValue }}
+ </p>
+ <p>
+ <a-icon type="user" />Create User：{{ data.userName }}
+ </p>
+ <p>
+ <a-icon type="team" />Team：{{ data.teamName }}
Review Comment:
The `data.teamName` can be deleted due to StreamPark WebUI is always show current TeamName.
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/entity/Variable.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.entity;
+
+import com.baomidou.mybatisplus.annotation.IdType;
+import com.baomidou.mybatisplus.annotation.TableId;
+import com.baomidou.mybatisplus.annotation.TableName;
+import lombok.Data;
+
+import javax.validation.constraints.NotBlank;
+import javax.validation.constraints.NotNull;
+import javax.validation.constraints.Size;
+
+import java.io.Serializable;
+import java.util.Date;
+
+@Data
+@TableName("t_variable")
+public class Variable implements Serializable {
+
+ private static final long serialVersionUID = -7720746591258904369L;
+
+ @TableId(type = IdType.AUTO)
+ private Long id;
+
+ @NotBlank(message = "{required}")
+ private String variableCode;
+
+ @NotBlank(message = "{required}")
+ @Size(max = 50, message = "{noMoreThan}")
+ private String variableValue;
+
+ @Size(max = 100, message = "{noMoreThan}")
+ private String description;
+
+ private Long creator;
+
+ private transient String userName;
+
+ @NotNull(message = "{required}")
+ private Long teamId;
+
+ private transient String teamName;
Review Comment:
The `teamName` can be deleted.
##########
streampark-console/streampark-console-webapp/src/views/system/variable/View.vue:
##########
@@ -0,0 +1,347 @@
+<!--
+
+ 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
+
+ https://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.
+
+-->
+
+<template>
+ <a-card :bordered="false">
+ <div
+ class="table-page-search-wrapper">
+ <a-form
+ layout="inline">
+ <a-row
+ :gutter="48">
+ <div
+ class="fold">
+ <a-col
+ :md="8"
+ :sm="24">
+ <a-form-item
+ label="Variable Code"
+ :label-col="{span: 4}"
+ :wrapper-col="{span: 18, offset: 2}">
+ <a-input
+ v-model="queryParams.variableCode" />
+ </a-form-item>
+ </a-col>
+ <a-col
+ :md="8"
+ :sm="24">
+ <a-form-item
+ label="Description"
+ :label-col="{span: 4}"
+ :wrapper-col="{span: 18, offset: 2}">
+ <a-input
+ v-model="queryParams.description" />
+ </a-form-item>
+ </a-col>
+ </div>
+ <a-col
+ :md="8"
+ :sm="24">
+ <span
+ class="table-page-search-bar">
+ <a-button
+ type="primary"
+ shape="circle"
+ icon="search"
+ @click="search" />
+ <a-button
+ type="primary"
+ shape="circle"
+ icon="rest"
+ @click="reset" />
+ <a-button
+ type="primary"
+ shape="circle"
+ icon="plus"
+ v-permit="'variable:add'"
+ @click="handleAdd" />
+ </span>
+ </a-col>
+ </a-row>
+ </a-form>
+ </div>
+
+ <!-- 表格区域 -->
+ <a-table
+ ref="TableInfo"
+ :columns="columns"
+ :data-source="dataSource"
+ :pagination="pagination"
+ :loading="loading"
+ :scroll="{ x: 900 }"
+ @change="handleTableChange">
+ <template
+ slot="email"
+ slot-scope="text">
+ <a-popover
+ placement="topLeft">
+ <template
+ slot="content">
+ <div>
+ {{ text }}
+ </div>
+ </template>
+ <p
+ style="width: 150px;margin-bottom: 0">
+ {{ text }}
+ </p>
+ </a-popover>
+ </template>
+ <template
+ slot="operation"
+ slot-scope="text, record">
+ <svg-icon
+ v-permit="'variable:update'"
+ v-if="(record.username !== 'admin' || userName === 'admin')"
+ name="edit"
+ border
+ @click.native="handleEdit(record)"
+ title="modify" />
+ <svg-icon
+ name="see"
+ border
+ @click.native="handleView(record)"
+ title="view" />
+ <a-popconfirm
+ v-permit="'variable:delete'"
+ v-if="record.username !== 'admin'"
Review Comment:
This condition is wired as well~
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/impl/VariableServiceImpl.java:
##########
@@ -0,0 +1,84 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.service.impl;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.ApplicationService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.mapper.VariableMapper;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.conditions.query.LambdaQueryWrapper;
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
+import com.baomidou.mybatisplus.extension.service.impl.ServiceImpl;
+import lombok.SneakyThrows;
+import lombok.extern.slf4j.Slf4j;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Propagation;
+import org.springframework.transaction.annotation.Transactional;
+
+import java.util.List;
+
+@Slf4j
+@Service
+@Transactional(propagation = Propagation.SUPPORTS, readOnly = true, rollbackFor = Exception.class)
+public class VariableServiceImpl extends ServiceImpl<VariableMapper, Variable> implements VariableService {
+
+ @Autowired
+ private ApplicationService applicationService;
+
+ @Override
+ @Transactional(rollbackFor = Exception.class)
+ public void createVariable(Variable variable) throws Exception {
+ this.save(variable);
+ }
+
+ @Override
+ public IPage<Variable> findVariables(Variable variable, RestRequest request) {
+ Page<Variable> page = new Page<>();
+ page.setCurrent(request.getPageNum());
+ page.setSize(request.getPageSize());
+ return this.baseMapper.findVariable(page, variable);
+ }
+
+ @Override
+ public void updateVariable(Variable variable) throws Exception {
+ updateById(variable);
+ }
+
+ @Override
+ public Variable findByVariableCode(Long teamId, String variableCode) {
+ return baseMapper.selectOne(new LambdaQueryWrapper<Variable>()
+ .eq(Variable::getVariableCode, variableCode)
+ .eq(Variable::getTeamId, teamId));
+ }
+
+ @Override
+ public List<Variable> findByTeamId(Long teamId) {
+ return baseMapper.selectByTeamId(teamId);
+ }
+
+ @Override
+ @SneakyThrows
+ public List<Application> findDependByCode(Variable variable) {
+ return null;
Review Comment:
What about define this method in next PR? I'm not sure how to use it in next PR.
##########
streampark-console/streampark-console-service/src/assembly/script/upgrade/mysql/1.2.4.sql:
##########
@@ -201,5 +205,21 @@ alter table `t_user`
modify `username` varchar(255) collate utf8mb4_general_ci not null comment 'user name',
add unique key `un_username` (`username`) using btree;
+drop table if exists `t_variable`;
+create table `t_variable` (
+ `id` bigint not null auto_increment,
+ `variable_code` varchar(100) collate utf8mb4_general_ci not null comment 'Variable code is used for parameter names passed to the program or as placeholders',
+ `variable_value` text collate utf8mb4_general_ci not null comment 'The specific value corresponding to the variable',
+ `variable_name` varchar(100) collate utf8mb4_general_ci not null comment 'The name of the variable is used for simple display and for searching',
+ `description` text collate utf8mb4_general_ci default null comment 'More detailed description of variables, only for display, not involved in program logic',
+ `creator` bigint collate utf8mb4_general_ci not null comment 'user_id of creator',
+ `team_id` bigint collate utf8mb4_general_ci not null comment 'team id',
+ `create_time` datetime not null default current_timestamp comment 'create time',
+ `modify_time` datetime not null default current_timestamp on update current_timestamp comment 'modify time',
+ primary key (`id`) using btree,
+ unique key `un_team_vcode_inx` (`team_id`,`variable_code`) using btree,
+ unique key `un_team_vname_inx` (`team_id`,`variable_name`) using btree
Review Comment:
This line should be deleted as well.
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/VariableController.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.controller;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.domain.RestResponse;
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shiro.authz.annotation.RequiresPermissions;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.DeleteMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotBlank;
+
+import java.util.List;
+
+@Slf4j
+@Validated
+@RestController
+@RequestMapping("variable")
+public class VariableController {
+
+ @Autowired
+ private CommonService commonService;
+
+ @Autowired
+ private VariableService variableService;
+
+ @PostMapping("list")
+ @RequiresPermissions("variable:view")
+ public RestResponse variableList(RestRequest restRequest, Variable variable) {
+ IPage<Variable> variableList = variableService.findVariables(variable, restRequest);
+ return RestResponse.success(variableList);
+ }
+
+ @PostMapping("post")
+ @RequiresPermissions("variable:add")
+ public RestResponse addVariable(@Valid Variable variable) throws Exception {
+ boolean isExists = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode()) != null;
+ if (isExists) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ variable.setCreator(commonService.getCurrentUser().getUserId());
+ this.variableService.createVariable(variable);
+ return RestResponse.success();
+ }
+
+ @PutMapping("update")
+ @RequiresPermissions("variable:update")
+ public RestResponse updateVariable(@Valid Variable variable) throws Exception {
+ Variable findVariable = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ throw new ApiAlertException("Sorry, the variable does not exist.");
+ }
+ if (findVariable.getId().longValue() != variable.getId().longValue()) {
+ throw new ApiAlertException("Sorry, the variable id is inconsistent.");
+ }
+ this.variableService.updateVariable(variable);
Review Comment:
This logic is wired, in general, we always findById, then check `findVariable.teamId== variable.teamId && findVariable.getVariableCode() == variable.getVariableCode()`.
Also, I'm not sure these checks should in Controller or Service. @wolfboys Do we have any related specifications before?
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996295729
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/VariableController.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.controller;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.domain.RestResponse;
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shiro.authz.annotation.RequiresPermissions;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.DeleteMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotBlank;
+
+import java.util.List;
+
+@Slf4j
+@Validated
+@RestController
+@RequestMapping("variable")
+public class VariableController {
+
+ @Autowired
+ private CommonService commonService;
+
+ @Autowired
+ private VariableService variableService;
+
+ @PostMapping("list")
+ @RequiresPermissions("variable:view")
+ public RestResponse variableList(RestRequest restRequest, Variable variable) {
+ IPage<Variable> variableList = variableService.findVariables(variable, restRequest);
+ return RestResponse.success(variableList);
+ }
+
+ @PostMapping("post")
+ @RequiresPermissions("variable:add")
+ public RestResponse addVariable(@Valid Variable variable) throws Exception {
+ boolean isExists = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode()) != null;
+ if (isExists) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ variable.setCreator(commonService.getCurrentUser().getUserId());
+ this.variableService.createVariable(variable);
+ return RestResponse.success();
+ }
+
+ @PutMapping("update")
+ @RequiresPermissions("variable:update")
+ public RestResponse updateVariable(@Valid Variable variable) throws Exception {
+ Variable findVariable = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ throw new ApiAlertException("Sorry, the variable does not exist.");
+ }
+ if (findVariable.getId().longValue() != variable.getId().longValue()) {
+ throw new ApiAlertException("Sorry, the variable id is inconsistent.");
+ }
+ this.variableService.updateVariable(variable);
Review Comment:
> This logic is wired, in general, we always findById, then check `findVariable.teamId== variable.teamId && findVariable.getVariableCode() == variable.getVariableCode()`.
>
> Also, I'm not sure these checks should in Controller or Service. @wolfboys Do we have any related specifications before?
@1996fanrui I will replace these checks with variableService.checkParam(variable)
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996295127
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/impl/VariableServiceImpl.java:
##########
@@ -0,0 +1,84 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.service.impl;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.ApplicationService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.mapper.VariableMapper;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.conditions.query.LambdaQueryWrapper;
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
+import com.baomidou.mybatisplus.extension.service.impl.ServiceImpl;
+import lombok.SneakyThrows;
+import lombok.extern.slf4j.Slf4j;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Propagation;
+import org.springframework.transaction.annotation.Transactional;
+
+import java.util.List;
+
+@Slf4j
+@Service
+@Transactional(propagation = Propagation.SUPPORTS, readOnly = true, rollbackFor = Exception.class)
+public class VariableServiceImpl extends ServiceImpl<VariableMapper, Variable> implements VariableService {
+
+ @Autowired
+ private ApplicationService applicationService;
+
+ @Override
+ @Transactional(rollbackFor = Exception.class)
+ public void createVariable(Variable variable) throws Exception {
+ this.save(variable);
+ }
+
+ @Override
+ public IPage<Variable> findVariables(Variable variable, RestRequest request) {
+ Page<Variable> page = new Page<>();
+ page.setCurrent(request.getPageNum());
+ page.setSize(request.getPageSize());
+ return this.baseMapper.findVariable(page, variable);
+ }
+
+ @Override
+ public void updateVariable(Variable variable) throws Exception {
+ updateById(variable);
+ }
+
+ @Override
+ public Variable findByVariableCode(Long teamId, String variableCode) {
+ return baseMapper.selectOne(new LambdaQueryWrapper<Variable>()
+ .eq(Variable::getVariableCode, variableCode)
+ .eq(Variable::getTeamId, teamId));
+ }
+
+ @Override
+ public List<Variable> findByTeamId(Long teamId) {
+ return baseMapper.selectByTeamId(teamId);
+ }
+
+ @Override
+ @SneakyThrows
+ public List<Application> findDependByCode(Variable variable) {
+ return null;
Review Comment:
Yes, it will be used in the next PR. When deleting a variable, it is necessary to detect whether the variable is referenced. If it is referenced, it will prompt that it cannot be deleted.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#issuecomment-1279742300
@1996fanrui Thanks for your review, I made changes based on your suggestion.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
wolfboys commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996378736
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/entity/Variable.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.entity;
+
+import com.baomidou.mybatisplus.annotation.IdType;
+import com.baomidou.mybatisplus.annotation.TableId;
+import com.baomidou.mybatisplus.annotation.TableName;
+import lombok.Data;
+
+import javax.validation.constraints.NotBlank;
+import javax.validation.constraints.NotNull;
+import javax.validation.constraints.Size;
+
+import java.io.Serializable;
+import java.util.Date;
+
+@Data
+@TableName("t_variable")
+public class Variable implements Serializable {
+
+ private static final long serialVersionUID = -7720746591258904369L;
+
+ @TableId(type = IdType.AUTO)
+ private Long id;
+
+ @NotBlank(message = "{required}")
+ private String variableCode;
+
+ @NotBlank(message = "{required}")
+ @Size(max = 50, message = "{noMoreThan}")
+ private String variableValue;
+
+ @Size(max = 100, message = "{noMoreThan}")
+ private String description;
+
+ private Long creator;
Review Comment:
> 一个好的名字应该描述意图，而非细节。 摘选自极客时间专栏《代码之丑》
>
> 
hi fanrui:
The "entity" package name is all the entities corresponding to the table. The user_id is easier to identify and recognize at the level of data model design. "creator" is difficult for me to know the creator's name or the creator's id, or anyone else's information , meaning unclear. It is also difficult to generate a complete association, the problem is that this is a specification problem. If this one is changed, other more fields such as: role_id, menu_id, team_id can be found everywhere in other tables, How should you name it? And these are the best practices and established habits of the java back-end, and it is not recommended to modify them.
----
entity包名下放的全是和表对应的实体, user_id 站在数据模型设计的层面更容易辨认和识别, creator 单看名字我很难知道是创建者名字还是创建者id, 还是其他任何人的信息, 表达的意思很迷糊. 也很难产生完整的联想, 问题是这是一个规范问题.如果这一处更改了, 其他的更多字段如: role_id, menu_id, team_id 在其他表中比比皆是,你该怎么命名? 而且这些是中台体系沉淀下来的最佳实践, 约定俗成的习惯, 不建议修改.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
wolfboys commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996316022
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/VariableService.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.service;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.system.entity.Variable;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import com.baomidou.mybatisplus.extension.service.IService;
+
+import java.util.List;
+
+public interface VariableService extends IService<Variable> {
+
+ /**
+ * find variable
+ *
+ * @param variable variable
+ * @param restRequest queryRequest
+ * @return IPage
+ */
+ IPage<Variable> findVariables(Variable variable, RestRequest restRequest);
+
+ /**
+ * get variables through team
+ * @param teamId
+ * @return
+ */
+ List<Variable> findByTeamId(Long teamId);
+
+ /**
+ * create variable
+ *
+ * @param variable variable
+ */
+ void createVariable(Variable variable) throws Exception;
+
+ /**
+ * delete variable
+ *
+ * @param variable variable
+ */
+ void deleteVariable(Variable variable) throws Exception;
+
+ /**
+ * update variable
+ *
+ * @param variable variable
+ */
+ void updateVariable(Variable variable) throws Exception;
+
+ Variable findByVariableCode(Long teamId, String variableCode);
Review Comment:
> Actually, the method is `findByTeamAndUserName` when I develop the team management. But @wolfboys change it from `findByTeamAndUserName` to `findByUserName` in #1792 . I don't know why.
>
> Hi @wolfboys , could you help explain why? Is it a mistake, or is there another reason?
>
> 
In fact, this method is used for business. The use of teamId has been clarified. I don’t think it is necessary to have a method name that is too long. For example, if you query users based on roleId, teamid, and userName, should the method name be written like this?
`findByRoleAndTeamAndUserName(...)`
if there are more parameters, how do you define the method name?
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] 1996fanrui commented on pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
1996fanrui commented on PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#issuecomment-1279932319
Hi @macksonmu @wolfboys, this comment isn't addressed, right? The team admin cannot access the variable page.
https://github.com/apache/incubator-streampark/pull/1831#discussion_r996319799
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
wolfboys commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996383062
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/entity/Variable.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.entity;
+
+import com.baomidou.mybatisplus.annotation.IdType;
+import com.baomidou.mybatisplus.annotation.TableId;
+import com.baomidou.mybatisplus.annotation.TableName;
+import lombok.Data;
+
+import javax.validation.constraints.NotBlank;
+import javax.validation.constraints.NotNull;
+import javax.validation.constraints.Size;
+
+import java.io.Serializable;
+import java.util.Date;
+
+@Data
+@TableName("t_variable")
+public class Variable implements Serializable {
+
+ private static final long serialVersionUID = -7720746591258904369L;
+
+ @TableId(type = IdType.AUTO)
+ private Long id;
+
+ @NotBlank(message = "{required}")
+ private String variableCode;
+
+ @NotBlank(message = "{required}")
+ @Size(max = 50, message = "{noMoreThan}")
+ private String variableValue;
+
+ @Size(max = 100, message = "{noMoreThan}")
+ private String description;
+
+ private Long creator;
Review Comment:
> In other word, in some places, userId is clear. But at here, It isn't clear.
creator_id, good, I agree with the new name
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
wolfboys commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996383145
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/entity/Variable.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.entity;
+
+import com.baomidou.mybatisplus.annotation.IdType;
+import com.baomidou.mybatisplus.annotation.TableId;
+import com.baomidou.mybatisplus.annotation.TableName;
+import lombok.Data;
+
+import javax.validation.constraints.NotBlank;
+import javax.validation.constraints.NotNull;
+import javax.validation.constraints.Size;
+
+import java.io.Serializable;
+import java.util.Date;
+
+@Data
+@TableName("t_variable")
+public class Variable implements Serializable {
+
+ private static final long serialVersionUID = -7720746591258904369L;
+
+ @TableId(type = IdType.AUTO)
+ private Long id;
+
+ @NotBlank(message = "{required}")
+ private String variableCode;
+
+ @NotBlank(message = "{required}")
+ @Size(max = 50, message = "{noMoreThan}")
+ private String variableValue;
+
+ @Size(max = 100, message = "{noMoreThan}")
+ private String description;
+
+ private Long creator;
Review Comment:
> In other word, in some places, userId is clear. But at here, It isn't clear.
Exactly as you said
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys merged pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
wolfboys merged PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996305141
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/VariableController.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.controller;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.domain.RestResponse;
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shiro.authz.annotation.RequiresPermissions;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.DeleteMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotBlank;
+
+import java.util.List;
+
+@Slf4j
+@Validated
+@RestController
+@RequestMapping("variable")
+public class VariableController {
+
+ @Autowired
+ private CommonService commonService;
+
+ @Autowired
+ private VariableService variableService;
+
+ @PostMapping("list")
+ @RequiresPermissions("variable:view")
+ public RestResponse variableList(RestRequest restRequest, Variable variable) {
+ IPage<Variable> variableList = variableService.findVariables(variable, restRequest);
+ return RestResponse.success(variableList);
+ }
+
+ @PostMapping("post")
+ @RequiresPermissions("variable:add")
+ public RestResponse addVariable(@Valid Variable variable) throws Exception {
+ boolean isExists = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode()) != null;
+ if (isExists) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ variable.setCreator(commonService.getCurrentUser().getUserId());
+ this.variableService.createVariable(variable);
+ return RestResponse.success();
+ }
+
+ @PutMapping("update")
+ @RequiresPermissions("variable:update")
+ public RestResponse updateVariable(@Valid Variable variable) throws Exception {
+ Variable findVariable = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ throw new ApiAlertException("Sorry, the variable does not exist.");
+ }
+ if (findVariable.getId().longValue() != variable.getId().longValue()) {
Review Comment:
I considered possible api calls, so I judged more, whether the id will be wrong when calling the api, do you think this is necessary?
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/VariableController.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.controller;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.domain.RestResponse;
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shiro.authz.annotation.RequiresPermissions;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.DeleteMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotBlank;
+
+import java.util.List;
+
+@Slf4j
+@Validated
+@RestController
+@RequestMapping("variable")
+public class VariableController {
+
+ @Autowired
+ private CommonService commonService;
+
+ @Autowired
+ private VariableService variableService;
+
+ @PostMapping("list")
+ @RequiresPermissions("variable:view")
+ public RestResponse variableList(RestRequest restRequest, Variable variable) {
+ IPage<Variable> variableList = variableService.findVariables(variable, restRequest);
+ return RestResponse.success(variableList);
+ }
+
+ @PostMapping("post")
+ @RequiresPermissions("variable:add")
+ public RestResponse addVariable(@Valid Variable variable) throws Exception {
+ boolean isExists = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode()) != null;
+ if (isExists) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ variable.setCreator(commonService.getCurrentUser().getUserId());
+ this.variableService.createVariable(variable);
+ return RestResponse.success();
+ }
+
+ @PutMapping("update")
+ @RequiresPermissions("variable:update")
+ public RestResponse updateVariable(@Valid Variable variable) throws Exception {
+ Variable findVariable = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ throw new ApiAlertException("Sorry, the variable does not exist.");
+ }
+ if (findVariable.getId().longValue() != variable.getId().longValue()) {
+ throw new ApiAlertException("Sorry, the variable id is inconsistent.");
+ }
+ this.variableService.updateVariable(variable);
+ return RestResponse.success();
+ }
+
+ @DeleteMapping("delete")
+ @RequiresPermissions("variable:delete")
+ public RestResponse deleteVariables(@Valid Variable variable) {
+ Variable findVariable = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
Review Comment:
I considered possible api calls, so I judged more, whether the id will be wrong when calling the api, do you think this is necessary?
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] 1996fanrui commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
1996fanrui commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996307165
##########
streampark-console/streampark-console-service/src/assembly/script/data/mysql-data.sql:
##########
@@ -104,7 +104,10 @@ insert into `t_menu` values (100050, 100048, 'update', null, null, 'member:updat
insert into `t_menu` values (100051, 100048, 'delete', null, null, 'member:delete', null, '1', 1, null, now(), now());
insert into `t_menu` values (100052, 100048, 'role view', null, null, 'role:view', null, '1', 1, null, now(), now());
insert into `t_menu` values (100053, 100001, 'types', null, null, 'user:types', null, '1', 1, null, now(), now());
-
+insert into `t_menu` VALUES (100054, 100000, 'Variable Management', '/system/variable', 'system/variable/View', 'variable:view', 'code', '0', 1, 3, now(), now());
+insert into `t_menu` VALUES (100055, 100054, 'add', NULL, NULL, 'variable:add', NULL, '1', 1, NULL, now(), now());
+insert into `t_menu` VALUES (100056, 100054, 'update', NULL, NULL, 'variable:update', NULL, '1', 1, NULL, now(), now());
+insert into `t_menu` VALUES (100057, 100054, 'delete', NULL, NULL, 'variable:delete', NULL, '1', 1, NULL, now(), now());
Review Comment:
If everyone agree with it, please @macksonmu help update the rule to #1477 , thanks~
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] 1996fanrui commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
1996fanrui commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996308692
##########
streampark-console/streampark-console-webapp/src/api/variable.js:
##########
@@ -0,0 +1,51 @@
+/*
+ * 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.
+ */
+
+import api from './index'
+import http from '@/utils/request'
+
+export function list (queryParam) {
+ return http.post(api.Variable.LIST, queryParam)
+}
+
+export function update (queryParam) {
+ return http.put(api.Variable.UPDATE, queryParam)
+}
+
+export function get (queryParam) {
+ return http.get(api.Variable.GET, queryParam)
Review Comment:
This method can be deleted as well.
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/impl/VariableServiceImpl.java:
##########
@@ -0,0 +1,120 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.service.impl;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.ApplicationService;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.mapper.VariableMapper;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.conditions.query.LambdaQueryWrapper;
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
+import com.baomidou.mybatisplus.extension.service.impl.ServiceImpl;
+import lombok.SneakyThrows;
+import lombok.extern.slf4j.Slf4j;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Propagation;
+import org.springframework.transaction.annotation.Transactional;
+
+import java.util.List;
+
+@Slf4j
+@Service
+@Transactional(propagation = Propagation.SUPPORTS, readOnly = true, rollbackFor = Exception.class)
+public class VariableServiceImpl extends ServiceImpl<VariableMapper, Variable> implements VariableService {
+
+ @Autowired
+ private ApplicationService applicationService;
+
+ @Autowired
+ private CommonService commonService;
+
+ @Override
+ @Transactional(rollbackFor = Exception.class)
+ public void createVariable(Variable variable) throws Exception {
+ if (isExists(variable)) {
Review Comment:
And `isExists` is just called inside `VariableServiceImpl`, why do you define it in the VariableService?
It is strongly recommended to delete this method. because:
- Ambiguous method name
- It is just called once
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/impl/VariableServiceImpl.java:
##########
@@ -0,0 +1,120 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.service.impl;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.ApplicationService;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.mapper.VariableMapper;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.conditions.query.LambdaQueryWrapper;
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
+import com.baomidou.mybatisplus.extension.service.impl.ServiceImpl;
+import lombok.SneakyThrows;
+import lombok.extern.slf4j.Slf4j;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Propagation;
+import org.springframework.transaction.annotation.Transactional;
+
+import java.util.List;
+
+@Slf4j
+@Service
+@Transactional(propagation = Propagation.SUPPORTS, readOnly = true, rollbackFor = Exception.class)
+public class VariableServiceImpl extends ServiceImpl<VariableMapper, Variable> implements VariableService {
+
+ @Autowired
+ private ApplicationService applicationService;
+
+ @Autowired
+ private CommonService commonService;
+
+ @Override
+ @Transactional(rollbackFor = Exception.class)
+ public void createVariable(Variable variable) throws Exception {
+ if (isExists(variable)) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ variable.setCreator(commonService.getCurrentUser().getUserId());
+ this.save(variable);
+ }
+
+ @Override
+ public IPage<Variable> findVariables(Variable variable, RestRequest request) {
+ Page<Variable> page = new Page<>();
+ page.setCurrent(request.getPageNum());
+ page.setSize(request.getPageSize());
+ return this.baseMapper.findVariable(page, variable);
+ }
+
+ @Override
+ public void updateVariable(Variable variable) throws Exception {
+ Variable findVariable = this.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ if (findVariable.getId().longValue() != variable.getId().longValue()) {
+ throw new ApiAlertException("Sorry, the variable id is inconsistent.");
+ }
+ updateById(variable);
+ }
+
+ @Override
+ public void deleteVariable(Variable variable) throws Exception {
+ Variable findVariable = this.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ if (findVariable.getId().longValue() != variable.getId().longValue()) {
+ throw new ApiAlertException("Sorry, the variable id is inconsistent.");
+ }
+ List<Application> dependApplications = this.findDependByCode(variable);
Review Comment:
If the `findDependByCode` just be called inside VariableServiceImpl, why do you define this method in the VariableService? As I understand, `findDependByCode` should be private.
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/VariableService.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.service;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.system.entity.Variable;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import com.baomidou.mybatisplus.extension.service.IService;
+
+import java.util.List;
+
+public interface VariableService extends IService<Variable> {
+
+ /**
+ * find variable
+ *
+ * @param variable variable
+ * @param restRequest queryRequest
+ * @return IPage
+ */
+ IPage<Variable> findVariables(Variable variable, RestRequest restRequest);
+
+ /**
+ * get variables through team
+ * @param teamId
+ * @return
+ */
+ List<Variable> findByTeamId(Long teamId);
+
+ /**
+ * create variable
+ *
+ * @param variable variable
+ */
+ void createVariable(Variable variable) throws Exception;
+
+ /**
+ * delete variable
+ *
+ * @param variable variable
+ */
+ void deleteVariable(Variable variable) throws Exception;
+
+ /**
+ * update variable
+ *
+ * @param variable variable
+ */
+ void updateVariable(Variable variable) throws Exception;
+
+ Variable findByVariableCode(Long teamId, String variableCode);
+
+ boolean isExists(Variable variable);
+
+ List<Application> findDependByCode(Variable variable);
Review Comment:
method is ByCode, but the parameter isn't code, it's Variable.
How about rename to f`indDependApplications(Variable variable)`?
##########
streampark-console/streampark-console-webapp/src/api/index.js:
##########
@@ -209,6 +209,15 @@ export default {
UPDATE: '/menu/update',
ROUTER: '/menu/router'
},
+ Variable: {
+ LIST: '/variable/list',
+ UPDATE: '/variable/update',
+ GET: '/variable/get',
Review Comment:
I checked, `VariableController` doesn't have the get method.
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/impl/VariableServiceImpl.java:
##########
@@ -0,0 +1,120 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.service.impl;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.ApplicationService;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.mapper.VariableMapper;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.conditions.query.LambdaQueryWrapper;
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
+import com.baomidou.mybatisplus.extension.service.impl.ServiceImpl;
+import lombok.SneakyThrows;
+import lombok.extern.slf4j.Slf4j;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Propagation;
+import org.springframework.transaction.annotation.Transactional;
+
+import java.util.List;
+
+@Slf4j
+@Service
+@Transactional(propagation = Propagation.SUPPORTS, readOnly = true, rollbackFor = Exception.class)
+public class VariableServiceImpl extends ServiceImpl<VariableMapper, Variable> implements VariableService {
+
+ @Autowired
+ private ApplicationService applicationService;
+
+ @Autowired
+ private CommonService commonService;
+
+ @Override
+ @Transactional(rollbackFor = Exception.class)
+ public void createVariable(Variable variable) throws Exception {
+ if (isExists(variable)) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ variable.setCreator(commonService.getCurrentUser().getUserId());
+ this.save(variable);
+ }
+
+ @Override
+ public IPage<Variable> findVariables(Variable variable, RestRequest request) {
+ Page<Variable> page = new Page<>();
+ page.setCurrent(request.getPageNum());
+ page.setSize(request.getPageSize());
+ return this.baseMapper.findVariable(page, variable);
+ }
+
+ @Override
+ public void updateVariable(Variable variable) throws Exception {
+ Variable findVariable = this.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ if (findVariable.getId().longValue() != variable.getId().longValue()) {
+ throw new ApiAlertException("Sorry, the variable id is inconsistent.");
+ }
+ updateById(variable);
+ }
+
+ @Override
+ public void deleteVariable(Variable variable) throws Exception {
+ Variable findVariable = this.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ if (findVariable.getId().longValue() != variable.getId().longValue()) {
+ throw new ApiAlertException("Sorry, the variable id is inconsistent.");
+ }
+ List<Application> dependApplications = this.findDependByCode(variable);
+ if (!(dependApplications == null || dependApplications.isEmpty())) {
Review Comment:
How about `dependApplications != null && !dependApplications.isEmpty()`?
Or could we ensure the return value of `findDependByCode(variable)` is always not null? We can return an empty list~.
If we can, the condition will be more simple, it can be `!dependApplications.isEmpty()`
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/impl/VariableServiceImpl.java:
##########
@@ -0,0 +1,120 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.service.impl;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.ApplicationService;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.mapper.VariableMapper;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.conditions.query.LambdaQueryWrapper;
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
+import com.baomidou.mybatisplus.extension.service.impl.ServiceImpl;
+import lombok.SneakyThrows;
+import lombok.extern.slf4j.Slf4j;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Propagation;
+import org.springframework.transaction.annotation.Transactional;
+
+import java.util.List;
+
+@Slf4j
+@Service
+@Transactional(propagation = Propagation.SUPPORTS, readOnly = true, rollbackFor = Exception.class)
+public class VariableServiceImpl extends ServiceImpl<VariableMapper, Variable> implements VariableService {
+
+ @Autowired
+ private ApplicationService applicationService;
+
+ @Autowired
+ private CommonService commonService;
+
+ @Override
+ @Transactional(rollbackFor = Exception.class)
+ public void createVariable(Variable variable) throws Exception {
+ if (isExists(variable)) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ variable.setCreator(commonService.getCurrentUser().getUserId());
+ this.save(variable);
+ }
+
+ @Override
+ public IPage<Variable> findVariables(Variable variable, RestRequest request) {
+ Page<Variable> page = new Page<>();
+ page.setCurrent(request.getPageNum());
+ page.setSize(request.getPageSize());
+ return this.baseMapper.findVariable(page, variable);
+ }
+
+ @Override
+ public void updateVariable(Variable variable) throws Exception {
+ Variable findVariable = this.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ if (findVariable.getId().longValue() != variable.getId().longValue()) {
+ throw new ApiAlertException("Sorry, the variable id is inconsistent.");
+ }
+ updateById(variable);
+ }
+
+ @Override
+ public void deleteVariable(Variable variable) throws Exception {
+ Variable findVariable = this.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ if (findVariable.getId().longValue() != variable.getId().longValue()) {
+ throw new ApiAlertException("Sorry, the variable id is inconsistent.");
+ }
+ List<Application> dependApplications = this.findDependByCode(variable);
+ if (!(dependApplications == null || dependApplications.isEmpty())) {
+ throw new ApiAlertException(String.format("Sorry, this variable is being used by [%s] applications.", dependApplications.size()));
+ }
+ this.removeById(findVariable.getId());
+ }
+
+ @Override
+ public Variable findByVariableCode(Long teamId, String variableCode) {
+ return baseMapper.selectOne(new LambdaQueryWrapper<Variable>()
+ .eq(Variable::getVariableCode, variableCode)
+ .eq(Variable::getTeamId, teamId));
+ }
+
+ @Override
+ public List<Variable> findByTeamId(Long teamId) {
+ return baseMapper.selectByTeamId(teamId);
+ }
+
+ @Override
+ public boolean isExists(Variable variable) {
+ return this.findByVariableCode(variable.getTeamId(), variable.getVariableCode()) != null;
+ }
Review Comment:
Could you delete this method? The method name isn't clear.
If you don't see the detailed code and just see the method name, do you know how this method checks if the variable exists? I see the method name, I thought it check by `variable.getId()`
Ambiguous method names introduce potential risks and bugs. It can easily lead to bugs if called by other developers.
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/VariableService.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.service;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.system.entity.Variable;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import com.baomidou.mybatisplus.extension.service.IService;
+
+import java.util.List;
+
+public interface VariableService extends IService<Variable> {
+
+ /**
+ * find variable
+ *
+ * @param variable variable
+ * @param restRequest queryRequest
+ * @return IPage
+ */
+ IPage<Variable> findVariables(Variable variable, RestRequest restRequest);
+
+ /**
+ * get variables through team
+ * @param teamId
+ * @return
+ */
+ List<Variable> findByTeamId(Long teamId);
+
+ /**
+ * create variable
+ *
+ * @param variable variable
+ */
+ void createVariable(Variable variable) throws Exception;
+
+ /**
+ * delete variable
+ *
+ * @param variable variable
+ */
+ void deleteVariable(Variable variable) throws Exception;
+
+ /**
+ * update variable
+ *
+ * @param variable variable
+ */
+ void updateVariable(Variable variable) throws Exception;
+
+ Variable findByVariableCode(Long teamId, String variableCode);
Review Comment:
findByTeamIdAndVariableCode?
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996311818
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/impl/VariableServiceImpl.java:
##########
@@ -0,0 +1,120 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.service.impl;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.ApplicationService;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.mapper.VariableMapper;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.conditions.query.LambdaQueryWrapper;
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
+import com.baomidou.mybatisplus.extension.service.impl.ServiceImpl;
+import lombok.SneakyThrows;
+import lombok.extern.slf4j.Slf4j;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Propagation;
+import org.springframework.transaction.annotation.Transactional;
+
+import java.util.List;
+
+@Slf4j
+@Service
+@Transactional(propagation = Propagation.SUPPORTS, readOnly = true, rollbackFor = Exception.class)
+public class VariableServiceImpl extends ServiceImpl<VariableMapper, Variable> implements VariableService {
+
+ @Autowired
+ private ApplicationService applicationService;
+
+ @Autowired
+ private CommonService commonService;
+
+ @Override
+ @Transactional(rollbackFor = Exception.class)
+ public void createVariable(Variable variable) throws Exception {
+ if (isExists(variable)) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ variable.setCreator(commonService.getCurrentUser().getUserId());
+ this.save(variable);
+ }
+
+ @Override
+ public IPage<Variable> findVariables(Variable variable, RestRequest request) {
+ Page<Variable> page = new Page<>();
+ page.setCurrent(request.getPageNum());
+ page.setSize(request.getPageSize());
+ return this.baseMapper.findVariable(page, variable);
+ }
+
+ @Override
+ public void updateVariable(Variable variable) throws Exception {
+ Variable findVariable = this.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ if (findVariable.getId().longValue() != variable.getId().longValue()) {
+ throw new ApiAlertException("Sorry, the variable id is inconsistent.");
+ }
+ updateById(variable);
+ }
+
+ @Override
+ public void deleteVariable(Variable variable) throws Exception {
+ Variable findVariable = this.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ if (findVariable.getId().longValue() != variable.getId().longValue()) {
+ throw new ApiAlertException("Sorry, the variable id is inconsistent.");
+ }
+ List<Application> dependApplications = this.findDependByCode(variable);
+ if (!(dependApplications == null || dependApplications.isEmpty())) {
Review Comment:
I deleted this judgment, I will judge in the next PR, because it is indeed related to the next PR.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996311799
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/impl/VariableServiceImpl.java:
##########
@@ -0,0 +1,120 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.service.impl;
+
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.ApplicationService;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.mapper.VariableMapper;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.conditions.query.LambdaQueryWrapper;
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
+import com.baomidou.mybatisplus.extension.service.impl.ServiceImpl;
+import lombok.SneakyThrows;
+import lombok.extern.slf4j.Slf4j;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Propagation;
+import org.springframework.transaction.annotation.Transactional;
+
+import java.util.List;
+
+@Slf4j
+@Service
+@Transactional(propagation = Propagation.SUPPORTS, readOnly = true, rollbackFor = Exception.class)
+public class VariableServiceImpl extends ServiceImpl<VariableMapper, Variable> implements VariableService {
+
+ @Autowired
+ private ApplicationService applicationService;
+
+ @Autowired
+ private CommonService commonService;
+
+ @Override
+ @Transactional(rollbackFor = Exception.class)
+ public void createVariable(Variable variable) throws Exception {
+ if (isExists(variable)) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ variable.setCreator(commonService.getCurrentUser().getUserId());
+ this.save(variable);
+ }
+
+ @Override
+ public IPage<Variable> findVariables(Variable variable, RestRequest request) {
+ Page<Variable> page = new Page<>();
+ page.setCurrent(request.getPageNum());
+ page.setSize(request.getPageSize());
+ return this.baseMapper.findVariable(page, variable);
+ }
+
+ @Override
+ public void updateVariable(Variable variable) throws Exception {
+ Variable findVariable = this.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ if (findVariable.getId().longValue() != variable.getId().longValue()) {
+ throw new ApiAlertException("Sorry, the variable id is inconsistent.");
+ }
+ updateById(variable);
+ }
+
+ @Override
+ public void deleteVariable(Variable variable) throws Exception {
+ Variable findVariable = this.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ throw new ApiAlertException("Sorry, the variable code already exists.");
+ }
+ if (findVariable.getId().longValue() != variable.getId().longValue()) {
+ throw new ApiAlertException("Sorry, the variable id is inconsistent.");
+ }
+ List<Application> dependApplications = this.findDependByCode(variable);
Review Comment:
I deleted this judgment, I will judge in the next PR, because it is indeed related to the next PR.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] 1996fanrui commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
1996fanrui commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996319799
##########
streampark-console/streampark-console-service/src/assembly/script/data/mysql-data.sql:
##########
@@ -104,7 +104,10 @@ insert into `t_menu` values (100050, 100048, 'update', null, null, 'member:updat
insert into `t_menu` values (100051, 100048, 'delete', null, null, 'member:delete', null, '1', 1, null, now(), now());
insert into `t_menu` values (100052, 100048, 'role view', null, null, 'role:view', null, '1', 1, null, now(), now());
insert into `t_menu` values (100053, 100001, 'types', null, null, 'user:types', null, '1', 1, null, now(), now());
-
+insert into `t_menu` VALUES (100054, 100000, 'Variable Management', '/system/variable', 'system/variable/View', 'variable:view', 'code', '0', 1, 3, now(), now());
+insert into `t_menu` VALUES (100055, 100054, 'add', NULL, NULL, 'variable:add', NULL, '1', 1, NULL, now(), now());
+insert into `t_menu` VALUES (100056, 100054, 'update', NULL, NULL, 'variable:update', NULL, '1', 1, NULL, now(), now());
+insert into `t_menu` VALUES (100057, 100054, 'delete', NULL, NULL, 'variable:delete', NULL, '1', 1, NULL, now(), now());
Review Comment:
Please rebase the latest dev branch, #1843 has been merged. Please add the variable management permission for `team admin`.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
wolfboys commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r996334563
##########
streampark-console/streampark-console-service/src/main/resources/db/schema-h2.sql:
##########
@@ -282,6 +282,22 @@ create table if not exists `t_team` (
unique (`team_name`)
);
+-- ----------------------------
+-- Table of t_variable
+-- ----------------------------
+create table if not exists `t_variable` (
+ `id` bigint generated by default as identity not null comment 'variable id',
+ `variable_code` varchar(100) not null comment 'Variable code is used for parameter names passed to the program or as placeholders',
+ `variable_value` text not null comment 'The specific value corresponding to the variable',
+ `description` text default null comment 'More detailed description of variables, only for display, not involved in program logic',
+ `creator` bigint not null comment 'user_id of creator',
Review Comment:
oh, column name need change to `user_id`
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
wolfboys commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r994665213
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/VariableController.java:
##########
@@ -0,0 +1,148 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.controller;
+
+import org.apache.streampark.console.base.domain.ResponseCode;
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.domain.RestResponse;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shiro.authz.annotation.RequiresPermissions;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.DeleteMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotBlank;
+
+import java.util.List;
+
+@Slf4j
+@Validated
+@RestController
+@RequestMapping("variable")
+public class VariableController {
+
+ @Autowired
+ CommonService commonService;
+
+ @Autowired
+ private VariableService variableService;
+
+ @PostMapping("list")
+ @RequiresPermissions("variable:view")
+ public RestResponse variableList(RestRequest restRequest, Variable variable) {
+ IPage<Variable> variableList = variableService.findVariables(variable, restRequest);
+ return RestResponse.success(variableList);
+ }
+
+ @PostMapping("post")
+ @RequiresPermissions("variable:add")
+ public RestResponse addVariable(@Valid Variable variable) throws Exception {
+ boolean isExists = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode()) != null;
+ if (isExists) {
+ return RestResponse.fail("Sorry, the variable Code already exists", ResponseCode.CODE_FAIL_ALERT);
Review Comment:
I suggest to change it to `throw new ApiAlertException("Sorry, the variable Code already exists")`
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/VariableController.java:
##########
@@ -0,0 +1,148 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.controller;
+
+import org.apache.streampark.console.base.domain.ResponseCode;
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.domain.RestResponse;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shiro.authz.annotation.RequiresPermissions;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.DeleteMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotBlank;
+
+import java.util.List;
+
+@Slf4j
+@Validated
+@RestController
+@RequestMapping("variable")
+public class VariableController {
+
+ @Autowired
+ CommonService commonService;
+
+ @Autowired
+ private VariableService variableService;
+
+ @PostMapping("list")
+ @RequiresPermissions("variable:view")
+ public RestResponse variableList(RestRequest restRequest, Variable variable) {
+ IPage<Variable> variableList = variableService.findVariables(variable, restRequest);
+ return RestResponse.success(variableList);
+ }
+
+ @PostMapping("post")
+ @RequiresPermissions("variable:add")
+ public RestResponse addVariable(@Valid Variable variable) throws Exception {
+ boolean isExists = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode()) != null;
+ if (isExists) {
+ return RestResponse.fail("Sorry, the variable Code already exists", ResponseCode.CODE_FAIL_ALERT);
+ }
+ isExists = this.variableService.findByVariableName(variable.getTeamId(), variable.getVariableName()) != null;
+ if (isExists) {
+ return RestResponse.fail("Sorry, the variable Name already exists", ResponseCode.CODE_FAIL_ALERT);
+ }
+ variable.setUserId(commonService.getCurrentUser().getUserId());
+ this.variableService.createVariable(variable);
+ return RestResponse.success();
+ }
+
+ @PutMapping("update")
+ @RequiresPermissions("variable:update")
+ public RestResponse updateVariable(@Valid Variable variable) throws Exception {
+ Variable findVariable = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ return RestResponse.fail("Sorry, the variable does not exist!", ResponseCode.CODE_FAIL_ALERT);
Review Comment:
the same
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/VariableController.java:
##########
@@ -0,0 +1,148 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.controller;
+
+import org.apache.streampark.console.base.domain.ResponseCode;
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.domain.RestResponse;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shiro.authz.annotation.RequiresPermissions;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.DeleteMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotBlank;
+
+import java.util.List;
+
+@Slf4j
+@Validated
+@RestController
+@RequestMapping("variable")
+public class VariableController {
+
+ @Autowired
+ CommonService commonService;
+
+ @Autowired
+ private VariableService variableService;
+
+ @PostMapping("list")
+ @RequiresPermissions("variable:view")
+ public RestResponse variableList(RestRequest restRequest, Variable variable) {
+ IPage<Variable> variableList = variableService.findVariables(variable, restRequest);
+ return RestResponse.success(variableList);
+ }
+
+ @PostMapping("post")
+ @RequiresPermissions("variable:add")
+ public RestResponse addVariable(@Valid Variable variable) throws Exception {
+ boolean isExists = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode()) != null;
+ if (isExists) {
+ return RestResponse.fail("Sorry, the variable Code already exists", ResponseCode.CODE_FAIL_ALERT);
+ }
+ isExists = this.variableService.findByVariableName(variable.getTeamId(), variable.getVariableName()) != null;
+ if (isExists) {
+ return RestResponse.fail("Sorry, the variable Name already exists", ResponseCode.CODE_FAIL_ALERT);
+ }
+ variable.setUserId(commonService.getCurrentUser().getUserId());
+ this.variableService.createVariable(variable);
+ return RestResponse.success();
+ }
+
+ @PutMapping("update")
+ @RequiresPermissions("variable:update")
+ public RestResponse updateVariable(@Valid Variable variable) throws Exception {
+ Variable findVariable = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ return RestResponse.fail("Sorry, the variable does not exist!", ResponseCode.CODE_FAIL_ALERT);
+ }
+ if (findVariable.getId() != variable.getId()) {
+ return RestResponse.fail("Sorry, the variable id is inconsistent!", ResponseCode.CODE_FAIL_ALERT);
Review Comment:
the same
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/VariableController.java:
##########
@@ -0,0 +1,148 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.controller;
+
+import org.apache.streampark.console.base.domain.ResponseCode;
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.domain.RestResponse;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shiro.authz.annotation.RequiresPermissions;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.DeleteMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotBlank;
+
+import java.util.List;
+
+@Slf4j
+@Validated
+@RestController
+@RequestMapping("variable")
+public class VariableController {
+
+ @Autowired
+ CommonService commonService;
Review Comment:
change to `private CommonService commonService`
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/VariableController.java:
##########
@@ -0,0 +1,148 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.controller;
+
+import org.apache.streampark.console.base.domain.ResponseCode;
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.domain.RestResponse;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shiro.authz.annotation.RequiresPermissions;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.DeleteMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotBlank;
+
+import java.util.List;
+
+@Slf4j
+@Validated
+@RestController
+@RequestMapping("variable")
+public class VariableController {
+
+ @Autowired
+ CommonService commonService;
+
+ @Autowired
+ private VariableService variableService;
+
+ @PostMapping("list")
+ @RequiresPermissions("variable:view")
+ public RestResponse variableList(RestRequest restRequest, Variable variable) {
+ IPage<Variable> variableList = variableService.findVariables(variable, restRequest);
+ return RestResponse.success(variableList);
+ }
+
+ @PostMapping("post")
+ @RequiresPermissions("variable:add")
+ public RestResponse addVariable(@Valid Variable variable) throws Exception {
+ boolean isExists = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode()) != null;
+ if (isExists) {
+ return RestResponse.fail("Sorry, the variable Code already exists", ResponseCode.CODE_FAIL_ALERT);
+ }
+ isExists = this.variableService.findByVariableName(variable.getTeamId(), variable.getVariableName()) != null;
+ if (isExists) {
+ return RestResponse.fail("Sorry, the variable Name already exists", ResponseCode.CODE_FAIL_ALERT);
Review Comment:
`throw new ApiAlertException(...)`
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/VariableController.java:
##########
@@ -0,0 +1,148 @@
+/*
+ * 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.
+ */
+
+package org.apache.streampark.console.system.controller;
+
+import org.apache.streampark.console.base.domain.ResponseCode;
+import org.apache.streampark.console.base.domain.RestRequest;
+import org.apache.streampark.console.base.domain.RestResponse;
+import org.apache.streampark.console.core.entity.Application;
+import org.apache.streampark.console.core.service.CommonService;
+import org.apache.streampark.console.system.entity.Variable;
+import org.apache.streampark.console.system.service.VariableService;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shiro.authz.annotation.RequiresPermissions;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.validation.annotation.Validated;
+import org.springframework.web.bind.annotation.DeleteMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotBlank;
+
+import java.util.List;
+
+@Slf4j
+@Validated
+@RestController
+@RequestMapping("variable")
+public class VariableController {
+
+ @Autowired
+ CommonService commonService;
+
+ @Autowired
+ private VariableService variableService;
+
+ @PostMapping("list")
+ @RequiresPermissions("variable:view")
+ public RestResponse variableList(RestRequest restRequest, Variable variable) {
+ IPage<Variable> variableList = variableService.findVariables(variable, restRequest);
+ return RestResponse.success(variableList);
+ }
+
+ @PostMapping("post")
+ @RequiresPermissions("variable:add")
+ public RestResponse addVariable(@Valid Variable variable) throws Exception {
+ boolean isExists = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode()) != null;
+ if (isExists) {
+ return RestResponse.fail("Sorry, the variable Code already exists", ResponseCode.CODE_FAIL_ALERT);
+ }
+ isExists = this.variableService.findByVariableName(variable.getTeamId(), variable.getVariableName()) != null;
+ if (isExists) {
+ return RestResponse.fail("Sorry, the variable Name already exists", ResponseCode.CODE_FAIL_ALERT);
+ }
+ variable.setUserId(commonService.getCurrentUser().getUserId());
+ this.variableService.createVariable(variable);
+ return RestResponse.success();
+ }
+
+ @PutMapping("update")
+ @RequiresPermissions("variable:update")
+ public RestResponse updateVariable(@Valid Variable variable) throws Exception {
+ Variable findVariable = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ return RestResponse.fail("Sorry, the variable does not exist!", ResponseCode.CODE_FAIL_ALERT);
+ }
+ if (findVariable.getId() != variable.getId()) {
+ return RestResponse.fail("Sorry, the variable id is inconsistent!", ResponseCode.CODE_FAIL_ALERT);
+ }
+ this.variableService.updateVariable(variable);
+ return RestResponse.success();
+ }
+
+ @DeleteMapping("delete")
+ @RequiresPermissions("variable:delete")
+ public RestResponse deleteVariables(@Valid Variable variable) {
+ Variable findVariable = this.variableService.findByVariableCode(variable.getTeamId(), variable.getVariableCode());
+ if (findVariable == null) {
+ return RestResponse.fail("Sorry, the variable does not exist!", ResponseCode.CODE_FAIL_ALERT);
Review Comment:
the same
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #1831: [Feature] Add basic functionality of variable modules such as create,…
Posted by GitBox <gi...@apache.org>.
wolfboys commented on code in PR #1831:
URL: https://github.com/apache/incubator-streampark/pull/1831#discussion_r994839182
##########
streampark-console/streampark-console-service/src/assembly/script/schema/mysql-schema.sql:
##########
@@ -319,6 +319,24 @@ create table `t_team` (
unique key `team_name_idx` (`team_name`) using btree
) engine = innodb default charset = utf8mb4 collate = utf8mb4_general_ci;
+-- ----------------------------
+-- Table of t_variable
+-- ----------------------------
+drop table if exists `t_variable`;
+create table `t_variable` (
+ `id` bigint not null auto_increment,
+ `variable_code` varchar(100) collate utf8mb4_general_ci not null comment 'variable code',
Review Comment:
about `comment` could it be clearer?
##########
streampark-console/streampark-console-service/src/assembly/script/upgrade/mysql/1.2.4.sql:
##########
@@ -201,5 +205,21 @@ alter table `t_user`
modify `username` varchar(255) collate utf8mb4_general_ci not null comment 'user name',
add unique key `un_username` (`username`) using btree;
+drop table if exists `t_variable`;
+create table `t_variable` (
+ `id` bigint not null auto_increment,
+ `variable_code` varchar(100) collate utf8mb4_general_ci not null comment 'variable code',
+ `variable_value` varchar(1024) collate utf8mb4_general_ci not null comment 'variable value',
Review Comment:
Suggest change type to: text
##########
streampark-console/streampark-console-service/src/assembly/script/schema/mysql-schema.sql:
##########
@@ -319,6 +319,24 @@ create table `t_team` (
unique key `team_name_idx` (`team_name`) using btree
) engine = innodb default charset = utf8mb4 collate = utf8mb4_general_ci;
+-- ----------------------------
+-- Table of t_variable
+-- ----------------------------
+drop table if exists `t_variable`;
+create table `t_variable` (
+ `id` bigint not null auto_increment,
+ `variable_code` varchar(100) collate utf8mb4_general_ci not null comment 'variable code',
+ `variable_value` varchar(1024) collate utf8mb4_general_ci not null comment 'variable value',
Review Comment:
Suggest change type to: text
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org