Copilot commented on code in PR #13907: URL: https://github.com/apache/cloudstack/pull/13907#discussion_r3903393129
########## engine/schema/src/main/java/org/apache/cloudstack/vm/bootgroup/InstanceBootGroupMemberVO.java: ########## @@ -0,0 +1,116 @@ +// 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.cloudstack.vm.bootgroup; + +import java.util.Date; +import java.util.UUID; + +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.EnumType; +import javax.persistence.Enumerated; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +import javax.persistence.Table; + +import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; + +@Entity +@Table(name = "instance_boot_group_member") +public class InstanceBootGroupMemberVO implements InstanceBootGroupMember { + + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + @Column(name = "id") + private long id; + + @Column(name = "uuid") + private String uuid; + + @Column(name = "boot_group_id") + private long bootGroupId; + + @Column(name = "member_type") + @Enumerated(EnumType.STRING) + private MemberType memberType; + + @Column(name = "member_id") + private long memberId; + + @Column(name = "order") + private int order; Review Comment: Mapping this field to a DB column named `order` is risky because `order` is a SQL reserved keyword and CloudStack builds SQL with unquoted column names in several places. This can break generated queries at runtime. Prefer renaming the DB column (and this mapping) to something like `boot_order`/`sort_order`. ########## engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql: ########## @@ -651,3 +651,91 @@ WHERE `name`='user.vm.readonly.details' AND `value` IS NOT NULL; -- usage records introduced in 4.22.1 (cumulative and per-VM) can coexist. See #13399. CALL `cloud_usage`.`IDEMPOTENT_DROP_INDEX`('id', 'cloud_usage.usage_volume'); CALL `cloud_usage`.`IDEMPOTENT_ADD_UNIQUE_INDEX`('cloud_usage.usage_volume', 'id', '(volume_id ASC, created ASC, vm_id ASC)'); + +-- InstanceBootGroup: ordered boot sequencing for VMs and InstanceGroups +CREATE TABLE IF NOT EXISTS `cloud`.`instance_boot_group` ( + `id` bigint unsigned NOT NULL UNIQUE AUTO_INCREMENT, + `uuid` varchar(40) NOT NULL, + `name` varchar(255) NOT NULL, + `description` varchar(4096) DEFAULT NULL, + `account_id` bigint unsigned NOT NULL COMMENT 'owner; foreign key to account table', + `domain_id` bigint unsigned NOT NULL, + `created` datetime NOT NULL, + `removed` datetime DEFAULT NULL COMMENT 'date the group was soft-deleted', + PRIMARY KEY (`id`), + CONSTRAINT `uc_instance_boot_group__uuid` UNIQUE (`uuid`), + CONSTRAINT `fk_instance_boot_group__account_id` FOREIGN KEY (`account_id`) REFERENCES `account` (`id`), + CONSTRAINT `fk_instance_boot_group__domain_id` FOREIGN KEY (`domain_id`) REFERENCES `domain` (`id`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + +CREATE TABLE IF NOT EXISTS `cloud`.`instance_boot_group_member` ( + `id` bigint unsigned NOT NULL AUTO_INCREMENT, + `uuid` varchar(40) NOT NULL, + `boot_group_id` bigint unsigned NOT NULL, + `member_type` varchar(32) NOT NULL COMMENT 'VirtualMachine or InstanceGroup', + `member_id` bigint unsigned NOT NULL, + `order` int NOT NULL DEFAULT 0, + `created` datetime NOT NULL, Review Comment: The column name `order` is a SQL reserved keyword and CloudStack’s query builder constructs SQL with unquoted column names (e.g. `Filter.addOrderBy` emits `table.order`). This can lead to SQL syntax errors when listing/sorting/filtering boot group members. Rename this column to a non-keyword (e.g. `boot_order`/`sort_order`) and propagate the rename through the VO/DAO/API param names. ########## engine/schema/src/main/java/com/cloud/vm/dao/InstanceBootGroupMemberDaoImpl.java: ########## @@ -0,0 +1,123 @@ +// 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 com.cloud.vm.dao; + +import java.util.List; + +import org.springframework.stereotype.Component; + +import com.cloud.utils.Pair; +import com.cloud.utils.db.GenericDaoBase; +import com.cloud.utils.db.SearchBuilder; +import com.cloud.utils.db.SearchCriteria; +import org.apache.cloudstack.vm.bootgroup.InstanceBootGroupMemberVO; +import org.apache.cloudstack.vm.bootgroup.InstanceBootGroupMember; + +@Component +public class InstanceBootGroupMemberDaoImpl extends GenericDaoBase<InstanceBootGroupMemberVO, Long> implements InstanceBootGroupMemberDao { + + private final SearchBuilder<InstanceBootGroupMemberVO> bootGroupSearch; + private final SearchBuilder<InstanceBootGroupMemberVO> bootGroupTypeSearch; + private final SearchBuilder<InstanceBootGroupMemberVO> memberSearch; + + public InstanceBootGroupMemberDaoImpl() { + bootGroupSearch = createSearchBuilder(); + bootGroupSearch.and("bootGroupId", bootGroupSearch.entity().getBootGroupId(), SearchCriteria.Op.EQ); + bootGroupSearch.done(); + + bootGroupTypeSearch = createSearchBuilder(); + bootGroupTypeSearch.and("bootGroupId", bootGroupTypeSearch.entity().getBootGroupId(), SearchCriteria.Op.EQ); + bootGroupTypeSearch.and("memberType", bootGroupTypeSearch.entity().getMemberType(), SearchCriteria.Op.EQ); + bootGroupTypeSearch.done(); + + memberSearch = createSearchBuilder(); + memberSearch.and("memberType", memberSearch.entity().getMemberType(), SearchCriteria.Op.EQ); + memberSearch.and("memberId", memberSearch.entity().getMemberId(), SearchCriteria.Op.EQ); + memberSearch.done(); + } + + @Override + public List<InstanceBootGroupMemberVO> listByBootGroupId(long bootGroupId) { + SearchCriteria<InstanceBootGroupMemberVO> sc = bootGroupSearch.create(); + sc.setParameters("bootGroupId", bootGroupId); + return listBy(sc, null); + } + + @Override + public int countByBootGroupId(long bootGroupId) { + SearchCriteria<InstanceBootGroupMemberVO> sc = bootGroupSearch.create(); + sc.setParameters("bootGroupId", bootGroupId); + return getCount(sc); + } + + @Override + public List<InstanceBootGroupMemberVO> listByBootGroupIdAndEqualOrHigherOrder(long bootGroupId, int order) { + SearchBuilder<InstanceBootGroupMemberVO> bootGroupOrderSearch = createSearchBuilder(); + bootGroupOrderSearch.and("bootGroupId", bootGroupOrderSearch.entity().getBootGroupId(), SearchCriteria.Op.EQ); + bootGroupOrderSearch.and("order", bootGroupOrderSearch.entity().getOrder(), SearchCriteria.Op.GTEQ); + bootGroupOrderSearch.done(); Review Comment: This DAO builds search criteria against the `order` column (via `entity().getOrder()`), which is a SQL reserved keyword. Since SQL emitted by CloudStack’s DB layer isn’t consistently identifier-quoted, this can result in syntax errors in generated queries. After renaming the DB column (e.g. `boot_order`), update this DAO and the VO accessor/mapping to match. -- 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]
