Details
diff --git a/catalog/src/main/java/org/killbill/billing/catalog/DefaultDuration.java b/catalog/src/main/java/org/killbill/billing/catalog/DefaultDuration.java
index cc1e343..726ea77 100644
--- a/catalog/src/main/java/org/killbill/billing/catalog/DefaultDuration.java
+++ b/catalog/src/main/java/org/killbill/billing/catalog/DefaultDuration.java
@@ -102,13 +102,14 @@ public class DefaultDuration extends ValidatingConfig<StandaloneCatalog> impleme
@Override
public ValidationErrors validate(final StandaloneCatalog catalog, final ValidationErrors errors) {
- //Validation: TimeUnit UNLIMITED iff number == -1
- if ((unit == TimeUnit.UNLIMITED && number != -1)) {
- errors.add(new ValidationError("Duration can only have 'UNLIMITED' unit if the number is omitted.",
- catalog.getCatalogURI(), DefaultPlanPhase.class, ""));
+ //Validation: TimeUnit UNLIMITED if number == -1
+ if ((unit == TimeUnit.UNLIMITED && number != DEFAULT_DURATION_NUMBER)) {
+ errors.add(new ValidationError("Duration can only have 'UNLIMITED' unit if the number is omitted",
+ catalog.getCatalogURI(), DefaultDuration.class, ""));
+ } else if ((unit != TimeUnit.UNLIMITED) && number == DEFAULT_DURATION_NUMBER) {
+ errors.add(new ValidationError("Finite Duration must have a well defined length",
+ catalog.getCatalogURI(), DefaultDuration.class, ""));
}
-
- //TODO MDW - Validation TimeUnit UNLIMITED iff number == -1
return errors;
}
diff --git a/catalog/src/main/java/org/killbill/billing/catalog/DefaultPlan.java b/catalog/src/main/java/org/killbill/billing/catalog/DefaultPlan.java
index e8a3a99..25fa3c4 100644
--- a/catalog/src/main/java/org/killbill/billing/catalog/DefaultPlan.java
+++ b/catalog/src/main/java/org/killbill/billing/catalog/DefaultPlan.java
@@ -187,24 +187,36 @@ public class DefaultPlan extends ValidatingConfig<StandaloneCatalog> implements
this.priceListName = this.priceListName != null ? this.priceListName : findPriceListForPlan(catalog);
}
-
@Override
public ValidationErrors validate(final StandaloneCatalog catalog, final ValidationErrors errors) {
if (effectiveDateForExistingSubscriptions != null &&
catalog.getEffectiveDate().getTime() > effectiveDateForExistingSubscriptions.getTime()) {
errors.add(new ValidationError(String.format("Price effective date %s is before catalog effective date '%s'",
effectiveDateForExistingSubscriptions,
- catalog.getEffectiveDate().getTime()),
- catalog.getCatalogURI(), DefaultInternationalPrice.class, ""));
+ catalog.getEffectiveDate()),
+ catalog.getCatalogURI(), DefaultPlan.class, ""));
}
- validateCollection(catalog, errors, initialPhases);
- finalPhase.validate(catalog, errors);
-
if (product == null) {
- errors.add(new ValidationError(String.format("Invalid product for plan '%s'", name), catalog.getCatalogURI(), DefaultProduct.class, ""));
+ errors.add(new ValidationError(String.format("Invalid product for plan '%s'", name), catalog.getCatalogURI(), DefaultPlan.class, ""));
}
+ for (DefaultPlanPhase cur : initialPhases) {
+ cur.validate(catalog, errors);
+ if (cur.getPhaseType() == PhaseType.EVERGREEN || cur.getPhaseType() == PhaseType.FIXEDTERM) {
+ errors.add(new ValidationError(String.format("Initial Phase %s of plan %s cannot be of type %s",
+ cur.getName(), name, cur.getPhaseType()),
+ catalog.getCatalogURI(), DefaultPlan.class, ""));
+ }
+ }
+
+ finalPhase.validate(catalog, errors);
+
+ if (finalPhase.getPhaseType() == PhaseType.TRIAL || finalPhase.getPhaseType() == PhaseType.DISCOUNT) {
+ errors.add(new ValidationError(String.format("Final Phase %s of plan %s cannot be of type %s",
+ finalPhase.getName(), name, finalPhase.getPhaseType()),
+ catalog.getCatalogURI(), DefaultPlan.class, ""));
+ }
return errors;
}
diff --git a/catalog/src/main/java/org/killbill/billing/catalog/DefaultPlanPhase.java b/catalog/src/main/java/org/killbill/billing/catalog/DefaultPlanPhase.java
index 37134ec..d49ba3e 100644
--- a/catalog/src/main/java/org/killbill/billing/catalog/DefaultPlanPhase.java
+++ b/catalog/src/main/java/org/killbill/billing/catalog/DefaultPlanPhase.java
@@ -138,6 +138,10 @@ public class DefaultPlanPhase extends ValidatingConfig<StandaloneCatalog> implem
@Override
public ValidationErrors validate(final StandaloneCatalog catalog, final ValidationErrors errors) {
+ if (plan == null) {
+ errors.add(new ValidationError(String.format("Invalid plan for phase '%s'", type), catalog.getCatalogURI(), DefaultPlanPhase.class, ""));
+ }
+
if (fixed == null && recurring == null && usages.length == 0) {
errors.add(new ValidationError(String.format("Phase %s of plan %s need to define at least either a fixed or recurrring or usage section.",
type.toString(), plan.getName()),
@@ -149,6 +153,8 @@ public class DefaultPlanPhase extends ValidatingConfig<StandaloneCatalog> implem
if (recurring != null) {
recurring.validate(catalog, errors);
}
+ duration.validate(catalog, errors);
+
validateCollection(catalog, errors, usages);
return errors;
}
diff --git a/catalog/src/main/java/org/killbill/billing/catalog/DefaultProduct.java b/catalog/src/main/java/org/killbill/billing/catalog/DefaultProduct.java
index bb5ac34..c8d4999 100644
--- a/catalog/src/main/java/org/killbill/billing/catalog/DefaultProduct.java
+++ b/catalog/src/main/java/org/killbill/billing/catalog/DefaultProduct.java
@@ -32,6 +32,7 @@ import org.killbill.billing.catalog.api.Limit;
import org.killbill.billing.catalog.api.Product;
import org.killbill.billing.catalog.api.ProductCategory;
import org.killbill.xmlloader.ValidatingConfig;
+import org.killbill.xmlloader.ValidationError;
import org.killbill.xmlloader.ValidationErrors;
@XmlAccessorType(XmlAccessType.NONE)
@@ -160,6 +161,10 @@ public class DefaultProduct extends ValidatingConfig<StandaloneCatalog> implemen
@Override
public ValidationErrors validate(final StandaloneCatalog catalog, final ValidationErrors errors) {
+ if (catalogName == null || !catalogName.equals(catalog.getCatalogName())) {
+ errors.add(new ValidationError(String.format("Invalid catalogName for product '%s'", name), catalog.getCatalogURI(), DefaultProduct.class, ""));
+
+ }
//TODO: MDW validation: inclusion and exclusion lists can only contain addon products
//TODO: MDW validation: a given product can only be in, at most, one of inclusion and exclusion lists
return errors;
diff --git a/catalog/src/main/java/org/killbill/billing/catalog/DefaultRecurring.java b/catalog/src/main/java/org/killbill/billing/catalog/DefaultRecurring.java
index f8599bf..14f956a 100644
--- a/catalog/src/main/java/org/killbill/billing/catalog/DefaultRecurring.java
+++ b/catalog/src/main/java/org/killbill/billing/catalog/DefaultRecurring.java
@@ -74,6 +74,16 @@ public class DefaultRecurring extends ValidatingConfig<StandaloneCatalog> implem
@Override
public ValidationErrors validate(final StandaloneCatalog catalog, final ValidationErrors errors) {
// Validation: check for nulls
+
+ if (plan == null) {
+ errors.add(new ValidationError(String.format("Invalid plan for recurring section"), catalog.getCatalogURI(), DefaultRecurring.class, ""));
+ }
+
+ if (phase == null) {
+ errors.add(new ValidationError(String.format("Invalid phase for recurring section"), catalog.getCatalogURI(), DefaultPlan.class, plan.getName().toString()));
+ }
+
+
if (billingPeriod == null) {
errors.add(new ValidationError(String.format("Recurring section of Phase %s of plan %s has a recurring price but no billing period", phase.getPhaseType().toString(), plan.getName()),
catalog.getCatalogURI(), DefaultPlanPhase.class, phase.getPhaseType().toString()));
diff --git a/catalog/src/main/java/org/killbill/billing/catalog/VersionedCatalog.java b/catalog/src/main/java/org/killbill/billing/catalog/VersionedCatalog.java
index 167bc55..aa53bda 100644
--- a/catalog/src/main/java/org/killbill/billing/catalog/VersionedCatalog.java
+++ b/catalog/src/main/java/org/killbill/billing/catalog/VersionedCatalog.java
@@ -60,6 +60,7 @@ import org.killbill.billing.catalog.api.StaticCatalog;
import org.killbill.billing.catalog.api.Unit;
import org.killbill.clock.Clock;
import org.killbill.xmlloader.ValidatingConfig;
+import org.killbill.xmlloader.ValidationError;
import org.killbill.xmlloader.ValidationErrors;
@XmlRootElement(name = "catalogs")
@@ -415,15 +416,22 @@ public class VersionedCatalog extends ValidatingConfig<VersionedCatalog> impleme
@Override
public ValidationErrors validate(final VersionedCatalog catalog, final ValidationErrors errors) {
+
+ final Set<Date> effectiveDates = new TreeSet<Date>();
+
for (final StandaloneCatalog c : versions) {
+ if (effectiveDates.contains(c.getEffectiveDate())) {
+ errors.add(new ValidationError(String.format("Catalog effective date '%s' already exists for a previous version", c.getEffectiveDate()),
+ c.getCatalogURI(), VersionedCatalog.class, ""));
+ } else {
+ effectiveDates.add(c.getEffectiveDate());
+ }
+ if (!c.getCatalogName().equals(catalogName)) {
+ errors.add(new ValidationError(String.format("Catalog name '%s' is not consistent across versions ", c.getCatalogName()),
+ c.getCatalogURI(), VersionedCatalog.class, ""));
+ }
errors.addAll(c.validate(c, errors));
}
- //TODO MDW validation - ensure all catalog versions have a single name
- //TODO MDW validation - ensure effective dates are different (actually do we want this?)
- //TODO MDW validation - check that all products are there
- //TODO MDW validation - check that all plans are there
- //TODO MDW validation - check that all currencies are there
- //TODO MDW validation - check that all pricelists are there
return errors;
}
diff --git a/catalog/src/test/java/org/killbill/billing/catalog/TestInternationalPrice.java b/catalog/src/test/java/org/killbill/billing/catalog/TestInternationalPrice.java
index 12cc090..103777a 100644
--- a/catalog/src/test/java/org/killbill/billing/catalog/TestInternationalPrice.java
+++ b/catalog/src/test/java/org/killbill/billing/catalog/TestInternationalPrice.java
@@ -34,7 +34,7 @@ public class TestInternationalPrice extends CatalogTestSuiteNoDB {
final StandaloneCatalog c = new MockCatalog();
c.setSupportedCurrencies(new Currency[]{Currency.GBP, Currency.EUR, Currency.USD, Currency.BRL, Currency.MXN});
final DefaultInternationalPrice p0 = new MockInternationalPrice();
- p0.setPrices(null);
+ p0.setPrices(new DefaultPrice[0]);
p0.initialize(c, new URI("foo:bar"));
final DefaultInternationalPrice p1 = new MockInternationalPrice();
p1.setPrices(new DefaultPrice[]{
@@ -63,7 +63,8 @@ public class TestInternationalPrice extends CatalogTestSuiteNoDB {
public void testPriceInitialization() throws URISyntaxException, CatalogApiException {
final StandaloneCatalog c = new MockCatalog();
c.setSupportedCurrencies(new Currency[]{Currency.GBP, Currency.EUR, Currency.USD, Currency.BRL, Currency.MXN});
- ((DefaultInternationalPrice) c.getCurrentPlans().iterator().next().getFinalPhase().getRecurring().getRecurringPrice()).setPrices(null);
+ ((DefaultInternationalPrice) c.getCurrentPlans().iterator().next().getFinalPhase().getRecurring().getRecurringPrice()).setPrices(new DefaultPrice[0]);
+ c.setUnits(new DefaultUnit[0]);
c.initialize(c, new URI("foo://bar"));
Assert.assertEquals(c.getCurrentPlans().iterator().next().getFinalPhase().getRecurring().getRecurringPrice().getPrice(Currency.GBP), new BigDecimal(0));
}
diff --git a/catalog/src/test/java/org/killbill/billing/catalog/TestPlan.java b/catalog/src/test/java/org/killbill/billing/catalog/TestPlan.java
index 86f5033..2f7cdaf 100644
--- a/catalog/src/test/java/org/killbill/billing/catalog/TestPlan.java
+++ b/catalog/src/test/java/org/killbill/billing/catalog/TestPlan.java
@@ -34,7 +34,7 @@ public class TestPlan extends CatalogTestSuiteNoDB {
final DefaultPlan p1 = MockPlan.createBicycleTrialEvergreen1USD();
p1.setEffectiveDateForExistingSubscriptions(new Date((new Date().getTime()) - (1000 * 60 * 60 * 24)));
final ValidationErrors errors = p1.validate(c, new ValidationErrors());
- Assert.assertEquals(errors.size(), 1);
+ Assert.assertEquals(errors.size(), 3);
errors.log(log);
}
diff --git a/catalog/src/test/java/org/killbill/billing/catalog/TestStandaloneCatalog.java b/catalog/src/test/java/org/killbill/billing/catalog/TestStandaloneCatalog.java
index 4a034f5..32d839a 100644
--- a/catalog/src/test/java/org/killbill/billing/catalog/TestStandaloneCatalog.java
+++ b/catalog/src/test/java/org/killbill/billing/catalog/TestStandaloneCatalog.java
@@ -32,11 +32,15 @@ public class TestStandaloneCatalog extends CatalogTestSuiteNoDB {
@Test(groups = "fast")
public void testLoadCatalogWithPlanInvalidProduct() throws Exception {
try {
- XMLLoader.getObjectFromString(Resources.getResource("CatalogWithPlanInvalidProduct.xml").toExternalForm(), StandaloneCatalog.class);
+ XMLLoader.getObjectFromString(Resources.getResource("CatalogWithValidationErrors.xml").toExternalForm(), StandaloneCatalog.class);
Assert.fail();
} catch (final ValidationException e) {
- Assert.assertEquals(e.getErrors().size(), 1);
+ Assert.assertEquals(e.getErrors().size(), 5);
Assert.assertEquals(e.getErrors().get(0).getDescription(), "Invalid product for plan 'standard'");
+ Assert.assertEquals(e.getErrors().get(1).getDescription(), "Duration can only have 'UNLIMITED' unit if the number is omitted");
+ Assert.assertEquals(e.getErrors().get(2).getDescription(), "Finite Duration must have a well defined length");
+ Assert.assertEquals(e.getErrors().get(3).getDescription(), "Initial Phase standard-trial-evergreen of plan standard-trial cannot be of type EVERGREEN");
+ Assert.assertEquals(e.getErrors().get(4).getDescription(), "Final Phase standard-trial-trial of plan standard-trial cannot be of type TRIAL");
}
}
diff --git a/jaxrs/src/test/java/org/killbill/billing/jaxrs/json/TestPlanDetailJson.java b/jaxrs/src/test/java/org/killbill/billing/jaxrs/json/TestPlanDetailJson.java
index f3e0934..243d97f 100644
--- a/jaxrs/src/test/java/org/killbill/billing/jaxrs/json/TestPlanDetailJson.java
+++ b/jaxrs/src/test/java/org/killbill/billing/jaxrs/json/TestPlanDetailJson.java
@@ -18,6 +18,7 @@ package org.killbill.billing.jaxrs.json;
import java.util.UUID;
+import org.killbill.billing.catalog.DefaultPrice;
import org.killbill.billing.catalog.api.BillingPeriod;
import org.killbill.billing.catalog.api.InternationalPrice;
import org.killbill.billing.catalog.api.Listing;
@@ -27,6 +28,7 @@ import org.killbill.billing.catalog.api.PriceList;
import org.killbill.billing.catalog.api.Product;
import org.killbill.billing.catalog.api.Recurring;
import org.killbill.billing.jaxrs.JaxrsTestSuiteNoDB;
+import org.mockito.Mock;
import org.mockito.Mockito;
import org.testng.Assert;
import org.testng.annotations.Test;
@@ -63,6 +65,7 @@ public class TestPlanDetailJson extends JaxrsTestSuiteNoDB {
Mockito.when(product.getName()).thenReturn(UUID.randomUUID().toString());
final InternationalPrice price = Mockito.mock(InternationalPrice.class);
+ Mockito.when(price.getPrices()).thenReturn(new DefaultPrice[0]);
final PlanPhase planPhase = Mockito.mock(PlanPhase.class);
final Recurring recurring = Mockito.mock(Recurring.class);
Mockito.when(recurring.getRecurringPrice()).thenReturn(price);