Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Improvement][ui] improving to find current version identifier(#15815) #15933

Open
wants to merge 18 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
3a9e080
[Improvement][ui] improving to find current version identifier(#15815)
pusl6 Apr 28, 2024
35f0aac
Merge branch 'apache:dev' into dev
pusl6 Apr 29, 2024
4ec8f9a
[Improvement][ui] improving to find current version identifier(#15815)
pusl6 Apr 30, 2024
f808e70
[Improvement][ui] improving to find current version identifier(#15815)
pusl6 Apr 30, 2024
00fbaca
Merge branch 'apache:dev' into dev
pusl6 May 6, 2024
b9fedfc
[Improvement][ui] improving to find current version identifier(#15815)
pusl6 May 6, 2024
c02e79d
[Improvement][ui] improving to find current version identifier(#15815)
pusl6 May 7, 2024
5043916
Merge branch 'dev' into dev
davidzollo May 8, 2024
db81822
[Improvement][ui] improving to find current version identifier(#15815)
pusl6 May 8, 2024
a524f5e
[Improvement][ui] improving to find current version identifier(#15815)
pusl6 May 11, 2024
a188699
[Improvement][ui] improving to find current version identifier(#15815)
pusl6 May 11, 2024
1e7b930
[Improvement][ui] improving to find current version identifier(#15815)
pusl6 May 15, 2024
d8b6741
[Improvement][ui] improving to find current version identifier(#15815)
pusl6 May 22, 2024
e84feb3
[Improvement][ui] improving to find current version identifier(#15815)
pusl6 May 28, 2024
e03491b
Merge branch 'dev' into dev
SbloodyS May 29, 2024
228a84c
[Improvement][ui] improving to find current version identifier(#15815)
pusl6 May 29, 2024
234d61a
Merge remote-tracking branch 'origin/dev' into dev
pusl6 May 29, 2024
b2abdd2
[Improvement][ui] improving to find current version identifier(#15815)
pusl6 Jun 4, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@

package org.apache.dolphinscheduler.api.controller;

import static org.apache.dolphinscheduler.api.enums.Status.QUERY_PLUGINS_ERROR;

import org.apache.dolphinscheduler.api.dto.ProductInfoDto;
import org.apache.dolphinscheduler.api.exceptions.ApiException;
import org.apache.dolphinscheduler.api.service.UiPluginService;
import org.apache.dolphinscheduler.api.utils.Result;
Expand All @@ -30,20 +29,16 @@

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpStatus;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RequestAttribute;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.ResponseStatus;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.bind.annotation.*;
SbloodyS marked this conversation as resolved.
Show resolved Hide resolved

import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.Parameter;
import io.swagger.v3.oas.annotations.Parameters;
import io.swagger.v3.oas.annotations.media.Schema;
import io.swagger.v3.oas.annotations.tags.Tag;

import static org.apache.dolphinscheduler.api.enums.Status.*;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid wildcard import.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use mvn spotless:apply to fix these.


/**
* ui plugin controller
* Some plugins (such as alert plugin) need to provide UI interfaces to users.
Expand Down Expand Up @@ -85,4 +80,22 @@ public Result queryUiPluginDetailById(@Parameter(hidden = true) @RequestAttribut
Map<String, Object> result = uiPluginService.queryUiPluginDetailById(pluginId);
return returnDataList(result);
}

/**
* obtain project version and address
*
// * @param loginUser login user
// * @param userId token for user
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// * @param loginUser login user
// * @param userId token for user

* @return product info
*/
@Operation(summary = "queryProductInfo", description = "QUERY_PRODUCT_INFO")
@PostMapping(value = "/queryProductInfo")
ruanwenjun marked this conversation as resolved.
Show resolved Hide resolved
@ResponseStatus(HttpStatus.OK)
@ApiException(VERSION_INFO_STATE_ERROR)
public Result<ProductInfoDto> queryProductInfo(
@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser,
@RequestParam(value = "userId") Integer userId) {
ProductInfoDto result = uiPluginService.queryProductInfo(loginUser, userId);
return Result.success(result);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public Result<ProductInfoDto> queryProductInfo(
@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser,
@RequestParam(value = "userId") Integer userId) {
ProductInfoDto result = uiPluginService.queryProductInfo(loginUser, userId);
return Result.success(result);
public Result<ProductInfoDto> queryProductInfo(
@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser) {
ProductInfoDto result = uiPluginService.queryProductInfo(loginUser);
return Result.success(result);

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* 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.dolphinscheduler.api.dto;

import lombok.Data;

import java.util.Date;
import java.util.List;

/**
* ProductInfoDto
*/
Comment on lines +25 to +27
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/**
* ProductInfoDto
*/
/**
* ProductInfoDto
*/

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My original idea for the throw code was to report errors when there is data but the version field is empty or null
Based on your proposal, I have made the following changes:

  1. Remove error prompts and directly display unknown when missing data
  2. Modify API URL path
  3. Remove method comments
    image

Copy link
Member

@ruanwenjun ruanwenjun Jun 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is meaningless, you need to remove.

@Data
public class ProductInfoDto {

private Integer id;
ruanwenjun marked this conversation as resolved.
Show resolved Hide resolved

private String version;

}
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ public enum Status {
QUERY_PROJECT_PREFERENCE_ERROR(10302, "query project preference error", "查询项目偏好设置错误"),
UPDATE_PROJECT_PREFERENCE_STATE_ERROR(10303, "Failed to update the state of the project preference", "更新项目偏好设置错误"),

VERSION_INFO_STATE_ERROR(10304, "Failed to obtain project version and address", "获取版本信息错误"),
UDF_FUNCTION_NOT_EXIST(20001, "UDF function not found", "UDF函数不存在"),
UDF_FUNCTION_EXISTS(20002, "UDF function already exists", "UDF函数已存在"),
RESOURCE_NOT_EXIST(20004, "resource not exist", "资源不存在"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@

package org.apache.dolphinscheduler.api.service;

import org.apache.dolphinscheduler.api.dto.ProductInfoDto;
import org.apache.dolphinscheduler.api.utils.Result;
import org.apache.dolphinscheduler.common.enums.PluginType;
import org.apache.dolphinscheduler.dao.entity.User;

import java.util.Map;

Expand All @@ -30,4 +33,6 @@ public interface UiPluginService {

Map<String, Object> queryUiPluginDetailById(int id);

ProductInfoDto queryProductInfo(User loginUser, int userId);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's weird that there exists a method queryProductInfo in uiPluginServer.

}
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,17 @@

package org.apache.dolphinscheduler.api.service.impl;

import org.apache.commons.lang3.StringUtils;
import org.apache.dolphinscheduler.api.dto.ProductInfoDto;
import org.apache.dolphinscheduler.api.enums.Status;
import org.apache.dolphinscheduler.api.exceptions.ServiceException;
import org.apache.dolphinscheduler.api.service.UiPluginService;
import org.apache.dolphinscheduler.common.constants.Constants;
import org.apache.dolphinscheduler.common.enums.PluginType;
import org.apache.dolphinscheduler.dao.entity.DsVersion;
import org.apache.dolphinscheduler.dao.entity.PluginDefine;
import org.apache.dolphinscheduler.dao.entity.User;
import org.apache.dolphinscheduler.dao.mapper.DsVersionMapper;
import org.apache.dolphinscheduler.dao.mapper.PluginDefineMapper;

import org.apache.commons.collections4.CollectionUtils;
Expand All @@ -32,6 +38,7 @@

import lombok.extern.slf4j.Slf4j;


import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;

Expand All @@ -45,6 +52,9 @@ public class UiPluginServiceImpl extends BaseServiceImpl implements UiPluginServ
@Autowired
PluginDefineMapper pluginDefineMapper;

@Autowired
DsVersionMapper dsVersionMapper;

@Override
public Map<String, Object> queryUiPluginsByType(PluginType pluginType) {
Map<String, Object> result = new HashMap<>();
Expand Down Expand Up @@ -82,4 +92,24 @@ public Map<String, Object> queryUiPluginDetailById(int id) {
return result;
}

@Override
public ProductInfoDto queryProductInfo(User loginUser, int userId) {

// check if user is existed
if (userId <= 0 || !(loginUser.getId() == userId)) {
throw new ServiceException(Status.REQUEST_PARAMS_NOT_VALID_ERROR,
"User id: " + userId + " should not less than or equals to 0.");
}
// persist to the database
ruanwenjun marked this conversation as resolved.
Show resolved Hide resolved
DsVersion dsVersion = dsVersionMapper.selectById(1);

if(StringUtils.isBlank(dsVersion.getVersion())){
throw new ServiceException(Status.VERSION_INFO_STATE_ERROR);
}
ProductInfoDto result = new ProductInfoDto();
result.setId(dsVersion.getId());
result.setVersion(dsVersion.getVersion());
return result;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't query the version by id, id might changed.

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,24 @@
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.Mockito.when;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

import org.apache.dolphinscheduler.api.dto.ProductInfoDto;
import org.apache.dolphinscheduler.api.enums.Status;
import org.apache.dolphinscheduler.api.service.UiPluginService;
import org.apache.dolphinscheduler.api.utils.Result;
import org.apache.dolphinscheduler.common.constants.Constants;
import org.apache.dolphinscheduler.common.enums.PluginType;
import org.apache.dolphinscheduler.common.utils.JSONUtils;
import org.apache.dolphinscheduler.dao.entity.User;

import org.mockito.Mockito;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.http.MediaType;
import org.springframework.test.web.servlet.MvcResult;
Expand All @@ -41,6 +48,9 @@

import com.google.common.collect.ImmutableMap;

import java.util.HashMap;
import java.util.Map;

/**
* ui plugin controller test
*/
Expand All @@ -53,6 +63,8 @@ public class UiPluginControllerTest extends AbstractControllerTest {
private static final ImmutableMap<String, Object> uiPluginServiceResult =
ImmutableMap.of(Constants.STATUS, Status.SUCCESS, Constants.DATA_LIST, "Test Data");

private static final Logger logger = LoggerFactory.getLogger(TenantControllerTest.class);

ruanwenjun marked this conversation as resolved.
Show resolved Hide resolved
@MockBean(name = "uiPluginService")
private UiPluginService uiPluginService;

Expand Down Expand Up @@ -91,4 +103,33 @@ public void testQueryUiPluginDetailById() throws Exception {
JSONUtils.parseObject(mvcResult.getResponse().getContentAsString(), Result.class);
assertThat(actualResponseContent.toString()).isEqualTo(expectResponseContent.toString());
}

@Test
public void testQueryProductInfo() throws Exception {
// Map<String, Object> mockResult = new HashMap<>();
// mockResult.put(Constants.STATUS, Status.SUCCESS);
SbloodyS marked this conversation as resolved.
Show resolved Hide resolved
ProductInfoDto mockResult = new ProductInfoDto();
Mockito.when(uiPluginService.queryProductInfo(Mockito.any(), Mockito.anyInt())).thenReturn(mockResult);

MultiValueMap<String, String> paramsMap = new LinkedMultiValueMap<>();
paramsMap.add("userId", "1");

MvcResult mvcResult = mockMvc.perform(post("/ui-plugins/queryProductInfo")
.header(SESSION_ID, sessionId)
.params(paramsMap))
.andExpect(status().isOk())
.andExpect(content().contentType(MediaType.APPLICATION_JSON))
.andReturn();

Result result = JSONUtils.parseObject(mvcResult.getResponse().getContentAsString(), Result.class);
Assertions.assertEquals(Status.SUCCESS.getCode(), result.getCode().intValue());
logger.info(mvcResult.getResponse().getContentAsString());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use log in UT

}

private User getLoginUser() {
User user = new User();
user.setId(1);
user.setUserName("admin");
return user;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't add useless code

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't add useless code

Based on your proposal, I have made the following modifications:

  1. Remove the userid from the input parameter
  2. Change the query for version to selectVersion ()
  3. Remove log() and redundant methods in UT
    In addition, the queryProductInfo method in uiPluginService was submitted this time and there are no duplicate named methods in the project, so the method name has not been changed

}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ export function useDropDown() {
useLogout()
} else if (key === 'password') {
router.push({ path: '/password' })
} else if (key === 'product') {
router.push({ path: '/product' })
} else if (key === 'profile') {
router.push({ path: '/profile' })
}
Expand Down
6 changes: 6 additions & 0 deletions dolphinscheduler-ui/src/layouts/content/use-dataList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
DesktopOutlined,
SafetyCertificateOutlined,
UserOutlined,
SelectOutlined,
LogoutOutlined,
FundProjectionScreenOutlined,
PartitionOutlined,
Expand Down Expand Up @@ -372,6 +373,11 @@ export function useDataList() {
icon: renderIcon(KeyOutlined),
disabled: userStore.getSecurityConfigType !== 'PASSWORD'
},
{
label: t('user_dropdown.product'),
key: 'product',
icon: renderIcon(SelectOutlined),
},
{
label: t('user_dropdown.logout'),
key: 'logout',
Expand Down
2 changes: 2 additions & 0 deletions dolphinscheduler-ui/src/locales/en_US/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import security from '@/locales/en_US/security'
import theme from '@/locales/en_US/theme'
import user_dropdown from '@/locales/en_US/user-dropdown'
import ui_setting from '@/locales/en_US/ui_setting'
import product from "@/locales/en_US/product";

export default {
login,
Expand All @@ -41,6 +42,7 @@ export default {
menu,
home,
password,
product,
profile,
monitor,
resource,
Expand Down
21 changes: 21 additions & 0 deletions dolphinscheduler-ui/src/locales/en_US/product.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* 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.
*/

export default {
product: 'Product Info',
product_version: 'Product Version'
}
1 change: 1 addition & 0 deletions dolphinscheduler-ui/src/locales/en_US/user-dropdown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,6 @@
export default {
profile: 'Profile',
password: 'Password',
product: 'Product',
SbloodyS marked this conversation as resolved.
Show resolved Hide resolved
logout: 'Logout'
}
2 changes: 2 additions & 0 deletions dolphinscheduler-ui/src/locales/zh_CN/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import security from '@/locales/zh_CN/security'
import theme from '@/locales/zh_CN/theme'
import user_dropdown from '@/locales/zh_CN/user-dropdown'
import ui_setting from '@/locales/zh_CN/ui_setting'
import product from "@/locales/zh_CN/product";

export default {
login,
Expand All @@ -41,6 +42,7 @@ export default {
menu,
home,
password,
product,
profile,
monitor,
resource,
Expand Down
21 changes: 21 additions & 0 deletions dolphinscheduler-ui/src/locales/zh_CN/product.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* 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.
*/

export default {
product: '产品信息',
product_version: '产品版本'
}
1 change: 1 addition & 0 deletions dolphinscheduler-ui/src/locales/zh_CN/user-dropdown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,6 @@
export default {
profile: '用户信息',
password: '密码管理',
product: '产品信息',
logout: '退出登录'
}
9 changes: 9 additions & 0 deletions dolphinscheduler-ui/src/router/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,15 @@ const basePage: RouteRecordRaw[] = [
title: '用户信息',
auth: []
}
},
{
path: '/product',
name: 'product',
component: components['product'],
meta: {
title: '产品信息',
auth: []
}
}
]
},
Expand Down