diff --git a/server/src/main/java/io/druid/server/coordinator/rules/ForeverLoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/ForeverLoadRule.java index 018f7cb48ec..87b3b8dccde 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/ForeverLoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/ForeverLoadRule.java @@ -19,6 +19,8 @@ package io.druid.server.coordinator.rules; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.collect.ImmutableMap; +import io.druid.client.DruidServer; import io.druid.timeline.DataSegment; import org.joda.time.DateTime; import org.joda.time.Interval; @@ -36,8 +38,8 @@ public class ForeverLoadRule extends LoadRule @JsonProperty("tieredReplicants") Map tieredReplicants ) { - validateTieredReplicants(tieredReplicants); - this.tieredReplicants = tieredReplicants; + this.tieredReplicants = tieredReplicants == null ? ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS) : tieredReplicants; + validateTieredReplicants(this.tieredReplicants); } @Override diff --git a/server/src/main/java/io/druid/server/coordinator/rules/IntervalLoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/IntervalLoadRule.java index f423fd2bbcb..b21ea130cbc 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/IntervalLoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/IntervalLoadRule.java @@ -19,7 +19,9 @@ package io.druid.server.coordinator.rules; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.collect.ImmutableMap; import com.metamx.common.logger.Logger; +import io.druid.client.DruidServer; import io.druid.timeline.DataSegment; import org.joda.time.DateTime; import org.joda.time.Interval; @@ -41,9 +43,9 @@ public class IntervalLoadRule extends LoadRule @JsonProperty("tieredReplicants") Map tieredReplicants ) { - validateTieredReplicants(tieredReplicants); + this.tieredReplicants = tieredReplicants == null ? ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS) : tieredReplicants; + validateTieredReplicants(this.tieredReplicants); this.interval = interval; - this.tieredReplicants = tieredReplicants; } @Override diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index 8d06f1ef80a..57a59e95943 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -242,7 +242,11 @@ public abstract class LoadRule implements Rule } protected void validateTieredReplicants(Map tieredReplicants){ + if(tieredReplicants.size() == 0) + throw new IAE("A rule with empty tiered replicants is invalid"); for (Map.Entry entry: tieredReplicants.entrySet()) { + if (entry.getValue() == null) + throw new IAE("Replicant value cannot be empty"); if (entry.getValue() < 0) throw new IAE("Replicant value [%d] is less than 0, which is not allowed", entry.getValue()); } diff --git a/server/src/main/java/io/druid/server/coordinator/rules/PeriodLoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/PeriodLoadRule.java index 6e6fdef52af..27b15a78141 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/PeriodLoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/PeriodLoadRule.java @@ -19,7 +19,9 @@ package io.druid.server.coordinator.rules; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.collect.ImmutableMap; import com.metamx.common.logger.Logger; +import io.druid.client.DruidServer; import io.druid.timeline.DataSegment; import org.joda.time.DateTime; import org.joda.time.Interval; @@ -42,9 +44,9 @@ public class PeriodLoadRule extends LoadRule @JsonProperty("tieredReplicants") Map tieredReplicants ) { - validateTieredReplicants(tieredReplicants); + this.tieredReplicants = tieredReplicants == null ? ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS) : tieredReplicants; + validateTieredReplicants(this.tieredReplicants); this.period = period; - this.tieredReplicants = tieredReplicants; } @Override diff --git a/server/src/test/java/io/druid/server/coordinator/rules/ForeverLoadRuleTest.java b/server/src/test/java/io/druid/server/coordinator/rules/ForeverLoadRuleTest.java new file mode 100644 index 00000000000..33ca4fc3235 --- /dev/null +++ b/server/src/test/java/io/druid/server/coordinator/rules/ForeverLoadRuleTest.java @@ -0,0 +1,90 @@ +/* +* Licensed to Metamarkets Group Inc. (Metamarkets) under one +* or more contributor license agreements. See the NOTICE file +* distributed with this work for additional information +* regarding copyright ownership. Metamarkets licenses this file +* to you under the Apache License, Version 2.0 (the +* "License"); you may not use this file except in compliance +* with the License. You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, +* software distributed under the License is distributed on an +* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +* KIND, either express or implied. See the License for the +* specific language governing permissions and limitations +* under the License. +*/ + +package io.druid.server.coordinator.rules; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.collect.ImmutableMap; +import com.metamx.common.IAE; +import io.druid.client.DruidServer; +import io.druid.jackson.DefaultObjectMapper; +import org.junit.Assert; +import org.junit.Test; + +import java.util.HashMap; +import java.util.Map; + +public class ForeverLoadRuleTest +{ + @Test + public void testSerdeNullTieredReplicants() throws Exception + { + ForeverLoadRule rule = new ForeverLoadRule( + null + ); + + ObjectMapper jsonMapper = new DefaultObjectMapper(); + Rule reread = jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); + + Assert.assertEquals(rule.getTieredReplicants(), ((ForeverLoadRule)reread).getTieredReplicants()); + Assert.assertEquals(ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), rule.getTieredReplicants()); + } + + @Test + public void testMappingNullTieredReplicants() throws Exception{ + String inputJson = "{\n" + + " \"type\": \"loadForever\"\n" + + "}"; + String expectedJson = " {\n" + + " \"tieredReplicants\": {\n" + + " \""+ DruidServer.DEFAULT_TIER +"\": "+ DruidServer.DEFAULT_NUM_REPLICANTS +"\n" + + " },\n" + + " \"type\": \"loadForever\"\n" + + " }"; + ObjectMapper jsonMapper = new DefaultObjectMapper(); + ForeverLoadRule inputForeverLoadRule = jsonMapper.readValue(inputJson, ForeverLoadRule.class); + ForeverLoadRule expectedForeverLoadRule = jsonMapper.readValue(expectedJson, ForeverLoadRule.class); + Assert.assertEquals(expectedForeverLoadRule.getTieredReplicants(), inputForeverLoadRule.getTieredReplicants()); + } + + @Test(expected = IAE.class) + public void testEmptyTieredReplicants() throws Exception + { + ForeverLoadRule rule = new ForeverLoadRule( + ImmutableMap.of() + ); + + ObjectMapper jsonMapper = new DefaultObjectMapper(); + Rule reread = jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); + } + + @Test(expected = IAE.class) + public void testEmptyReplicantValue() throws Exception + { + // Immutable map does not allow null values + Map tieredReplicants= new HashMap<>(); + tieredReplicants.put("tier", null); + ForeverLoadRule rule = new ForeverLoadRule( + tieredReplicants + ); + + ObjectMapper jsonMapper = new DefaultObjectMapper(); + Rule reread = jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); + } +} diff --git a/server/src/test/java/io/druid/server/coordinator/rules/IntervalLoadRuleTest.java b/server/src/test/java/io/druid/server/coordinator/rules/IntervalLoadRuleTest.java index 4509eaa0c3f..9bc5e355a33 100644 --- a/server/src/test/java/io/druid/server/coordinator/rules/IntervalLoadRuleTest.java +++ b/server/src/test/java/io/druid/server/coordinator/rules/IntervalLoadRuleTest.java @@ -21,8 +21,8 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableMap; import io.druid.client.DruidServer; import io.druid.jackson.DefaultObjectMapper; -import junit.framework.Assert; import org.joda.time.Interval; +import org.junit.Assert; import org.junit.Test; /** @@ -42,4 +42,37 @@ import org.junit.Test; Assert.assertEquals(rule, reread); } + + @Test + public void testSerdeNullTieredReplicants() throws Exception + { + IntervalLoadRule rule = new IntervalLoadRule( + new Interval("0/3000"), null + ); + + ObjectMapper jsonMapper = new DefaultObjectMapper(); + Rule reread = jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); + + Assert.assertEquals(rule, reread); + Assert.assertEquals(ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), rule.getTieredReplicants()); + } + + @Test + public void testMappingNullTieredReplicants() throws Exception{ + String inputJson = " {\n" + + " \"interval\": \"0000-01-01T00:00:00.000-05:50:36/3000-01-01T00:00:00.000-06:00\",\n" + + " \"type\": \"loadByInterval\"\n" + + " }"; + String expectedJson = "{\n" + + " \"interval\": \"0000-01-01T00:00:00.000-05:50:36/3000-01-01T00:00:00.000-06:00\",\n" + + " \"tieredReplicants\": {\n" + + " \""+ DruidServer.DEFAULT_TIER +"\": "+ DruidServer.DEFAULT_NUM_REPLICANTS +"\n" + + " },\n" + + " \"type\": \"loadByInterval\"\n" + + " }"; + ObjectMapper jsonMapper = new DefaultObjectMapper(); + IntervalLoadRule inputIntervalLoadRule = jsonMapper.readValue(inputJson, IntervalLoadRule.class); + IntervalLoadRule expectedIntervalLoadRule = jsonMapper.readValue(expectedJson, IntervalLoadRule.class); + Assert.assertEquals(expectedIntervalLoadRule, inputIntervalLoadRule); + } } diff --git a/server/src/test/java/io/druid/server/coordinator/rules/PeriodLoadRuleTest.java b/server/src/test/java/io/druid/server/coordinator/rules/PeriodLoadRuleTest.java index f8da3febdcf..7e1d4d2975f 100644 --- a/server/src/test/java/io/druid/server/coordinator/rules/PeriodLoadRuleTest.java +++ b/server/src/test/java/io/druid/server/coordinator/rules/PeriodLoadRuleTest.java @@ -17,7 +17,10 @@ package io.druid.server.coordinator.rules; +import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableMap; +import io.druid.client.DruidServer; +import io.druid.jackson.DefaultObjectMapper; import io.druid.timeline.DataSegment; import io.druid.timeline.partition.NoneShardSpec; import org.joda.time.DateTime; @@ -74,4 +77,39 @@ public class PeriodLoadRuleTest ) ); } + + @Test + public void testSerdeNullTieredReplicants() throws Exception + { + PeriodLoadRule rule = new PeriodLoadRule( + new Period("P1D"), null + ); + + ObjectMapper jsonMapper = new DefaultObjectMapper(); + Rule reread = jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); + + Assert.assertEquals(rule.getPeriod(), ((PeriodLoadRule)reread).getPeriod()); + Assert.assertEquals(rule.getTieredReplicants(), ((PeriodLoadRule)reread).getTieredReplicants()); + Assert.assertEquals(ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), rule.getTieredReplicants()); + } + + @Test + public void testMappingNullTieredReplicants() throws Exception{ + String inputJson = "{\n" + + " \"period\": \"P1D\",\n" + + " \"type\": \"loadByPeriod\"\n" + + " }"; + String expectedJson = "{\n" + + " \"period\": \"P1D\",\n" + + " \"tieredReplicants\": {\n" + + " \""+ DruidServer.DEFAULT_TIER +"\": "+ DruidServer.DEFAULT_NUM_REPLICANTS +"\n" + + " },\n" + + " \"type\": \"loadByPeriod\"\n" + + " }"; + ObjectMapper jsonMapper = new DefaultObjectMapper(); + PeriodLoadRule inputPeriodLoadRule = jsonMapper.readValue(inputJson, PeriodLoadRule.class); + PeriodLoadRule expectedPeriodLoadRule = jsonMapper.readValue(expectedJson, PeriodLoadRule.class); + Assert.assertEquals(expectedPeriodLoadRule.getTieredReplicants(), inputPeriodLoadRule.getTieredReplicants()); + Assert.assertEquals(expectedPeriodLoadRule.getPeriod(), inputPeriodLoadRule.getPeriod()); + } }