TeslaCN commented on a change in pull request #1980: URL: https://github.com/apache/shardingsphere-elasticjob/pull/1980#discussion_r710263507
########## File path: elasticjob-api/src/main/java/org/apache/shardingsphere/elasticjob/api/JobExtraConfigurationFactory.java ########## @@ -0,0 +1,30 @@ +/* + * 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.shardingsphere.elasticjob.api; + +/** + * Job extra configuration factory. + */ +public interface JobExtraConfigurationFactory { + + /** + * get JobExtraConfiguration. Review comment: The first letter should be uppercase. ########## File path: elasticjob-lite/elasticjob-lite-core/src/main/java/org/apache/shardingsphere/elasticjob/lite/internal/annotation/JobAnnotationBuilder.java ########## @@ -0,0 +1,72 @@ +/* + * 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.shardingsphere.elasticjob.lite.internal.annotation; + +import com.google.common.base.Preconditions; +import com.google.common.base.Strings; +import org.apache.shardingsphere.elasticjob.annotation.ElasticJobConfiguration; +import org.apache.shardingsphere.elasticjob.annotation.ElasticJobProp; +import org.apache.shardingsphere.elasticjob.api.JobConfiguration; +import org.apache.shardingsphere.elasticjob.api.JobExtraConfiguration; +import org.apache.shardingsphere.elasticjob.api.JobExtraConfigurationFactory; +import org.apache.shardingsphere.elasticjob.infra.exception.JobAnnotationException; + +public class JobAnnotationBuilder { + + /** + * generate JobConfiguration from @ElasticJobConfiguration. + * @param type The job of @ElasticJobConfiguration annotation class + * @return JobConfiguration + */ + public static JobConfiguration generateJobConfiguration(final Class<?> type) { + ElasticJobConfiguration annotation = type.getAnnotation(ElasticJobConfiguration.class); + Preconditions.checkArgument(null != annotation, "@ElasticJobConfiguration not found by class '%s'.", type); + Preconditions.checkArgument(!Strings.isNullOrEmpty(annotation.jobName()), "@ElasticJobConfiguration jobName not be empty by class '%s'.", type); + JobConfiguration.Builder jobConfigurationBuilder = JobConfiguration.newBuilder(annotation.jobName(), annotation.shardingTotalCount()) + .shardingItemParameters(annotation.shardingItemParameters()) + .cron(Strings.isNullOrEmpty(annotation.cron()) ? null : annotation.cron()) + .timeZone(Strings.isNullOrEmpty(annotation.timeZone()) ? null : annotation.timeZone()) + .jobParameter(annotation.jobParameter()) + .monitorExecution(annotation.monitorExecution()) + .failover(annotation.failover()) + .misfire(annotation.misfire()) + .maxTimeDiffSeconds(annotation.maxTimeDiffSeconds()) + .reconcileIntervalMinutes(annotation.reconcileIntervalMinutes()) + .jobShardingStrategyType(annotation.jobShardingStrategyType()) + .jobExecutorServiceHandlerType(annotation.jobExecutorServiceHandlerType()) + .jobErrorHandlerType(Strings.isNullOrEmpty(annotation.jobErrorHandlerType()) ? null : annotation.jobErrorHandlerType()) + .jobListenerTypes(annotation.jobListenerTypes()) + .description(annotation.description()) + .disabled(annotation.disabled()) + .overwrite(annotation.overwrite()); + for (Class<? extends JobExtraConfigurationFactory> clazz : annotation.extraConfigurations()) { + try { + JobExtraConfiguration jobExtraConfiguration = clazz.newInstance().getJobExtraConfiguration(); Review comment: I prefer doing a null check here. What do you think? ########## File path: elasticjob-lite/elasticjob-lite-core/src/main/java/org/apache/shardingsphere/elasticjob/lite/internal/annotation/JobAnnotationBuilder.java ########## @@ -0,0 +1,72 @@ +/* + * 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.shardingsphere.elasticjob.lite.internal.annotation; + +import com.google.common.base.Preconditions; +import com.google.common.base.Strings; +import org.apache.shardingsphere.elasticjob.annotation.ElasticJobConfiguration; +import org.apache.shardingsphere.elasticjob.annotation.ElasticJobProp; +import org.apache.shardingsphere.elasticjob.api.JobConfiguration; +import org.apache.shardingsphere.elasticjob.api.JobExtraConfiguration; +import org.apache.shardingsphere.elasticjob.api.JobExtraConfigurationFactory; +import org.apache.shardingsphere.elasticjob.infra.exception.JobAnnotationException; + +public class JobAnnotationBuilder { + + /** + * generate JobConfiguration from @ElasticJobConfiguration. + * @param type The job of @ElasticJobConfiguration annotation class + * @return JobConfiguration + */ + public static JobConfiguration generateJobConfiguration(final Class<?> type) { + ElasticJobConfiguration annotation = type.getAnnotation(ElasticJobConfiguration.class); + Preconditions.checkArgument(null != annotation, "@ElasticJobConfiguration not found by class '%s'.", type); + Preconditions.checkArgument(!Strings.isNullOrEmpty(annotation.jobName()), "@ElasticJobConfiguration jobName not be empty by class '%s'.", type); + JobConfiguration.Builder jobConfigurationBuilder = JobConfiguration.newBuilder(annotation.jobName(), annotation.shardingTotalCount()) + .shardingItemParameters(annotation.shardingItemParameters()) + .cron(Strings.isNullOrEmpty(annotation.cron()) ? null : annotation.cron()) + .timeZone(Strings.isNullOrEmpty(annotation.timeZone()) ? null : annotation.timeZone()) + .jobParameter(annotation.jobParameter()) + .monitorExecution(annotation.monitorExecution()) + .failover(annotation.failover()) + .misfire(annotation.misfire()) + .maxTimeDiffSeconds(annotation.maxTimeDiffSeconds()) + .reconcileIntervalMinutes(annotation.reconcileIntervalMinutes()) + .jobShardingStrategyType(annotation.jobShardingStrategyType()) + .jobExecutorServiceHandlerType(annotation.jobExecutorServiceHandlerType()) + .jobErrorHandlerType(Strings.isNullOrEmpty(annotation.jobErrorHandlerType()) ? null : annotation.jobErrorHandlerType()) + .jobListenerTypes(annotation.jobListenerTypes()) + .description(annotation.description()) + .disabled(annotation.disabled()) + .overwrite(annotation.overwrite()); + for (Class<? extends JobExtraConfigurationFactory> clazz : annotation.extraConfigurations()) { + try { + JobExtraConfiguration jobExtraConfiguration = clazz.newInstance().getJobExtraConfiguration(); + jobConfigurationBuilder.addExtraConfigurations(jobExtraConfiguration); + } catch (IllegalAccessException | InstantiationException exception) { + throw (JobAnnotationException) new JobAnnotationException("new JobExtraConfigurationFactory instance by class '%s' failure", clazz).initCause(exception); + } + } + for (ElasticJobProp prop :annotation.props()) { + jobConfigurationBuilder.setProperty(prop.key(), prop.value()); + } + Review comment: Remove the redundant blank line. ########## File path: elasticjob-api/src/test/java/org/apache/shardingsphere/elasticjob/annotation/ElasticJobConfigurationTest.java ########## @@ -0,0 +1,50 @@ +/* + * 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.shardingsphere.elasticjob.annotation; + +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; + +import java.util.Arrays; +import java.util.LinkedList; +import java.util.Queue; +import org.apache.shardingsphere.elasticjob.annotation.job.impl.SimpleTestJob; +import org.apache.shardingsphere.elasticjob.api.JobExtraConfigurationFactory; +import org.junit.Test; + +public class ElasticJobConfigurationTest { + + @Test + public void assertAnnotationJob() { + ElasticJobConfiguration annotation = SimpleTestJob.class.getAnnotation(ElasticJobConfiguration.class); + assertEquals(annotation.jobName(), "SimpleTestJob"); Review comment: Use `assertThat` instead of `assertEquals`. ########## File path: elasticjob-api/src/main/java/org/apache/shardingsphere/elasticjob/annotation/ElasticJobConfiguration.java ########## @@ -0,0 +1,155 @@ +/* + * 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.shardingsphere.elasticjob.annotation; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import org.apache.shardingsphere.elasticjob.api.JobExtraConfigurationFactory; + +/** + * The annotation that specify a jo of elastic. Review comment: A typo here. ########## File path: elasticjob-api/src/main/java/org/apache/shardingsphere/elasticjob/annotation/ElasticJobConfiguration.java ########## @@ -0,0 +1,155 @@ +/* + * 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.shardingsphere.elasticjob.annotation; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import org.apache.shardingsphere.elasticjob.api.JobExtraConfigurationFactory; + +/** + * The annotation that specify a jo of elastic. + */ +@Documented +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.TYPE) +public @interface ElasticJobConfiguration { + + /** + * Job name. + * @return jobName + */ + String jobName(); + + /** + * CRON expression, control the job trigger time. + * @return cron + */ + String cron() default ""; + + /** + * time zone of CRON. + * @return timeZone + */ + String timeZone() default ""; + + /** + * Sharding total count. + * @return shardingTotalCount + */ + int shardingTotalCount(); + + /** + * Sharding item parameters. + * @return shardingItemParameters + */ + String shardingItemParameters() default ""; + + /** + * Job parameter. + * @return jobParameter + */ + String jobParameter() default ""; + + /** + * Monitor job execution status. + * @return monitorExecution + */ + boolean monitorExecution() default true; + + /** + * Enable or disable job failover. + * @return failover + */ + boolean failover() default false; + + /** + * Enable or disable the missed task to re-execute. + * @return misfire + */ + boolean misfire() default true; + + /** + * The maximum value for time difference between server and registry center in seconds. + * @return maxTimeDiffSeconds + */ + int maxTimeDiffSeconds() default -1; + + /** + * Service scheduling interval in minutes for repairing job server inconsistent state. + * @return reconcileIntervalMinutes + */ + int reconcileIntervalMinutes() default 10; + + /** + * Job sharding strategy type. + * @return jobShardingStrategyType + */ + String jobShardingStrategyType() default "AVG_ALLOCATION"; Review comment: It's better to keep the default value same as JobConfiguration. ########## File path: elasticjob-api/src/main/java/org/apache/shardingsphere/elasticjob/annotation/ElasticJobConfiguration.java ########## @@ -0,0 +1,155 @@ +/* + * 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.shardingsphere.elasticjob.annotation; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import org.apache.shardingsphere.elasticjob.api.JobExtraConfigurationFactory; + +/** + * The annotation that specify a jo of elastic. + */ +@Documented +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.TYPE) +public @interface ElasticJobConfiguration { + + /** + * Job name. + * @return jobName + */ + String jobName(); + + /** + * CRON expression, control the job trigger time. + * @return cron + */ + String cron() default ""; + + /** + * time zone of CRON. + * @return timeZone + */ + String timeZone() default ""; + + /** + * Sharding total count. + * @return shardingTotalCount + */ + int shardingTotalCount(); + + /** + * Sharding item parameters. + * @return shardingItemParameters + */ + String shardingItemParameters() default ""; + + /** + * Job parameter. + * @return jobParameter + */ + String jobParameter() default ""; + + /** + * Monitor job execution status. + * @return monitorExecution + */ + boolean monitorExecution() default true; + + /** + * Enable or disable job failover. + * @return failover + */ + boolean failover() default false; + + /** + * Enable or disable the missed task to re-execute. + * @return misfire + */ + boolean misfire() default true; + + /** + * The maximum value for time difference between server and registry center in seconds. + * @return maxTimeDiffSeconds + */ + int maxTimeDiffSeconds() default -1; + + /** + * Service scheduling interval in minutes for repairing job server inconsistent state. + * @return reconcileIntervalMinutes + */ + int reconcileIntervalMinutes() default 10; + + /** + * Job sharding strategy type. + * @return jobShardingStrategyType + */ + String jobShardingStrategyType() default "AVG_ALLOCATION"; + + /** + * Job thread pool handler type. + * @return jobExecutorServiceHandlerType + */ + String jobExecutorServiceHandlerType() default "CPU"; Review comment: It's better to keep the default value same as JobConfiguration. -- 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]
