killbill-uncached

Code review for entitlement

8/21/2013 8:34:05 PM

Details

diff --git a/entitlement/src/main/java/com/ning/billing/entitlement/api/DefaultEntitlementApi.java b/entitlement/src/main/java/com/ning/billing/entitlement/api/DefaultEntitlementApi.java
index ba0121c..c89ef98 100644
--- a/entitlement/src/main/java/com/ning/billing/entitlement/api/DefaultEntitlementApi.java
+++ b/entitlement/src/main/java/com/ning/billing/entitlement/api/DefaultEntitlementApi.java
@@ -123,7 +123,6 @@ public class DefaultEntitlementApi implements EntitlementApi {
 
 
             final InternalCallContext contextWithValidAccountRecordId = internalCallContextFactory.createInternalCallContext(bundle.getAccountId(), callContext);
-            final BlockingState currentBaseState = blockingStateDao.getBlockingStateForService(baseSubscription.getId(), EntitlementService.ENTITLEMENT_SERVICE_NAME, contextWithValidAccountRecordId);
 
             final Account account = accountApi.getAccountById(bundle.getAccountId(), context);
             final LocalDate baseEntitlementEffectiveEndDate = getEffectiveEndDate(bundle.getAccountId(), baseSubscription, account.getTimeZone(), contextWithValidAccountRecordId);
@@ -144,7 +143,7 @@ public class DefaultEntitlementApi implements EntitlementApi {
             final DateTime requestedDate = dateHelper.fromLocalDateAndReferenceTime(effectiveDate, baseSubscription.getStartDate(), contextWithValidAccountRecordId);
             final SubscriptionBase subscription = subscriptionInternalApi.createSubscription(baseSubscription.getBundleId(), planPhaseSpecifier, requestedDate, context);
 
-            return new DefaultEntitlement(dateHelper, subscription, bundle.getAccountId(), bundle.getExternalKey(), baseEntitlementState, null, account.getTimeZone(),
+            return new DefaultEntitlement(dateHelper, subscription, bundle.getAccountId(), bundle.getExternalKey(), EntitlementState.ACTIVE, null, account.getTimeZone(),
                                           this, internalCallContextFactory, blockingStateDao, clock, checker);
         } catch (SubscriptionBaseApiException e) {
             throw new EntitlementApiException(e);
@@ -179,8 +178,6 @@ public class DefaultEntitlementApi implements EntitlementApi {
             final SubscriptionBaseBundle bundle = subscriptionInternalApi.getBundleFromId(subscription.getBundleId(), context);
 
             final Account account = accountApi.getAccountById(bundle.getAccountId(), context);
-            final BlockingState currentState = blockingStateDao.getBlockingStateForService(subscription.getId(), EntitlementService.ENTITLEMENT_SERVICE_NAME, context);
-
 
             final LocalDate entitlementEffectiveEndDate = getEffectiveEndDate(bundle.getAccountId(), subscription, account.getTimeZone(), context);
             final EntitlementState entitlementState = getStateForEntitlement(entitlementEffectiveEndDate, subscription, account.getTimeZone(), context);
@@ -242,7 +239,14 @@ public class DefaultEntitlementApi implements EntitlementApi {
                 public Entitlement apply(@Nullable final SubscriptionBase input) {
 
                     final LocalDate effectiveEndDate = getEffectiveEndDate(accountId, input, account.getTimeZone(), context);
-                    final EntitlementState entitlementState = getStateForEntitlement(effectiveEndDate, input, account.getTimeZone(), context);
+
+                    EntitlementState entitlementState;
+                    try {
+                        entitlementState = getStateForEntitlement(effectiveEndDate, input, account.getTimeZone(), context);
+                    } catch (EntitlementApiException e) {
+                        log.warn("Failed to extract blocking state for subscription " + input.getId().toString());
+                        entitlementState = EntitlementState.CANCELLED;
+                    }
 
                     return new DefaultEntitlement(dateHelper, input, accountId, externalKey,
                                                   entitlementState,
@@ -263,25 +267,25 @@ public class DefaultEntitlementApi implements EntitlementApi {
         LocalDate result = null;
 
         final BlockingState subEntitlementState = blockingStateDao.getBlockingStateForService(subscriptionBase.getId(), EntitlementService.ENTITLEMENT_SERVICE_NAME, context);
-        if (subEntitlementState != null && subEntitlementState.getStateName().equals(ENT_STATE_CANCELLED)) {
+        if (subEntitlementState != null && ENT_STATE_CANCELLED.equals(subEntitlementState.getStateName())) {
             result = new LocalDate(subEntitlementState.getEffectiveDate(), accountTimeZone);
         }
 
         final BlockingState bundleEntitlementState = blockingStateDao.getBlockingStateForService(subscriptionBase.getBundleId(), EntitlementService.ENTITLEMENT_SERVICE_NAME, context);
-        if (bundleEntitlementState != null && bundleEntitlementState .getStateName().equals(ENT_STATE_CANCELLED)) {
+        if (bundleEntitlementState != null && ENT_STATE_CANCELLED.equals(bundleEntitlementState.getStateName())) {
             final LocalDate localDate = new LocalDate(bundleEntitlementState.getEffectiveDate(), accountTimeZone);
-            result = result == null || result.compareTo(localDate) < 0 ? localDate : result;
+            result = (result == null) || (result.compareTo(localDate) < 0) ? result : localDate;
         }
 
         final BlockingState accountEntitlementState = blockingStateDao.getBlockingStateForService(accountId, EntitlementService.ENTITLEMENT_SERVICE_NAME, context);
-        if (accountEntitlementState != null && accountEntitlementState .getStateName().equals(ENT_STATE_CANCELLED)) {
+        if (accountEntitlementState != null && ENT_STATE_CANCELLED.equals(accountEntitlementState.getStateName())) {
             final LocalDate localDate = new LocalDate(accountEntitlementState.getEffectiveDate(), accountTimeZone);
-            result = result == null || result.compareTo(localDate) < 0 ? localDate : result;
+            result = (result == null) || (result.compareTo(localDate) < 0) ? result : localDate;
         }
         return result;
     }
 
-    private EntitlementState getStateForEntitlement(final LocalDate entitlementEndDate, final SubscriptionBase subscriptionBase, final DateTimeZone accountTimeZone, final InternalTenantContext context) {
+    private EntitlementState getStateForEntitlement(final LocalDate entitlementEndDate, final SubscriptionBase subscriptionBase, final DateTimeZone accountTimeZone, final InternalTenantContext context) throws EntitlementApiException {
 
         // Current state for the ENTITLEMENT_SERVICE_NAME is set to cancelled
         if (entitlementEndDate != null &&
@@ -294,8 +298,7 @@ public class DefaultEntitlementApi implements EntitlementApi {
             BlockingAggregator blocking = checker.getBlockedStatus(subscriptionBase, context);
             return blocking != null && blocking.isBlockEntitlement() ? EntitlementState.BLOCKED : EntitlementState.ACTIVE;
         } catch (BlockingApiException e) {
-            log.warn("Failed to extract blocking state for subscription " + subscriptionBase.getId().toString());
-            return null;
+            throw new EntitlementApiException(e);
         }
     }
 
diff --git a/entitlement/src/main/java/com/ning/billing/entitlement/api/DefaultSubscriptionApi.java b/entitlement/src/main/java/com/ning/billing/entitlement/api/DefaultSubscriptionApi.java
index 77a174c..2510a62 100644
--- a/entitlement/src/main/java/com/ning/billing/entitlement/api/DefaultSubscriptionApi.java
+++ b/entitlement/src/main/java/com/ning/billing/entitlement/api/DefaultSubscriptionApi.java
@@ -175,7 +175,7 @@ public class DefaultSubscriptionApi implements SubscriptionApi  {
     private SubscriptionBundle getSubscriptionBundleFromEntitlements(final UUID bundleId, final List<Entitlement> entitlements, final TenantContext context) throws SubscriptionBaseApiException, AccountApiException {
         final InternalTenantContext internalTenantContext = internalCallContextFactory.createInternalTenantContext(context);
         final SubscriptionBaseBundle baseBundle = subscriptionInternalApi.getBundleFromId(bundleId, internalTenantContext);
-        final List<Subscription> subscriptions = new ArrayList<Subscription>();
+        final List<Subscription> subscriptions = new ArrayList<Subscription>(entitlements.size());
         subscriptions.addAll(Collections2.transform(entitlements, new Function<Entitlement, Subscription>() {
             @Override
             public Subscription apply(final Entitlement input) {
diff --git a/entitlement/src/main/java/com/ning/billing/entitlement/api/DefaultSubscriptionBundleTimeline.java b/entitlement/src/main/java/com/ning/billing/entitlement/api/DefaultSubscriptionBundleTimeline.java
index d16e1cf..ecfa16f 100644
--- a/entitlement/src/main/java/com/ning/billing/entitlement/api/DefaultSubscriptionBundleTimeline.java
+++ b/entitlement/src/main/java/com/ning/billing/entitlement/api/DefaultSubscriptionBundleTimeline.java
@@ -349,7 +349,7 @@ public class DefaultSubscriptionBundleTimeline implements SubscriptionBundleTime
             case RE_CREATE:
                 return SubscriptionEventType.RESUME_BILLING;
             /*
-             * Those can be ignore:
+             * Those can be ignored:
              */
             // Marker event
             case UNCANCEL:
diff --git a/entitlement/src/main/java/com/ning/billing/entitlement/api/EntitlementDateHelper.java b/entitlement/src/main/java/com/ning/billing/entitlement/api/EntitlementDateHelper.java
index 9d2add8..1a619c4 100644
--- a/entitlement/src/main/java/com/ning/billing/entitlement/api/EntitlementDateHelper.java
+++ b/entitlement/src/main/java/com/ning/billing/entitlement/api/EntitlementDateHelper.java
@@ -74,13 +74,13 @@ public class EntitlementDateHelper {
         return adjustDateTimeToNotBeInFutureIfLocaDateIsToday(t2);
     }
 
-    private final DateTime adjustDateTimeToNotBeInFutureIfLocaDateIsToday(final DateTime input) {
+    private final DateTime adjustDateTimeToNotBeInFutureIfLocaDateIsToday(final DateTime inputUtc) {
         // If the LocalDate is TODAY but after adding the reference time we end up in the future, we correct it to be NOW,
         // so change occurs immediately
-        if (isBeforeOrEqualsToday(input, DateTimeZone.UTC) && input.compareTo(clock.getUTCNow()) > 0) {
+        if (isBeforeOrEqualsToday(inputUtc, DateTimeZone.UTC) && inputUtc.compareTo(clock.getUTCNow()) > 0) {
             return clock.getUTCNow();
         } else {
-            return input;
+            return inputUtc;
         }
     }
 
diff --git a/entitlement/src/main/java/com/ning/billing/entitlement/glue/DefaultEntitlementModule.java b/entitlement/src/main/java/com/ning/billing/entitlement/glue/DefaultEntitlementModule.java
index 8d96e35..0b27e0d 100644
--- a/entitlement/src/main/java/com/ning/billing/entitlement/glue/DefaultEntitlementModule.java
+++ b/entitlement/src/main/java/com/ning/billing/entitlement/glue/DefaultEntitlementModule.java
@@ -18,6 +18,8 @@ package com.ning.billing.entitlement.glue;
 
 import org.skife.config.ConfigSource;
 
+import com.ning.billing.entitlement.DefaultEntitlementService;
+import com.ning.billing.entitlement.EntitlementService;
 import com.ning.billing.entitlement.api.DefaultEntitlementApi;
 import com.ning.billing.entitlement.api.DefaultSubscriptionApi;
 import com.ning.billing.entitlement.api.EntitlementApi;
@@ -45,6 +47,7 @@ public class DefaultEntitlementModule extends AbstractModule implements Entitlem
         installEntitlementApi();
         installSubscriptionApi();
         installBlockingChecker();
+        bind(EntitlementService.class).to(DefaultEntitlementService.class).asEagerSingleton();
     }
 
     @Override
diff --git a/entitlement/src/test/java/com/ning/billing/entitlement/api/TestEntitlementDateHelper.java b/entitlement/src/test/java/com/ning/billing/entitlement/api/TestEntitlementDateHelper.java
index 248651e..fbb1aea 100644
--- a/entitlement/src/test/java/com/ning/billing/entitlement/api/TestEntitlementDateHelper.java
+++ b/entitlement/src/test/java/com/ning/billing/entitlement/api/TestEntitlementDateHelper.java
@@ -49,7 +49,7 @@ public class TestEntitlementDateHelper extends EntitlementTestSuiteNoDB {
     public void testWithAccountInUtc() throws EntitlementApiException {
 
         final LocalDate initialDate = new LocalDate(2013, 8, 7);
-        clock.setDay(initialDate.plusDays(1));
+        //clock.setDay(initialDate.plusDays(1));
 
         Mockito.when(account.getTimeZone()).thenReturn(DateTimeZone.UTC);
 
@@ -64,7 +64,7 @@ public class TestEntitlementDateHelper extends EntitlementTestSuiteNoDB {
     public void testWithAccountInUtcMinus8() throws EntitlementApiException {
 
         final LocalDate inputDate = new LocalDate(2013, 8, 7);
-        clock.setDay(inputDate.plusDays(3));
+        //.setDay(inputDate.plusDays(3));
 
         final DateTimeZone timeZoneUtcMinus8 = DateTimeZone.forOffsetHours(-8);
         Mockito.when(account.getTimeZone()).thenReturn(timeZoneUtcMinus8);
@@ -84,10 +84,10 @@ public class TestEntitlementDateHelper extends EntitlementTestSuiteNoDB {
     public void testWithAccountInUtcPlus5() throws EntitlementApiException {
 
         final LocalDate inputDate = new LocalDate(2013, 8, 7);
-        clock.setDay(inputDate.plusDays(1));
+        //clock.setDay(inputDate.plusDays(1));
 
-        final DateTimeZone timeZoneUtcMinus8 = DateTimeZone.forOffsetHours(+5);
-        Mockito.when(account.getTimeZone()).thenReturn(timeZoneUtcMinus8);
+        final DateTimeZone timeZoneUtcPlus5 = DateTimeZone.forOffsetHours(+5);
+        Mockito.when(account.getTimeZone()).thenReturn(timeZoneUtcPlus5);
 
         // We also use a reference time of 20, 28, 10, 0 -> DateTime in accountTimeZone will be (2013, 8, 7, 20, 28, 10)
         final DateTime refererenceDateTime = new DateTime(2013, 1, 1, 20, 28, 10, 0, DateTimeZone.UTC);
diff --git a/jaxrs/src/main/java/com/ning/billing/jaxrs/json/SubscriptionJsonNoEvents.java b/jaxrs/src/main/java/com/ning/billing/jaxrs/json/SubscriptionJsonNoEvents.java
index ae14fc1..70e5cff 100644
--- a/jaxrs/src/main/java/com/ning/billing/jaxrs/json/SubscriptionJsonNoEvents.java
+++ b/jaxrs/src/main/java/com/ning/billing/jaxrs/json/SubscriptionJsonNoEvents.java
@@ -26,6 +26,7 @@ import org.joda.time.LocalDate;
 import com.ning.billing.entitlement.api.Subscription;
 import com.ning.billing.util.audit.AuditLog;
 
+import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
 
 public class SubscriptionJsonNoEvents extends EntitlementJsonNoEvents {
@@ -38,7 +39,7 @@ public class SubscriptionJsonNoEvents extends EntitlementJsonNoEvents {
     private final Integer bcd;
     //private final Map<String, String> currentStatesForServices;
 
-
+    @JsonCreator
     public SubscriptionJsonNoEvents(@JsonProperty("accountId") @Nullable final String accountId,
                                     @JsonProperty("bundleId") @Nullable final String bundleId,
                                     @JsonProperty("entitlementId") @Nullable final String entitlementId,
diff --git a/jaxrs/src/main/java/com/ning/billing/jaxrs/resources/JaxRsResourceBase.java b/jaxrs/src/main/java/com/ning/billing/jaxrs/resources/JaxRsResourceBase.java
index 2e3209a..1310ec5 100644
--- a/jaxrs/src/main/java/com/ning/billing/jaxrs/resources/JaxRsResourceBase.java
+++ b/jaxrs/src/main/java/com/ning/billing/jaxrs/resources/JaxRsResourceBase.java
@@ -27,6 +27,7 @@ import javax.ws.rs.core.Response;
 import javax.ws.rs.core.UriInfo;
 
 import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
 import org.joda.time.LocalDate;
 import org.joda.time.format.DateTimeFormatter;
 import org.joda.time.format.ISODateTimeFormat;
@@ -205,10 +206,10 @@ public abstract class JaxRsResourceBase implements JaxrsResource {
 
         if (account == null && inputDate == null) {
             // We have no inputDate and so accountTimeZone so we default to LocalDate as seen in UTC
-            return new LocalDate(clock.getUTCNow());
+            return new LocalDate(clock.getUTCNow(), DateTimeZone.UTC);
         } else if (account == null && inputDate != null) {
             // We were given a date but can't get timezone, default in UTC
-            return new LocalDate(inputDate);
+            return new LocalDate(inputDate, DateTimeZone.UTC);
         } else if (account != null && inputDate == null) {
             // We have no inputDate but for accountTimeZone so default to LocalDate as seen in account timezone
             return new LocalDate(clock.getUTCNow(), account.getTimeZone());