refactoring: code review comment changes

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
This commit is contained in:
Abhishek Kumar 2019-05-30 16:42:14 +05:30
parent 6d82e63e53
commit f78b99cc8f
2 changed files with 34 additions and 36 deletions

View File

@ -183,14 +183,14 @@ public class DomainChecker extends AdapterBase implements SecurityChecker {
@Override
public boolean checkAccess(Account account, DiskOffering dof, DataCenter zone) throws PermissionDeniedException {
boolean isAccess = false;
boolean hasAccess = false;
// Check fo domains
if (account == null || dof == null) {
isAccess = true;
hasAccess = true;
} else {
//admin has all permissions
if (_accountService.isRootAdmin(account.getId())) {
isAccess = true;
hasAccess = true;
}
//if account is normal user or domain admin
//check if account's domain is a child of offering's domain (Note: This is made consistent with the list command for disk offering)
@ -200,11 +200,11 @@ public class DomainChecker extends AdapterBase implements SecurityChecker {
|| account.getType() == Account.ACCOUNT_TYPE_PROJECT) {
final List<Long> doDomainIds = diskOfferingDetailsDao.findDomainIds(dof.getId());
if (doDomainIds.isEmpty()) {
isAccess = true;
hasAccess = true;
} else {
for (Long domainId : doDomainIds) {
if (_domainDao.isChildDomain(domainId, account.getDomainId())) {
isAccess = true;
hasAccess = true;
break;
}
}
@ -212,23 +212,23 @@ public class DomainChecker extends AdapterBase implements SecurityChecker {
}
}
// Check for zones
if (isAccess && dof != null && zone != null) {
if (hasAccess && dof != null && zone != null) {
final List<Long> doZoneIds = diskOfferingDetailsDao.findZoneIds(dof.getId());
isAccess = doZoneIds.isEmpty() || doZoneIds.contains(zone.getId());
hasAccess = doZoneIds.isEmpty() || doZoneIds.contains(zone.getId());
}
return isAccess;
return hasAccess;
}
@Override
public boolean checkAccess(Account account, ServiceOffering so, DataCenter zone) throws PermissionDeniedException {
boolean isAccess = false;
boolean hasAccess = false;
// Check fo domains
if (account == null || so == null) {
isAccess = true;
hasAccess = true;
} else {
//admin has all permissions
if (_accountService.isRootAdmin(account.getId())) {
isAccess = true;
hasAccess = true;
}
//if account is normal user or domain admin
//check if account's domain is a child of offering's domain (Note: This is made consistent with the list command for service offering)
@ -238,11 +238,11 @@ public class DomainChecker extends AdapterBase implements SecurityChecker {
|| account.getType() == Account.ACCOUNT_TYPE_PROJECT) {
final List<Long> soDomainIds = serviceOfferingDetailsDao.findDomainIds(so.getId());
if (soDomainIds.isEmpty()) {
isAccess = true;
hasAccess = true;
} else {
for (Long domainId : soDomainIds) {
if (_domainDao.isChildDomain(domainId, account.getDomainId())) {
isAccess = true;
hasAccess = true;
break;
}
}
@ -250,23 +250,23 @@ public class DomainChecker extends AdapterBase implements SecurityChecker {
}
}
// Check for zones
if (isAccess && so != null && zone != null) {
if (hasAccess && so != null && zone != null) {
final List<Long> soZoneIds = serviceOfferingDetailsDao.findZoneIds(so.getId());
isAccess = soZoneIds.isEmpty() || soZoneIds.contains(zone.getId());
hasAccess = soZoneIds.isEmpty() || soZoneIds.contains(zone.getId());
}
return isAccess;
return hasAccess;
}
@Override
public boolean checkAccess(Account account, NetworkOffering nof, DataCenter zone) throws PermissionDeniedException {
boolean isAccess = false;
// Check fo domains
boolean hasAccess = false;
// Check for domains
if (account == null || nof == null) {
isAccess = true;
hasAccess = true;
} else {
//admin has all permissions
if (_accountService.isRootAdmin(account.getId())) {
isAccess = true;
hasAccess = true;
}
//if account is normal user or domain admin
//check if account's domain is a child of offering's domain (Note: This is made consistent with the list command for disk offering)
@ -276,11 +276,11 @@ public class DomainChecker extends AdapterBase implements SecurityChecker {
|| account.getType() == Account.ACCOUNT_TYPE_PROJECT) {
final List<Long> noDomainIds = networkOfferingDetailsDao.findDomainIds(nof.getId());
if (noDomainIds.isEmpty()) {
isAccess = true;
hasAccess = true;
} else {
for (Long domainId : noDomainIds) {
if (_domainDao.isChildDomain(domainId, account.getDomainId())) {
isAccess = true;
hasAccess = true;
break;
}
}
@ -288,23 +288,23 @@ public class DomainChecker extends AdapterBase implements SecurityChecker {
}
}
// Check for zones
if (isAccess && nof != null && zone != null) {
if (hasAccess && nof != null && zone != null) {
final List<Long> doZoneIds = networkOfferingDetailsDao.findZoneIds(nof.getId());
isAccess = doZoneIds.isEmpty() || doZoneIds.contains(zone.getId());
hasAccess = doZoneIds.isEmpty() || doZoneIds.contains(zone.getId());
}
return isAccess;
return hasAccess;
}
@Override
public boolean checkAccess(Account account, VpcOffering vof, DataCenter zone) throws PermissionDeniedException {
boolean isAccess = false;
// Check fo domains
boolean hasAccess = false;
// Check for domains
if (account == null || vof == null) {
isAccess = true;
hasAccess = true;
} else {
//admin has all permissions
if (_accountService.isRootAdmin(account.getId())) {
isAccess = true;
hasAccess = true;
}
//if account is normal user or domain admin
//check if account's domain is a child of offering's domain (Note: This is made consistent with the list command for disk offering)
@ -314,11 +314,11 @@ public class DomainChecker extends AdapterBase implements SecurityChecker {
|| account.getType() == Account.ACCOUNT_TYPE_PROJECT) {
final List<Long> voDomainIds = vpcOfferingDetailsDao.findDomainIds(vof.getId());
if (voDomainIds.isEmpty()) {
isAccess = true;
hasAccess = true;
} else {
for (Long domainId : voDomainIds) {
if (_domainDao.isChildDomain(domainId, account.getDomainId())) {
isAccess = true;
hasAccess = true;
break;
}
}
@ -326,11 +326,11 @@ public class DomainChecker extends AdapterBase implements SecurityChecker {
}
}
// Check for zones
if (isAccess && vof != null && zone != null) {
if (hasAccess && vof != null && zone != null) {
final List<Long> doZoneIds = vpcOfferingDetailsDao.findZoneIds(vof.getId());
isAccess = doZoneIds.isEmpty() || doZoneIds.contains(zone.getId());
hasAccess = doZoneIds.isEmpty() || doZoneIds.contains(zone.getId());
}
return isAccess;
return hasAccess;
}
@Override

View File

@ -2609,7 +2609,6 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q
Pair<List<DiskOfferingJoinVO>, Integer> result = _diskOfferingJoinDao.searchAndCount(sc, searchFilter);
// Remove offerings that are not associated with caller's domain
// TODO: Better approach
if (account.getType() != Account.ACCOUNT_TYPE_ADMIN && CollectionUtils.isNotEmpty(result.first())) {
ListIterator<DiskOfferingJoinVO> it = result.first().listIterator();
while (it.hasNext()) {
@ -2823,7 +2822,6 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q
//Couldn't figure out a smart way to filter offerings based on tags in sql so doing it in Java.
List<ServiceOfferingJoinVO> filteredOfferings = filterOfferingsOnCurrentTags(result.first(), currentVmOffering);
// Remove offerings that are not associated with caller's domain
// TODO: Better approach
if (caller.getType() != Account.ACCOUNT_TYPE_ADMIN && CollectionUtils.isNotEmpty(filteredOfferings)) {
ListIterator<ServiceOfferingJoinVO> it = filteredOfferings.listIterator();
while (it.hasNext()) {