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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
