JoaoJandre commented on code in PR #11016:
URL: https://github.com/apache/cloudstack/pull/11016#discussion_r2172567727
##########
server/src/main/java/com/cloud/api/ApiResponseHelper.java:
##########
@@ -5550,4 +5552,87 @@ public void
updateTemplateIsoResponsesForIcons(List<TemplateResponse> responses,
response.setResourceIconResponse(iconResponse);
}
}
+
+ private void populateDomainFieldsOnConsoleSessionResponse(ConsoleSession
consoleSession, ConsoleSessionResponse consoleSessionResponse) {
+ if (consoleSession == null) {
+ return;
+ }
+
+ Domain domain =
ApiDBUtils.findDomainById(consoleSession.getDomainId());
+ if (domain != null) {
+ consoleSessionResponse.setDomain(domain.getName());
+ consoleSessionResponse.setDomainPath(domain.getPath());
+ consoleSessionResponse.setDomainId(domain.getUuid());
+ }
+ }
+
+ private void populateUserFieldsOnConsoleSessionResponse(ConsoleSession
consoleSession, ConsoleSessionResponse consoleSessionResponse) {
+ if (consoleSession == null) {
+ return;
+ }
+
+ User user = findUserById(consoleSession.getUserId());
+ if (user != null) {
+ consoleSessionResponse.setUser(user.getUsername());
+ consoleSessionResponse.setUserId(user.getUuid());
+ }
+ }
+
+ private void populateAccountFieldsOnConsoleSessionResponse(ConsoleSession
consoleSession, ConsoleSessionResponse consoleSessionResponse) {
+ if (consoleSession == null) {
+ return;
+ }
+
+ Account account =
ApiDBUtils.findAccountById(consoleSession.getAccountId());
+ if (account != null) {
+ consoleSessionResponse.setAccount(account.getAccountName());
+ consoleSessionResponse.setAccountId(account.getUuid());
+ }
+ }
+
+ private void populateHostFieldsOnConsoleSessionResponse(ConsoleSession
consoleSession, ConsoleSessionResponse consoleSessionResponse) {
+ if (consoleSession == null) {
+ return;
+ }
+
+ Host host = findHostById(consoleSession.getHostId());
+ if (host != null) {
+ consoleSessionResponse.setHostId(host.getUuid());
+ consoleSessionResponse.setHostName(host.getName());
+ }
+ }
+
+ private void populateInstanceFieldsOnConsoleSessionResponse(ConsoleSession
consoleSession, ConsoleSessionResponse consoleSessionResponse) {
+ if (consoleSession == null) {
+ return;
+ }
+
+ VMInstanceVO instance =
ApiDBUtils.findVMInstanceById(consoleSession.getInstanceId());
+ if (instance != null) {
+ consoleSessionResponse.setInstanceId(instance.getUuid());
+ consoleSessionResponse.setInstanceName(instance.getInstanceName());
+ }
+ }
+
+ @Override
+ public ConsoleSessionResponse createConsoleSessionResponse(ConsoleSession
consoleSession, ResponseView responseView) {
+ ConsoleSessionResponse consoleSessionResponse = new
ConsoleSessionResponse();
+ consoleSessionResponse.setId(consoleSession.getUuid());
+ consoleSessionResponse.setCreated(consoleSession.getCreated());
+ consoleSessionResponse.setAcquired(consoleSession.getAcquired());
+ consoleSessionResponse.setRemoved(consoleSession.getRemoved());
+
consoleSessionResponse.setConsoleEndpointCreatorAddress(consoleSession.getConsoleEndpointCreatorAddress());
+
consoleSessionResponse.setClientAddress(consoleSession.getClientAddress());
+
+ populateDomainFieldsOnConsoleSessionResponse(consoleSession,
consoleSessionResponse);
+ populateUserFieldsOnConsoleSessionResponse(consoleSession,
consoleSessionResponse);
+ populateAccountFieldsOnConsoleSessionResponse(consoleSession,
consoleSessionResponse);
+ populateInstanceFieldsOnConsoleSessionResponse(consoleSession,
consoleSessionResponse);
Review Comment:
Instead of having to check if the console session is null inside every one
of these methods, we could check it before calling all of them.
##########
engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java:
##########
@@ -80,4 +82,59 @@ public int expungeByVmList(List<Long> vmIds, Long batchSize)
{
sc.setParameters("vmIds", vmIds.toArray());
return batchExpunge(sc, batchSize);
}
+
+ @Override
+ public Pair<List<ConsoleSessionVO>, Integer> listConsoleSessions(Long id,
List<Long> domainIds, Long accountId, Long userId, Long hostId,
+ Date
startDate, Date endDate, Long instanceId,
+ String
consoleEndpointCreatorAddress, String clientAddress,
+ boolean
activeOnly, Long pageSizeVal, Long startIndex) {
+ Filter filter = new Filter(ConsoleSessionVO.class, "created", false,
startIndex, pageSizeVal);
+ SearchCriteria<ConsoleSessionVO> searchCriteria =
createListConsoleSessionsSearchCriteria(id, domainIds, accountId, userId,
hostId,
+ startDate, endDate, instanceId, consoleEndpointCreatorAddress,
clientAddress, activeOnly);
+
+ return searchAndCount(searchCriteria, filter, true);
+ }
+
+ private SearchCriteria<ConsoleSessionVO>
createListConsoleSessionsSearchCriteria(Long id, List<Long> domainIds, Long
accountId, Long userId, Long hostId,
+
Date startDate, Date endDate, Long instanceId,
+
String consoleEndpointCreatorAddress, String clientAddress,
+
boolean activeOnly) {
+ SearchCriteria<ConsoleSessionVO> searchCriteria =
createListConsoleSessionsSearchBuilder(activeOnly).create();
+
+ searchCriteria.setParametersIfNotNull("id", id);
+ searchCriteria.setParametersIfNotNull("domainIds",
domainIds.toArray());
+ searchCriteria.setParametersIfNotNull("accountId", accountId);
+ searchCriteria.setParametersIfNotNull("userId", userId);
+ searchCriteria.setParametersIfNotNull("hostId", hostId);
+ searchCriteria.setParametersIfNotNull("instanceId", instanceId);
+ searchCriteria.setParametersIfNotNull("startDate", startDate);
+ searchCriteria.setParametersIfNotNull("endDate", endDate);
+ searchCriteria.setParametersIfNotNull("creatorAddress",
consoleEndpointCreatorAddress);
+ searchCriteria.setParametersIfNotNull("clientAddress", clientAddress);
Review Comment:
We could extract these strings to constants
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -181,6 +194,78 @@ private void reallyRun() {
}
}
+ @Override
+ public ListResponse<ConsoleSessionResponse>
listConsoleSessions(ListConsoleSessionsCmd cmd) {
+ Pair<List<ConsoleSessionVO>, Integer> consoleSessions =
listConsoleSessionsInternal(cmd);
+ ListResponse<ConsoleSessionResponse> response = new ListResponse<>();
+
+ ResponseObject.ResponseView responseView =
ResponseObject.ResponseView.Restricted;
+ Long callerId = CallContext.current().getCallingAccountId();
+ if (accountManager.isRootAdmin(callerId)) {
+ responseView = ResponseObject.ResponseView.Full;
+ }
+
+ List<ConsoleSessionResponse> consoleSessionResponses = new
ArrayList<>();
+ for (ConsoleSessionVO consoleSession : consoleSessions.first()) {
+ ConsoleSessionResponse consoleSessionResponse =
responseGenerator.createConsoleSessionResponse(consoleSession, responseView);
+ consoleSessionResponses.add(consoleSessionResponse);
+ }
+
+ response.setResponses(consoleSessionResponses,
consoleSessions.second());
+ return response;
+ }
+
+ protected Pair<List<ConsoleSessionVO>, Integer>
listConsoleSessionsInternal(ListConsoleSessionsCmd cmd) {
+ CallContext caller = CallContext.current();
+ long domainId =
getBaseDomainIdToListConsoleSessions(cmd.getDomainId());
+ Long accountId = cmd.getAccountId();
+ Long userId = cmd.getUserId();
+ boolean isRecursive = cmd.isRecursive();
+
+ boolean isCallerNormalUser =
accountManager.isNormalUser(caller.getCallingAccountId());
+ if (isCallerNormalUser) {
+ accountId = caller.getCallingAccountId();
+ userId = caller.getCallingUserId();
+ }
+
+ List<Long> domainIds = isRecursive ?
domainDao.getDomainAndChildrenIds(domainId) : List.of(domainId);
+
+ return consoleSessionDao.listConsoleSessions(cmd.getId(), domainIds,
accountId, userId,
+ cmd.getHostId(), cmd.getStartDate(), cmd.getEndDate(),
cmd.getInstanceId(),
+ cmd.getConsoleEndpointCreatorAddress(),
cmd.getClientAddress(), cmd.isActiveOnly(),
+ cmd.getPageSizeVal(), cmd.getStartIndex());
+ }
+
+ /**
+ * Determines the base domain ID for listing console sessions.
+ *
+ * If no domain ID is provided, returns the caller's domain ID. Otherwise,
+ * checks if the caller has access to that domain and returns the provided
domain ID.
+ *
+ * @param domainId The domain ID to check, can be null
+ * @return The base domain ID to use for listing console sessions
+ * @throws PermissionDeniedException if the caller does not have access to
the specified domain
+ */
+ protected long getBaseDomainIdToListConsoleSessions(Long domainId) {
+ Account caller = CallContext.current().getCallingAccount();
+ if (domainId == null) {
+ return caller.getDomainId();
+ }
+
+ Domain domain = domainDao.findById(domainId);
+ if (domain == null) {
+ throw new InvalidParameterValueException(String.format("Unable to
find domain with ID [%s]. Verify the informed domain and try again.",
domainId));
+ }
+
+ accountManager.checkAccess(caller, domain);
+ return domainId;
+ }
+
+ @Override
+ public ConsoleSession listConsoleSessionById(long id) {
+ return consoleSessionDao.findByIdIncludingRemoved(id);
+ }
Review Comment:
Is this method necessary? Is there somewhere where you cannot reference the
DAO but can reference the Manager? If not... why not just call the DAO method?
--
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: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
