Bugfixes

[wienczny]

Hi,
I've used your the api wrapper for a project and like it's API. I had to fix some bugs and add some features like calls for whitlists, rejects and templates. There was a bug when sending attachements. Some additional changes were made:

1. AbstractService has been refactored to use https://jersey.java.net/ which makes handling requests and responses easier.
2. There are automated test against Mandrill using a test API-key. Just add mandrill_apikey with an test key to your environment or settings.xml
3. Use Objects.toStringHelper instead of StringBuilder to improve String rendering

Best regards

[duergner]

Thanks for your contribution. I'll review and integrate it on the weekend.

[duergner]

Why have you refactored the AbstractService to use baseUrl and JerseyClient as static fields and not as instance fields?

[wienczny]

I think the reason was preparation for connection pooling. There are two alternatives to static fields:

1. Refactor AbstractService to be a client class without child classes. This would almost remove the services and move their methods to the client. This would look like:

   Client client = new Client(baseurl, apikey)
   MessageService messageService = new MessageService();
   List response = client.sendMessage(new MessageSendPayload.Builder().withTo(EMAIL).withSubject("Mandrill Test")
   .withText("This is an automatic test for emails send using Mandrill"));

2. Have a factory that creates all services and assigns initial values to them:

   Factory factory = new Factory(baseurl, apikey);
   MessageService messageService = factory.createMessageService();
   // messageService has been assigned baseurl, apikey and jersey
   messageService.sendMessage(new MessageSendPayload.Builder().withTo(EMAIL).withSubject("Mandrill Test")
   .withText("This is an automatic test for emails send using Mandrill"))

Both alternatives have their advantages. Which do you prefer?

[duergner]

Ok I understand that; my original implementation was mainly used within a Spring DI/IoC container and therefore connection pooling was always done as I always injected the same HttpClient into the different services; the AbstractService was actually defined as abstract bean and reused by all the specific services when created.

I think moving over to a factory based approach would be best than as one could still simply use the plain service beans inside a DI/IoC container.

[duergner]

BTW: you might want to change the author to yourself on the classes you added and feel free to add yourself as author on the classes you modified

[wienczny]

I'll try to update this PR over this weekend ;-)

[wienczny]

I added my copyright to every file I did more the cleanups in and hope everything is fine ;)
I'm not sure about the factories name. Maybe MandrillServiceFactory would cause less collisions. What do you think?

[duergner]

Looks good so far (only the failing Travis CI build is strange as I added the 'mandrill_apikey' environment variable; strange)

Yeah MandrillServiceFactory might be better as it would be more descriptive. So if you want to change that feel free.

[wienczny]

I'm building it on https://drone.io/github.com/wienczny/mandrill-java-api-wrapper/
Have a look at AbstractTest to see where this value could come from.
Did you use a secure variable? Secure variables are not added to pull request builds http://docs.travis-ci.com/user/environment-variables/#Secure-Variables
I'm trying to test it on travis.

[duergner]

I think the build issue is nothing you need to worry about; I'll get that fixed in my build :-)

[alphaduke0]

Hey there is this going to be in included soon? Would love to see that ;-)