No way to override Primitive Serialization and deserialization of Payload. #108

[shahzorkhan123]

Shahzor Khan @ 30-Mar-15 5:38:46 PM
No way to override Primitive Serialization and deserialization of Payload. #108

     Added a serializer/deserializer extensibility point, to convert Json
     Payload value to Primitive type. And allowed skipping the validation.

 Shahzor Khan @ 30-Mar-15 6:14:05 PM
     Fixed testcase failures

 Shahzor Khan @ 30-Mar-15 8:46:51 PM
     Added Testcase BinaryPayloadAsStringRoundtripJsonLightTest

     1-Passes values as Binary, and expect the standard Base64 string,
     2-Passes values as string, and expect pass through,

[msftclas]

Hi @shahzorkhan123, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

[karataliu]

As you're using global static variable to store the converter here, all models would shall the same value converter. This would breaks when a service wants to deal with 2+ models with different configurations at the same time.

We can consider leverage model.SetAnnotationValue & model.GetAnnotationValue to make it a per model settings. Please refer to src\Microsoft.OData.Edm\ExtensionMethods::SetPrimitiveValueConverter for more information.

Also the tests run in PrimitiveValuesRoundtripJsonLightTests class would randomly fails as one case would call global SetPrimitivePayloadValueConverter which changes global serialization/deserialization behaviour.

[shahzorkhan123]

@karataliu: I didn't added it there, because this is very limited to Payload serialization/deserialization, and shouldn't be added Edm Lib.
And the only hidden annotations on model are the internal namespace annotations.The internal namespace should not be visible outside EdmLib.

I can however expose the an API to
AddCustomPropertyToModel(string propertyName, object value), and
object GetCustomPropertyToModel(object value),
And use that to store the PayloadConverter on the model, and retrieve from it.

Please suggest.

[karataliu]

Yes, I didn't mean you should put those inside EdmLib. But just to show that this is a way to make some object bound to certain model.

My point is the custom payload value converter should not be a global static configuration, but a model specific one.

I'd suggest APIs to be something like the following in ExtensionMethods.cs of ODataLib:

public static void SetPrimitivePayloadValueConverter(this IEdmModel, IPrimitivePayloadValueConverter)
public static IPrimitivePayloadValueConverter GetPrimitivePayloadValueConverter(this IEdmModel)

You might maintain a static global dictionary to map the model with certain converter, but the dictionary size would keep growing if you're to continue adding new models to it, as some service may use dynamic models. But if using model.SetAnnotationValue that won't be an issue.

[shahzorkhan123]

Updated as per discussion.

[karataliu]

We're reviewing the code. Thanks.

[karataliu]

Merged in commit 11c4467