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

[Matthias Wessendorf]

On Mon, Nov 10, 2014 at 9:00 AM, Sebastien Blanc <scm.blanc at gmail.com>
wrote:

> +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 ?
>

I am fine with having smaller PRs (easier to review)


>
>
> 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
>>
>
>
> _______________________________________________
> aerogear-dev mailing list
> aerogear-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>



-- 
Matthias Wessendorf

blog: http://matthiaswessendorf.wordpress.com/
sessions: http://www.slideshare.net/mwessendorf
twitter: http://twitter.com/mwessendorf
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/aerogear-dev/attachments/20141110/54368cc1/attachment-0001.html