korlov42 commented on code in PR #2916: URL: https://github.com/apache/ignite-3/pull/2916#discussion_r1418491141
########## modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/RenameZoneCommandValidationTest.java: ########## @@ -0,0 +1,52 @@ +/* + * 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.ignite.internal.catalog.commands; + +import static org.apache.ignite.internal.catalog.CatalogService.DEFAULT_ZONE_NAME; +import static org.apache.ignite.internal.testframework.IgniteTestUtils.assertThrows; + +import org.apache.ignite.internal.catalog.CatalogValidationException; +import org.junit.jupiter.api.Test; + +/** + * Tests to verify validation of {@link RenameZoneCommand}. + */ +public class RenameZoneCommandValidationTest extends AbstractCommandValidationTest { + private static final String ZONE_NAME = "test_zone"; + + @Test + void testValidateZoneNamesOnRenameZone() { Review Comment: let's check every string from `org.apache.ignite.internal.catalog.commands.AbstractCommandValidationTest#nullAndBlankStrings` ########## modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/RenameZoneCommandValidationTest.java: ########## @@ -0,0 +1,52 @@ +/* + * 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.ignite.internal.catalog.commands; + +import static org.apache.ignite.internal.catalog.CatalogService.DEFAULT_ZONE_NAME; +import static org.apache.ignite.internal.testframework.IgniteTestUtils.assertThrows; + +import org.apache.ignite.internal.catalog.CatalogValidationException; +import org.junit.jupiter.api.Test; + +/** + * Tests to verify validation of {@link RenameZoneCommand}. + */ +public class RenameZoneCommandValidationTest extends AbstractCommandValidationTest { Review Comment: what about validation regarding particular version of catalog? see `org.apache.ignite.internal.catalog.commands.DropTableCommandValidationTest#exceptionIsThrownIfTableWithGivenNameNotFound` for example this is applied to other test classes as well ########## modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/AbstractZoneCommandBuilder.java: ########## @@ -17,27 +17,17 @@ package org.apache.ignite.internal.catalog.commands; +import org.apache.ignite.internal.catalog.CatalogCommand; + /** - * DROP ZONE statement. + * Abstract builder of zone-related commands. + * + * <p>Every zone-related command, disregard it going to create new zone or modify existing one, + * should specify name of the zone to be processed. */ -public class DropZoneParams extends AbstractZoneCommandParams { - /** Constructor. */ - private DropZoneParams(String zoneName) { - super(zoneName); - } - - /** Creates parameters builder. */ - public static Builder builder() { - return new Builder(); - } +public interface AbstractZoneCommandBuilder<T extends AbstractZoneCommandBuilder<T>> { + T zoneName(String zoneName); Review Comment: missing javadoc ########## modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/RenameZoneCommand.java: ########## @@ -0,0 +1,117 @@ +/* + * 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.ignite.internal.catalog.commands; + +import static org.apache.ignite.internal.catalog.CatalogParamsValidationUtils.validateIdentifier; +import static org.apache.ignite.internal.catalog.CatalogService.DEFAULT_ZONE_NAME; +import static org.apache.ignite.internal.catalog.commands.CatalogUtils.zoneOrThrow; + +import java.util.List; +import org.apache.ignite.internal.catalog.Catalog; +import org.apache.ignite.internal.catalog.CatalogCommand; +import org.apache.ignite.internal.catalog.CatalogValidationException; +import org.apache.ignite.internal.catalog.DistributionZoneExistsValidationException; +import org.apache.ignite.internal.catalog.descriptors.CatalogZoneDescriptor; +import org.apache.ignite.internal.catalog.storage.AlterZoneEntry; +import org.apache.ignite.internal.catalog.storage.UpdateEntry; +import org.apache.ignite.lang.ErrorGroups.DistributionZones; + +/** + * A command that renames a zone with specified name. + */ +public class RenameZoneCommand extends AbstractZoneCommand { + /** Returns builder to create a command to rename zone. */ + public static RenameZoneCommandBuilder builder() { + return new RenameZoneCommand.Builder(); + } + + private final String newZoneName; + + /** + * Constructor. + * + * @param zoneName Name of the zone. + * @param newZoneName New name of the zone. + * @throws CatalogValidationException if any of restrictions above is violated. + */ + private RenameZoneCommand(String zoneName, String newZoneName) throws CatalogValidationException { + super(zoneName); + + this.newZoneName = newZoneName; + + validate(); + } + + @Override + public List<UpdateEntry> get(Catalog catalog) { + CatalogZoneDescriptor zone = zoneOrThrow(catalog, zoneName); + + if (catalog.zone(newZoneName) != null) { + throw new DistributionZoneExistsValidationException("Distribution zone already exists [zoneName=" + newZoneName + ']'); Review Comment: message pattern, see comment above ########## modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/DropZoneCommandValidationTest.java: ########## @@ -0,0 +1,48 @@ +/* + * 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.ignite.internal.catalog.commands; + +import static org.apache.ignite.internal.catalog.CatalogService.DEFAULT_ZONE_NAME; +import static org.apache.ignite.internal.testframework.IgniteTestUtils.assertThrows; + +import org.apache.ignite.internal.catalog.CatalogValidationException; +import org.junit.jupiter.api.Test; + +/** + * Tests to verify validation of {@link DropZoneCommand}. + */ +@SuppressWarnings("ThrowableNotThrown") +public class DropZoneCommandValidationTest extends AbstractCommandValidationTest { + @Test + void testValidateZoneNameOnDropZone() { Review Comment: let's check every string from `org.apache.ignite.internal.catalog.commands.AbstractCommandValidationTest#nullAndBlankStrings` ########## modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateZoneCommand.java: ########## @@ -0,0 +1,246 @@ +/* + * 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.ignite.internal.catalog.commands; + +import static org.apache.ignite.internal.catalog.CatalogParamsValidationUtils.validateField; +import static org.apache.ignite.internal.catalog.CatalogParamsValidationUtils.validateZoneDataNodesAutoAdjustParametersCompatibility; +import static org.apache.ignite.internal.catalog.CatalogParamsValidationUtils.validateZoneFilter; +import static org.apache.ignite.internal.catalog.commands.CatalogUtils.DEFAULT_DATA_REGION; +import static org.apache.ignite.internal.catalog.commands.CatalogUtils.DEFAULT_FILTER; +import static org.apache.ignite.internal.catalog.commands.CatalogUtils.DEFAULT_PARTITION_COUNT; +import static org.apache.ignite.internal.catalog.commands.CatalogUtils.DEFAULT_REPLICA_COUNT; +import static org.apache.ignite.internal.catalog.commands.CatalogUtils.DEFAULT_STORAGE_ENGINE; +import static org.apache.ignite.internal.catalog.commands.CatalogUtils.IMMEDIATE_TIMER_VALUE; +import static org.apache.ignite.internal.catalog.commands.CatalogUtils.INFINITE_TIMER_VALUE; +import static org.apache.ignite.internal.catalog.commands.CatalogUtils.MAX_PARTITION_COUNT; +import static org.apache.ignite.internal.catalog.commands.CatalogUtils.fromParams; + +import java.util.List; +import java.util.Objects; +import org.apache.ignite.internal.catalog.Catalog; +import org.apache.ignite.internal.catalog.CatalogCommand; +import org.apache.ignite.internal.catalog.CatalogValidationException; +import org.apache.ignite.internal.catalog.DistributionZoneExistsValidationException; +import org.apache.ignite.internal.catalog.descriptors.CatalogZoneDescriptor; +import org.apache.ignite.internal.catalog.storage.NewZoneEntry; +import org.apache.ignite.internal.catalog.storage.ObjectIdGenUpdateEntry; +import org.apache.ignite.internal.catalog.storage.UpdateEntry; +import org.jetbrains.annotations.Nullable; + +/** + * A command that creates a new zone. + */ +public class CreateZoneCommand extends AbstractZoneCommand { + /** Returns builder to create a command to create a zone with specified name. */ + public static CreateZoneCommandBuilder builder() { + return new Builder(); + } + + private final @Nullable Integer partitions; + + private final @Nullable Integer replicas; + + private final @Nullable Integer dataNodesAutoAdjust; + + private final @Nullable Integer dataNodesAutoAdjustScaleUp; + + private final @Nullable Integer dataNodesAutoAdjustScaleDown; + + private final @Nullable String filter; + + private final @Nullable DataStorageParams dataStorageParams; + + /** + * Constructor. + * + * @param zoneName Name of the zone. + * @param partitions Number of partitions. + * @param replicas Number of replicas. + * @param dataNodesAutoAdjust Timeout in seconds between node added or node left topology event itself and data nodes switch. + * @param dataNodesAutoAdjustScaleUp Timeout in seconds between node added topology event itself and data nodes switch. + * @param dataNodesAutoAdjustScaleDown Timeout in seconds between node left topology event itself and data nodes switch. + * @param filter Nodes filter. + * @param dataStorageParams Data storage params. + * @throws CatalogValidationException if any of restrictions above is violated. + */ + private CreateZoneCommand( + String zoneName, + @Nullable Integer partitions, + @Nullable Integer replicas, + @Nullable Integer dataNodesAutoAdjust, + @Nullable Integer dataNodesAutoAdjustScaleUp, + @Nullable Integer dataNodesAutoAdjustScaleDown, + @Nullable String filter, + @Nullable DataStorageParams dataStorageParams + ) throws CatalogValidationException { + super(zoneName); + + this.partitions = partitions; + this.replicas = replicas; + this.dataNodesAutoAdjust = dataNodesAutoAdjust; + this.dataNodesAutoAdjustScaleUp = dataNodesAutoAdjustScaleUp; + this.dataNodesAutoAdjustScaleDown = dataNodesAutoAdjustScaleDown; + this.filter = filter; + this.dataStorageParams = dataStorageParams; + + validate(); + } + + @Override + public List<UpdateEntry> get(Catalog catalog) { + if (catalog.zone(zoneName) != null) { + throw new DistributionZoneExistsValidationException("Distribution zone already exists [zoneName=" + zoneName + ']'); Review Comment: The patter for this type of exception is `<Object> with name '{}' already exists` (see `org.apache.ignite.internal.catalog.CatalogParamsValidationUtils#ensureNoTableIndexOrSysViewExistsWithGivenName`) -- 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]
