You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@inlong.apache.org by GitBox <gi...@apache.org> on 2022/07/05 03:26:50 UTC
[GitHub] [inlong] healchow opened a new pull request, #4869: [INLONG-4867][Manager] Add restrictions to the naming of some key fields
healchow opened a new pull request, #4869:
URL: https://github.com/apache/inlong/pull/4869
### Prepare a Pull Request
- Fixes #4867
### Motivation
Add restrictions to the naming of some key fields.
### Verifying this change
- [X] This change is a trivial rework/code cleanup without any test coverage.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] healchow merged pull request #4869: [INLONG-4867][Manager] Add restrictions to the naming of some key fields
Posted by GitBox <gi...@apache.org>.
healchow merged PR #4869:
URL: https://github.com/apache/inlong/pull/4869
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] chestnut-c commented on a diff in pull request #4869: [INLONG-4867][Manager] Add restrictions to the naming of some key fields
Posted by GitBox <gi...@apache.org>.
chestnut-c commented on code in PR #4869:
URL: https://github.com/apache/inlong/pull/4869#discussion_r913360254
##########
inlong-manager/manager-common/src/main/java/org/apache/inlong/manager/common/pojo/cluster/ClusterRequest.java:
##########
@@ -44,18 +44,18 @@ public class ClusterRequest {
@ApiModelProperty(value = "Primary key")
private Integer id;
- @NotBlank
+ @NotBlank(message = "cluster name cannot be blank")
@ApiModelProperty(value = "Cluster name")
private String name;
- @NotBlank
+ @NotBlank(message = "cluster type cannot be blank")
@ApiModelProperty(value = "Cluster type, including TUBE, PULSAR, DATA_PROXY, etc.")
private String type;
@ApiModelProperty(value = "Cluster url")
private String url;
- @NotBlank
+ @NotBlank(message = "clusterTags cannot be blank")
@ApiModelProperty(value = "Cluster tags, separated by commas")
private String clusterTags;
Review Comment:
Why can't tag be empty？nothing to do with business
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] healchow commented on a diff in pull request #4869: [INLONG-4867][Manager] Add restrictions to the naming of some key fields
Posted by GitBox <gi...@apache.org>.
healchow commented on code in PR #4869:
URL: https://github.com/apache/inlong/pull/4869#discussion_r913415065
##########
inlong-manager/manager-common/src/main/java/org/apache/inlong/manager/common/pojo/group/InlongGroupRequest.java:
##########
@@ -46,10 +46,9 @@
public class InlongGroupRequest {
@ApiModelProperty(value = "Inlong group id", required = true)
- @Length(min = 4, max = 200)
- @Pattern(regexp = "^(?![0-9]+$)[a-z][a-z0-9_-]{1,200}$",
- message = "inlongGroupId must starts with a lowercase letter "
- + "and contains only lowercase letters, digits, `-` or `_`")
+ @Length(min = 4, max = 100, message = "inlongGroupId length must be between 4 and 100")
Review Comment:
Done.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] healchow commented on a diff in pull request #4869: [INLONG-4867][Manager] Add restrictions to the naming of some key fields
Posted by GitBox <gi...@apache.org>.
healchow commented on code in PR #4869:
URL: https://github.com/apache/inlong/pull/4869#discussion_r913436020
##########
inlong-manager/manager-common/src/main/java/org/apache/inlong/manager/common/pojo/group/InlongGroupRequest.java:
##########
@@ -46,10 +46,9 @@
public class InlongGroupRequest {
@ApiModelProperty(value = "Inlong group id", required = true)
- @Length(min = 4, max = 200)
- @Pattern(regexp = "^(?![0-9]+$)[a-z][a-z0-9_-]{1,200}$",
- message = "inlongGroupId must starts with a lowercase letter "
- + "and contains only lowercase letters, digits, `-` or `_`")
+ @Length(min = 4, max = 100, message = "inlongGroupId length must be between 4 and 100")
+ @Pattern(regexp = "^[a-z0-9_-]{4,100}$",
+ message = "inlongGroupId only supports lowercase letters, numbers, '_', or '_'")
Review Comment:
This tool will remove at https://github.com/apache/inlong/pull/4873.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] gong commented on a diff in pull request #4869: [INLONG-4867][Manager] Add restrictions to the naming of some key fields
Posted by GitBox <gi...@apache.org>.
gong commented on code in PR #4869:
URL: https://github.com/apache/inlong/pull/4869#discussion_r913429996
##########
inlong-manager/manager-common/src/main/java/org/apache/inlong/manager/common/pojo/group/InlongGroupRequest.java:
##########
@@ -46,10 +46,9 @@
public class InlongGroupRequest {
@ApiModelProperty(value = "Inlong group id", required = true)
- @Length(min = 4, max = 200)
- @Pattern(regexp = "^(?![0-9]+$)[a-z][a-z0-9_-]{1,200}$",
- message = "inlongGroupId must starts with a lowercase letter "
- + "and contains only lowercase letters, digits, `-` or `_`")
+ @Length(min = 4, max = 100, message = "inlongGroupId length must be between 4 and 100")
+ @Pattern(regexp = "^[a-z0-9_-]{4,100}$",
+ message = "inlongGroupId only supports lowercase letters, numbers, '_', or '_'")
Review Comment:
`SmallTools.isLowerOrNum` should modify regexp
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] healchow commented on a diff in pull request #4869: [INLONG-4867][Manager] Add restrictions to the naming of some key fields
Posted by GitBox <gi...@apache.org>.
healchow commented on code in PR #4869:
URL: https://github.com/apache/inlong/pull/4869#discussion_r913361219
##########
inlong-manager/manager-common/src/main/java/org/apache/inlong/manager/common/pojo/cluster/ClusterRequest.java:
##########
@@ -44,18 +44,18 @@ public class ClusterRequest {
@ApiModelProperty(value = "Primary key")
private Integer id;
- @NotBlank
+ @NotBlank(message = "cluster name cannot be blank")
@ApiModelProperty(value = "Cluster name")
private String name;
- @NotBlank
+ @NotBlank(message = "cluster type cannot be blank")
@ApiModelProperty(value = "Cluster type, including TUBE, PULSAR, DATA_PROXY, etc.")
private String type;
@ApiModelProperty(value = "Cluster url")
private String url;
- @NotBlank
+ @NotBlank(message = "clusterTags cannot be blank")
@ApiModelProperty(value = "Cluster tags, separated by commas")
private String clusterTags;
Review Comment:
If a cluster has no tag, it will not be found by any tag.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] leosanqing commented on a diff in pull request #4869: [INLONG-4867][Manager] Add restrictions to the naming of some key fields
Posted by GitBox <gi...@apache.org>.
leosanqing commented on code in PR #4869:
URL: https://github.com/apache/inlong/pull/4869#discussion_r913410073
##########
inlong-manager/manager-common/src/main/java/org/apache/inlong/manager/common/pojo/group/InlongGroupRequest.java:
##########
@@ -46,10 +46,9 @@
public class InlongGroupRequest {
@ApiModelProperty(value = "Inlong group id", required = true)
- @Length(min = 4, max = 200)
- @Pattern(regexp = "^(?![0-9]+$)[a-z][a-z0-9_-]{1,200}$",
- message = "inlongGroupId must starts with a lowercase letter "
- + "and contains only lowercase letters, digits, `-` or `_`")
+ @Length(min = 4, max = 100, message = "inlongGroupId length must be between 4 and 100")
Review Comment:
Please add @NotBlank，Otherwise, if null, the length check will be bypassed
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] healchow commented on a diff in pull request #4869: [INLONG-4867][Manager] Add restrictions to the naming of some key fields
Posted by GitBox <gi...@apache.org>.
healchow commented on code in PR #4869:
URL: https://github.com/apache/inlong/pull/4869#discussion_r913392357
##########
inlong-manager/manager-common/src/main/java/org/apache/inlong/manager/common/pojo/source/SourceRequest.java:
##########
@@ -35,21 +39,26 @@
@JsonTypeInfo(use = Id.NAME, visible = true, property = "sourceType")
public class SourceRequest {
+ @NotNull(groups = UpdateValidation.class)
+ @ApiModelProperty(value = "Primary key")
private Integer id;
- @NotBlank(message = "inlongGroupId cannot be null")
+ @NotBlank(message = "inlongGroupId cannot be blank")
@ApiModelProperty("Inlong group id")
private String inlongGroupId;
- @NotBlank(message = "inlongStreamId cannot be null")
+ @NotBlank(message = "inlongStreamId cannot be blank")
@ApiModelProperty("Inlong stream id")
private String inlongStreamId;
- @NotBlank(message = "sourceType cannot be null")
+ @NotBlank(message = "sourceType cannot be blank")
@ApiModelProperty("Source type, including: FILE, KAFKA, etc.")
private String sourceType;
- @NotBlank(message = "sourceName cannot be null")
+ @NotBlank(message = "sourceName cannot be blank")
+ @Length(min = 1, max = 100, message = "sourceName length must be between 1 and 100")
+ @Pattern(regexp = "^[a-z0-9_-]{1,100}$",
+ message = "sourceName only supports lowercase letters, numbers, '_', or '_'")
Review Comment:
Change 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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] leosanqing commented on a diff in pull request #4869: [INLONG-4867][Manager] Add restrictions to the naming of some key fields
Posted by GitBox <gi...@apache.org>.
leosanqing commented on code in PR #4869:
URL: https://github.com/apache/inlong/pull/4869#discussion_r913415443
##########
inlong-manager/manager-web/src/main/java/org/apache/inlong/manager/web/controller/StreamTransformController.java:
##########
@@ -58,27 +59,28 @@ public Response<Integer> save(@Validated @RequestBody TransformRequest request)
}
@RequestMapping(value = "/list", method = RequestMethod.GET)
- @ApiOperation(value = "Query stream transform list")
+ @ApiOperation(value = "Get stream transform list")
public Response<List<TransformResponse>> list(@RequestParam("inlongGroupId") String groupId,
@RequestParam("inlongStreamId") String streamId) {
return Response.success(streamTransformService.listTransform(groupId, streamId));
}
@RequestMapping(value = "/update", method = RequestMethod.POST)
@OperationLog(operation = OperationType.UPDATE)
- @ApiOperation(value = "Modify stream source")
- public Response<Boolean> update(@Validated @RequestBody TransformRequest request) {
- return Response.success(
- streamTransformService.update(request, LoginUserUtils.getLoginUserDetail().getUsername()));
+ @ApiOperation(value = "Update stream transform")
+ public Response<Boolean> update(@Validated(UpdateValidation.class) @RequestBody TransformRequest request) {
+ String operator = LoginUserUtils.getLoginUserDetail().getUsername();
+ return Response.success(streamTransformService.update(request, operator));
}
@RequestMapping(value = "/delete", method = RequestMethod.DELETE)
@OperationLog(operation = OperationType.UPDATE)
- @ApiOperation(value = "Modify stream source")
+ @ApiOperation(value = "Delete stream transform")
public Response<Boolean> delete(@RequestParam("inlongGroupId") String groupId,
- @RequestParam("inlongStreamId") String streamId, @RequestParam("transformName") String transformName) {
- return Response.success(
- streamTransformService.delete(groupId, streamId, transformName,
- LoginUserUtils.getLoginUserDetail().getUsername()));
+ @RequestParam("inlongStreamId") String streamId,
Review Comment:
Could add @NotBlank,the following validation only checks Null, '''' will bypass the validation.
If you do this, remember to add @Validated to the class, otherwise the validation will not work
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] gong commented on a diff in pull request #4869: [INLONG-4867][Manager] Add restrictions to the naming of some key fields
Posted by GitBox <gi...@apache.org>.
gong commented on code in PR #4869:
URL: https://github.com/apache/inlong/pull/4869#discussion_r913431993
##########
inlong-manager/manager-common/src/main/java/org/apache/inlong/manager/common/pojo/transform/TransformRequest.java:
##########
@@ -32,7 +36,9 @@
@ApiModel("Request of stream transform")
public class TransformRequest {
- private int id;
+ @NotNull(groups = UpdateValidation.class)
+ @ApiModelProperty(value = "Primary key")
+ private Integer id;
@NotBlank(message = "inlongGroupId cannot be blank")
@ApiModelProperty("Inlong group id")
Review Comment:
`inlong group` should be `inlong stream`
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] chestnut-c commented on a diff in pull request #4869: [INLONG-4867][Manager] Add restrictions to the naming of some key fields
Posted by GitBox <gi...@apache.org>.
chestnut-c commented on code in PR #4869:
URL: https://github.com/apache/inlong/pull/4869#discussion_r913362983
##########
inlong-manager/manager-common/src/main/java/org/apache/inlong/manager/common/pojo/source/SourceRequest.java:
##########
@@ -35,21 +39,26 @@
@JsonTypeInfo(use = Id.NAME, visible = true, property = "sourceType")
public class SourceRequest {
+ @NotNull(groups = UpdateValidation.class)
+ @ApiModelProperty(value = "Primary key")
private Integer id;
- @NotBlank(message = "inlongGroupId cannot be null")
+ @NotBlank(message = "inlongGroupId cannot be blank")
@ApiModelProperty("Inlong group id")
private String inlongGroupId;
- @NotBlank(message = "inlongStreamId cannot be null")
+ @NotBlank(message = "inlongStreamId cannot be blank")
@ApiModelProperty("Inlong stream id")
private String inlongStreamId;
- @NotBlank(message = "sourceType cannot be null")
+ @NotBlank(message = "sourceType cannot be blank")
@ApiModelProperty("Source type, including: FILE, KAFKA, etc.")
private String sourceType;
- @NotBlank(message = "sourceName cannot be null")
+ @NotBlank(message = "sourceName cannot be blank")
+ @Length(min = 1, max = 100, message = "sourceName length must be between 1 and 100")
+ @Pattern(regexp = "^[a-z0-9_-]{1,100}$",
+ message = "sourceName only supports lowercase letters, numbers, '_', or '_'")
Review Comment:
Two "_" are redundant
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] healchow commented on a diff in pull request #4869: [INLONG-4867][Manager] Add restrictions to the naming of some key fields
Posted by GitBox <gi...@apache.org>.
healchow commented on code in PR #4869:
URL: https://github.com/apache/inlong/pull/4869#discussion_r913424374
##########
inlong-manager/manager-web/src/main/java/org/apache/inlong/manager/web/controller/StreamTransformController.java:
##########
@@ -58,27 +59,28 @@ public Response<Integer> save(@Validated @RequestBody TransformRequest request)
}
@RequestMapping(value = "/list", method = RequestMethod.GET)
- @ApiOperation(value = "Query stream transform list")
+ @ApiOperation(value = "Get stream transform list")
public Response<List<TransformResponse>> list(@RequestParam("inlongGroupId") String groupId,
@RequestParam("inlongStreamId") String streamId) {
return Response.success(streamTransformService.listTransform(groupId, streamId));
}
@RequestMapping(value = "/update", method = RequestMethod.POST)
@OperationLog(operation = OperationType.UPDATE)
- @ApiOperation(value = "Modify stream source")
- public Response<Boolean> update(@Validated @RequestBody TransformRequest request) {
- return Response.success(
- streamTransformService.update(request, LoginUserUtils.getLoginUserDetail().getUsername()));
+ @ApiOperation(value = "Update stream transform")
+ public Response<Boolean> update(@Validated(UpdateValidation.class) @RequestBody TransformRequest request) {
+ String operator = LoginUserUtils.getLoginUserDetail().getUsername();
+ return Response.success(streamTransformService.update(request, operator));
}
@RequestMapping(value = "/delete", method = RequestMethod.DELETE)
@OperationLog(operation = OperationType.UPDATE)
- @ApiOperation(value = "Modify stream source")
+ @ApiOperation(value = "Delete stream transform")
public Response<Boolean> delete(@RequestParam("inlongGroupId") String groupId,
- @RequestParam("inlongStreamId") String streamId, @RequestParam("transformName") String transformName) {
- return Response.success(
- streamTransformService.delete(groupId, streamId, transformName,
- LoginUserUtils.getLoginUserDetail().getUsername()));
+ @RequestParam("inlongStreamId") String streamId,
Review Comment:
Done.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org