Skip to content

Commit

Permalink
jaxrs: remove ability to pass DateTime when LocalDate is expected
Browse files Browse the repository at this point in the history 
This fixes #488.

Signed-off-by: Pierre-Alexandre Meyer <pierre@mouraf.org>
  • Loading branch information
@pierre
pierre committed Feb 11, 2016
1 parent 56857be commit 5effd5e
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 82 deletions.
10 changes: 3 additions & 7 deletions jaxrs/src/main/java/org/killbill/billing/jaxrs/resources/BundleResource.java
View file
Expand Up @@ -205,12 +205,10 @@ public Response pauseBundle(@PathParam(ID_PARAM_NAME) final String id,
@HeaderParam(HDR_REASON) final String reason,
@HeaderParam(HDR_COMMENT) final String comment,
@javax.ws.rs.core.Context final HttpServletRequest request) throws SubscriptionApiException, EntitlementApiException, AccountApiException {

final Iterable<PluginProperty> pluginProperties = extractPluginProperties(pluginPropertiesString);
final CallContext callContext = context.createContext(createdBy, reason, comment, request);
final UUID bundleId = UUID.fromString(id);
final SubscriptionBundle bundle = subscriptionApi.getSubscriptionBundle(bundleId, callContext);
final LocalDate inputLocalDate = toLocalDate(bundle.getAccountId(), requestedDate, callContext);
final LocalDate inputLocalDate = toLocalDate(requestedDate, callContext);
entitlementApi.pause(bundleId, inputLocalDate, pluginProperties, callContext);
return Response.status(Status.OK).build();
}
Expand All @@ -230,12 +228,10 @@ public Response resumeBundle(@PathParam(ID_PARAM_NAME) final String id,
@HeaderParam(HDR_REASON) final String reason,
@HeaderParam(HDR_COMMENT) final String comment,
@javax.ws.rs.core.Context final HttpServletRequest request) throws SubscriptionApiException, EntitlementApiException, AccountApiException {

final Iterable<PluginProperty> pluginProperties = extractPluginProperties(pluginPropertiesString);
final CallContext callContext = context.createContext(createdBy, reason, comment, request);
final UUID bundleId = UUID.fromString(id);
final SubscriptionBundle bundle = subscriptionApi.getSubscriptionBundle(bundleId, callContext);
final LocalDate inputLocalDate = toLocalDate(bundle.getAccountId(), requestedDate, callContext);
final LocalDate inputLocalDate = toLocalDate(requestedDate, callContext);
entitlementApi.resume(bundleId, inputLocalDate, pluginProperties, callContext);

This comment has been minimized.

Sign in to view

Copy link
@sbrossie

sbrossie Feb 11, 2016

Member

Don't we want to also use the toLocalDateDefaultToday for all the subscription endpoints?

This comment has been minimized.

Sign in to view

Copy link
@sbrossie

sbrossie Feb 11, 2016

Member

Oh scratch that, got it. We need to pass null to entitlementApi to force an event with utcNow... got it...

This comment has been minimized.

Sign in to view

Copy link
@pierre

pierre Feb 11, 2016

Author Member

Yup! Same with invoice (except when we need something for the effective date of the adjustment or credit).
return Response.status(Status.OK).build();
}
Expand Down Expand Up @@ -362,7 +358,7 @@ public Response transferBundle(final BundleJson json,
final UUID bundleId = UUID.fromString(id);

final SubscriptionBundle bundle = subscriptionApi.getSubscriptionBundle(bundleId, callContext);
final LocalDate inputLocalDate = toLocalDate(bundle.getAccountId(), requestedDate, callContext);
final LocalDate inputLocalDate = toLocalDate(requestedDate, callContext);

final UUID newBundleId = entitlementApi.transferEntitlementsOverrideBillingPolicy(bundle.getAccountId(), UUID.fromString(json.getAccountId()), bundle.getExternalKey(), inputLocalDate, policy, pluginProperties, callContext);
return uriBuilder.buildResponse(BundleResource.class, "getBundle", newBundleId, uriInfo.getBaseUri().toString());
Expand Down
10 changes: 5 additions & 5 deletions jaxrs/src/main/java/org/killbill/billing/jaxrs/resources/InvoiceResource.java
View file
Expand Up @@ -302,7 +302,7 @@ public Response createFutureInvoice(@QueryParam(QUERY_ACCOUNT_ID) final String a
@javax.ws.rs.core.Context final HttpServletRequest request,
@javax.ws.rs.core.Context final UriInfo uriInfo) throws AccountApiException, InvoiceApiException {
final CallContext callContext = context.createContext(createdBy, reason, comment, request);
final LocalDate inputDate = toLocalDate(UUID.fromString(accountId), targetDate, callContext);
final LocalDate inputDate = toLocalDate(targetDate, callContext);

try {
final Invoice generatedInvoice = invoiceApi.triggerInvoiceGeneration(UUID.fromString(accountId), inputDate, null,
Expand Down Expand Up @@ -339,10 +339,10 @@ public Response generateDryRunInvoice(@Nullable final InvoiceDryRunJson dryRunSu
} else if (DryRunType.SUBSCRIPTION_ACTION.name().equals(dryRunSubscriptionSpec.getDryRunType()) && dryRunSubscriptionSpec.getEffectiveDate() != null) {
inputDate = dryRunSubscriptionSpec.getEffectiveDate();
} else {
inputDate = toLocalDate(UUID.fromString(accountId), targetDate, callContext);
inputDate = toLocalDate(targetDate, callContext);
}
} else {
inputDate = toLocalDate(UUID.fromString(accountId), targetDate, callContext);
inputDate = toLocalDate(targetDate, callContext);
}

// Passing a null or empty body means we are trying to generate an invoice with a (future) targetDate
Expand Down Expand Up @@ -426,7 +426,7 @@ public Response adjustInvoiceItem(final InvoiceItemJson json,
final CallContext callContext = context.createContext(createdBy, reason, comment, request);

final UUID accountId = UUID.fromString(json.getAccountId());
final LocalDate requestedDate = toLocalDate(accountId, requestedDateTimeString, callContext);
final LocalDate requestedDate = toLocalDateDefaultToday(accountId, requestedDateTimeString, callContext);
final InvoiceItem adjustmentItem;
if (json.getAmount() == null) {
adjustmentItem = invoiceApi.insertInvoiceItemAdjustment(accountId,
Expand Down Expand Up @@ -474,7 +474,7 @@ public Response createExternalCharges(final Iterable<InvoiceItemJson> externalCh
final Iterable<InvoiceItemJson> sanitizedExternalChargesJson = cloneRefundItemsWithValidCurrency(account.getCurrency(), externalChargesJson);

// Get the effective date of the external charge, in the account timezone
final LocalDate requestedDate = toLocalDate(account, requestedDateTimeString, callContext);
final LocalDate requestedDate = toLocalDateDefaultToday(account, requestedDateTimeString, callContext);

final Iterable<InvoiceItem> externalCharges = Iterables.<InvoiceItemJson, InvoiceItem>transform(sanitizedExternalChargesJson,
new Function<InvoiceItemJson, InvoiceItem>() {
Expand Down
53 changes: 10 additions & 43 deletions jaxrs/src/main/java/org/killbill/billing/jaxrs/resources/JaxRsResourceBase.java
View file
Expand Up @@ -39,8 +39,6 @@
import javax.ws.rs.core.StreamingOutput;
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.DateTimeFormat;
import org.joda.time.format.DateTimeFormatter;
Expand Down Expand Up @@ -91,6 +89,7 @@
import com.fasterxml.jackson.core.JsonGenerator;
import com.google.common.base.Function;
import com.google.common.base.Joiner;
import com.google.common.base.MoreObjects;
import com.google.common.base.Preconditions;
import com.google.common.base.Predicate;
import com.google.common.base.Strings;
Expand Down Expand Up @@ -298,51 +297,19 @@ protected void validatePaymentMethodForAccount(final UUID accountId, final UUID
}
}

protected LocalDate toLocalDate(final UUID accountId, final String inputDate, final TenantContext context) throws AccountApiException {
final LocalDate maybeResult = extractLocalDate(inputDate);
if (maybeResult != null) {
return maybeResult;
}
Account account = accountId != null ? accountUserApi.getAccountById(accountId, context) : null;
final DateTime inputDateTime = inputDate != null ? DATE_TIME_FORMATTER.parseDateTime(inputDate) : clock.getUTCNow();
return toLocalDate(account, inputDateTime, context);
protected LocalDate toLocalDateDefaultToday(final UUID accountId, @Nullable final String inputDate, final TenantContext context) throws AccountApiException {
final Account account = accountId != null ? accountUserApi.getAccountById(accountId, context) : null;
return toLocalDateDefaultToday(account, inputDate, context);
}

protected LocalDate toLocalDate(final Account account, final String inputDate, final TenantContext context) {

final LocalDate maybeResult = extractLocalDate(inputDate);
if (maybeResult != null) {
return maybeResult;
}
final DateTime inputDateTime = inputDate != null ? DATE_TIME_FORMATTER.parseDateTime(inputDate) : clock.getUTCNow();
return toLocalDate(account, inputDateTime, context);
protected LocalDate toLocalDateDefaultToday(final Account account, @Nullable final String inputDate, final TenantContext context) {
// TODO Switch to cached normalized timezone when available
return MoreObjects.firstNonNull(toLocalDate(inputDate, context), clock.getToday(account.getTimeZone()));
}

private LocalDate toLocalDate(final Account account, final DateTime inputDate, final TenantContext context) {
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(), 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, 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());
} else {
// Precise LocalDate as requested
return new LocalDate(inputDate, account.getTimeZone());
}
}

private LocalDate extractLocalDate(final String inputDate) {
if (inputDate != null) {
try {
final LocalDate localDate = LocalDate.parse(inputDate, LOCAL_DATE_FORMATTER);
return localDate;
} catch (final IllegalArgumentException expectedAndIgnore) {
}
}
return null;
// API for subscription and invoice generation: keep null, the lower layers will default to now()
protected LocalDate toLocalDate(@Nullable final String inputDate, final TenantContext context) {
return inputDate == null ? null : LocalDate.parse(inputDate, LOCAL_DATE_FORMATTER);
}

protected Iterable<PluginProperty> extractPluginProperties(@Nullable final Iterable<PluginPropertyJson> pluginProperties) {
Expand Down
9 changes: 4 additions & 5 deletions jaxrs/src/main/java/org/killbill/billing/jaxrs/resources/SubscriptionResource.java
View file
Expand Up @@ -92,7 +92,6 @@

import com.google.common.base.Preconditions;
import com.google.common.base.Predicate;
import com.google.common.base.Strings;
import com.google.common.collect.Iterables;
import com.google.inject.Inject;
import com.wordnik.swagger.annotations.Api;
Expand Down Expand Up @@ -187,7 +186,7 @@ public Entitlement doOperation(final CallContext ctx) throws InterruptedExceptio
ProductCategory.valueOf(entitlement.getProductCategory()),
BillingPeriod.valueOf(entitlement.getBillingPeriod()), entitlement.getPriceList(), phaseType);

final LocalDate inputLocalDate = requestedDate == null ? null : toLocalDate(account, requestedDate, callContext);
final LocalDate inputLocalDate = toLocalDate(requestedDate, callContext);
final PlanSpecifier planSpec = new PlanSpecifier(entitlement.getProductName(),
ProductCategory.valueOf(entitlement.getProductCategory()),
BillingPeriod.valueOf(entitlement.getBillingPeriod()), entitlement.getPriceList());
Expand Down Expand Up @@ -314,7 +313,7 @@ public List<PlanPhasePriceOverride> getOverrides() {
entitlementSpecifierList.add(specifier);
}

final LocalDate inputLocalDate = toLocalDate(account, requestedDate, callContext);
final LocalDate inputLocalDate = toLocalDate(requestedDate, callContext);
return entitlementApi.createBaseEntitlementWithAddOns(account.getId(), baseEntitlement.getExternalKey(), entitlementSpecifierList,
inputLocalDate, pluginProperties, callContext);
}
Expand Down Expand Up @@ -392,7 +391,7 @@ public Response doOperation(final CallContext ctx) throws EntitlementApiExceptio
final UUID uuid = UUID.fromString(subscriptionId);

final Entitlement current = entitlementApi.getEntitlementForId(uuid, callContext);
final LocalDate inputLocalDate = toLocalDate(current.getAccountId(), requestedDate, callContext);
final LocalDate inputLocalDate = toLocalDate(requestedDate, callContext);
final Entitlement newEntitlement;

final Account account = accountUserApi.getAccountById(current.getAccountId(), callContext);
Expand Down Expand Up @@ -467,7 +466,7 @@ public Response doOperation(final CallContext ctx)
final UUID uuid = UUID.fromString(subscriptionId);

final Entitlement current = entitlementApi.getEntitlementForId(uuid, ctx);
final LocalDate inputLocalDate = toLocalDate(current.getAccountId(), requestedDate, callContext);
final LocalDate inputLocalDate = toLocalDate(requestedDate, callContext);
final Entitlement newEntitlement;
if (billingPolicyString == null && entitlementPolicyString == null) {
newEntitlement = current.cancelEntitlementWithDate(inputLocalDate, useRequestedDateForBilling, pluginProperties, ctx);
Expand Down
24 changes: 11 additions & 13 deletions jaxrs/src/test/java/org/killbill/billing/jaxrs/TestDateConversion.java
View file
@@ -1,7 +1,9 @@
/*
* Copyright 2010-2013 Ning, Inc.
* Copyright 2014-2016 Groupon, Inc
* Copyright 2014-2016 The Billing Project, LLC
*
* Ning licenses this file to you under the Apache License, version 2.0
* The Billing Project 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:
*
Expand Down Expand Up @@ -55,30 +57,26 @@ public void beforeTest() {
//
// BASIC Tests to understand how toLocalDate converts different inputs (null, LocalDate, DateTime)
//
@Test(groups = "fast")
public void testDateTimeConversion() throws AccountApiException {
final UUID accountId = setupAccount(DateTimeZone.forOffsetHours(-8));
final String input = "2013-08-26T06:50:20Z";
final LocalDate result = toLocalDate(accountId, input, null);
Assert.assertTrue(result.compareTo(new LocalDate(2013, 8, 25)) == 0);
}


@Test(groups = "fast")
public void testNullConversion() throws AccountApiException {
final String input = null;

final LocalDate result = toLocalDate(input, null);
Assert.assertNull(result);

final UUID accountId = setupAccount(DateTimeZone.forOffsetHours(-8));
((ClockMock) clock).setTime(new DateTime("2013-08-26T06:50:20Z"));
final String input = null;
final LocalDate result = toLocalDate(accountId, input, null);
Assert.assertTrue(result.compareTo(new LocalDate(2013, 8, 25)) == 0);
final LocalDate result2 = toLocalDateDefaultToday(accountId, input, null);
Assert.assertTrue(result2.compareTo(new LocalDate(2013, 8, 25)) == 0);
((ClockMock) clock).resetDeltaFromReality();
}

@Test(groups = "fast")
public void testLocalDateConversion() throws AccountApiException {
final UUID accountId = setupAccount(DateTimeZone.forOffsetHours(-8));
final String input = "2013-08-25";
final LocalDate result = toLocalDate(accountId, input, null);
final LocalDate result = toLocalDate(input, null);
Assert.assertTrue(result.compareTo(new LocalDate(2013, 8, 25)) == 0);
}

Expand Down
4 changes: 2 additions & 2 deletions profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestEntitlement.java
View file
Expand Up @@ -246,7 +246,7 @@ public void testEntitlementWithAddOns() throws Exception {
subscriptions.add(base);
subscriptions.add(addOn1);
subscriptions.add(addOn2);
final Bundle bundle = killBillClient.createSubscriptionWithAddOns(subscriptions, initialDate, 10, "createdBy", "", "");
final Bundle bundle = killBillClient.createSubscriptionWithAddOns(subscriptions, initialDate.toLocalDate(), 10, "createdBy", "", "");
assertNotNull(bundle);
assertEquals(bundle.getExternalKey(), "base");
assertEquals(bundle.getSubscriptions().size(), 3);
Expand All @@ -268,7 +268,7 @@ public void testCreateEntitlementInTheFuture() throws Exception {
input.setProductCategory(ProductCategory.BASE);
input.setBillingPeriod(BillingPeriod.MONTHLY);
input.setPriceList(PriceListSet.DEFAULT_PRICELIST_NAME);
final Subscription entitlementJson = killBillClient.createSubscription(input, initialDate.plusMonths(1), -1, createdBy, reason, comment);
final Subscription entitlementJson = killBillClient.createSubscription(input, initialDate.toLocalDate().plusMonths(1), -1, createdBy, reason, comment);

Assert.assertEquals(entitlementJson.getProductName(), input.getProductName());
Assert.assertEquals(entitlementJson.getProductCategory(), input.getProductCategory());
Expand Down

1 comment on commit 5effd5e

@sbrossie
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Please sign in to comment.