Support clients and recurring payments

[jcassee]

Hi Mats,

It would be nice to be able to support clients and recurring payments. Would you accept a PR to add the functionality? If so, do you have any tips, constraints, guidelines, etc. for the implementation?

[jcassee]

Adding yet another constructor to CreatePayment for customerId and recurringType feels wrong. What about converting the classes that build requests to fluent interface builders:

CreatePayment payment = new CreatePayment()
        .withMethod(...)
        .withAmount(...)
        .withDescription(...)
        .withX(...);

[stil4m]

Hi Joost, of course PRs are welcome. I haven't looked into Mollie's recurring payment API yet, but I aggree with you that adding another constructor would be wrong.

I favour your builder suggestion. I would suggest a inner class for the builder and a hidden constructor for the CreatePayment such as:

CreatePayment payment = new CreatePayment.Builder().withX(...).withY(...).build();

In the current implementation the main focus is that you should not pass in null for mandatory fields (otherwise it would be an Optional). My main concern with the builder is that you will be able to create CreatePayment instances that are not valid. You will receive an error when you go to the mollie server and back. This is annoying if the library client made a mistake and only found when the payments are made in an test/acceptance/integration environment. It may help to do verification in the build() method and throw an exception, but that would not solve the problem. Any suggestions on this?

[stil4m]

Add API URL for the context of this converstation: https://www.mollie.com/en/docs/recurring

[jcassee]

Making sure the request is always valid is a good concern for the library. Why would validation on the build() method not work?

[stil4m]

It would help. But it still requires unit/integration testing by the library user to verify if the object is build correctly. This 'problem' was less big in the old scenario. But we just have to take that for granted.

I'm looking forward to the PR. If you need help, just let me know.

[jcassee]

Alright, I will get to it.

Out of curiosity, how does the current code avoid the need for testing?

[stil4m]

Well, it does not. But you can see that you will make a mistake when you call the constructor in such a way (due to the convention to use Optional over null values):

x = new CreatePayment("Foo", null);

You should not pass in null values. If you do, you make the explicit choice. With a builder pattern it is less obvious that you will build the object without the mandatory second argument. For example:

x  = new CreatePayment.Builder()
    .withFoo("Foo")
    .build()

In this example it is less clear that you will make a mistake to create x.

IMHO I find that you ar more dependent on a unit/integration test in the second case than in the first case.

[jcassee]

Okay, that makes sense. Thanks!

[jcassee]

@stil4m: Question: what is the use-case for calling code to provide their own ObjectMapper instance?

[stil4m]

What do you refer to when you mention calling code and own?

[jcassee]

I meant, it is possible to inject an ObjectMapper using ClientBuilder.withMapper. Why would you want to do that?

[jcassee]

By the way, I am hacking away, and I am discovering that I have different design sensibilities. For example:

• using @Nonnull / @CheckForNull annotations instead of Optional
• not using builders because this increases the code size, and they are just HTTP request bodies anyway, not domain entities
• simplifying the Client, ClientBuilder, etc. setup

The API is becoming pretty much backward-incompatible, and I am unsure whether you would agree with it. I can either 1) submit my changes as a pull request, but it would have to become something like a 2.0 version, or 2) just create a separate Mollie library. What would you prefer?

[stil4m]

I meant, it is possible to inject an ObjectMapper using ClientBuilder.withMapper. Why would you want to do that?

Currently the default ObjectMapper makes use of the Jdk8Module to be able map Optional and DateTime. Suppose Java 9 comes out, people will be able to inject an ObjectMapper with a Jdk9Module. In addition users of the library may embed their own domain objects in the meta data of a payment. As well for this they may want to inject an ObjectMapper with custom encoding/decoding functionality.

using @Nonnull / @CheckForNull annotations instead of Optional.

I agree with adding the annotations, but why drop the Optional? In my opinion it is a nice pattern which allows you to reason about data instead of having to program defensively with the null.

The API is becoming pretty much backward-incompatible

Yeah. I was already set for this when we discussed the builder pattern for CreatePayment.

1. ... 2) ... What would you prefer?

I prefer 1. I think it is a good thing that there remains 1 mollie library. Otherwise we will become a JavaScript/npm community :p.

One last note: I really like that you help me out with this. The original design was only a figment of my mind, with multiple insights I think we can make something nice.

[jcassee]

Alright, I will push some example code shortly.

I think it would be cool to get Mollie to sanction the library as an official one, just like Python, Ruby, etc. It's really strange to have an API without an official Java client, right?

[Daanvm]

Hi @jcassee and @stil4m! First of all: we really appreciate the work that you're doing with this Java client!

About making this client an official Mollie client: I'd need to discuss that with my colleagues, and of course Mats has to agree to transfer the client to the @mollie Github account. Note that this client is already listen on our modules page. If you really want to make this an official Mollie client, please send an e-mail to [email protected] and we'll look into it!

[stil4m]

I like the idea of an official Java client. I do think it is better to have one Java library in the ecosystem. I'm not sure how transferring the code to Mollie will have a positive effect. Firstly, there is another Java client. Is it fair to promote this library to become the official one? Maybe the other one is better. Secondly, AFAIK (and correct me if I am wrong) the stack at mollie is PHP/Javascript/Python and maybe some other things. Do they want to maintain a Java package? Otherwise putting in pull requests might become less trivial, who does quality control. Currently their are clear architectural choices, do these ideologies remain when the code is transferred? Lastly, currently the library is available through a maven repository and CI is set up with Travis CI. Doesn't seem that this standard is applied to the other language specific libraries. As you can see in #13, these integration/regression test do provide value.
If these concerns get tackled, I will be happy to transfer the repository. But at the moment I need some convincing.

[jcassee]

As someone who looked at both Java clients, this one is better. :-) I agree that there should be no regressions in the the quality assurance set-up. I imagine Mollie would want to keep the delivery process in place?

[tubbynl]

after some short research i also found this library to be better (code style, tests) than the other Java cliënt mentioned on the mollie list. I've requested a few weeks ago that this library was added to the https://www.mollie.com/nl/modules listing.

@jcassee i also need the recurring thingies in this cliënt, can i help you with something? I already did some basic stuff on the Customers API on my fork

[tubbynl]

Moving of this java client to the mollie github is an other discussion/ticket; but i agree that the current pipeline should be retained (CI, Maven repo could be some public?) and somehow authorship/administrator privileges should be assigned to @stil4m

[stil4m]

I was sitting in the train so I though lets give it a spin. I've created a branch where the api supports fetching/creating/updating customers and can create/fetch payments for a customer.

Additionally you can create recurring payments for a customer (recurring or not). I decided not to use the /payments endpoint to create recurring payments, but only on /customers/X/payments.

It is available as an snapshot through the maven repository on GitHub.

The architecture of the library did not change. @jcassee can you give it a look? Maybe you can reuse parts for the rewrite.
@Daanvm I found 2 bugs in the Mollie api related to recurring payments. What is the best way to communicate these, report this?

[stil4m]

@tubbynl I'll look into pushing the library to a public repo see #21.

[tubbynl]

really cool @stil4m

creating dedicated tests per concept as you did for Customers also makes more sense now the amount of tests grow.

i would suggest to rename 'CustomersIntegerationTest' to 'CustomersIntegrationTest' so it's less Guantanamo-Bay-like ;)

[Daanvm]

@stil4m The best way to report bugs or issues is sending a mail to [email protected]. I'm curious what you've found!

[stil4m]

Just merged my customers and customer payments functionality to fix the currently breaking functionality mentioned in #23. I'll deploy this as 2.2.0. @jcassee I'm still interested in your changes. Please create a PR once finished and ping me.

[jcassee]

Great! I will.