killbill / killbill Goto Github PK

H2 has a MySQL compatibility mode, see:

http://www.h2database.com/html/features.html#feature_list

We should try it out as an alternative for the MysqlTestingHelper, this would greatly speed up slow tests with the in-memory mode of H2.

The task is actually bigger:

• make a reference from tags to tag_definitions via the tag_definition_id
• implement audit and history for tag_definitions
• add system tags in the tag_definitions database on startup (which will take care of the initial history too)

Our customers are confronted with datetime and dates. Not sure if we could only expose one of the two, this is part of the discussion:

Today, when Creating a subscription, this happens at given point in time, and this is there seen in our system as a datetime.
When billed we use a day granularity and export a BCD, which is just a number and therefore would give different results depending on where one live on the planet.
Unfortunately our code needs to juggle between both worlds, and the issues are:

First and foremost, the User experience is confusing; we need to bring clarity into what BCD means and how it should be interpreted
Second our APIs are cumbersome, for instance changing a Plan requires a datetime and sometimes will fail because the time part is not acceptable
Third our code is complicated because of all the conversion we are making and thgis translates all the way from the catalog which exposes datetime for versioning to entitlement subsystem which also exposes datetime at the API level to the invoice subsystem which rounds everything at the date level by using customer timezone.

For instance one case, is to specify what happens when an invoice has a charge (unrelated to bundle or subscription); how does overdue system work?

Catalog has a Fixed Item type but we really need two of them:

• One is for setup which is non repairable and non pro-ratable
• One would be fixed over a period.

When asking for plugin specific information for external payments, the call will almost always fail since not all of the payments are kept in memory.

We create ids in various places today. For example, JAX-RS will create the id for an AccountEmail in AccountEmailJson but account ids are created in DefaultAccountUserApi by AccountModelDao.

We should centralize id generation.

Overdue Apis are not public anymore. Should we re-export them in public Apis?
How does that play with a potential overdue module?

Multiple BP could be created or cancellation could happen multiple times-- we have seen that later case.

I'm working on building a little server and have encountered some minor pom discrepancies with the server module.
Server uses a different slf4j version (1.6.5) than skelton (1.6.4)
Server uses a different Guava version (11.0.2) than the killbill master pom (12.0.0)

ruby -e "$(curl -fsSL http://kill-bill.org/schema)" > ddl.sql seems to fail with curl: (22) The requested URL returned error: 404 Not Found
.

Unable to install.

Should we allow additional flexibility for changing plan-- symmetrical to what we offer for cancellation APis.Today entitlement will exactly match billing and we have 2 APis:

• Provide a (local) date and let the subscriptionBase to decide when the change occurs based on catalog policies
• Provide a (local) date and override the billing policy.

Payment method is not necessarily the same for all attempts for a payment. To be correct, it should be moved from payments to payment_attempts

Fix 'bad API'

An example id the chargeback API that does not match other APIs (should be /1.0/accounts//chargebacks)

We have quite a lot of simple mappers (e.g. HistoryRecordIdMapper) that could be replaced with the generic LowerToCamelBeanMapper.

Another issues to track in this ticket is that we currently have a possible failure scenario with REST API generation of invoices. Invoice can be generated but failure could happend before charge through date is set.

Modularity and Visibility of Tests

The logics is scattered across different places-- Invoice Generator and DAO layer mainly. Code is hard to maintain, and to work around it we wrote lots of tests, but tests themselves are messy and lack visibility. So making changes in that code is very time consuming because both code and tests take a long time to fix.

Modifications of Existing Invoices

Another issue, is the modifications of existing invoices, creating a bad customer experience and creating issues with weird use of CBA. There are several places where we modify existing invoices 1. Repair mode: That one could be avoided by repairing on the newly created invoice 2. Refund with Invoice adjustment/Item Adjustment 3. Apply a credit on existing invoice 4. Apply a charge on existing invoice

Item 1 could be worked around it by applying the repair on the newly created invoice. Items 3-- provided we need to support them-- have to modify that specific invoice by definition. The question, is whether we can live without those and always create new invoice for those?

Also note that items 1,2,3 may trigger CBA (positive amount) while 4 may use CBA

In EntitySqlDaoWrapperInvocationHandler, we take extra care of propagating the right exception to the caller.

We should do the same in EntitySqlDaoTransactionalJdbiWrapper#execute. Any BillingExceptionBase exception explicitly thrown in transactions are wrapped in TransactionFailedException. It would be nice to unwrap them, although BillingExceptionBase is a checked exception and #execute doesn't declare any to be thrown.

Ideally, these utilities could be extracted into a library (java client).

I think if server throws an exception on any operation, the client should get a correctly formatted json which can be parsed to at least BillingExceptionBase, and checked against the error code returned.

Or a better alternative is the 'Response' object (from asyncHttpClient) can detect the success/failure and offer a way (i.e. response.isError()) to see if there was an error, and offer one more method "public BillingExceptionBase getError()" in addition to getResponseBody().
The getError() method can return null when no error received by client.

. Ensure consistency across API; for e.g make sure APIs don't return null but instead throw exceptions
. Each api should have proper java doc

Ideally, we should be able to swap out Guice for another CDI framework (e.g. if we deploy Kill Bill in JBoss).

In a case of repair and when the catalog version changed junction seems to to use the new rate for the repair which is not necessarily what we want: The customer did not know the catalog would change in the future.

That would decrease the load on the DB server when nothing happens.

The util module has become very large - it may be the time to start breaking it up into smaller libraries. These libraries will be especially useful when developing OSGI bundles.

A candidate list of libraries could be:

• GlobalLocker library (used in Analytics bundle)
• Embedded DB tester library (used in Zuora, Analytics and Meter bundles)
• Custom JDBI mappers and binders library (used in Analytics bundle)
• Persistent event bus/notification library

The list is work in progress.

GET /1.0/<object_type>/custom_fields/<object_id> N/A Retrieves the custom fields associated with that object
200 (OK), 204 (NO_CONTENT)

PUT /1.0/<object_type>/custom_fields/<object_id> N/A
Modify the custom field associated to that object
204 (NO_CONTENT), 400 (BAD_REQUEST)

DELETE /1.0/<object_type>/custom_fields/<object_id>?name=foo&name=... N/A
Removes the custom field
204 (NO_CONTENT), 400 (BAD_REQUEST)

Guava library (BusEvent) catches exceptions when posting events and does not allow to extend the dispatch() method; that prevent us from handling exceptions which is a prblem; filed ISSUE-780. To be followed-- hopefully...

In InvoiceUserApi, add:

public Invoice getInvoiceByInvoiceItem(UUID invoiceItemId, TenantContext context) throws InvoiceApiException;

In InvoiceResource, add getInvoiceByInvoiceItem (@Path("/byInvoiceItem/{invoiceItemId:" + UUID_PATTERN + "}/")).

• Clients to regenerate (Java, Python) or update (Ruby)
• Slate to update

There are two issues here:

1. Query to fetch available events from disk is very expansive
2. Ordering of events is not respected

Proposal:

Use a separate global lock for NotificationQ/BusEvent at the account level. The queue would fetch all available events for that account with that lock (Global SQL lock) and process them in order. That solves the ordering issue and make the query much simpler to write

However this adds complexity, because we now have multiple level of locks-- the one from the Qs and the one taken by Invoice and Payment; the ordering should always be 1 )Q lock followed by 2) InvoicePayment lock

We would like to bring some more uniformity across our tests; there are several axes that need rework:
1 - Guice / non Guice; and for guice, do we do field injection? @guice annotation, or private test module?
2 - Outside mock classes? Should we create mock APIs for all services or rely on Mockito?
3 - Per module mock class? Should we use mock classes or Mockito?

For instance Invoice item type REPAIR_ADJ is not actually an adjustment. It should be renamed to reduce chance of confusion.

. Audits occur for both internal / external calls
. Create abstraction layer for audit/history
-> (At the same time catch TransactionFailedException and throw cause BillingException)

. EntitySqlDao < AuditSqlDao, HistorySqlDao, Transactional
. Get rid of Transmogrifier (use annotation @CreateSqlObject)

• Search by custom fields
• Search by last4 of CC
• Search by Tags

http://10.18.56.145:8080/1.0/kb/accounts/bc018d37-b758-45fb-9135-b1996d43bcc8/timeline
...
"invoices": [
{
"accountId": "bc018d37-b758-45fb-9135-b1996d43bcc8",
"amount": 0,
"balance": 0.0,
"bundleKeys": "2025736",
"cba": 0.0,
"creditAdj": 0.0,
"credits": [
{
"accountId": "bc018d37-b758-45fb-9135-b1996d43bcc8",
"creditAmount": -0.01,
"effectiveDate": "2012-08-09T00:00:00.000-08:00",
"invoiceId": "e8242530-8fee-420a-8fb4-ab8bfe8961e0",
"invoiceNumber": null,
"reason": null,
"requestedDate": null
}
],
"invoiceDate": "2012-07-16",
"invoiceId": "42f52d03-d2da-4912-bd2f-81695bb97627",
"invoiceNumber": "1",
"refundAdj": 0.0,
"targetDate": "2012-07-16"
},
...

If triggered it will add auto_pay_off tag to account throw alert and drop the payment (without creating a payment attempt) also create an ALERT log message.

Also need a control tag that can be added to the account that overrides the bound.

An example is testENT_REPAIR_BP_RECREATE_MISSING_AO

• Change the default jdbi configuration options. User/Pass should be killbill/killbill, not root/root so the defaults "just work" with the 5 minutes tutorial (one can double click on the jetty console war)
• Change the default polling interval for the notification queue. Currently, if you start killbill with wrong credentials for the db, you get a stacktrace every 500ms. Note: we need to fix our tests so they have that as a default
• Add a simple HTML page at the root (127.0.0.1:8080), so that people "see" something when killbill starts. We could maybe add a form on that page to create a tenant, with links to the docs
• Consider switching to H2 as a default. More testing is needed here, and potentially code too (analytics queries don't work with H2 at the moment)
• Documentation: in the 5 minutes demo, add a set of 'useful' curls -- such as create and account, bundle, subscription, and then see your invoices and payment automatically processed

EntitlementJsonNoEvents
BundleJsonNoSubscriptions
InvoiceJsonWithBundleKeys
...

yuck.

In the past it dod not matter so much but now those classes are the base classes for the ruby client library through the use of the java parser...

Should we use Entity class everywhere Integration with History and AuditLog should be the same everywhere

The (local) date used in entitlement APIs has two function:

• On the entitlement side and for cancellation, the date is respected
• On the billing side (subscription base), the current code does the following:
• If policy is IMM (whether policy was passed at API level or extrapolated from catalog), we either use the requestedDate or default to clock.getUtcNow() if date is in future
• If policy is EOT: if there is a CTD we use the min(CTD, requestedDate); if there is no CTD (trial example) we default requestedDate

This seems hawky and hard to document.

Adding the account record id in the log (custom prefix) would be nice too.

We can start with an API call that allows rebalance the CBA, starting from oldest unpaid invoice and using a greedy algorithm.