This is an automated email from the ASF dual-hosted git repository.
pearl11594 pushed a commit to branch dedicate-backup-offering-to-domain
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to
refs/heads/dedicate-backup-offering-to-domain by this push:
new 79ff42b0592 Add tests and UI support and update response params
79ff42b0592 is described below
commit 79ff42b0592a2864351344bb73e7fbf22cb06836
Author: Pearl Dsilva <[email protected]>
AuthorDate: Thu Dec 4 10:38:31 2025 -0500
Add tests and UI support and update response params
---
.../admin/backup/UpdateBackupOfferingCmd.java | 11 +++-
.../api/response/BackupOfferingResponse.java | 17 +++++
.../backup/dao/BackupOfferingDaoImpl.java | 23 ++++++-
.../cloudstack/backup/BackupManagerImpl.java | 47 ++++++++++++-
.../cloudstack/backup/BackupManagerTest.java | 76 ++++++++++++++++++++++
ui/src/config/section/offering.js | 6 +-
ui/src/views/offering/ImportBackupOffering.vue | 69 +++++++++++++++++++-
7 files changed, 240 insertions(+), 9 deletions(-)
diff --git
a/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/UpdateBackupOfferingCmd.java
b/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/UpdateBackupOfferingCmd.java
index 682f2f595ad..2b8643819d4 100644
---
a/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/UpdateBackupOfferingCmd.java
+++
b/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/UpdateBackupOfferingCmd.java
@@ -37,6 +37,7 @@ import com.cloud.user.Account;
import com.cloud.utils.exception.CloudRuntimeException;
import java.util.List;
+import java.util.function.LongFunction;
@APICommand(name = "updateBackupOffering", description = "Updates a backup
offering.", responseObject = BackupOfferingResponse.class,
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since =
"4.16.0")
@@ -114,7 +115,15 @@ public class UpdateBackupOfferingCmd extends BaseCmd
implements DomainAndZoneIdR
}
public List<Long> getDomainIds() {
- return resolveDomainIds(domainIds, id,
backupManager::getBackupOfferingDomains, "backup offering");
+ // backupManager may be null in unit tests where the command is spied
without injection.
+ // Avoid creating a method reference to a null receiver which causes
NPE. When backupManager
+ // is null, pass null as the defaultDomainsProvider so
resolveDomainIds will simply return
+ // an empty list or parse the explicit domainIds string.
+ LongFunction<List<Long>> defaultDomainsProvider = null;
+ if (backupManager != null) {
+ defaultDomainsProvider = backupManager::getBackupOfferingDomains;
+ }
+ return resolveDomainIds(domainIds, id, defaultDomainsProvider, "backup
offering");
}
@Override
diff --git
a/api/src/main/java/org/apache/cloudstack/api/response/BackupOfferingResponse.java
b/api/src/main/java/org/apache/cloudstack/api/response/BackupOfferingResponse.java
index 4120f68d9da..0669afa604b 100644
---
a/api/src/main/java/org/apache/cloudstack/api/response/BackupOfferingResponse.java
+++
b/api/src/main/java/org/apache/cloudstack/api/response/BackupOfferingResponse.java
@@ -61,6 +61,14 @@ public class BackupOfferingResponse extends BaseResponse {
@Param(description = "zone name")
private String zoneName;
+ @SerializedName(ApiConstants.DOMAIN_ID)
+ @Param(description = "the domain ID(s) this disk offering belongs to.
Ignore this information as it is not currently applicable.")
+ private String domainId;
+
+ @SerializedName(ApiConstants.DOMAIN)
+ @Param(description = "the domain name(s) this disk offering belongs to.
Ignore this information as it is not currently applicable.")
+ private String domain;
+
@SerializedName(ApiConstants.CROSS_ZONE_INSTANCE_CREATION)
@Param(description = "the backups with this offering can be used to create
Instances on all Zones", since = "4.22.0")
private Boolean crossZoneInstanceCreation;
@@ -108,4 +116,13 @@ public class BackupOfferingResponse extends BaseResponse {
public void setCreated(Date created) {
this.created = created;
}
+
+ public void setDomainId(String domainId) {
+ this.domainId = domainId;
+ }
+
+ public void setDomain(String domain) {
+ this.domain = domain;
+ }
+
}
diff --git
a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupOfferingDaoImpl.java
b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupOfferingDaoImpl.java
index a41e4e70d33..708faeef464 100644
---
a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupOfferingDaoImpl.java
+++
b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupOfferingDaoImpl.java
@@ -20,6 +20,8 @@ package org.apache.cloudstack.backup.dao;
import javax.annotation.PostConstruct;
import javax.inject.Inject;
+import com.cloud.domain.DomainVO;
+import com.cloud.domain.dao.DomainDao;
import org.apache.cloudstack.api.response.BackupOfferingResponse;
import org.apache.cloudstack.backup.BackupOffering;
import org.apache.cloudstack.backup.BackupOfferingVO;
@@ -30,10 +32,16 @@ import com.cloud.utils.db.GenericDaoBase;
import com.cloud.utils.db.SearchBuilder;
import com.cloud.utils.db.SearchCriteria;
+import java.util.List;
+
public class BackupOfferingDaoImpl extends GenericDaoBase<BackupOfferingVO,
Long> implements BackupOfferingDao {
@Inject
DataCenterDao dataCenterDao;
+ @Inject
+ BackupOfferingDetailsDao backupOfferingDetailsDao;
+ @Inject
+ DomainDao domainDao;
private SearchBuilder<BackupOfferingVO> backupPoliciesSearch;
@@ -51,8 +59,9 @@ public class BackupOfferingDaoImpl extends
GenericDaoBase<BackupOfferingVO, Long
@Override
public BackupOfferingResponse newBackupOfferingResponse(BackupOffering
offering, Boolean crossZoneInstanceCreation) {
- DataCenterVO zone = dataCenterDao.findById(offering.getZoneId());
+ DataCenterVO zone = dataCenterDao.findById(offering.getZoneId());
+ List<Long> domainIds =
backupOfferingDetailsDao.findDomainIds(offering.getId());
BackupOfferingResponse response = new BackupOfferingResponse();
response.setId(offering.getUuid());
response.setName(offering.getName());
@@ -64,6 +73,18 @@ public class BackupOfferingDaoImpl extends
GenericDaoBase<BackupOfferingVO, Long
response.setZoneId(zone.getUuid());
response.setZoneName(zone.getName());
}
+ if (domainIds != null && !domainIds.isEmpty()) {
+ String domainUUIDs =
domainIds.stream().map(Long::valueOf).map(domainId -> {
+ DomainVO domain = domainDao.findById(domainId);
+ return domain != null ? domain.getUuid() : "";
+ }).filter(name -> !name.isEmpty()).reduce((a, b) -> a + "," +
b).orElse("");
+ String domainNames =
domainIds.stream().map(Long::valueOf).map(domainId -> {
+ DomainVO domain = domainDao.findById(domainId);
+ return domain != null ? domain.getName() : "";
+ }).filter(name -> !name.isEmpty()).reduce((a, b) -> a + "," +
b).orElse("");
+ response.setDomain(domainNames);
+ response.setDomainId(domainUUIDs);
+ }
if (crossZoneInstanceCreation) {
response.setCrossZoneInstanceCreation(true);
}
diff --git
a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java
b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java
index e1c51813ad6..2d9f7bbcc87 100644
--- a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java
+++ b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java
@@ -314,7 +314,7 @@ public class BackupManagerImpl extends ManagerBase
implements BackupManager {
if
(org.apache.commons.collections.CollectionUtils.isNotEmpty(filteredDomainIds)) {
List<BackupOfferingDetailsVO> detailsVOList = new ArrayList<>();
for (Long domainId : filteredDomainIds) {
- detailsVOList.add(new
BackupOfferingDetailsVO(savedOffering.getId(),
org.apache.cloudstack.api.ApiConstants.DOMAIN_ID, String.valueOf(domainId),
false));
+ detailsVOList.add(new
BackupOfferingDetailsVO(savedOffering.getId(), ApiConstants.DOMAIN_ID,
String.valueOf(domainId), false));
}
if (!detailsVOList.isEmpty()) {
backupOfferingDetailsDao.saveDetails(detailsVOList);
@@ -2179,6 +2179,7 @@ public class BackupManagerImpl extends ManagerBase
implements BackupManager {
String name = updateBackupOfferingCmd.getName();
String description = updateBackupOfferingCmd.getDescription();
Boolean allowUserDrivenBackups =
updateBackupOfferingCmd.getAllowUserDrivenBackups();
+ List<Long> domainIds = updateBackupOfferingCmd.getDomainIds();
BackupOfferingVO backupOfferingVO = backupOfferingDao.findById(id);
if (backupOfferingVO == null) {
@@ -2209,16 +2210,58 @@ public class BackupManagerImpl extends ManagerBase
implements BackupManager {
fields.add("allowUserDrivenBackups: " + allowUserDrivenBackups);
}
- if (!backupOfferingDao.update(id, offering)) {
+ if
(org.apache.commons.collections.CollectionUtils.isNotEmpty(domainIds)) {
+ for (final Long domainId: domainIds) {
+ if (domainDao.findById(domainId) == null) {
+ throw new InvalidParameterValueException("Please specify a
valid domain id");
+ }
+ }
+ }
+ List<Long> filteredDomainIds = filterChildSubDomains(domainIds);
+ Collections.sort(filteredDomainIds);
+
+ boolean success =backupOfferingDao.update(id, offering);
+ if (!success) {
logger.warn(String.format("Couldn't update Backup offering (%s)
with [%s].", backupOfferingVO, String.join(", ", fields)));
}
+ if (success) {
+ List<Long> existingDomainIds =
backupOfferingDetailsDao.findDomainIds(id);
+ Collections.sort(existingDomainIds);
+ updateBackupOfferingDomainDetails(id, filteredDomainIds,
existingDomainIds);
+ }
+
BackupOfferingVO response = backupOfferingDao.findById(id);
CallContext.current().setEventDetails(String.format("Backup Offering
updated [%s].",
ReflectionToStringBuilderUtils.reflectOnlySelectedFields(response, "id",
"name", "description", "userDrivenBackupAllowed", "externalId")));
return response;
}
+ private void updateBackupOfferingDomainDetails(Long id, List<Long>
filteredDomainIds, List<Long> existingDomainIds) {
+ if (existingDomainIds == null) {
+ existingDomainIds = new ArrayList<>();
+ }
+ List<BackupOfferingDetailsVO> detailsVO = new ArrayList<>();
+ if(!filteredDomainIds.equals(existingDomainIds)) {
+ SearchBuilder<BackupOfferingDetailsVO> sb =
backupOfferingDetailsDao.createSearchBuilder();
+ sb.and("offeringId", sb.entity().getResourceId(),
SearchCriteria.Op.EQ);
+ sb.and("detailName", sb.entity().getName(), SearchCriteria.Op.EQ);
+ sb.done();
+ SearchCriteria<BackupOfferingDetailsVO> sc = sb.create();
+ sc.setParameters("offeringId", String.valueOf(id));
+ sc.setParameters("detailName", ApiConstants.DOMAIN_ID);
+ backupOfferingDetailsDao.remove(sc);
+ for (Long domainId : filteredDomainIds) {
+ detailsVO.add(new BackupOfferingDetailsVO(id,
ApiConstants.DOMAIN_ID, String.valueOf(domainId), false));
+ }
+ }
+ if (!detailsVO.isEmpty()) {
+ for (BackupOfferingDetailsVO detailVO : detailsVO) {
+ backupOfferingDetailsDao.persist(detailVO);
+ }
+ }
+ }
+
Map<String, String> getDetailsFromBackupDetails(Long backupId) {
Map<String, String> details =
backupDetailsDao.listDetailsKeyPairs(backupId, true);
if (details == null) {
diff --git
a/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java
b/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java
index 6f6879b6fe3..9b4661ff371 100644
--- a/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java
+++ b/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java
@@ -85,6 +85,7 @@ import org.apache.cloudstack.api.response.BackupResponse;
import org.apache.cloudstack.backup.dao.BackupDao;
import org.apache.cloudstack.backup.dao.BackupDetailsDao;
import org.apache.cloudstack.backup.dao.BackupOfferingDao;
+import org.apache.cloudstack.backup.dao.BackupOfferingDetailsDao;
import org.apache.cloudstack.backup.dao.BackupScheduleDao;
import org.apache.cloudstack.context.CallContext;
import org.apache.cloudstack.framework.config.ConfigKey;
@@ -241,6 +242,9 @@ public class BackupManagerTest {
@Mock
private GuestOSDao _guestOSDao;
+ @Mock
+ private BackupOfferingDetailsDao backupOfferingDetailsDao;
+
private Gson gson;
private String[] hostPossibleValues = {"127.0.0.1", "hostname"};
@@ -352,6 +356,7 @@ public class BackupManagerTest {
when(cmd.getName()).thenReturn("New name");
when(cmd.getDescription()).thenReturn("New description");
when(cmd.getAllowUserDrivenBackups()).thenReturn(true);
+
when(backupOfferingDetailsDao.findDomainIds(id)).thenReturn(Collections.emptyList());
BackupOffering updated = backupManager.updateBackupOffering(cmd);
assertEquals("New name", updated.getName());
@@ -2156,4 +2161,75 @@ public class BackupManagerTest {
verify(vmInstanceDao, times(1)).findByIdIncludingRemoved(vmId);
verify(volumeDao, times(1)).findByInstance(vmId);
}
+
+ @Test
+ public void getBackupOfferingDomainsTestOfferingNotFound() {
+ Long offeringId = 1L;
+ when(backupOfferingDao.findById(offeringId)).thenReturn(null);
+
+ InvalidParameterValueException exception =
Assert.assertThrows(InvalidParameterValueException.class,
+ () -> backupManager.getBackupOfferingDomains(offeringId));
+ assertEquals("Unable to find backup offering null",
exception.getMessage());
+ }
+
+ @Test
+ public void getBackupOfferingDomainsTestReturnsDomains() {
+ Long offeringId = 1L;
+ BackupOfferingVO offering = Mockito.mock(BackupOfferingVO.class);
+ when(backupOfferingDao.findById(offeringId)).thenReturn(offering);
+
when(backupOfferingDetailsDao.findDomainIds(offeringId)).thenReturn(List.of(10L,
20L));
+
+ List<Long> result = backupManager.getBackupOfferingDomains(offeringId);
+
+ assertEquals(2, result.size());
+ assertTrue(result.contains(10L));
+ assertTrue(result.contains(20L));
+ }
+
+ @Test
+ public void testUpdateBackupOfferingThrowsWhenDomainIdInvalid() {
+ Long id = 1234L;
+ UpdateBackupOfferingCmd cmd =
Mockito.spy(UpdateBackupOfferingCmd.class);
+ when(cmd.getId()).thenReturn(id);
+ when(cmd.getDomainIds()).thenReturn(List.of(99L));
+
+ when(domainDao.findById(99L)).thenReturn(null);
+
+ InvalidParameterValueException exception =
Assert.assertThrows(InvalidParameterValueException.class,
+ () -> backupManager.updateBackupOffering(cmd));
+ assertEquals("Please specify a valid domain id",
exception.getMessage());
+ }
+
+ @Test
+ public void testUpdateBackupOfferingPersistsDomainDetailsWhenProvided() {
+ Long id = 1234L;
+ Long domainId = 11L;
+ UpdateBackupOfferingCmd cmd =
Mockito.spy(UpdateBackupOfferingCmd.class);
+ when(cmd.getId()).thenReturn(id);
+ when(cmd.getDomainIds()).thenReturn(List.of(domainId));
+
+ DomainVO domain = Mockito.mock(DomainVO.class);
+ when(domainDao.findById(domainId)).thenReturn(domain);
+
+
when(backupOfferingDetailsDao.findDomainIds(id)).thenReturn(Collections.emptyList());
+
+ SearchBuilder<BackupOfferingDetailsVO> sb =
Mockito.mock(SearchBuilder.class);
+ SearchCriteria<BackupOfferingDetailsVO> sc =
Mockito.mock(SearchCriteria.class);
+ BackupOfferingDetailsVO entity =
Mockito.mock(BackupOfferingDetailsVO.class);
+ when(backupOfferingDetailsDao.createSearchBuilder()).thenReturn(sb);
+ when(sb.entity()).thenReturn(entity);
+ when(sb.and(Mockito.anyString(), (Object) any(),
Mockito.any())).thenReturn(sb);
+ when(sb.create()).thenReturn(sc);
+
+ BackupOfferingVO offering = Mockito.mock(BackupOfferingVO.class);
+ BackupOfferingVO offeringUpdate = Mockito.mock(BackupOfferingVO.class);
+ when(backupOfferingDao.findById(id)).thenReturn(offering);
+ when(backupOfferingDao.createForUpdate(id)).thenReturn(offeringUpdate);
+ when(backupOfferingDao.update(id, offeringUpdate)).thenReturn(true);
+
+ BackupOffering updated = backupManager.updateBackupOffering(cmd);
+
+ verify(backupOfferingDetailsDao,
times(1)).persist(Mockito.any(BackupOfferingDetailsVO.class));
+ }
+
}
diff --git a/ui/src/config/section/offering.js
b/ui/src/config/section/offering.js
index 4a32619b8c2..bc95772d6f7 100644
--- a/ui/src/config/section/offering.js
+++ b/ui/src/config/section/offering.js
@@ -340,9 +340,9 @@ export default {
icon: 'cloud-upload-outlined',
docHelp: 'adminguide/virtual_machines.html#backup-offerings',
permission: ['listBackupOfferings'],
- searchFilters: ['zoneid'],
- columns: ['name', 'description', 'zonename'],
- details: ['name', 'id', 'description', 'externalid', 'zone',
'allowuserdrivenbackups', 'created'],
+ searchFilters: ['zoneid', 'domainid'],
+ columns: ['name', 'description', 'domain', 'zonename'],
+ details: ['name', 'id', 'description', 'externalid', 'domain', 'zone',
'allowuserdrivenbackups', 'created'],
related: [{
name: 'vm',
title: 'label.instances',
diff --git a/ui/src/views/offering/ImportBackupOffering.vue
b/ui/src/views/offering/ImportBackupOffering.vue
index b8ac7d8e8e6..f680eacd4a7 100644
--- a/ui/src/views/offering/ImportBackupOffering.vue
+++ b/ui/src/views/offering/ImportBackupOffering.vue
@@ -85,6 +85,33 @@
</template>
<a-switch v-model:checked="form.allowuserdrivenbackups"/>
</a-form-item>
+ <a-form-item name="ispublic" ref="ispublic"
:label="$t('label.ispublic')" v-if="isAdmin()">
+ <a-switch v-model:checked="form.ispublic" />
+ </a-form-item>
+ <a-form-item name="domainid" ref="domainid" v-if="!form.ispublic">
+ <template #label>
+ <tooltip-label :title="$t('label.domainid')"
:tooltip="apiParams.domainid.description"/>
+ </template>
+ <a-select
+ mode="multiple"
+ :getPopupContainer="(trigger) => trigger.parentNode"
+ v-model:value="form.domainid"
+ showSearch
+ optionFilterProp="label"
+ :filterOption="(input, option) => {
+ return option.label.toLowerCase().indexOf(input.toLowerCase())
>= 0
+ }"
+ :loading="domains.loading"
+ :placeholder="apiParams.domainid.description">
+ <a-select-option v-for="(opt, optIndex) in domains.opts"
:key="optIndex" :label="opt.path || opt.name || opt.description">
+ <span>
+ <resource-icon v-if="opt && opt.icon"
:image="opt.icon.base64image" size="1x" style="margin-right: 5px"/>
+ <block-outlined v-else style="margin-right: 5px" />
+ {{ opt.path || opt.name || opt.description }}
+ </span>
+ </a-select-option>
+ </a-select>
+ </a-form-item>
<div :span="24" class="action-button">
<a-button :loading="loading" @click="closeAction">{{
this.$t('label.cancel') }}</a-button>
<a-button :loading="loading" ref="submit" type="primary"
@click="handleSubmit">{{ this.$t('label.ok') }}</a-button>
@@ -96,6 +123,7 @@
<script>
import { ref, reactive, toRaw } from 'vue'
import { getAPI, postAPI } from '@/api'
+import { isAdmin } from '@/role'
import ResourceIcon from '@/components/view/ResourceIcon'
import TooltipLabel from '@/components/widgets/TooltipLabel'
@@ -108,6 +136,10 @@ export default {
data () {
return {
loading: false,
+ domains: {
+ loading: false,
+ opts: []
+ },
zones: {
loading: false,
opts: []
@@ -129,17 +161,23 @@ export default {
initForm () {
this.formRef = ref()
this.form = reactive({
- allowuserdrivenbackups: true
+ allowuserdrivenbackups: true,
+ ispublic: true
})
this.rules = reactive({
name: [{ required: true, message:
this.$t('message.error.required.input') }],
description: [{ required: true, message:
this.$t('message.error.required.input') }],
zoneid: [{ required: true, message: this.$t('message.error.select') }],
- externalid: [{ required: true, message:
this.$t('message.error.select') }]
+ externalid: [{ required: true, message:
this.$t('message.error.select') }],
+ domainid: [{ type: 'array', message: this.$t('message.error.select') }]
})
},
+ isAdmin () {
+ return isAdmin()
+ },
fetchData () {
this.fetchZone()
+ this.fetchDomainData()
},
fetchZone () {
this.zones.loading = true
@@ -151,6 +189,19 @@ export default {
this.zones.loading = false
})
},
+ fetchDomainData () {
+ const params = {}
+ params.listAll = true
+ params.details = 'min'
+ this.domains.loading = true
+ getAPI('listDomains', params).then(json => {
+ this.domains.opts = json.listdomainsresponse.domain || []
+ }).catch(error => {
+ this.$notifyError(error)
+ }).finally(() => {
+ this.domains.loading = false
+ })
+ },
fetchExternal (zoneId) {
if (!zoneId) {
this.externals.opts = []
@@ -179,6 +230,20 @@ export default {
params[key] = input
}
}
+ if (values.ispublic !== true) {
+ var domainIndexes = values.domainid
+ var domainId = null
+ if (domainIndexes && domainIndexes.length > 0) {
+ var domainIds = []
+ for (var i = 0; i < domainIndexes.length; i++) {
+ domainIds =
domainIds.concat(this.domains.opts[domainIndexes[i]].id)
+ }
+ domainId = domainIds.join(',')
+ }
+ if (domainId) {
+ params.domainid = domainId
+ }
+ }
params.allowuserdrivenbackups = values.allowuserdrivenbackups
this.loading = true
const title = this.$t('label.import.offering')
