Skip to content

Commit

Permalink
SONAR-7330 WS api/rules/search apply ES+DB pattern
Browse files Browse the repository at this point in the history
  • Loading branch information
@teryk
teryk committed Feb 26, 2016
1 parent f4af3e9 commit 6940e82
Show file tree
Hide file tree
Showing 15 changed files with 691 additions and 77 deletions.
2 changes: 2 additions & 0 deletions ...er/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel4.java
View file
Expand Up @@ -257,6 +257,7 @@
import org.sonar.server.rule.RuleUpdater; import org.sonar.server.rule.RuleUpdater;
import org.sonar.server.rule.ws.ActiveRuleCompleter; import org.sonar.server.rule.ws.ActiveRuleCompleter;
import org.sonar.server.rule.ws.RepositoriesAction; import org.sonar.server.rule.ws.RepositoriesAction;
import org.sonar.server.rule.ws.RuleMapper;
import org.sonar.server.rule.ws.RuleMapping; import org.sonar.server.rule.ws.RuleMapping;
import org.sonar.server.rule.ws.RulesWs; import org.sonar.server.rule.ws.RulesWs;
import org.sonar.server.rule.ws.TagsAction; import org.sonar.server.rule.ws.TagsAction;
Expand Down Expand Up @@ -450,6 +451,7 @@ protected void configureLevel() {
org.sonar.server.rule.ws.ListAction.class, org.sonar.server.rule.ws.ListAction.class,
TagsAction.class, TagsAction.class,
RuleMapping.class, RuleMapping.class,
RuleMapper.class,
ActiveRuleCompleter.class, ActiveRuleCompleter.class,
RepositoriesAction.class, RepositoriesAction.class,
org.sonar.server.rule.ws.AppAction.class, org.sonar.server.rule.ws.AppAction.class,
Expand Down
48 changes: 42 additions & 6 deletions server/sonar-server/src/main/java/org/sonar/server/qualityprofile/db/ActiveRuleDao.java
View file
Expand Up @@ -19,8 +19,15 @@
*/ */
package org.sonar.server.qualityprofile.db; package org.sonar.server.qualityprofile.db;


import com.google.common.base.Function;
import com.google.common.base.Optional;
import com.google.common.base.Preconditions; import com.google.common.base.Preconditions;
import java.util.List;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.sonar.api.utils.System2; import org.sonar.api.utils.System2;
import org.sonar.db.DatabaseUtils;
import org.sonar.db.DbSession; import org.sonar.db.DbSession;
import org.sonar.db.qualityprofile.ActiveRuleDto; import org.sonar.db.qualityprofile.ActiveRuleDto;
import org.sonar.db.qualityprofile.ActiveRuleKey; import org.sonar.db.qualityprofile.ActiveRuleKey;
Expand All @@ -33,9 +40,7 @@
import org.sonar.server.rule.db.RuleDao; import org.sonar.server.rule.db.RuleDao;
import org.sonar.server.search.IndexDefinition; import org.sonar.server.search.IndexDefinition;


import javax.annotation.CheckForNull; import static java.util.Collections.emptyList;

import java.util.List;


public class ActiveRuleDao extends BaseDao<ActiveRuleMapper, ActiveRuleDto, ActiveRuleKey> { public class ActiveRuleDao extends BaseDao<ActiveRuleMapper, ActiveRuleDto, ActiveRuleKey> {


Expand All @@ -59,9 +64,6 @@ public ActiveRuleDao(QualityProfileDao profileDao, RuleDao ruleDao, System2 syst
this.profileDao = profileDao; this.profileDao = profileDao;
} }


/**
* @deprecated do not use ids but keys
*/
@CheckForNull @CheckForNull
@Deprecated @Deprecated
public ActiveRuleDto selectById(DbSession session, int activeRuleId) { public ActiveRuleDto selectById(DbSession session, int activeRuleId) {
Expand Down Expand Up @@ -126,6 +128,23 @@ public List<ActiveRuleParamDto> selectAllParams(DbSession dbSession) {
return mapper(dbSession).selectAllParams(); return mapper(dbSession).selectAllParams();
} }


public Optional<ActiveRuleDto> selectByActiveRuleKey(DbSession dbSession, ActiveRuleKey key) {
return Optional.fromNullable(mapper(dbSession).selectByKey(key.qProfile(), key.ruleKey().repository(), key.ruleKey().rule()));
}

public List<ActiveRuleDto> selectByActiveRuleKeys(final DbSession dbSession, final List<ActiveRuleKey> keys) {
if (keys.isEmpty()) {

This comment has been minimized.

Sign in to view

Copy link
@julienlancelot

julienlancelot Feb 26, 2016

Contributor

This check is already handle by DatabaseUtils.executeLargeInputs
return emptyList();
}

return DatabaseUtils.executeLargeInputs(keys, new Function<List<ActiveRuleKey>, List<ActiveRuleDto>>() {
@Override
public List<ActiveRuleDto> apply(@Nonnull List<ActiveRuleKey> input) {
return mapper(dbSession).selectByKeys(input);
}
});
}

/** /**
* Nested DTO ActiveRuleParams * Nested DTO ActiveRuleParams
*/ */
Expand Down Expand Up @@ -187,6 +206,23 @@ public List<ActiveRuleParamDto> selectParamsByActiveRuleKey(DbSession session, A
return mapper(session).selectParamsByActiveRuleId(activeRule.getId()); return mapper(session).selectParamsByActiveRuleId(activeRule.getId());
} }


public List<ActiveRuleParamDto> selectParamsByActiveRuleId(DbSession dbSession, Integer activeRuleId) {

This comment has been minimized.

Sign in to view

Copy link
@julienlancelot

julienlancelot Feb 26, 2016

Contributor

Why not have used selectParamsByActiveRuleKey ?
return mapper(dbSession).selectParamsByActiveRuleId(activeRuleId);
}

public List<ActiveRuleParamDto> selectParamsByActiveRuleIds(final DbSession dbSession, List<Integer> activeRuleIds) {
if (activeRuleIds.isEmpty()) {
return emptyList();
}

return DatabaseUtils.executeLargeInputs(activeRuleIds, new Function<List<Integer>, List<ActiveRuleParamDto>>() {
@Override
public List<ActiveRuleParamDto> apply(@Nullable List<Integer> input) {
return mapper(dbSession).selectParamsByActiveRuleIds(input);
}
});
}

@CheckForNull @CheckForNull
public ActiveRuleParamDto selectParamByKeyAndName(ActiveRuleKey key, String name, DbSession session) { public ActiveRuleParamDto selectParamByKeyAndName(ActiveRuleKey key, String name, DbSession session) {
Preconditions.checkNotNull(key, ACTIVE_RULE_KEY_CANNOT_BE_NULL); Preconditions.checkNotNull(key, ACTIVE_RULE_KEY_CANNOT_BE_NULL);
Expand Down
89 changes: 72 additions & 17 deletions server/sonar-server/src/main/java/org/sonar/server/rule/ws/ActiveRuleCompleter.java
View file
Expand Up @@ -19,19 +19,32 @@
*/ */
package org.sonar.server.rule.ws; package org.sonar.server.rule.ws;


import com.google.common.base.Function;
import com.google.common.base.Optional;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps; import com.google.common.collect.Maps;
import java.util.Arrays;
import java.util.Collection; import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map; import java.util.Map;
import javax.annotation.CheckForNull; import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import org.sonar.api.resources.Language; import org.sonar.api.resources.Language;
import org.sonar.api.resources.Languages; import org.sonar.api.resources.Languages;
import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.RuleKey;
import org.sonar.api.server.ServerSide; import org.sonar.api.server.ServerSide;
import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Logger;
import org.sonar.api.utils.log.Loggers; import org.sonar.api.utils.log.Loggers;
import org.sonar.db.DbSession;
import org.sonar.db.qualityprofile.ActiveRuleDto;
import org.sonar.db.qualityprofile.ActiveRuleKey; import org.sonar.db.qualityprofile.ActiveRuleKey;
import org.sonar.db.qualityprofile.ActiveRuleParamDto;
import org.sonar.db.qualityprofile.QualityProfileDto; import org.sonar.db.qualityprofile.QualityProfileDto;
import org.sonar.db.rule.RuleDto;
import org.sonar.server.db.DbClient;
import org.sonar.server.qualityprofile.ActiveRule; import org.sonar.server.qualityprofile.ActiveRule;
import org.sonar.server.qualityprofile.QProfileLoader; import org.sonar.server.qualityprofile.QProfileLoader;
import org.sonar.server.rule.Rule; import org.sonar.server.rule.Rule;
Expand All @@ -42,6 +55,8 @@


import static com.google.common.base.Strings.nullToEmpty; import static com.google.common.base.Strings.nullToEmpty;
import static com.google.common.collect.Sets.newHashSet; import static com.google.common.collect.Sets.newHashSet;
import static java.util.Collections.singletonList;
import static org.sonar.server.ws.WsUtils.checkFoundWithOptional;


/** /**
* Add details about active rules to api/rules/search and api/rules/show * Add details about active rules to api/rules/search and api/rules/show
Expand All @@ -52,36 +67,57 @@ public class ActiveRuleCompleter {


private static final Logger LOG = Loggers.get(ActiveRuleCompleter.class); private static final Logger LOG = Loggers.get(ActiveRuleCompleter.class);


private final DbClient dbClient;
private final QProfileLoader loader; private final QProfileLoader loader;
private final Languages languages; private final Languages languages;


public ActiveRuleCompleter(QProfileLoader loader, Languages languages) { public ActiveRuleCompleter(DbClient dbClient, QProfileLoader loader, Languages languages) {
this.dbClient = dbClient;
this.loader = loader; this.loader = loader;
this.languages = languages; this.languages = languages;
} }


void completeSearch(RuleQuery query, Collection<Rule> rules, SearchResponse.Builder searchResponse) { void completeSearch(DbSession dbSession, RuleQuery query, List<RuleDto> rules, SearchResponse.Builder searchResponse) {
Collection<String> harvestedProfileKeys = writeActiveRules(searchResponse, query, rules); Collection<String> harvestedProfileKeys = writeActiveRules(dbSession, searchResponse, query, rules);
searchResponse.setQProfiles(buildQProfiles(harvestedProfileKeys)); searchResponse.setQProfiles(buildQProfiles(harvestedProfileKeys));
} }


private Collection<String> writeActiveRules(SearchResponse.Builder response, RuleQuery query, Collection<Rule> rules) { private Collection<String> writeActiveRules(DbSession dbSession, SearchResponse.Builder response, RuleQuery query, Collection<RuleDto> rules) {
Collection<String> qProfileKeys = newHashSet(); Collection<String> qProfileKeys = newHashSet();
Rules.Actives.Builder activesBuilder = response.getActivesBuilder(); Rules.Actives.Builder activesBuilder = response.getActivesBuilder();


String profileKey = query.getQProfileKey(); String profileKey = query.getQProfileKey();
if (profileKey != null) { if (profileKey != null) {
// Load details of active rules on the selected profile // Load details of active rules on the selected profile
for (Rule rule : rules) { for (RuleDto rule : rules) {
ActiveRule activeRule = loader.getActiveRule(ActiveRuleKey.of(profileKey, rule.key())); ActiveRule activeRule = loader.getActiveRule(ActiveRuleKey.of(profileKey, rule.getKey()));
if (activeRule != null) { if (activeRule != null) {
qProfileKeys = writeActiveRules(rule.key(), Arrays.asList(activeRule), activesBuilder); Optional<ActiveRuleDto> activeRuleDto = dbClient.activeRuleDao().selectByActiveRuleKey(dbSession, activeRule.key());
checkFoundWithOptional(activeRuleDto, "Active rule with key '%s' not found", activeRule.key().toString());
List<ActiveRuleParamDto> activeRuleParamDtos = dbClient.activeRuleDao().selectParamsByActiveRuleId(dbSession, activeRuleDto.get().getId());

This comment has been minimized.

Sign in to view

Copy link
@julienlancelot

julienlancelot Feb 26, 2016

Contributor

Would have been better to use selectParamsByActiveRuleKey, no ?

This comment has been minimized.

Sign in to view

Copy link
@julienlancelot

julienlancelot Feb 26, 2016

Contributor

Moreover, doing one SQL query by rule to load parameter is too much, no ?
Would have it been possible to load all params from all rules just before ?

This comment has been minimized.

Sign in to view

Copy link
@julienlancelot

julienlancelot Feb 26, 2016

Contributor

Forget my last point, optimisation can be done later
ListMultimap<ActiveRuleKey, ActiveRuleParamDto> activeRuleParamByActiveRuleKey = ArrayListMultimap.create(1, activeRuleParamDtos.size());
activeRuleParamByActiveRuleKey.putAll(activeRule.key(), activeRuleParamDtos);
qProfileKeys = writeActiveRules(rule.getKey(), singletonList(activeRule), activeRuleParamByActiveRuleKey, activesBuilder);
} }
} }
} else { } else {
// Load details of all active rules // Load details of all active rules
for (Rule rule : rules) { for (RuleDto rule : rules) {
qProfileKeys = writeActiveRules(rule.key(), loader.findActiveRulesByRule(rule.key()), activesBuilder); List<ActiveRule> activeRules = loader.findActiveRulesByRule(rule.getKey());
List<ActiveRuleDto> activeRuleDtos = dbClient.activeRuleDao().selectByActiveRuleKeys(dbSession, Lists.transform(activeRules, ActiveRuleToKey.INSTANCE));
Map<Integer, ActiveRuleKey> activeRuleIdsByKey = new HashMap<>();
for (ActiveRuleDto activeRuleDto : activeRuleDtos) {
activeRuleIdsByKey.put(activeRuleDto.getId(), activeRuleDto.getKey());
}

List<ActiveRuleParamDto> activeRuleParamDtos = dbClient.activeRuleDao().selectParamsByActiveRuleIds(dbSession, Lists.transform(activeRuleDtos, ActiveRuleDtoToId.INSTANCE));
ListMultimap<ActiveRuleKey, ActiveRuleParamDto> activeRuleParamsByActiveRuleKey = ArrayListMultimap.create(activeRules.size(), 10);
for (ActiveRuleParamDto activeRuleParamDto : activeRuleParamDtos) {
ActiveRuleKey activeRuleKey = activeRuleIdsByKey.get(activeRuleParamDto.getId());
activeRuleParamsByActiveRuleKey.put(activeRuleKey, activeRuleParamDto);
}

qProfileKeys = writeActiveRules(rule.getKey(), activeRules, activeRuleParamsByActiveRuleKey, activesBuilder);
} }
} }


Expand All @@ -91,15 +127,16 @@ private Collection<String> writeActiveRules(SearchResponse.Builder response, Rul


void completeShow(Rule rule, ShowResponse.Builder response) { void completeShow(Rule rule, ShowResponse.Builder response) {
for (ActiveRule activeRule : loader.findActiveRulesByRule(rule.key())) { for (ActiveRule activeRule : loader.findActiveRulesByRule(rule.key())) {
response.addActives(buildActiveRuleResponse(activeRule)); response.addActives(buildActiveRuleResponse(activeRule, Collections.<ActiveRuleParamDto>emptyList()));
} }
} }


private static Collection<String> writeActiveRules(RuleKey ruleKey, Collection<ActiveRule> activeRules, Rules.Actives.Builder activesBuilder) { private static Collection<String> writeActiveRules(RuleKey ruleKey, Collection<ActiveRule> activeRules,
ListMultimap<ActiveRuleKey, ActiveRuleParamDto> activeRuleParamsByActiveRuleKey, Rules.Actives.Builder activesBuilder) {
Collection<String> qProfileKeys = newHashSet(); Collection<String> qProfileKeys = newHashSet();
Rules.ActiveList.Builder activeRulesListResponse = Rules.ActiveList.newBuilder(); Rules.ActiveList.Builder activeRulesListResponse = Rules.ActiveList.newBuilder();
for (ActiveRule activeRule : activeRules) { for (ActiveRule activeRule : activeRules) {
activeRulesListResponse.addActiveList(buildActiveRuleResponse(activeRule)); activeRulesListResponse.addActiveList(buildActiveRuleResponse(activeRule, activeRuleParamsByActiveRuleKey.get(activeRule.key())));
qProfileKeys.add(activeRule.key().qProfile()); qProfileKeys.add(activeRule.key().qProfile());
} }
activesBuilder activesBuilder
Expand All @@ -108,7 +145,7 @@ private static Collection<String> writeActiveRules(RuleKey ruleKey, Collection<A
return qProfileKeys; return qProfileKeys;
} }


private static Rules.Active buildActiveRuleResponse(ActiveRule activeRule) { private static Rules.Active buildActiveRuleResponse(ActiveRule activeRule, List<ActiveRuleParamDto> parameters) {
Rules.Active.Builder activeRuleResponse = Rules.Active.newBuilder(); Rules.Active.Builder activeRuleResponse = Rules.Active.newBuilder();
activeRuleResponse.setQProfile(activeRule.key().qProfile()); activeRuleResponse.setQProfile(activeRule.key().qProfile());
activeRuleResponse.setInherit(activeRule.inheritance().toString()); activeRuleResponse.setInherit(activeRule.inheritance().toString());
Expand All @@ -118,10 +155,10 @@ private static Rules.Active buildActiveRuleResponse(ActiveRule activeRule) {
activeRuleResponse.setParent(parentKey.toString()); activeRuleResponse.setParent(parentKey.toString());
} }
Rules.Active.Param.Builder paramBuilder = Rules.Active.Param.newBuilder(); Rules.Active.Param.Builder paramBuilder = Rules.Active.Param.newBuilder();
for (Map.Entry<String, String> param : activeRule.params().entrySet()) { for (ActiveRuleParamDto parameter : parameters) {
activeRuleResponse.addParams(paramBuilder.clear() activeRuleResponse.addParams(paramBuilder.clear()
.setKey(param.getKey()) .setKey(parameter.getKey())
.setValue(nullToEmpty(param.getValue()))); .setValue(nullToEmpty(parameter.getValue())));
} }


return activeRuleResponse.build(); return activeRuleResponse.build();
Expand Down Expand Up @@ -176,4 +213,22 @@ private void writeProfile(Map<String, Rules.QProfile> profilesResponse, QualityP
profilesResponse.put(profile.getKey(), profileResponse.build()); profilesResponse.put(profile.getKey(), profileResponse.build());
} }


private enum ActiveRuleToKey implements Function<ActiveRule, ActiveRuleKey> {
INSTANCE;

@Override
public ActiveRuleKey apply(@Nonnull ActiveRule input) {
return input.key();
}
}

private enum ActiveRuleDtoToId implements Function<ActiveRuleDto, Integer> {
INSTANCE;

@Override
public Integer apply(@Nonnull ActiveRuleDto input) {
return input.getId();
}
}

} }

0 comments on commit 6940e82

Please sign in to comment.