[aerogear-dev] Proposal to change the Java Sender Builder API

[Sebastien Blanc]

+1 Dan, thx for all these useful feedback
@all : Shall we do separate PR/Tickets for these items or one "bigger" Java
Sender overhaul PR ?


On Mon, Nov 10, 2014 at 5:45 AM, Daniel Bevenius <daniel.bevenius at gmail.com>
wrote:

> +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 at 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 at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>>
>
>
> _______________________________________________
> aerogear-dev mailing list
> aerogear-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/aerogear-dev/attachments/20141110/81aa205b/attachment.html