kezhenxu94 commented on code in PR #11929:
URL: https://github.com/apache/dolphinscheduler/pull/11929#discussion_r970513962
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ExecutorServiceImpl.java:
##########
@@ -903,7 +903,8 @@ protected int
createComplementDependentCommand(List<Schedule> schedules, Command
List<DependentProcessDefinition> dependentProcessDefinitionList =
getComplementDependentDefinitionList(dependentCommand.getProcessDefinitionCode(),
CronUtils.getMaxCycle(schedules.get(0).getCrontab()),
dependentCommand.getWorkerGroup());
-
+ // If the id is Integer, the auto-increment id will be obtained and
cloned, causing duplicate writes
Review Comment:
How did you get this conclusion? From the documentation of
`BeanUtils.cloneBean`, it copies all fields regardless of its type
##########
dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/CommandMapperTest.java:
##########
@@ -179,6 +182,14 @@ public void testQueryCommandPageBySlot() {
toTestQueryCommandPageBySlot(masterCount, thisMasterSlot);
}
+ @Test
+ public void testClone() throws Exception {
+ Command command1 = createCommand();
+ Command command2 = createCommand();
+ Command cloneBean = (Command) BeanUtils.cloneBean(command1);
+ Assert.assertEquals(cloneBean.getId(), command1.getId());
+ }
Review Comment:
This is pointless, you are testing a third-party method, this is not our
responsibility, instead, you should add a test case that verifies
`createComplementDependentCommand` doesn't copy the id to the new dependent
command
--
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]
