From: Teryk Bellahsene Date: Fri, 26 Feb 2016 09:44:38 +0000 (+0100) Subject: SONAR-7330 WS api/rules/update apply ES+DB pattern X-Git-Tag: 5.5-M6~107 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=61a5ee1acd76d492128623d76a6712c88c2b8919;p=sonarqube.git SONAR-7330 WS api/rules/update apply ES+DB pattern --- diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/RuleService.java b/server/sonar-server/src/main/java/org/sonar/server/rule/RuleService.java index d14d83c3cc4..5010d7fb505 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/RuleService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/RuleService.java @@ -96,11 +96,6 @@ public class RuleService { return index.terms(RuleNormalizer.RuleField.ALL_TAGS.field(), query, size); } - public void update(RuleUpdate update) { - checkPermission(); - ruleUpdater.update(update, userSession); - } - public RuleKey create(NewRule newRule) { checkPermission(); return ruleCreator.create(newRule); diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/RuleUpdater.java b/server/sonar-server/src/main/java/org/sonar/server/rule/RuleUpdater.java index 1d0424c1ba2..d7248f5b522 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/RuleUpdater.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/RuleUpdater.java @@ -57,24 +57,18 @@ public class RuleUpdater { /** * Update manual rules and custom rules (rules instantiated from templates) */ - public boolean update(RuleUpdate update, UserSession userSession) { + public boolean update(DbSession dbSession, RuleUpdate update, UserSession userSession) { if (update.isEmpty()) { return false; } - DbSession dbSession = dbClient.openSession(false); - try { - Context context = newContext(update); - // validate only the changes, not all the rule fields - apply(update, context, userSession); - dbClient.deprecatedRuleDao().update(dbSession, context.rule); - updateParameters(dbSession, update, context); - dbSession.commit(); - return true; - - } finally { - dbSession.close(); - } + Context context = newContext(update); + // validate only the changes, not all the rule fields + apply(update, context, userSession); + dbClient.deprecatedRuleDao().update(dbSession, context.rule); + updateParameters(dbSession, update, context); + dbSession.commit(); + return true; } /** diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/ws/UpdateAction.java b/server/sonar-server/src/main/java/org/sonar/server/rule/ws/UpdateAction.java index 7a2380c10b2..61b11d32750 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/ws/UpdateAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/ws/UpdateAction.java @@ -19,8 +19,12 @@ */ package org.sonar.server.rule.ws; +import com.google.common.base.Optional; import com.google.common.base.Splitter; import com.google.common.collect.Sets; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import org.apache.commons.lang.StringUtils; import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.RuleStatus; @@ -31,12 +35,18 @@ import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; import org.sonar.api.utils.KeyValueFormat; -import org.sonar.server.exceptions.NotFoundException; -import org.sonar.server.rule.Rule; -import org.sonar.server.rule.RuleService; +import org.sonar.core.permission.GlobalPermissions; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; +import org.sonar.db.rule.RuleDto; +import org.sonar.db.rule.RuleParamDto; import org.sonar.server.rule.RuleUpdate; +import org.sonar.server.rule.RuleUpdater; +import org.sonar.server.user.UserSession; import org.sonarqube.ws.Rules.UpdateResponse; +import static java.util.Collections.singletonList; +import static org.sonar.server.ws.WsUtils.checkFoundWithOptional; import static org.sonar.server.ws.WsUtils.writeProtobuf; public class UpdateAction implements RulesWsAction { @@ -54,12 +64,16 @@ public class UpdateAction implements RulesWsAction { public static final String PARAM_STATUS = "status"; public static final String PARAMS = "params"; - private final RuleService service; - private final RuleMapping mapping; + private final DbClient dbClient; + private final RuleUpdater ruleUpdater; + private final RuleMapper mapper; + private final UserSession userSession; - public UpdateAction(RuleService service, RuleMapping mapping) { - this.service = service; - this.mapping = mapping; + public UpdateAction(DbClient dbClient, RuleUpdater ruleUpdater, RuleMapper mapper, UserSession userSession) { + this.dbClient = dbClient; + this.ruleUpdater = ruleUpdater; + this.mapper = mapper; + this.userSession = userSession; } @Override @@ -125,16 +139,22 @@ public class UpdateAction implements RulesWsAction { @Override public void handle(Request request, Response response) throws Exception { - RuleUpdate update = readRequest(request); - service.update(update); - UpdateResponse updateResponse = buildResponse(update.getRuleKey()); + checkPermission(); + DbSession dbSession = dbClient.openSession(false); + try { + RuleUpdate update = readRequest(dbSession, request); + ruleUpdater.update(dbSession, update, userSession); + UpdateResponse updateResponse = buildResponse(dbSession, update.getRuleKey()); - writeProtobuf(updateResponse, request, response); + writeProtobuf(updateResponse, request, response); + } finally { + dbClient.closeSession(dbSession); + } } - private RuleUpdate readRequest(Request request) { + private RuleUpdate readRequest(DbSession dbSession, Request request) { RuleKey key = RuleKey.parse(request.mandatoryParam(PARAM_KEY)); - RuleUpdate update = createRuleUpdate(key); + RuleUpdate update = createRuleUpdate(dbSession, key); readTags(request, update); readMarkdownNote(request, update); readDebt(request, update); @@ -162,14 +182,13 @@ public class UpdateAction implements RulesWsAction { return update; } - private RuleUpdate createRuleUpdate(RuleKey key) { - Rule rule = service.getByKey(key); - if (rule == null) { - throw new NotFoundException("This rule does not exists : " + key); - } - if (rule.templateKey() != null) { + private RuleUpdate createRuleUpdate(DbSession dbSession, RuleKey key) { + Optional optionalRule = dbClient.ruleDao().selectByKey(dbSession, key); + checkFoundWithOptional(optionalRule, "This rule does not exists : " + key); + RuleDto rule = optionalRule.get(); + if (rule.getTemplateId() != null) { return RuleUpdate.createForCustomRule(key); - } else if (rule.isManual()) { + } else if (RuleKey.MANUAL_REPOSITORY_KEY.equals(rule.getRepositoryKey())) { return RuleUpdate.createForManualRule(key); } else { return RuleUpdate.createForPluginRule(key); @@ -210,11 +229,31 @@ public class UpdateAction implements RulesWsAction { } } - private UpdateResponse buildResponse(RuleKey ruleKey) { - Rule rule = service.getNonNullByKey(ruleKey); + private UpdateResponse buildResponse(DbSession dbSession, RuleKey key) { + Optional optionalRule = dbClient.ruleDao().selectByKey(dbSession, key); + checkFoundWithOptional(optionalRule, "Rule not found: " + key); + RuleDto rule = optionalRule.get(); + List templateRules = new ArrayList<>(); + if (rule.getTemplateId() != null) { + Optional templateRule = dbClient.ruleDao().selectById(rule.getTemplateId(), dbSession); + if (templateRule.isPresent()) { + templateRules.add(templateRule.get()); + } + } + List ruleParameters = dbClient.ruleDao().selectRuleParamsByRuleIds(dbSession, singletonList(rule.getId())); UpdateResponse.Builder responseBuilder = UpdateResponse.newBuilder(); - responseBuilder.setRule(mapping.buildRuleResponse(rule, null /* TODO replace by SearchOptions immutable constant */)); + SearchAction.SearchResult searchResult = new SearchAction.SearchResult() + .setRules(singletonList(rule)) + .setTemplateRules(templateRules) + .setRuleParameters(ruleParameters) + .setTotal(1L); + responseBuilder.setRule(mapper.toWsRule(rule, searchResult, Collections.emptySet())); return responseBuilder.build(); } + + private void checkPermission() { + userSession.checkLoggedIn(); + userSession.checkPermission(GlobalPermissions.QUALITY_PROFILE_ADMIN); + } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesMediumTest.java index 24675023257..ff9194766d7 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesMediumTest.java @@ -35,7 +35,6 @@ import org.sonar.api.rule.Severity; import org.sonar.api.server.debt.DebtRemediationFunction; import org.sonar.api.server.rule.RuleParamType; import org.sonar.api.server.rule.RulesDefinition; -import org.sonar.api.utils.MessageException; import org.sonar.core.permission.GlobalPermissions; import org.sonar.db.DbSession; import org.sonar.db.qualityprofile.ActiveRuleKey; @@ -461,7 +460,7 @@ public class RegisterRulesMediumTest { assertThat(rule.tags()).isEmpty(); // User adds tag - TESTER.get(RuleUpdater.class).update(RuleUpdate.createForPluginRule(RuleTesting.XOO_X1).setTags(newHashSet("tag2")), userSessionRule); + TESTER.get(RuleUpdater.class).update(dbSession, RuleUpdate.createForPluginRule(RuleTesting.XOO_X1).setTags(newHashSet("tag2")), userSessionRule); dbSession.clearCache(); rule = ruleIndex.getByKey(RuleTesting.XOO_X1); assertThat(rule.systemTags()).containsOnly("tag1"); diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/RuleServiceMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/rule/RuleServiceMediumTest.java index 21e1aac476c..544f17680f0 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/rule/RuleServiceMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/rule/RuleServiceMediumTest.java @@ -156,41 +156,6 @@ public class RuleServiceMediumTest { assertThat(tags).containsOnly("sys1", "sys2"); } - @Test - public void update_rule() { - userSessionRule.login("me").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN); - - RuleKey key = RuleKey.of("java", "S001"); - - dao.insert(dbSession, RuleTesting.newDto(key)); - dbSession.commit(); - - RuleUpdate update = RuleUpdate.createForCustomRule(key); - update.setMarkdownNote("my *note*"); - service.update(update); - - dbSession.clearCache(); - - RuleDto rule = dao.getNullableByKey(dbSession, key); - assertThat(rule.getNoteData()).isEqualTo("my *note*"); - assertThat(rule.getNoteUserLogin()).isEqualTo("me"); - } - - @Test(expected = UnauthorizedException.class) - public void do_not_update_if_not_granted() { - userSessionRule.setGlobalPermissions(GlobalPermissions.SCAN_EXECUTION); - - RuleKey key = RuleKey.of("java", "S001"); - - dao.insert(dbSession, RuleTesting.newDto(key) - .setTags(Sets.newHashSet("security")) - .setSystemTags(Sets.newHashSet("java8", "javadoc"))); - dbSession.commit(); - - RuleUpdate update = RuleUpdate.createForPluginRule(key).setMarkdownNote("my *note*"); - service.update(update); - } - @Test public void create_rule() { userSessionRule.login().setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN); diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/RuleUpdaterMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/rule/RuleUpdaterMediumTest.java index 4de502c673f..eeb89016853 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/rule/RuleUpdaterMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/rule/RuleUpdaterMediumTest.java @@ -86,7 +86,7 @@ public class RuleUpdaterMediumTest { RuleUpdate update = RuleUpdate.createForPluginRule(RULE_KEY).setTags(Sets.newHashSet("java9")); try { - updater.update(update, userSessionRule); + updater.update(dbSession, update, userSessionRule); fail(); } catch (IllegalArgumentException e) { assertThat(e).hasMessage("Rule with REMOVED status cannot be updated: squid:S001"); @@ -107,7 +107,7 @@ public class RuleUpdaterMediumTest { RuleUpdate update = RuleUpdate.createForPluginRule(RULE_KEY); assertThat(update.isEmpty()).isTrue(); - updater.update(update, userSessionRule); + updater.update(dbSession, update, userSessionRule); dbSession.clearCache(); RuleDto rule = ruleDao.getNullableByKey(dbSession, RULE_KEY); @@ -136,7 +136,7 @@ public class RuleUpdaterMediumTest { RuleUpdate update = RuleUpdate.createForPluginRule(RULE_KEY); update.setMarkdownNote("my *note*"); - updater.update(update, userSessionRule); + updater.update(dbSession, update, userSessionRule); dbSession.clearCache(); RuleDto rule = ruleDao.getNullableByKey(dbSession, RULE_KEY); @@ -159,7 +159,7 @@ public class RuleUpdaterMediumTest { dbSession.commit(); RuleUpdate update = RuleUpdate.createForPluginRule(RULE_KEY).setMarkdownNote(null); - updater.update(update, userSessionRule); + updater.update(dbSession, update, userSessionRule); dbSession.clearCache(); RuleDto rule = ruleDao.getNullableByKey(dbSession, RULE_KEY); @@ -179,7 +179,7 @@ public class RuleUpdaterMediumTest { // java8 is a system tag -> ignore RuleUpdate update = RuleUpdate.createForPluginRule(RULE_KEY).setTags(Sets.newHashSet("bug", "java8")); - updater.update(update, userSessionRule); + updater.update(dbSession, update, userSessionRule); dbSession.clearCache(); RuleDto rule = ruleDao.getNullableByKey(dbSession, RULE_KEY); @@ -199,7 +199,7 @@ public class RuleUpdaterMediumTest { dbSession.commit(); RuleUpdate update = RuleUpdate.createForPluginRule(RULE_KEY).setTags(null); - updater.update(update, userSessionRule); + updater.update(dbSession, update, userSessionRule); dbSession.clearCache(); RuleDto rule = ruleDao.getNullableByKey(dbSession, RULE_KEY); @@ -225,7 +225,7 @@ public class RuleUpdaterMediumTest { DefaultDebtRemediationFunction fn = new DefaultDebtRemediationFunction(DebtRemediationFunction.Type.CONSTANT_ISSUE, null, "1min"); RuleUpdate update = RuleUpdate.createForPluginRule(RULE_KEY) .setDebtRemediationFunction(fn); - updater.update(update, userSessionRule); + updater.update(dbSession, update, userSessionRule); dbSession.clearCache(); // verify debt is overridden @@ -253,7 +253,7 @@ public class RuleUpdaterMediumTest { RuleUpdate update = RuleUpdate.createForPluginRule(RULE_KEY) .setDebtRemediationFunction(new DefaultDebtRemediationFunction(DebtRemediationFunction.Type.LINEAR, "2d", null)); - updater.update(update, userSessionRule); + updater.update(dbSession, update, userSessionRule); dbSession.clearCache(); // verify debt is overridden @@ -281,7 +281,7 @@ public class RuleUpdaterMediumTest { RuleUpdate update = RuleUpdate.createForPluginRule(RULE_KEY) .setDebtRemediationFunction(new DefaultDebtRemediationFunction(DebtRemediationFunction.Type.CONSTANT_ISSUE, null, "10min")); - updater.update(update, userSessionRule); + updater.update(dbSession, update, userSessionRule); dbSession.clearCache(); // verify debt is overridden @@ -308,7 +308,7 @@ public class RuleUpdaterMediumTest { dbSession.commit(); RuleUpdate update = RuleUpdate.createForPluginRule(RULE_KEY).setDebtRemediationFunction(null); - updater.update(update, userSessionRule); + updater.update(dbSession, update, userSessionRule); dbSession.clearCache(); // verify debt is coming from default values @@ -352,7 +352,7 @@ public class RuleUpdaterMediumTest { .setSeverity("MAJOR") .setStatus(RuleStatus.READY) .setParameters(ImmutableMap.of("regex", "b.*")); - updater.update(update, userSessionRule); + updater.update(dbSession, update, userSessionRule); dbSession.clearCache(); @@ -394,7 +394,7 @@ public class RuleUpdaterMediumTest { .setMarkdownDescription("New description") .setSeverity("MAJOR") .setStatus(RuleStatus.READY); - updater.update(update, userSessionRule); + updater.update(dbSession, update, userSessionRule); dbSession.clearCache(); @@ -437,7 +437,7 @@ public class RuleUpdaterMediumTest { // Update custom rule parameter 'regex', add 'message' and remove 'format' RuleUpdate update = RuleUpdate.createForCustomRule(customRule.getKey()) .setParameters(ImmutableMap.of("regex", "b.*", "message", "a message")); - updater.update(update, userSessionRule); + updater.update(dbSession, update, userSessionRule); dbSession.clearCache(); @@ -482,7 +482,7 @@ public class RuleUpdaterMediumTest { .setName("") .setMarkdownDescription("New desc"); try { - updater.update(update, userSessionRule); + updater.update(dbSession, update, userSessionRule); fail(); } catch (Exception e) { assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("The name is missing"); @@ -506,7 +506,7 @@ public class RuleUpdaterMediumTest { .setName("New name") .setMarkdownDescription(""); try { - updater.update(update, userSessionRule); + updater.update(dbSession, update, userSessionRule); fail(); } catch (Exception e) { assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("The description is missing"); @@ -529,7 +529,7 @@ public class RuleUpdaterMediumTest { .setName("New name") .setMarkdownDescription("New description") .setSeverity(Severity.CRITICAL); - updater.update(update, userSessionRule); + updater.update(dbSession, update, userSessionRule); dbSession.clearCache();