From 7eb7671675b1bde5fab8e907b5057fb904981bc7 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Fri, 17 May 2013 07:44:31 +0200 Subject: [PATCH] SONAR-3755 slight refactoring --- .../issue/InternalRubyIssueService.java | 16 ++++----- .../java/org/sonar/server/issue/Result.java | 33 ++++++++++++------- .../controllers/action_plans_controller.rb | 6 ++-- .../api/action_plans_controller.rb | 11 ++++--- 4 files changed, 40 insertions(+), 26 deletions(-) diff --git a/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java b/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java index ed12f859991..8a7c2ae3dc3 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java @@ -142,7 +142,7 @@ public class InternalRubyIssueService implements ServerComponent { Result result = createActionPlanResult(parameters); if (result.ok()) { - result.setObject(actionPlanService.create(result.get())); + result.set(actionPlanService.create(result.get())); } return result; } @@ -152,7 +152,7 @@ public class InternalRubyIssueService implements ServerComponent { DefaultActionPlan existingActionPlan = (DefaultActionPlan) actionPlanService.findByKey(key); if (existingActionPlan == null) { - Result result = new Result(); + Result result = Result.of(); result.addError("issues_action_plans.errors.action_plan_does_not_exists", key); return result; } else { @@ -161,7 +161,7 @@ public class InternalRubyIssueService implements ServerComponent { DefaultActionPlan actionPlan = (DefaultActionPlan) result.get(); actionPlan.setKey(existingActionPlan.key()); actionPlan.setUserLogin(existingActionPlan.userLogin()); - result.setObject(actionPlanService.update(actionPlan)); + result.set(actionPlanService.update(actionPlan)); } return result; } @@ -171,7 +171,7 @@ public class InternalRubyIssueService implements ServerComponent { // TODO verify authorization Result result = createResultForExistingActionPlan(actionPlanKey); if (result.ok()) { - result.setObject(actionPlanService.setStatus(actionPlanKey, ActionPlan.STATUS_CLOSED)); + result.set(actionPlanService.setStatus(actionPlanKey, ActionPlan.STATUS_CLOSED)); } return result; } @@ -180,7 +180,7 @@ public class InternalRubyIssueService implements ServerComponent { // TODO verify authorization Result result = createResultForExistingActionPlan(actionPlanKey); if (result.ok()) { - result.setObject(actionPlanService.setStatus(actionPlanKey, ActionPlan.STATUS_OPEN)); + result.set(actionPlanService.setStatus(actionPlanKey, ActionPlan.STATUS_OPEN)); } return result; } @@ -201,7 +201,7 @@ public class InternalRubyIssueService implements ServerComponent { @VisibleForTesting Result createActionPlanResult(Map parameters, @Nullable String oldName) { - Result result = new Result(); + Result result = Result.of(); String name = parameters.get("name"); String description = parameters.get("description"); @@ -253,13 +253,13 @@ public class InternalRubyIssueService implements ServerComponent { .setDescription(description) .setUserLogin(UserSession.get().login()) .setDeadLine(deadLine); - result.setObject(actionPlan); + result.set(actionPlan); } return result; } private Result createResultForExistingActionPlan(String actionPlanKey) { - Result result = new Result(); + Result result = Result.of(); if (findActionPlan(actionPlanKey) == null) { result.addError("issues_action_plans.errors.action_plan_does_not_exists", actionPlanKey); } diff --git a/sonar-server/src/main/java/org/sonar/server/issue/Result.java b/sonar-server/src/main/java/org/sonar/server/issue/Result.java index e8a51948f91..587114e8b3a 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/Result.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/Result.java @@ -17,46 +17,55 @@ * along with this program; if not, write to the Free Software Foundation, * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ - package org.sonar.server.issue; +import javax.annotation.Nullable; import java.util.Arrays; import java.util.List; import static com.google.common.collect.Lists.newArrayList; +/** + * @since 3.6 + */ public class Result { - private T object; - private List errors; + private T object = null; + private List errors = newArrayList(); + ; + + private Result(@Nullable T object) { + this.object = object; + } - public Result() { - errors = newArrayList(); + public static Result of(@Nullable T t) { + return new Result(t); } - public Result(T object) { - this(); - this.object = object; + public static Result of() { + return new Result(null); } - public void setObject(T object){ + public Result set(@Nullable T object) { this.object = object; + return this; } public T get() { return object; } - public void addError(String text, Object... params){ + public Result addError(String text, Object... params) { Message message = new Message(text, params); errors.add(message); + return this; } - public List errors(){ + public List errors() { return errors; } - public boolean ok(){ + public boolean ok() { return errors.isEmpty(); } diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/action_plans_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/action_plans_controller.rb index 1ded3b55754..557c4eabdb0 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/action_plans_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/action_plans_controller.rb @@ -22,7 +22,6 @@ class ActionPlansController < ApplicationController SECTION=Navigation::SECTION_RESOURCE before_filter :load_resource - verify :method => :post, :only => [:save, :delete, :change_status], :redirect_to => {:action => :index} def index load_action_plans() @@ -35,7 +34,8 @@ class ActionPlansController < ApplicationController end def save - @action_plan = ActionPlan.find params[:plan_id] unless params[:plan_id].blank? + verify_post_request + @action_plan = ActionPlan.find params[:plan_id] if params[:plan_id].present? unless @action_plan @action_plan = ActionPlan.new( :kee => Java::JavaUtil::UUID.randomUUID().toString(), @@ -72,12 +72,14 @@ class ActionPlansController < ApplicationController end def delete + verify_post_request action_plan = ActionPlan.find params[:plan_id] action_plan.destroy redirect_to :action => 'index', :id => @resource.id end def change_status + verify_post_request action_plan = ActionPlan.find params[:plan_id] if action_plan action_plan.status = action_plan.open? ? ActionPlan::STATUS_CLOSED : ActionPlan::STATUS_OPEN diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/action_plans_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/action_plans_controller.rb index bb8ed4d0afd..e0c67868374 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/action_plans_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/action_plans_controller.rb @@ -20,8 +20,6 @@ class Api::ActionPlansController < Api::ApiController - before_filter :admin_required, :only => [ :create, :delete, :update, :close, :open ] - # # GET /api/action_plans/search?project= # @@ -44,8 +42,8 @@ class Api::ActionPlansController < Api::ApiController # POST /api/action_plans/create # # -- Mandatory parameters - # 'name' is the action plan name - # 'project' is the project key to link the action plan to + # 'name' is the name of the action plan + # 'project' is the key of the project to link the action plan to # # -- Optional parameters # 'description' is the plain-text description @@ -56,6 +54,7 @@ class Api::ActionPlansController < Api::ApiController # def create verify_post_request + access_denied unless has_role?(:admin) require_parameters :project, :name result = Internal.issues.createActionPlan(params) @@ -78,6 +77,7 @@ class Api::ActionPlansController < Api::ApiController # def delete verify_post_request + access_denied unless has_role?(:admin) require_parameters :key result = Internal.issues.deleteActionPlan(params[:key]) @@ -102,6 +102,7 @@ class Api::ActionPlansController < Api::ApiController # def update verify_post_request + access_denied unless has_role?(:admin) require_parameters :key result = Internal.issues.updateActionPlan(params[:key], params) @@ -124,6 +125,7 @@ class Api::ActionPlansController < Api::ApiController # def close verify_post_request + access_denied unless has_role?(:admin) require_parameters :key result = Internal.issues.closeActionPlan(params[:key]) @@ -146,6 +148,7 @@ class Api::ActionPlansController < Api::ApiController # def open verify_post_request + access_denied unless has_role?(:admin) require_parameters :key result = Internal.issues.openActionPlan(params[:key]) -- 2.39.5