Skip to content

Commit

Permalink
Merge pull request searchbox-io#391 from SethAbrams/fix-aggregation-l…
Browse files Browse the repository at this point in the history
…ogic-inconsistency

Fix aggregation logic inconsistency
  • Loading branch information
ferhatsb authored May 1, 2018
2 parents f6a3ee4 + 6e4239d commit 1eefdd8
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ public class CardinalityAggregation extends MetricAggregation {

public CardinalityAggregation(String name, JsonObject cardinalityAggregation) {
super(name, cardinalityAggregation);
cardinality = cardinalityAggregation.get(String.valueOf(VALUE)).getAsLong();
if(cardinalityAggregation.has(String.valueOf(VALUE)) && !cardinalityAggregation.get(String.valueOf(VALUE)).isJsonNull()) {
cardinality = cardinalityAggregation.get(String.valueOf(VALUE)).getAsLong();
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package io.searchbox.search.aggregation;

import com.google.gson.JsonObject;
import io.searchbox.core.search.aggregation.CardinalityAggregation;
import org.junit.Test;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;

public class CardinalityAggregationTest {
@Test
public void givenMissingValueProperty_cardinalityAggregationConstructor_doesNotAssignToCardinalityField() {
JsonObject cardinalityAggregationJson = new JsonObject();
cardinalityAggregationJson.addProperty("notValueField", "hi");
cardinalityAggregationJson.addProperty("anotherField", "hello");
CardinalityAggregation cardinalityAggregation = new CardinalityAggregation("aggName", cardinalityAggregationJson);
assertNull("Cardinality field should default to null since value is unassigned", cardinalityAggregation.getCardinality());
}

@Test
public void givenNullValueProperty_cardinalityAggregationConstructor_doesNotAssignToCardinalityField() {
JsonObject cardinalityAggregationJson = new JsonObject();
cardinalityAggregationJson.addProperty("notValueField", "hi");
cardinalityAggregationJson.addProperty("anotherField", "hello");
String nullString = null;
cardinalityAggregationJson.addProperty("value", nullString);
CardinalityAggregation cardinalityAggregation = new CardinalityAggregation("aggName", cardinalityAggregationJson);
assertNull("Cardinality field should default to null since value is null", cardinalityAggregation.getCardinality());
}

@Test
public void givenNonNullValueProperty_cardinalityAggregationConstructor_assignsToCardinalityField() {
JsonObject cardinalityAggregationJson = new JsonObject();
cardinalityAggregationJson.addProperty("notValueField", "hi");
cardinalityAggregationJson.addProperty("anotherField", "hello");
Long setValue = 100L;
cardinalityAggregationJson.addProperty("value", setValue);
CardinalityAggregation cardinalityAggregation = new CardinalityAggregation("aggName", cardinalityAggregationJson);
assertEquals(setValue, cardinalityAggregation.getCardinality());
}
}

0 comments on commit 1eefdd8

Please sign in to comment.