It seems there is room to improve the current CarbonMessage implementation.
Following are few main areas where we can improve its functionality.
- At the moment there are too many ways to get the message body from the
CarbonMessage
- getMessageBody()
- getInputStream() - This is a http specific impl.
- getMessageDateSource()
The above 3 methods serve 3 different purposes. This design has been done
mainly thinking about the HTTP messages. In a message passthrough scenario,
content is kept in the content queue and accessed as a stream. When the
message content is accessed within intermediate layer (like ballerina), it
will be retrieved through messageDataSource.
- There are too many types of CarbonMessages in carbon-messaging that
are only used by a specific transport implementation
- BinaryCarbonMessage
- ControlCarbonMessage
- DefaultCarbonMessage
- MapCarbonMessage
- SerializableCarbonMessage
- StatusCarbonMessage
- StreamingCarbonMessage
- TextCarbonMessage
- Most of above CarbonMessages do not adhere to the contract that is
imposed by the CarbonMessage interface.
I agree on the fact that we need to refactor these different types of
CarbonMessage implementations. I think what Asitha suggested is a viable
approach to handle different types of messages.
Considering these facts, I believe we should refactor CarbonMessage before
others starting using it in their implementations. Please let us know your
thoughts as well :)
+1 for refactoring the carbon message and relevant implementations to
support non-HTTP use cases. In the meantime, we should support HTTP
messaging in an efficient manner since that will be the mostly used
scenario of ballerina.
On Wed, Jul 5, 2017 at 4:55 PM, Irunika Weeraratne <irun...@wso2.com> wrote:
> Hi Shafreen and Asitha,
> In the first place, they were introduced after a long discussion in
> WebSocket scenarios. This was because when the WebSocket messages are
> arrived to transport level we already knew what kind of message it is.
> Eg: Text, Binary, Control, Status
>
> So I made sense to create the same type of carbon message in the
> transport. Then carbon message types were used to identify the scenarios we
> had to address in the application layer.
> Eg: If the carbon message is an instance of TextCarbonMessage we know what
> to do in the first place. Also converting text into byte buffer in the
> transport and then converting it again into text in the application layer
> doesn't make sense.
>
> Because of that, those message types were introduced. IMO while we reduce
> the no. of carbon message types we might need to keep some of the types
> because it will affect the performance.
> Also, we can always override the methods in default carbon message if in
> order to work with default scenarios.
>
> WDYT?
>
> Thanks,
> Irunika
>
> *Irunika Weeraratne*
> *Software Engineer | WSO2, Inc. <http://wso2.com/>*
> *Email : irun...@wso2.com <irun...@wso2.com>*
> *LinkedIn : https://lk.linkedin.com/in/irunika
> <https://lk.linkedin.com/in/irunika>*
> *Mobile : +94712403314 <071%20240%203314>*
> *Lean . Enterprise . Middleware*
>
>
> On Wed, Jul 5, 2017 at 3:54 PM, Asitha Nanayakkara <asi...@wso2.com>
> wrote:
>
>> Hi Shafreen,
>>
>> I would suggest having a single Carbon Message with the option to set
>> different types of data sources as the data source for the message. For
>> that, we might need to go through the existing other message types and
>> think what kind of functionality to be added to carbon message. If our
>> intention is not to extend carbon message, we can make it a final class. If
>> not we have to design it to be extended by other users.
>>
>> When reading the payload multiple times (building the message) we need to
>> clone messages (since data is represented as a stream). In pass through
>> scenarios, we should avoid cloning the message and use the stream once.
>> IMO we can implement this functionality by way of having package level
>> methods for transport to read from the internal blocking queue and write to
>> the output stream. If we invoke other public methods they will make a clone
>> of the message data and use. This way we avoid unnecessary cloning in a
>> pass-through scenario. And reading from the blocking queue can only be done
>> from the transport. Outside users have to use message data source to read
>> the content, which might clone the data.
>>
>> WDYT?
>>
>> Regards,
>> Asitha
>>
>> On Wed, Jul 5, 2017 at 3:14 PM, Shafreen Anfar <shafr...@wso2.com> wrote:
>>
>>> Hi All,
>>>
>>> It seems there is room to improve the current CarbonMessage
>>> implementation. Following are few main areas where we can improve its
>>> functionality.
>>>
>>> - At the moment there are too many ways to get the message body from
>>> the CarbonMessage
>>> - getMessageBody()
>>> - getInputStream() - This is a http specific impl.
>>> - getMessageDateSource()
>>>
>>>
>>> - There are too many types of CarbonMessages in carbon-messaging
>>> that are only used by a specific transport implementation
>>> - BinaryCarbonMessage
>>> - ControlCarbonMessage
>>> - DefaultCarbonMessage
>>> - MapCarbonMessage
>>> - SerializableCarbonMessage
>>> - StatusCarbonMessage
>>> - StreamingCarbonMessage
>>> - TextCarbonMessage
>>>
>>>
>>> - Most of above CarbonMessages do not adhere to the contract that is
>>> imposed by the CarbonMessage interface.
>>>
>>> Considering these facts, I believe we should refactor CarbonMessage
>>> before others starting using it in their implementations. Please let us
>>> know your thoughts as well :)
>>>
>>> --
>>> Regards,
>>> *Shafreen*
>>> Software Engineer
>>> WSO2 Inc
>>> Mobile : 077-556-395-1
>>>
>>
>>
>>
>> --
>> *Asitha Nanayakkara* <http://asitha.github.io/>
>> Senior Software Engineer
>> WSO2, Inc. <http://wso2.com/>
>> Mob: +94 77 853 0682 <+94%2077%20853%200682>
>> [image: https://wso2.com/signature] <https://wso2.com/signature>
>>
>>
>
--
Thank you and Best Regards,
Chanaka Fernando
Senior Technical Lead
m: +94 773337238
https://wso2.com <https://wso2.com/signature>
_______________________________________________
Dev mailing list
Dev@wso2.org
http://wso2.org/cgi-bin/mailman/listinfo/dev
