Skip to content

Commit 275abaf

Browse files
hsato03Henrique Sato
hsato03
and
Henrique Sato
authored
Refactor updateDiskOffering API (#8446)
Co-authored-by: Henrique Sato <henrique.sato@scclouds.com.br>
1 parent e74a72b commit 275abaf

File tree

2 files changed

+289
-76
lines changed

2 files changed

+289
-76
lines changed

‎server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java‎

Lines changed: 132 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -3919,22 +3919,9 @@ public DiskOffering updateDiskOffering(final UpdateDiskOfferingCmd cmd) {
39193919
List<Long> existingZoneIds = diskOfferingDetailsDao.findZoneIds(diskOfferingId);
39203920
Collections.sort(existingZoneIds);
39213921

3922-
// check if valid domain
3923-
if (CollectionUtils.isNotEmpty(domainIds)) {
3924-
for (final Long domainId: domainIds) {
3925-
if (_domainDao.findById(domainId) == null) {
3926-
throw new InvalidParameterValueException("Please specify a valid domain id");
3927-
}
3928-
}
3929-
}
3922+
validateDomain(domainIds);
39303923

3931-
// check if valid zone
3932-
if (CollectionUtils.isNotEmpty(zoneIds)) {
3933-
for (Long zoneId : zoneIds) {
3934-
if (_zoneDao.findById(zoneId) == null)
3935-
throw new InvalidParameterValueException("Please specify a valid zone id");
3936-
}
3937-
}
3924+
validateZone(zoneIds);
39383925

39393926
Long userId = CallContext.current().getCallingUserId();
39403927
if (userId == null) {
@@ -3957,35 +3944,16 @@ public DiskOffering updateDiskOffering(final UpdateDiskOfferingCmd cmd) {
39573944
Collections.sort(filteredZoneIds);
39583945

39593946
if (account.getType() == Account.Type.DOMAIN_ADMIN) {
3960-
if (!filteredZoneIds.equals(existingZoneIds)) { // Domain-admins cannot update zone(s) for offerings
3961-
throw new InvalidParameterValueException(String.format("Unable to update zone(s) for disk offering: %s by admin: %s as it is domain-admin", diskOfferingHandle.getUuid(), user.getUuid()));
3962-
}
3963-
if (existingDomainIds.isEmpty()) {
3964-
throw new InvalidParameterValueException(String.format("Unable to update public disk offering: %s by user: %s because it is domain-admin", diskOfferingHandle.getUuid(), user.getUuid()));
3965-
} else {
3966-
if (filteredDomainIds.isEmpty()) {
3967-
throw new InvalidParameterValueException(String.format("Unable to update disk offering: %s to a public offering by user: %s because it is domain-admin", diskOfferingHandle.getUuid(), user.getUuid()));
3968-
}
3969-
}
3947+
checkDomainAdminUpdateOfferingRestrictions(diskOfferingHandle, user, filteredZoneIds, existingZoneIds, existingDomainIds, filteredDomainIds);
3948+
39703949
if (StringUtils.isNotBlank(tags) && !ALLOW_DOMAIN_ADMINS_TO_CREATE_TAGGED_OFFERINGS.valueIn(account.getAccountId())) {
39713950
throw new InvalidParameterValueException(String.format("User [%s] is unable to update disk offering tags.", user.getUuid()));
39723951
}
39733952

3974-
List<Long> nonChildDomains = new ArrayList<>();
3975-
for (Long domainId : existingDomainIds) {
3976-
if (!_domainDao.isChildDomain(account.getDomainId(), domainId)) {
3977-
if (name != null || displayText != null || sortKey != null) { // Domain-admins cannot update name, display text, sort key for offerings with domain which are not child domains for domain-admin
3978-
throw new InvalidParameterValueException(String.format("Unable to update disk offering: %s as it has linked domain(s) which are not child domain for domain-admin: %s", diskOfferingHandle.getUuid(), user.getUuid()));
3979-
}
3980-
nonChildDomains.add(domainId);
3981-
}
3982-
}
3983-
for (Long domainId : filteredDomainIds) {
3984-
if (!_domainDao.isChildDomain(account.getDomainId(), domainId)) {
3985-
Domain domain = _entityMgr.findById(Domain.class, domainId);
3986-
throw new InvalidParameterValueException(String.format("Unable to update disk offering: %s by domain-admin: %s with domain: %3$s which is not a child domain", diskOfferingHandle.getUuid(), user.getUuid(), domain.getUuid()));
3987-
}
3988-
}
3953+
List<Long> nonChildDomains = getAccountNonChildDomains(diskOfferingHandle, account, user, cmd, existingDomainIds);
3954+
3955+
checkIfDomainIsChildDomain(diskOfferingHandle, account, user, filteredDomainIds);
3956+
39893957
filteredDomainIds.addAll(nonChildDomains); // Final list must include domains which were not child domain for domain-admin but specified for this offering prior to update
39903958
} else if (account.getType() != Account.Type.ADMIN) {
39913959
throw new InvalidParameterValueException(String.format("Unable to update disk offering: %s by id user: %s because it is not root-admin or domain-admin", diskOfferingHandle.getUuid(), user.getUuid()));
@@ -4001,22 +3969,7 @@ public DiskOffering updateDiskOffering(final UpdateDiskOfferingCmd cmd) {
40013969
}
40023970

40033971
final DiskOfferingVO diskOffering = _diskOfferingDao.createForUpdate(diskOfferingId);
4004-
4005-
if (name != null) {
4006-
diskOffering.setName(name);
4007-
}
4008-
4009-
if (displayText != null) {
4010-
diskOffering.setDisplayText(displayText);
4011-
}
4012-
4013-
if (sortKey != null) {
4014-
diskOffering.setSortKey(sortKey);
4015-
}
4016-
4017-
if (displayDiskOffering != null) {
4018-
diskOffering.setDisplayOffering(displayDiskOffering);
4019-
}
3972+
updateDiskOfferingIfCmdAttributeNotNull(diskOffering, cmd);
40203973

40213974
updateOfferingTagsIfIsNotNull(tags, diskOffering);
40223975

@@ -4039,26 +3992,7 @@ public DiskOffering updateDiskOffering(final UpdateDiskOfferingCmd cmd) {
40393992
}
40403993
List<DiskOfferingDetailVO> detailsVO = new ArrayList<>();
40413994
if(detailsUpdateNeeded) {
4042-
SearchBuilder<DiskOfferingDetailVO> sb = diskOfferingDetailsDao.createSearchBuilder();
4043-
sb.and("offeringId", sb.entity().getResourceId(), SearchCriteria.Op.EQ);
4044-
sb.and("detailName", sb.entity().getName(), SearchCriteria.Op.EQ);
4045-
sb.done();
4046-
SearchCriteria<DiskOfferingDetailVO> sc = sb.create();
4047-
sc.setParameters("offeringId", String.valueOf(diskOfferingId));
4048-
if(!filteredDomainIds.equals(existingDomainIds)) {
4049-
sc.setParameters("detailName", ApiConstants.DOMAIN_ID);
4050-
diskOfferingDetailsDao.remove(sc);
4051-
for (Long domainId : filteredDomainIds) {
4052-
detailsVO.add(new DiskOfferingDetailVO(diskOfferingId, ApiConstants.DOMAIN_ID, String.valueOf(domainId), false));
4053-
}
4054-
}
4055-
if(!filteredZoneIds.equals(existingZoneIds)) {
4056-
sc.setParameters("detailName", ApiConstants.ZONE_ID);
4057-
diskOfferingDetailsDao.remove(sc);
4058-
for (Long zoneId : filteredZoneIds) {
4059-
detailsVO.add(new DiskOfferingDetailVO(diskOfferingId, ApiConstants.ZONE_ID, String.valueOf(zoneId), false));
4060-
}
4061-
}
3995+
updateDiskOfferingDetails(detailsVO, diskOfferingId, filteredDomainIds, existingDomainIds, filteredZoneIds, existingZoneIds);
40623996
}
40633997
if (!detailsVO.isEmpty()) {
40643998
for (DiskOfferingDetailVO detailVO : detailsVO) {
@@ -4069,6 +4003,128 @@ public DiskOffering updateDiskOffering(final UpdateDiskOfferingCmd cmd) {
40694003
return _diskOfferingDao.findById(diskOfferingId);
40704004
}
40714005

4006+
protected void validateDomain(List<Long> domainIds) {
4007+
if (CollectionUtils.isEmpty(domainIds)) {
4008+
return;
4009+
}
4010+
4011+
for (final Long domainId: domainIds) {
4012+
if (_domainDao.findById(domainId) == null) {
4013+
throw new InvalidParameterValueException("Please specify a valid domain id.");
4014+
}
4015+
}
4016+
}
4017+
4018+
protected void validateZone(List<Long> zoneIds) {
4019+
if (CollectionUtils.isEmpty(zoneIds)) {
4020+
return;
4021+
}
4022+
4023+
for (Long zoneId : zoneIds) {
4024+
if (_zoneDao.findById(zoneId) == null) {
4025+
throw new InvalidParameterValueException("Please specify a valid zone id.");
4026+
}
4027+
}
4028+
}
4029+
4030+
protected void updateDiskOfferingIfCmdAttributeNotNull(DiskOfferingVO diskOffering, UpdateDiskOfferingCmd cmd) {
4031+
if (cmd.getDiskOfferingName() != null) {
4032+
diskOffering.setName(cmd.getDiskOfferingName());
4033+
}
4034+
4035+
if (cmd.getDisplayText() != null) {
4036+
diskOffering.setDisplayText(cmd.getDisplayText());
4037+
}
4038+
4039+
if (cmd.getSortKey() != null) {
4040+
diskOffering.setSortKey(cmd.getSortKey());
4041+
}
4042+
4043+
if (cmd.getDisplayOffering() != null) {
4044+
diskOffering.setDisplayOffering(cmd.getDisplayOffering());
4045+
}
4046+
}
4047+
4048+
protected void updateDiskOfferingDetails(List<DiskOfferingDetailVO> detailsVO, Long diskOfferingId, List<Long> filteredDomainIds,
4049+
List<Long> existingDomainIds, List<Long> filteredZoneIds, List<Long> existingZoneIds) {
4050+
SearchBuilder<DiskOfferingDetailVO> sb = diskOfferingDetailsDao.createSearchBuilder();
4051+
sb.and("offeringId", sb.entity().getResourceId(), SearchCriteria.Op.EQ);
4052+
sb.and("detailName", sb.entity().getName(), SearchCriteria.Op.EQ);
4053+
sb.done();
4054+
SearchCriteria<DiskOfferingDetailVO> sc = sb.create();
4055+
sc.setParameters("offeringId", String.valueOf(diskOfferingId));
4056+
4057+
updateDiskOfferingDetailsDomainIds(detailsVO, sc, diskOfferingId, filteredDomainIds, existingDomainIds);
4058+
updateDiskOfferingDetailsZoneIds(detailsVO, sc, diskOfferingId, filteredZoneIds, existingZoneIds);
4059+
}
4060+
4061+
protected void updateDiskOfferingDetailsDomainIds(List<DiskOfferingDetailVO> detailsVO, SearchCriteria<DiskOfferingDetailVO> sc, Long diskOfferingId, List<Long> filteredDomainIds, List<Long> existingDomainIds) {
4062+
if (filteredDomainIds.equals(existingDomainIds)) {
4063+
return;
4064+
}
4065+
4066+
sc.setParameters("detailName", ApiConstants.DOMAIN_ID);
4067+
diskOfferingDetailsDao.remove(sc);
4068+
for (Long domainId : filteredDomainIds) {
4069+
detailsVO.add(new DiskOfferingDetailVO(diskOfferingId, ApiConstants.DOMAIN_ID, String.valueOf(domainId), false));
4070+
}
4071+
}
4072+
4073+
protected void updateDiskOfferingDetailsZoneIds(List<DiskOfferingDetailVO> detailsVO, SearchCriteria<DiskOfferingDetailVO> sc, Long diskOfferingId, List<Long> filteredZoneIds, List<Long> existingZoneIds) {
4074+
if (filteredZoneIds.equals(existingZoneIds)) {
4075+
return;
4076+
}
4077+
4078+
sc.setParameters("detailName", ApiConstants.ZONE_ID);
4079+
diskOfferingDetailsDao.remove(sc);
4080+
for (Long zoneId : filteredZoneIds) {
4081+
detailsVO.add(new DiskOfferingDetailVO(diskOfferingId, ApiConstants.ZONE_ID, String.valueOf(zoneId), false));
4082+
}
4083+
}
4084+
4085+
protected void checkDomainAdminUpdateOfferingRestrictions(DiskOffering diskOffering, User user, List<Long> filteredZoneIds, List<Long> existingZoneIds,
4086+
List<Long> existingDomainIds, List<Long> filteredDomainIds) {
4087+
if (!filteredZoneIds.equals(existingZoneIds)) {
4088+
throw new InvalidParameterValueException(String.format("Unable to update zone(s) for disk offering [%s] by admin [%s] as it is domain-admin.", diskOffering.getUuid(), user.getUuid()));
4089+
}
4090+
if (existingDomainIds.isEmpty()) {
4091+
throw new InvalidParameterValueException(String.format("Unable to update public disk offering [%s] by user [%s] because it is domain-admin.", diskOffering.getUuid(), user.getUuid()));
4092+
}
4093+
if (filteredDomainIds.isEmpty()) {
4094+
throw new InvalidParameterValueException(String.format("Unable to update disk offering [%s] to a public offering by user [%s] because it is domain-admin.", diskOffering.getUuid(), user.getUuid()));
4095+
}
4096+
}
4097+
4098+
protected List<Long> getAccountNonChildDomains(DiskOffering diskOffering, Account account, User user,
4099+
UpdateDiskOfferingCmd cmd, List<Long> existingDomainIds) {
4100+
List<Long> nonChildDomains = new ArrayList<>();
4101+
String name = cmd.getDiskOfferingName();
4102+
String displayText = cmd.getDisplayText();
4103+
Integer sortKey = cmd.getSortKey();
4104+
for (Long domainId : existingDomainIds) {
4105+
if (_domainDao.isChildDomain(account.getDomainId(), domainId)) {
4106+
continue;
4107+
}
4108+
4109+
if (ObjectUtils.anyNotNull(name, displayText, sortKey)) {
4110+
throw new InvalidParameterValueException(String.format("Unable to update disk offering [%s] as it has linked domain(s) which are not child domain for domain-admin [%s].", diskOffering.getUuid(), user.getUuid()));
4111+
}
4112+
nonChildDomains.add(domainId);
4113+
}
4114+
return nonChildDomains;
4115+
}
4116+
4117+
protected void checkIfDomainIsChildDomain(DiskOffering diskOffering, Account account, User user, List<Long> filteredDomainIds) {
4118+
for (Long domainId : filteredDomainIds) {
4119+
if (_domainDao.isChildDomain(account.getDomainId(), domainId)) {
4120+
continue;
4121+
}
4122+
4123+
Domain domain = _entityMgr.findById(Domain.class, domainId);
4124+
throw new InvalidParameterValueException(String.format("Unable to update disk offering [%s] by domain-admin [%s] with domain [%3$s] which is not a child domain.", diskOffering.getUuid(), user.getUuid(), domain.getUuid()));
4125+
}
4126+
}
4127+
40724128
/**
40734129
* Check the tags parameters to the disk/service offering
40744130
* <ul>

0 commit comments

Comments
 (0)