Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Json serialization issure, not compatible types #391

Open
charconstpointer opened this issue Apr 3, 2019 · 8 comments
Open

Json serialization issure, not compatible types #391

charconstpointer opened this issue Apr 3, 2019 · 8 comments

Comments

@charconstpointer
Copy link

Hey, have you encoutered any problems with newtonsoft’s serializer? Let’s say i have an event SomeEvent:IEvent in two+ of my microserivces, whenever i’m listening for this event with rabbitmq i get the message but because of the object’s namespace Newtonsoft cant parse it properly and is throwing exceptions

for example

Type specified in JSON ‘Orders.OrderCreated’ is not compatible with ‘Notifications.OrderCreated’ i thought it would just try to match the props? Can you please point me in the right direction?

Using version 1.10.4

Thanks

@alexdawes
Copy link

Newtonsoft encodes the type in a special $type field, and this is used for deserialisation to the correct type. This is as intended, as otherwise during deserialisation the consumer would have to take a guess at the type of the request and could easily get this wrong. Trying to match the props does not give the required guarantee that the type is correct.

You will likely need to share your OrderCreated class in a common library that both consumer and producer can use.

@cocowalla
Copy link

Also, if you want to customise serialisation/deserialisation (sounds like you might need to), you can use a JsonConverter

@charconstpointer
Copy link
Author

Hey, thank you for responding, @alexdawes your solution is what i ended up doing, but i with this project i was trying to follow these guys https://github.com/devmentors, and they are keeping their models, events and commands in projects that require it not in a common 'global' project,

for exmaple https://github.com/devmentors/DNC-DShop.Services.Orders/tree/master/src/DShop.Services.Orders
and https://github.com/devmentors/DNC-DShop.Services.Notifications/tree/master/src/DShop.Services.Notifications

any idea how they bypassed it?

@charconstpointer
Copy link
Author

Please check projects above, take OrderCreated for an exmaple, Notifications app subscribes to OrderCreated which is an Event class owned by Notifications app, but the event itself is emitted by Orders app, two different classes, somehow are serialized properly

@alexdawes
Copy link

Interesting!

The Notifications app uses a [MessageNamespace("orders")] attribute to mark this as coming from the Orders app: https://github.com/devmentors/DNC-DShop.Services.Notifications/blob/master/src/DShop.Services.Notifications/Messages/Events/OrderCreated.cs#L7

This is then used within a custom RabbitMQ NamingConventions in the common lib (https://github.com/devmentors/DNC-DShop.Common/blob/master/src/DShop.Common/RabbitMq/Extensions.cs#L92) to ensure that the Exchange and Routing Key can be common between the two messaging parties.

However, I've no idea how the app is avoiding the Newtonsoft serialisation issue that you are seeing. It could be that I am simply wrong, and either it is possible to override the JsonSerialiserSettings being used by Rabbit, or else the $type field can be forcibly ignored somehow allowing type coersion.

Probably worth pointing out that I am not a RawRabbit expert - your ticket interested me and I thought I could help out, but its possible my understanding of things is simply wrong.

For what its worth as well, the setup used by DShop seems a little flimsy and liable to fall over if types change or are moved between namespaces etc. I therefore would still recommend sharing commands/events via a common lib where possible, even if you can work it out, as at least then you can publish new versions of the common lib for clients to use if there are breaking API changes.

I'll try to take a more in depth look at things later, as I'm keen to understand whats being done here.

@alexdawes
Copy link

Ahh I may have worked out where I was wrong.

RawRabbit uses TypeNameHandling.Auto: https://github.com/pardahlman/RawRabbit/blob/2.0/src/RawRabbit/DependencyInjection/RawRabbitDependencyRegisterExtension.cs#L57

This means that the $type key is only used during serialisation when the object being serialised is not the same as its declared type: https://www.newtonsoft.com/json/help/html/T_Newtonsoft_Json_TypeNameHandling.htm

My guess here is that DShop is using types where this is not the case, and so $type is never used and hence deserialisation doesnt find it and complain. As mentioned above, looks like routing is manually handled by the use of attributes.

@cocowalla In your example above were your OrderCreated objects derived types, or else passed into the BusClient as ICommands or something similar?

@simon-davis
Copy link

I have ran into the same issue but have set TypeNameHandling.None using a custom serializer which made Publish/Subscribe work perfectly.

However... with Request/Respond the deserialization is reading the type from the Headers.message_type and using object Deserialize(BasicDeliverEventArgs args) which is using the fully qualified type from headers and causes a casting issue. I am still trying to resolve this in a tidy manner, any suggestions?

@alexdawes
Copy link

Interesting - @simon-davis did you get anywhere with this?

From a quick glance in the source code, I cant actually see any use of PropertyHeaders.MessageType (in RawRabbit.Common) so its hard to find where this header is actually being set or used (during deserialisation). Maybe its part of RabbitMQ.Client instead, but that wouldnt explain why the deserialisation is different between Publish/Subscribe and Request/Response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants