search

sargue / mailgun

Java library to easily send emails using the Mailgun service
MIT License
152 stars 40 forks source link

Specifically use JerseyClientBuilder #6

Closed jphollanti closed 8 years ago

jphollanti commented 8 years ago

in Mail.java a new client is built with: Client client = ClientBuilder.newClient();

Since MailMultipart depends specifically on Jersey features, it would be better to use: Client client = JerseyClientBuilder.createClient();

This becomes an issue if you happen to have e.g. RESTEasy in classpath as well. Then you might end up in a situation where a RESTEasy client tries to register a Jersey specific multi part feature: client.register(MultiPartFeature.class);

sargue commented 8 years ago

Hi! Thanks for reporting.

Mmm, that's a tricky one. I understand the problem but I don't feel comfortable just changing that line (which is easy anyway). The problem does not go away.

I don't think that having two JAX-RS implementations on the classpath is a good idea. Probably Mailgun should be implementation agnostic (not sure if that is possible) or have more than one version or some module/plugin mechanism.

What about the Entity.entity static calls? Same problem, isn't it?

jphollanti commented 8 years ago

Yeah, i think you're right, the normal get requests and such are implementation agnostic but multipart form is implementation specific. IMO it's not a big deal to have multiple implementations in your classpath (though it's far from ideal, it can easily happen and there's not much you can do about it). A plugin mechanism would be cool, but i'm not sure if it's worth the extra effort? The current code does a great job of being implementation agnostic but since it's not fully possible it ends up being anyway Jersey specific, which in turn might break things.

I'm not sure if this way of using Entity.entity is Jersey specific. It might even work as is.

sargue commented 8 years ago

Well, the change shouldn't be problematic so I will try it. Let's see what happens.