From 87b720fbd3f2369694fcfe2ecc8886be8eba92eb Mon Sep 17 00:00:00 2001 From: Stephane Gamard Date: Tue, 1 Jul 2014 16:30:33 +0200 Subject: [PATCH] fix quality flaws --- .../qualityprofile/index/ActiveRuleIndex.java | 19 ----------- .../index/ActiveRuleNormalizer.java | 6 ++-- .../sonar/server/rule/index/RuleIndex.java | 34 ++++++++++--------- .../org/sonar/server/search/BaseIndex.java | 16 ++++----- 4 files changed, 29 insertions(+), 46 deletions(-) diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/index/ActiveRuleIndex.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/index/ActiveRuleIndex.java index a4d0ec05646..4dbfd203599 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/index/ActiveRuleIndex.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/index/ActiveRuleIndex.java @@ -1,22 +1,3 @@ -/* - * SonarQube, open source software quality management tool. - * Copyright (C) 2008-2014 SonarSource - * mailto:contact AT sonarsource DOT com - * - * SonarQube is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * SonarQube is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ /* * SonarQube, open source software quality management tool. * Copyright (C) 2008-2014 SonarSource diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/index/ActiveRuleNormalizer.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/index/ActiveRuleNormalizer.java index 1c5784a2282..3306df57e7c 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/index/ActiveRuleNormalizer.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/index/ActiveRuleNormalizer.java @@ -102,7 +102,8 @@ public class ActiveRuleNormalizer extends BaseNormalizer requests = new ArrayList(); try { - requests.addAll(normalize(db.activeRuleDao().getNullableByKey(dbSession, key))); + ActiveRuleDto activeRule = db.activeRuleDao().getByKey(dbSession, key); + requests.addAll(normalize(activeRule)); for (ActiveRuleParamDto param : db.activeRuleDao().findParamsByActiveRuleKey(dbSession, key)) { requests.addAll(normalizeNested(param, key)); } @@ -129,14 +130,13 @@ public class ActiveRuleNormalizer extends BaseNormalizer { private void setSorting(RuleQuery query, SearchRequestBuilder esSearch) { /* integrate Query Sort */ + String queryText = query.getQueryText(); if (query.getSortField() != null) { FieldSortBuilder sort = SortBuilders.fieldSort(query.getSortField().sortField()); if (query.isAscendingSort()) { @@ -142,7 +142,7 @@ public class RuleIndex extends BaseIndex { sort.order(SortOrder.DESC); } esSearch.addSort(sort); - } else if (query.getQueryText() != null && !query.getQueryText().isEmpty()) { + } else if (queryText != null && !queryText.isEmpty()) { esSearch.addSort(SortBuilders.scoreSort()); } else { esSearch.addSort(RuleNormalizer.RuleField.UPDATED_AT.sortField(), SortOrder.DESC); @@ -175,6 +175,7 @@ public class RuleIndex extends BaseIndex { protected QueryBuilder getQuery(RuleQuery query, QueryOptions options) { // No contextual query case + String queryText = query.getQueryText(); if (query.getQueryText() == null || query.getQueryText().isEmpty()) { return QueryBuilders.matchAllQuery(); } @@ -228,8 +229,8 @@ public class RuleIndex extends BaseIndex { this.addTermFilter(fb, RuleNormalizer.RuleField._TAGS.field(), query.getTags()); // Construct the debt filter on effective char and subChar - Collection characteristics = query.getDebtCharacteristics(); - if (characteristics != null && !characteristics.isEmpty()) { + Collection debtCharacteristics = query.getDebtCharacteristics(); + if (debtCharacteristics != null && !debtCharacteristics.isEmpty()) { fb.must( FilterBuilders.orFilter( // Match only when NOT NONE overriden @@ -237,8 +238,8 @@ public class RuleIndex extends BaseIndex { FilterBuilders.notFilter( FilterBuilders.termsFilter(RuleNormalizer.RuleField.SUB_CHARACTERISTIC.field(), DebtCharacteristic.NONE)), FilterBuilders.orFilter( - FilterBuilders.termsFilter(RuleNormalizer.RuleField.SUB_CHARACTERISTIC.field(), characteristics), - FilterBuilders.termsFilter(RuleNormalizer.RuleField.CHARACTERISTIC.field(), characteristics)) + FilterBuilders.termsFilter(RuleNormalizer.RuleField.SUB_CHARACTERISTIC.field(), debtCharacteristics), + FilterBuilders.termsFilter(RuleNormalizer.RuleField.CHARACTERISTIC.field(), debtCharacteristics)) ), // Match only when NOT NONE overriden @@ -247,14 +248,15 @@ public class RuleIndex extends BaseIndex { FilterBuilders.termsFilter(RuleNormalizer.RuleField.SUB_CHARACTERISTIC.field(), ""), FilterBuilders.notFilter(FilterBuilders.existsFilter(RuleNormalizer.RuleField.SUB_CHARACTERISTIC.field()))), FilterBuilders.orFilter( - FilterBuilders.termsFilter(RuleNormalizer.RuleField.DEFAULT_SUB_CHARACTERISTIC.field(), characteristics), - FilterBuilders.termsFilter(RuleNormalizer.RuleField.DEFAULT_CHARACTERISTIC.field(), characteristics))) + FilterBuilders.termsFilter(RuleNormalizer.RuleField.DEFAULT_SUB_CHARACTERISTIC.field(), debtCharacteristics), + FilterBuilders.termsFilter(RuleNormalizer.RuleField.DEFAULT_CHARACTERISTIC.field(), debtCharacteristics))) ) ); } // Debt char exist filter - if (Boolean.TRUE.equals(query.getHasDebtCharacteristic())) { + Boolean hasDebtCharacteristic = query.getHasDebtCharacteristic(); + if (hasDebtCharacteristic != null && hasDebtCharacteristic) { fb.must(FilterBuilders.existsFilter(RuleNormalizer.RuleField.SUB_CHARACTERISTIC.field())); } @@ -263,10 +265,10 @@ public class RuleIndex extends BaseIndex { .gte(query.getAvailableSince())); } - Collection statuses = query.getStatuses(); - if (statuses != null && !statuses.isEmpty()) { + Collection statusValues = query.getStatuses(); + if (statusValues != null && !statusValues.isEmpty()) { Collection stringStatus = new ArrayList(); - for (RuleStatus status : statuses) { + for (RuleStatus status : statusValues) { stringStatus.add(status.name()); } this.addTermFilter(fb, RuleNormalizer.RuleField.STATUS.field(), stringStatus); @@ -296,10 +298,10 @@ public class RuleIndex extends BaseIndex { } /** Implementation of activation query */ - if (query.getActivation() == Boolean.TRUE) { + if (query.getActivation().equals(Boolean.TRUE)) { fb.must(FilterBuilders.hasChildFilter(IndexDefinition.ACTIVE_RULE.getIndexType(), childQuery)); - } else if (query.getActivation() == Boolean.FALSE) { + } else if (query.getActivation().equals(Boolean.FALSE)) { fb.mustNot(FilterBuilders.hasChildFilter(IndexDefinition.ACTIVE_RULE.getIndexType(), childQuery)); } @@ -394,7 +396,7 @@ public class RuleIndex extends BaseIndex { } /** - * @deprecated do not use ids but keys + * @deprecated please use getByKey(RuleKey key) */ @Deprecated @CheckForNull @@ -413,7 +415,7 @@ public class RuleIndex extends BaseIndex { } /** - * @deprecated do not use ids but keys + * @deprecated please use getByKey(RuleKey key) */ @Deprecated public List getByIds(Collection ids) { diff --git a/sonar-server/src/main/java/org/sonar/server/search/BaseIndex.java b/sonar-server/src/main/java/org/sonar/server/search/BaseIndex.java index ddd10ef0e85..c6d1ffc9790 100644 --- a/sonar-server/src/main/java/org/sonar/server/search/BaseIndex.java +++ b/sonar-server/src/main/java/org/sonar/server/search/BaseIndex.java @@ -285,9 +285,9 @@ public abstract class BaseIndex, KEY extends Serial } protected boolean needMultiField(IndexField field) { - return ((field.type() == IndexField.Type.TEXT + return (field.type() == IndexField.Type.TEXT || field.type() == IndexField.Type.STRING) - && (field.sortable() || field.searchable())); + && (field.sortable() || field.searchable()); } protected Map mapSortField(IndexField field) { @@ -386,7 +386,7 @@ public abstract class BaseIndex, KEY extends Serial return null; } - protected void updateDocument(Collection requests, KEY key) throws Exception { + protected void updateDocument(Collection requests, KEY key) { LOG.debug("UPDATE _id:{} in index {}", key, this.getIndexName()); BulkRequestBuilder bulkRequest = getClient().prepareBulk(); for (UpdateRequest request : requests) { @@ -481,8 +481,8 @@ public abstract class BaseIndex, KEY extends Serial this.deleteDocument(additionalKey); } } catch (Exception e) { - LOG.error("Could not DELETE _id = '{}' for index '{}': {}", - this.getKeyValue(key), this.getIndexName(), e.getMessage()); + throw new IllegalStateException("Could not DELETE _id = '" + this.getKeyValue(key) + "' " + + "for index '" + this.getIndexName() + "': " + e.getMessage()); } } @@ -494,8 +494,8 @@ public abstract class BaseIndex, KEY extends Serial this.deleteDocument(additionalItem.getKey()); } } catch (Exception e) { - LOG.error("Could not DELETE _id:{} for index {}: {}", - this.getKeyValue(item.getKey()), this.getIndexName(), e.getMessage()); + throw new IllegalStateException("Could not DELETE _id = '" + this.getKeyValue(item.getKey()) + "' " + + "for index '" + this.getIndexName() + "': " + e.getMessage()); } } @@ -588,7 +588,7 @@ public abstract class BaseIndex, KEY extends Serial stats.put(aggregation.getName(), facetValue); } } else if (aggregation.getClass().isAssignableFrom(InternalValueCount.class)) { - InternalValueCount count = ((InternalValueCount) aggregation); + InternalValueCount count = (InternalValueCount) aggregation; FacetValue facetValue = new FacetValue(count.getName(), (int) count.getValue()); stats.put(count.getName(), facetValue); } -- 2.39.5