From 8f98442cd975402936d8c8be0066d4e2db96f30d Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Thu, 13 Jun 2024 08:11:54 +0200 Subject: [PATCH 1/4] protect agains missing service offering --- .../main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java index e309a7534a3a..68b08a0d0648 100644 --- a/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java @@ -188,8 +188,11 @@ public VolumeResponse newVolumeResponse(ResponseView view, VolumeJoinVO volume) if (volume.getDiskOfferingId() > 0) { DiskOffering computeOnlyDiskOffering = ApiDBUtils.findComputeOnlyDiskOfferingById(volume.getDiskOfferingId()); + ServiceOffering serviceOffering = null; if (computeOnlyDiskOffering != null) { - ServiceOffering serviceOffering = ApiDBUtils.findServiceOfferingByComputeOnlyDiskOffering(volume.getDiskOfferingId()); + serviceOffering = ApiDBUtils.findServiceOfferingByComputeOnlyDiskOffering(volume.getDiskOfferingId()); + } + if (serviceOffering != null) { volResponse.setServiceOfferingId(String.valueOf(serviceOffering.getId())); volResponse.setServiceOfferingName(serviceOffering.getName()); volResponse.setServiceOfferingDisplayText(serviceOffering.getDisplayText()); From 1517d16f73bbc8e43b9fdfb7a81c9a28f6839059 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Thu, 13 Jun 2024 08:45:21 +0200 Subject: [PATCH 2/4] search removed before assuming none --- .../cloud/service/dao/ServiceOfferingDao.java | 2 +- .../service/dao/ServiceOfferingDaoImpl.java | 4 ++-- .../main/java/com/cloud/api/ApiDBUtils.java | 4 ++-- .../api/query/dao/VolumeJoinDaoImpl.java | 19 +++++++++++++++---- 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/service/dao/ServiceOfferingDao.java b/engine/schema/src/main/java/com/cloud/service/dao/ServiceOfferingDao.java index e2fc5b49ae84..3b6fa8fa1033 100644 --- a/engine/schema/src/main/java/com/cloud/service/dao/ServiceOfferingDao.java +++ b/engine/schema/src/main/java/com/cloud/service/dao/ServiceOfferingDao.java @@ -54,5 +54,5 @@ List createSystemServiceOfferings(String name, String uniqueN List listPublicByCpuAndMemory(Integer cpus, Integer memory); - ServiceOfferingVO findServiceOfferingByComputeOnlyDiskOffering(long diskOfferingId); + ServiceOfferingVO findServiceOfferingByComputeOnlyDiskOffering(long diskOfferingId, boolean includingRemoved); } diff --git a/engine/schema/src/main/java/com/cloud/service/dao/ServiceOfferingDaoImpl.java b/engine/schema/src/main/java/com/cloud/service/dao/ServiceOfferingDaoImpl.java index 5c8e49938295..ef6d4e719899 100644 --- a/engine/schema/src/main/java/com/cloud/service/dao/ServiceOfferingDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/service/dao/ServiceOfferingDaoImpl.java @@ -284,10 +284,10 @@ public List listPublicByCpuAndMemory(Integer cpus, Integer me } @Override - public ServiceOfferingVO findServiceOfferingByComputeOnlyDiskOffering(long diskOfferingId) { + public ServiceOfferingVO findServiceOfferingByComputeOnlyDiskOffering(long diskOfferingId, boolean includingRemoved) { SearchCriteria sc = SearchComputeOfferingByComputeOnlyDiskOffering.create(); sc.setParameters("disk_offering_id", diskOfferingId); - List vos = listBy(sc); + List vos = includingRemoved ? listIncludingRemovedBy(sc) : listBy(sc); if (vos.size() == 0) { return null; } diff --git a/server/src/main/java/com/cloud/api/ApiDBUtils.java b/server/src/main/java/com/cloud/api/ApiDBUtils.java index 97ecd982f53c..c45e60f8dd51 100644 --- a/server/src/main/java/com/cloud/api/ApiDBUtils.java +++ b/server/src/main/java/com/cloud/api/ApiDBUtils.java @@ -1109,8 +1109,8 @@ public static DiskOfferingVO findDiskOfferingById(Long diskOfferingId) { return null; } - public static ServiceOfferingVO findServiceOfferingByComputeOnlyDiskOffering(Long diskOfferingId) { - ServiceOfferingVO off = s_serviceOfferingDao.findServiceOfferingByComputeOnlyDiskOffering(diskOfferingId); + public static ServiceOfferingVO findServiceOfferingByComputeOnlyDiskOffering(Long diskOfferingId, boolean includingRemoved) { + ServiceOfferingVO off = s_serviceOfferingDao.findServiceOfferingByComputeOnlyDiskOffering(diskOfferingId, includingRemoved); return off; } public static DomainVO findDomainById(Long domainId) { diff --git a/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java index 68b08a0d0648..470b0bc638a2 100644 --- a/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java @@ -32,6 +32,7 @@ import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.log4j.Logger; +import org.jetbrains.annotations.Nullable; import org.springframework.stereotype.Component; import com.cloud.api.ApiDBUtils; @@ -188,10 +189,7 @@ public VolumeResponse newVolumeResponse(ResponseView view, VolumeJoinVO volume) if (volume.getDiskOfferingId() > 0) { DiskOffering computeOnlyDiskOffering = ApiDBUtils.findComputeOnlyDiskOfferingById(volume.getDiskOfferingId()); - ServiceOffering serviceOffering = null; - if (computeOnlyDiskOffering != null) { - serviceOffering = ApiDBUtils.findServiceOfferingByComputeOnlyDiskOffering(volume.getDiskOfferingId()); - } + ServiceOffering serviceOffering = getServiceOfferingForDiskOffering(volume, computeOnlyDiskOffering); if (serviceOffering != null) { volResponse.setServiceOfferingId(String.valueOf(serviceOffering.getId())); volResponse.setServiceOfferingName(serviceOffering.getName()); @@ -286,6 +284,19 @@ public VolumeResponse newVolumeResponse(ResponseView view, VolumeJoinVO volume) return volResponse; } + private static ServiceOffering getServiceOfferingForDiskOffering(VolumeJoinVO volume, DiskOffering computeOnlyDiskOffering) { + ServiceOffering serviceOffering = null; + // first try existing ones + if (computeOnlyDiskOffering != null) { + serviceOffering = ApiDBUtils.findServiceOfferingByComputeOnlyDiskOffering(volume.getDiskOfferingId(), false); + } + // else try removed ones + if (serviceOffering == null) { + serviceOffering = ApiDBUtils.findServiceOfferingByComputeOnlyDiskOffering(volume.getDiskOfferingId(), true); + } + return serviceOffering; + } + @Override public VolumeResponse setVolumeResponse(ResponseView view, VolumeResponse volData, VolumeJoinVO vol) { long tag_id = vol.getTagId(); From 19214aa03abb83ea9fc3fc3af9e4855c82f16620 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Thu, 13 Jun 2024 08:50:00 +0200 Subject: [PATCH 3/4] inport --- .../src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java index 470b0bc638a2..9eb60fa6dabb 100644 --- a/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java @@ -32,7 +32,6 @@ import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.log4j.Logger; -import org.jetbrains.annotations.Nullable; import org.springframework.stereotype.Component; import com.cloud.api.ApiDBUtils; From bb8e8bb8ff888ba7bdad4c9fa262bd24cdd1c022 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Fri, 14 Jun 2024 08:41:12 +0200 Subject: [PATCH 4/4] javadoc --- .../com/cloud/api/query/dao/VolumeJoinDaoImpl.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java index 9eb60fa6dabb..1060fd840b5a 100644 --- a/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java @@ -283,13 +283,20 @@ public VolumeResponse newVolumeResponse(ResponseView view, VolumeJoinVO volume) return volResponse; } + /** + * gets the {@see ServiceOffering} for the {@see Volume} with {@see DiskOffering} + * It will first try existing ones + * If not found it will try to get a removed one + * + * @param volume + * @param computeOnlyDiskOffering + * @return the resulting offering or null + */ private static ServiceOffering getServiceOfferingForDiskOffering(VolumeJoinVO volume, DiskOffering computeOnlyDiskOffering) { ServiceOffering serviceOffering = null; - // first try existing ones if (computeOnlyDiskOffering != null) { serviceOffering = ApiDBUtils.findServiceOfferingByComputeOnlyDiskOffering(volume.getDiskOfferingId(), false); } - // else try removed ones if (serviceOffering == null) { serviceOffering = ApiDBUtils.findServiceOfferingByComputeOnlyDiskOffering(volume.getDiskOfferingId(), true); }