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

[Design] Use DI for the serializer provider and all the serializers #301

Closed
lewischeng-ms opened this issue Feb 25, 2016 · 7 comments
Closed
Assignees
Milestone

Comments

@lewischeng-ms
Copy link
Contributor

So that users can easily replace them with their customized ones. Now it is so hard to override the serializer behavior.

@rayao
Copy link
Contributor

rayao commented Mar 6, 2016

I'm not very familiar with WebApi configuration flow, so maybe this question is stupid.
How the serializer/deserializer providers get context info? It seems to me RestierFormatting attribute applies to RestierController, so it only gets a controller descriptor, but although every Api uses RestierController type, different Api may have some specific serializer/deserializer, and that means the serializer providers must know route info (to get an Api instance for example). During attribute initialization there's only controller specific configuration settings, so it's impossible there. And at runtime, only GetODataPayloadSerializer gets routing info, GetEdmTypeSerializer gets nothing contextual, same for deserializer.

@lewischeng-ms
Copy link
Contributor Author

I think your concerns are reasonable. GetEdmTypeSerializer and GetEdmTypeDeserializer won't get the context info HOWEVER each single serializer/deserializer will get the context info from a parameter called writeContext when being called (e.g., https://github.com/OData/RESTier/blob/master/src/Microsoft.Restier.WebApi/Formatter/Serialization/RestierComplexTypeSerializer.cs). But what I am really worried about here is how to access the ApiBuilder and the ApiConfiguration.

@lewischeng-ms lewischeng-ms self-assigned this Mar 7, 2016
@rayao
Copy link
Contributor

rayao commented Mar 10, 2016

You're right, serializer/deserializer have access to RequestMessage, with ApiFactory in Properties.
It seems to me we'd have to move the Api into Request.Properties also.

@ivanko2000
Copy link

"Pinging" this issue in case it went on the back burner...

I am currently trying to implement a deserializer for an entity with binary property (so when POSTing a new instance one could pass the JSON for entity definition and binary content for one of the properties in "multipart/form-data"). So far I haven't been successful in making it work, due to a combination of me being a newbie and the above mentioned

Now it is so hard to override the serializer behavior

@chinadragon0515
Copy link
Contributor

@ivanko2000 thanks for your patience, Lewis is busy on other critical items, I discussed with him and take this from him today, it is my TODO items in the next several days. And plan to be part of 0.5.0 release.

@chinadragon0515
Copy link
Contributor

chinadragon0515 commented Apr 28, 2016

@lewischeng-ms @rayao I am investigating this item and thinking about the design we may have,

Update per discussion with Lewis.

Facts,

  1. Format is initialized when application is started, and a formatter need to be set.
  2. For the serializer provider and serializer, there are some cross reference like for EntityTypeSerializer it needs Provider for further get other needed serializer.

So here is what I am thinking,

  1. We will have two provider for Formatter initialization, and these two are not part of DI services.
  2. We will create two new providers for DI services which has the same logic we have now.
  3. In the two providers for formatter initialization, when method GetODataPayloadSerializer is called, it will get ApiContext from request and store in local variable.
    Then it will get two new providers service, set the ApiContext too, then call new provider to return the services.

Here we need to set Api or context into request but not Api factory as it will cause a new instance.

  1. User can override new provider to customize the logic with his own serializer.
  2. Serializer and Deserializer itself are not registered as DI services.

Comments? Suggestions?

@chinadragon0515
Copy link
Contributor

The issue is fixed and merged, doc is added, close the issue.

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