CLOUDSTACK-1190: make APIChecker throw one sensible exception.
Project: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/commit/5750fd96 Tree: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/tree/5750fd96 Diff: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/diff/5750fd96 Branch: refs/heads/master Commit: 5750fd9631c8064df0f9bb95117aacf497e00c23 Parents: 579b027 Author: Min Chen <[email protected]> Authored: Fri Feb 8 11:05:11 2013 -0800 Committer: Min Chen <[email protected]> Committed: Tue Mar 5 09:44:08 2013 -0800 ---------------------------------------------------------------------- .../cloud/exception/PermissionDeniedException.java | 4 +++ .../com/cloud/exception/RequestLimitException.java | 3 +- api/src/org/apache/cloudstack/acl/APIChecker.java | 3 +- .../ratelimit/ApiRateLimitServiceImpl.java | 2 +- server/src/com/cloud/api/ApiServer.java | 20 +++++++------- 5 files changed, 17 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/5750fd96/api/src/com/cloud/exception/PermissionDeniedException.java ---------------------------------------------------------------------- diff --git a/api/src/com/cloud/exception/PermissionDeniedException.java b/api/src/com/cloud/exception/PermissionDeniedException.java index b95d49b..718de0d 100644 --- a/api/src/com/cloud/exception/PermissionDeniedException.java +++ b/api/src/com/cloud/exception/PermissionDeniedException.java @@ -32,6 +32,10 @@ public class PermissionDeniedException extends CloudRuntimeException { super(message); } + public PermissionDeniedException(String message, Throwable cause){ + super(message, cause); + } + protected PermissionDeniedException() { super(); } http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/5750fd96/api/src/com/cloud/exception/RequestLimitException.java ---------------------------------------------------------------------- diff --git a/api/src/com/cloud/exception/RequestLimitException.java b/api/src/com/cloud/exception/RequestLimitException.java index ebaac80..4d7504e 100644 --- a/api/src/com/cloud/exception/RequestLimitException.java +++ b/api/src/com/cloud/exception/RequestLimitException.java @@ -17,13 +17,12 @@ package com.cloud.exception; import com.cloud.utils.SerialVersionUID; -import com.cloud.utils.exception.CloudRuntimeException; /** * Exception thrown if number of requests is over api rate limit set. * */ -public class RequestLimitException extends CloudRuntimeException { +public class RequestLimitException extends PermissionDeniedException { private static final long serialVersionUID = SerialVersionUID.AccountLimitException; http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/5750fd96/api/src/org/apache/cloudstack/acl/APIChecker.java ---------------------------------------------------------------------- diff --git a/api/src/org/apache/cloudstack/acl/APIChecker.java b/api/src/org/apache/cloudstack/acl/APIChecker.java index 2e2b73b..0d0dfd1 100644 --- a/api/src/org/apache/cloudstack/acl/APIChecker.java +++ b/api/src/org/apache/cloudstack/acl/APIChecker.java @@ -17,7 +17,6 @@ package org.apache.cloudstack.acl; import com.cloud.exception.PermissionDeniedException; -import com.cloud.exception.RequestLimitException; import com.cloud.user.User; import com.cloud.utils.component.Adapter; @@ -27,5 +26,5 @@ public interface APIChecker extends Adapter { // If true, apiChecker has checked the operation // If false, apiChecker is unable to handle the operation or not implemented // On exception, checkAccess failed don't allow - boolean checkAccess(User user, String apiCommandName) throws PermissionDeniedException, RequestLimitException; + boolean checkAccess(User user, String apiCommandName) throws PermissionDeniedException; } http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/5750fd96/plugins/api/rate-limit/src/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java ---------------------------------------------------------------------- diff --git a/plugins/api/rate-limit/src/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java b/plugins/api/rate-limit/src/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java index 1f84ca1..d23a11d 100644 --- a/plugins/api/rate-limit/src/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java +++ b/plugins/api/rate-limit/src/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java @@ -139,7 +139,7 @@ public class ApiRateLimitServiceImpl extends AdapterBase implements APIChecker, @Override - public boolean checkAccess(User user, String apiCommandName) throws PermissionDeniedException, RequestLimitException { + public boolean checkAccess(User user, String apiCommandName) throws PermissionDeniedException { Long accountId = user.getAccountId(); Account account = _accountService.getAccount(accountId); if ( _accountService.isRootAdmin(account.getType())){ http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/5750fd96/server/src/com/cloud/api/ApiServer.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/api/ApiServer.java b/server/src/com/cloud/api/ApiServer.java index 60d65da..8b14d82 100755 --- a/server/src/com/cloud/api/ApiServer.java +++ b/server/src/com/cloud/api/ApiServer.java @@ -5,7 +5,7 @@ // to you under the Apache License, Version 2.0 (the // "License"); you may not use this file except in compliance // with the License. You may obtain a copy of the License at -// +// // http://www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to in writing, @@ -159,7 +159,7 @@ public class ApiServer implements HttpRequestHandler { @Inject List<APIChecker> _apiAccessCheckers; @Inject private RegionManager _regionMgr = null; - + private static int _workerCount = 0; private static ApiServer s_instance = null; private static final DateFormat _dateFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ssZ"); @@ -596,14 +596,14 @@ public class ApiServer implements HttpRequestHandler { try{ checkCommandAvailable(user, commandName); } - catch (PermissionDeniedException ex){ - s_logger.debug("The given command:" + commandName + " does not exist or it is not available for user with id:" + userId); - throw new ServerApiException(ApiErrorCode.UNSUPPORTED_ACTION_ERROR, "The given command does not exist or it is not available for user"); - } catch (RequestLimitException ex){ s_logger.debug(ex.getMessage()); throw new ServerApiException(ApiErrorCode.API_LIMIT_EXCEED, ex.getMessage()); } + catch (PermissionDeniedException ex){ + s_logger.debug("The given command:" + commandName + " does not exist or it is not available for user with id:" + userId); + throw new ServerApiException(ApiErrorCode.UNSUPPORTED_ACTION_ERROR, "The given command does not exist or it is not available for user"); + } return true; } else { // check against every available command to see if the command exists or not @@ -1005,7 +1005,7 @@ public class ApiServer implements HttpRequestHandler { } catch (Exception e) { s_logger.error("Exception responding to http request", e); - } + } return responseText; } @@ -1017,7 +1017,7 @@ public class ApiServer implements HttpRequestHandler { if (ex == null){ // this call should not be invoked with null exception return getSerializedApiError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, "Some internal error happened", apiCommandParams, responseType); - } + } try { if (ex.getErrorCode() == ApiErrorCode.UNSUPPORTED_ACTION_ERROR || apiCommandParams == null || apiCommandParams.isEmpty()) { responseName = "errorresponse"; @@ -1043,7 +1043,7 @@ public class ApiServer implements HttpRequestHandler { if (idList != null) { for (int i=0; i < idList.size(); i++) { apiResponse.addProxyObject(idList.get(i)); - } + } } // Also copy over the cserror code and the function/layer in which // it was thrown. @@ -1053,7 +1053,7 @@ public class ApiServer implements HttpRequestHandler { responseText = ApiResponseSerializer.toSerializedString(apiResponse, responseType); } catch (Exception e) { - s_logger.error("Exception responding to http request", e); + s_logger.error("Exception responding to http request", e); } return responseText; }
