From f5bfa86e0f4c92d61f66dc965010db78823ebdad Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Tue, 3 Jun 2014 23:18:21 +0200 Subject: [PATCH] SONAR-5007 ignore warnings when restoring a backup --- .../qualityprofile/QProfileBackuper.java | 9 ++++- .../server/qualityprofile/RuleActivation.java | 36 ++++++++++++------- .../RuleActivationContextFactory.java | 13 +++---- .../server/qualityprofile/RuleActivator.java | 25 +++++-------- .../org/sonar/server/search/QueryOptions.java | 7 ++-- .../RuleActivatorMediumTest.java | 16 ++++----- .../ws/QProfilesWsMediumTest.java | 7 ++-- .../server/rule/RuleBackendMediumTest.java | 2 +- .../rule/index/RuleIndexMediumTest.java | 6 ++-- .../sonar/server/search/QueryOptionsTest.java | 6 ++++ 10 files changed, 73 insertions(+), 54 deletions(-) diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileBackuper.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileBackuper.java index 22569765d44..4d93115cd37 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileBackuper.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileBackuper.java @@ -26,6 +26,7 @@ import org.apache.commons.lang.StringUtils; import org.codehaus.staxmate.SMInputFactory; import org.codehaus.staxmate.in.SMHierarchicCursor; import org.codehaus.staxmate.in.SMInputCursor; +import org.slf4j.LoggerFactory; import org.sonar.api.ServerComponent; import org.sonar.api.rule.RuleKey; import org.sonar.api.utils.text.XmlWriter; @@ -35,6 +36,7 @@ import org.sonar.core.qualityprofile.db.ActiveRuleKey; import org.sonar.core.qualityprofile.db.QualityProfileDto; import org.sonar.core.qualityprofile.db.QualityProfileKey; import org.sonar.server.db.DbClient; +import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.qualityprofile.index.ActiveRuleIndex; import org.sonar.server.search.IndexClient; @@ -173,7 +175,12 @@ public class QProfileBackuper implements ServerComponent { RuleActivation activation = new RuleActivation(ActiveRuleKey.of(profileKey, ruleKey)); activation.setSeverity(severity); activation.setParameters(parameters); - activator.activate(dbSession, activation); + try { + activator.activate(dbSession, activation); + } catch (BadRequestException e) { + // TODO should return warnings instead of logging warnings + LoggerFactory.getLogger(getClass()).warn(e.getMessage()); + } rulesToDeactivate.remove(ruleKey); } for (RuleKey ruleKey : rulesToDeactivate) { diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivation.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivation.java index 255562e851e..e7c856962f5 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivation.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivation.java @@ -39,29 +39,44 @@ public class RuleActivation { this.key = key; } - public boolean isCascade(){ + /** + * For internal use + */ + boolean isCascade() { return this.cascade; } - public RuleActivation isCascade(boolean b){ + /** + * For internal use + */ + RuleActivation isCascade(boolean b) { this.cascade = b; return this; } - public boolean isReset() { - return severity==null && parameters.isEmpty(); + /** + * For internal use + */ + boolean isReset() { + return severity == null && parameters.isEmpty(); } public RuleActivation setSeverity(@Nullable String s) { - if (s != null) { - if (!Severity.ALL.contains(s)) { - throw new IllegalArgumentException("Unknown severity: " + s); - } + if (s != null && !Severity.ALL.contains(s)) { + throw new IllegalArgumentException("Unknown severity: " + s); } this.severity = s; return this; } + /** + * Optional severity. Use the parent severity or default rule severity if null. + */ + @CheckForNull + public String getSeverity() { + return severity; + } + public RuleActivation setParameter(String key, @Nullable String value) { String sanitizedValue = Strings.emptyToNull(value); if (value == null) { @@ -85,9 +100,4 @@ public class RuleActivation { public Map getParameters() { return parameters; } - - @CheckForNull - public String getSeverity() { - return severity; - } } diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivationContextFactory.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivationContextFactory.java index 5188850c22a..272786895c0 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivationContextFactory.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivationContextFactory.java @@ -31,6 +31,7 @@ import org.sonar.core.qualityprofile.db.QualityProfileDto; import org.sonar.core.qualityprofile.db.QualityProfileKey; import org.sonar.core.rule.RuleDto; import org.sonar.server.db.DbClient; +import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.rule.Rule; import java.util.Collection; @@ -51,7 +52,7 @@ public class RuleActivationContextFactory implements ServerComponent { QualityProfileDto profile = initProfile(key, context, session, false); initActiveRules(key, context, session, false); if (!profile.getLanguage().equals(rule.getLanguage())) { - throw new IllegalArgumentException(String.format("Rule %s and profile %s have different languages", rule.getKey(), profile.getKey())); + throw new BadRequestException(String.format("Rule %s and profile %s have different languages", rule.getKey(), profile.getKey())); } if (profile.getParent() != null) { @@ -66,16 +67,16 @@ public class RuleActivationContextFactory implements ServerComponent { private RuleDto initRule(RuleKey ruleKey, RuleActivationContext context, DbSession dbSession) { RuleDto rule = db.ruleDao().getNullableByKey(dbSession, ruleKey); if (rule == null) { - throw new IllegalArgumentException("Rule not found: " + ruleKey); + throw new BadRequestException("Rule not found: " + ruleKey); } if (RuleStatus.REMOVED == rule.getStatus()) { - throw new IllegalArgumentException("Rule was removed: " + ruleKey); + throw new BadRequestException("Rule was removed: " + ruleKey); } if (Cardinality.MULTIPLE.equals(rule.getCardinality())) { - throw new IllegalArgumentException("Rule template can't be activated on a Quality profile: " + ruleKey); + throw new BadRequestException("Rule template can't be activated on a Quality profile: " + ruleKey); } if (Rule.MANUAL_REPOSITORY_KEY.equals(rule.getRepositoryKey())) { - throw new IllegalArgumentException("Manual rule can't be activated on a Quality profile: " + ruleKey); + throw new BadRequestException("Manual rule can't be activated on a Quality profile: " + ruleKey); } context.setRule(rule); context.setRuleParams(db.ruleDao().findRuleParamsByRuleKey(dbSession, rule.getKey())); @@ -85,7 +86,7 @@ public class RuleActivationContextFactory implements ServerComponent { private QualityProfileDto initProfile(ActiveRuleKey key, RuleActivationContext context, DbSession session, boolean parent) { QualityProfileDto profile = db.qualityProfileDao().getByKey(key.qProfile(),session); if (profile == null) { - throw new IllegalArgumentException("Quality profile not found: " + key.qProfile()); + throw new BadRequestException("Quality profile not found: " + key.qProfile()); } if (parent) { context.setParentProfile(profile); diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java index dcab4ba292f..de87d2ac1da 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java @@ -77,6 +77,9 @@ public class RuleActivator implements ServerComponent { /** * Activate a rule on a Quality profile. Update configuration (severity/parameters) if the rule is already * activated. + * + * @throws org.sonar.server.exceptions.BadRequestException if the profile, the rule or a rule parameter does + * not exist */ List activate(RuleActivation activation) { DbSession dbSession = db.openSession(false); @@ -96,7 +99,6 @@ public class RuleActivator implements ServerComponent { * Activate the rule WITHOUT committing db session */ List activate(DbSession dbSession, RuleActivation activation) { - RuleActivationContext context = contextFactory.create(activation.getKey(), dbSession); List changes = Lists.newArrayList(); ActiveRuleChange change; @@ -145,7 +147,6 @@ public class RuleActivator implements ServerComponent { } private List cascadeActivation(DbSession session, RuleActivation activation) { - List changes = Lists.newArrayList(); // get all inherited profiles @@ -284,20 +285,15 @@ public class RuleActivator implements ServerComponent { try { // TODO pb because limited to QueryOptions.MAX_LIMIT RuleResult result = ruleIndex.search(ruleQuery, - QueryOptions.DEFAULT.setOffset(0) - .setLimit(Integer.MAX_VALUE) - .setFieldsToReturn(ImmutableSet.of(RuleNormalizer.RuleField.IS_TEMPLATE.field(), RuleNormalizer.RuleField.SEVERITY.field())) - ); + new QueryOptions() + .setMaxLimit() + .setFieldsToReturn(ImmutableSet.of(RuleNormalizer.RuleField.IS_TEMPLATE.field()))); for (Rule rule : result.getHits()) { if (!rule.isTemplate()) { ActiveRuleKey key = ActiveRuleKey.of(profileKey, rule.key()); RuleActivation activation = new RuleActivation(key); - if (severity != null && !severity.isEmpty()) { - activation.setSeverity(severity); - } else { - activation.setSeverity(rule.severity()); - } + activation.setSeverity(severity); for (ActiveRuleChange active : this.activate(dbSession, activation)) { results.put("activated", active.getKey().ruleKey().toString()); } @@ -318,12 +314,7 @@ public class RuleActivator implements ServerComponent { DbSession dbSession = db.openSession(false); try { - RuleResult result = ruleIndex.search(ruleQuery, - QueryOptions.DEFAULT.setOffset(0) - // TODO pb because limited to QueryOptions.MAX_LIMIT - .setLimit(Integer.MAX_VALUE) - .setFieldsToReturn(ImmutableSet.of(RuleNormalizer.RuleField.IS_TEMPLATE.field(), RuleNormalizer.RuleField.SEVERITY.field())) - ); + RuleResult result = ruleIndex.search(ruleQuery, new QueryOptions()); for (Rule rule : result.getHits()) { ActiveRuleKey key = ActiveRuleKey.of(profile, rule.key()); diff --git a/sonar-server/src/main/java/org/sonar/server/search/QueryOptions.java b/sonar-server/src/main/java/org/sonar/server/search/QueryOptions.java index 881b4d87470..ae19e77a69f 100644 --- a/sonar-server/src/main/java/org/sonar/server/search/QueryOptions.java +++ b/sonar-server/src/main/java/org/sonar/server/search/QueryOptions.java @@ -34,8 +34,6 @@ import java.util.Set; */ public class QueryOptions { - public static final QueryOptions DEFAULT = new QueryOptions().setLimit(Integer.MAX_VALUE); - public static final int DEFAULT_OFFSET = 0; public static final int DEFAULT_LIMIT = 10; public static final int MAX_LIMIT = 500; @@ -103,6 +101,11 @@ public class QueryOptions { return this; } + public QueryOptions setMaxLimit() { + this.limit = MAX_LIMIT; + return this; + } + public Set getFieldsToReturn() { return fieldsToReturn; } diff --git a/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorMediumTest.java b/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorMediumTest.java index 67a30dd5ede..5a626bf2e4d 100644 --- a/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorMediumTest.java +++ b/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorMediumTest.java @@ -188,7 +188,7 @@ public class RuleActivatorMediumTest { try { ruleActivator.activate(activation); fail(); - } catch (IllegalArgumentException e) { + } catch (BadRequestException e) { assertThat(e).hasMessage("Rule template can't be activated on a Quality profile: xoo:template1"); verifyZeroActiveRules(XOO_PROFILE_KEY); } @@ -202,7 +202,7 @@ public class RuleActivatorMediumTest { try { ruleActivator.activate(activation); fail(); - } catch (IllegalArgumentException e) { + } catch (BadRequestException e) { assertThat(e).hasMessage("Rule squid:j1 and profile P1:xoo have different languages"); verifyZeroActiveRules(XOO_PROFILE_KEY); } @@ -216,7 +216,7 @@ public class RuleActivatorMediumTest { try { ruleActivator.activate(activation); fail(); - } catch (IllegalArgumentException e) { + } catch (BadRequestException e) { assertThat(e).hasMessage("Rule not found: xoo:x3"); verifyZeroActiveRules(XOO_PROFILE_KEY); } @@ -235,7 +235,7 @@ public class RuleActivatorMediumTest { try { ruleActivator.activate(activation); fail(); - } catch (IllegalArgumentException e) { + } catch (BadRequestException e) { assertThat(e).hasMessage("Rule was removed: xoo:x1"); verifyZeroActiveRules(XOO_PROFILE_KEY); } @@ -248,7 +248,7 @@ public class RuleActivatorMediumTest { try { ruleActivator.activate(activation); fail(); - } catch (IllegalArgumentException e) { + } catch (BadRequestException e) { assertThat(e).hasMessage("Manual rule can't be activated on a Quality profile: manual:m1"); verifyZeroActiveRules(XOO_PROFILE_KEY); } @@ -262,7 +262,7 @@ public class RuleActivatorMediumTest { try { ruleActivator.activate(activation); fail(); - } catch (IllegalArgumentException e) { + } catch (BadRequestException e) { assertThat(e).hasMessage("Quality profile not found: other:js"); } } @@ -311,7 +311,7 @@ public class RuleActivatorMediumTest { try { ruleActivator.deactivate(key); fail(); - } catch (IllegalArgumentException e) { + } catch (BadRequestException e) { assertThat(e).hasMessage("Rule not found: xoo:x3"); verifyZeroActiveRules(XOO_PROFILE_KEY); } @@ -323,7 +323,7 @@ public class RuleActivatorMediumTest { try { ruleActivator.deactivate(key); fail(); - } catch (IllegalArgumentException e) { + } catch (BadRequestException e) { assertThat(e).hasMessage("Quality profile not found: other:js"); } } diff --git a/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsMediumTest.java b/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsMediumTest.java index a9aa46ae360..dc3b6c5d26b 100644 --- a/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsMediumTest.java +++ b/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsMediumTest.java @@ -36,6 +36,7 @@ import org.sonar.core.qualityprofile.db.ActiveRuleKey; import org.sonar.core.qualityprofile.db.QualityProfileDto; import org.sonar.core.rule.RuleDto; import org.sonar.server.db.DbClient; +import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.qualityprofile.index.ActiveRuleIndex; import org.sonar.server.rule.index.RuleIndex; import org.sonar.server.rule.index.RuleQuery; @@ -226,10 +227,10 @@ public class QProfilesWsMediumTest { WsTester.TestRequest request = wsTester.newGetRequest(QProfilesWs.API_ENDPOINT, RuleActivationActions.ACTIVATE_ACTION); request.setParam(RuleActivationActions.PROFILE_KEY, profile.getKey().toString()); request.setParam(RuleActivationActions.RULE_KEY, rule.getKey().toString()); - WsTester.Result result = request.execute(); + request.execute(); session.clearCache(); fail(); - } catch (IllegalArgumentException e) { + } catch (BadRequestException e) { assertThat(e.getMessage()).isEqualTo("Rule blah:toto and profile test:java have different languages"); } } @@ -378,7 +379,7 @@ public class QProfilesWsMediumTest { // 2. Assert ActiveRule with BLOCKER severity assertThat(tester.get(RuleIndex.class).search( new RuleQuery().setSeverities(ImmutableSet.of("BLOCKER")), - QueryOptions.DEFAULT).getHits()).hasSize(2); + new QueryOptions()).getHits()).hasSize(2); // 1. Activate Rule with query returning 2 hits WsTester.TestRequest request = wsTester.newGetRequest(QProfilesWs.API_ENDPOINT, BulkRuleActivationActions.BULK_ACTIVATE_ACTION); diff --git a/sonar-server/src/test/java/org/sonar/server/rule/RuleBackendMediumTest.java b/sonar-server/src/test/java/org/sonar/server/rule/RuleBackendMediumTest.java index b3944a21efe..d09ba714c27 100644 --- a/sonar-server/src/test/java/org/sonar/server/rule/RuleBackendMediumTest.java +++ b/sonar-server/src/test/java/org/sonar/server/rule/RuleBackendMediumTest.java @@ -313,7 +313,7 @@ public class RuleBackendMediumTest { assertThat(index.getByKey(removedKey)).isNotNull(); // 2. assert find does not get REMOVED - List rules = index.search(new RuleQuery(), QueryOptions.DEFAULT).getHits(); + List rules = index.search(new RuleQuery(), new QueryOptions()).getHits(); assertThat(rules).hasSize(1); assertThat(rules.get(0).key()).isEqualTo(readyKey); } diff --git a/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexMediumTest.java b/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexMediumTest.java index 6b5b496bd0e..77940ab5bfd 100644 --- a/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexMediumTest.java +++ b/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexMediumTest.java @@ -776,13 +776,13 @@ public class RuleIndexMediumTest { dbSession.commit(); // 0. find all rules; - assertThat(index.search(new RuleQuery(), QueryOptions.DEFAULT).getHits()).hasSize(2); + assertThat(index.search(new RuleQuery(), new QueryOptions()).getHits()).hasSize(2); // 1. find all rules available since a date; RuleQuery availableSinceQuery = new RuleQuery() .setAvailableSince(since); - List hits = index.search(availableSinceQuery, QueryOptions.DEFAULT).getHits(); + List hits = index.search(availableSinceQuery, new QueryOptions()).getHits(); assertThat(hits).hasSize(1); // 2. find no new rules since tomorrow. @@ -791,7 +791,7 @@ public class RuleIndexMediumTest { c.add(Calendar.DATE, 1); // number of days to add RuleQuery availableSinceNowQuery = new RuleQuery() .setAvailableSince(c.getTime()); - assertThat(index.search(availableSinceNowQuery, QueryOptions.DEFAULT).getHits()).hasSize(0); + assertThat(index.search(availableSinceNowQuery, new QueryOptions()).getHits()).hasSize(0); } private RuleDto newRuleDto(RuleKey ruleKey) { diff --git a/sonar-server/src/test/java/org/sonar/server/search/QueryOptionsTest.java b/sonar-server/src/test/java/org/sonar/server/search/QueryOptionsTest.java index 08247322765..35b201f3577 100644 --- a/sonar-server/src/test/java/org/sonar/server/search/QueryOptionsTest.java +++ b/sonar-server/src/test/java/org/sonar/server/search/QueryOptionsTest.java @@ -76,6 +76,12 @@ public class QueryOptionsTest { assertThat(options.getLimit()).isEqualTo(QueryOptions.MAX_LIMIT); } + @Test + public void set_max_limit() throws Exception { + options.setMaxLimit(); + assertThat(options.getLimit()).isEqualTo(QueryOptions.MAX_LIMIT); + } + @Test public void max_page_size() throws Exception { options.setPage(3, QueryOptions.MAX_LIMIT + 10); -- 2.39.5