+1 I think that making the application id and master secret part of the
Sender makes sense.
Looking at the code example you posted, I was a little surprised to see
that the SenderClient.Builder does not build a SenderClient but instead a
JavaSender. Inspecting the code I can see that SenderClient implements
JavaSender of course, but having to think about this just truck me as there
might be room for improvement.
Would it be possible to move the methods declared in SenderClient that are
missing from JavaSender into that interface?
For example, it looks like the proxy methods and the truststore methods
only exist in SenderClient.
Was there a reason for leaving them out of the JavaSender interface?
If we moved the methods up into the interface we could then make the naming
consistent, like JavaSender and DefaultJavaSender. I'm lousy at coming up
with class/interface names so this might be horrible but would PushSender
be a better name for the interface?
This might be a personal preference but I much prefer using a static method
instead when creating a new Builder instance. There is one such static
method in SenderClient but the Builder's constructor is still public making
the usage above valid. What I'd prefer is that the Builder's constructor be
private and provide static methods to start the build process:
PushSender client = DefaultPushSender.withRootServerURL("
http://aerogear.example.com/ag-push")
.proxy("proxy", 8080)
.proxyType(Proxy.Type.HTTP)
.build();
PushSender client =
DefaultPushSender.withConfig("/path/on/classpath/push-config.json").build();
PushSender client = DefaultPushSender.withConfig(inputstream).build();
Again, these might be personal preferences and others might disagree but I
thought I'd post them just the same incase that offer different view.
/Dan
On 8 November 2014 13:59, Sebastien Blanc <scm.blanc(a)gmail.com> wrote:
Hi,
Currently in the Java Sender Library the pushApplicationId and the
masterSecret are part of the UnifiedPushMessage object. I would like to
move these 2 fields to the Sender object and add it to its Builder API,
to have something like :
JavaSender defaultJavaSender = new
SenderClient.Builder("http://localhost:8080/ag-push")
.pushApplicationId("c7fc6525-5506-4ca9-9cf1-55cc261ddb9c")
.masterSecret("8b2f43a9-23c8-44fe-bee9-d6b0af9e316b")
.build();
Why ?
- Regarding design, these 2 fields are more part of the sender rather
than the message itself
- This is a first step to externalize the configuration, so we could
have later, for instance, `JavaSender defaultJavaSender = new
SenderClient.Config("path.to.config(or resource stream")
What does it change ?
- It only concerns the Java Sender, we do not touch the Rest Sender
API here.
- A Sender instance is now bounded to a particular PushApp, so if we
want to send a message to another PushApp we should :
- Create a new instance of the Sender
- Expose setters to update the config.
So , wdyt ?
_______________________________________________
aerogear-dev mailing list
aerogear-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/aerogear-dev