OData / RESTier

A turn-key library for building RESTful services
http://odata.github.io/RESTier
Other
472 stars 135 forks source link

Feature request: access to Microsoft.OData.IContainerBuilder is needed during MapRestier execution #688

Closed vvdev closed 10 months ago

vvdev commented 3 years ago

Hello.

It would be great and helpful to be able to register some services, and especially OData ServicePrototypes when we call MapRestier.

Currently, to override, for example, ODataMessageReaderSettings.MessageQuotas (of type ODataMessageQuotas) we have to parse collection of existing service registrations, replace existing with new, ensure that we keep other possibly preconfigured values when override values we need and so on.

If we could access OData ContainerBuilder we would be able to do custom advanced configuration in a way recommended by OData team, example: https://github.com/OData/WebApi/issues/2163#issuecomment-632512766

I think it should not be too complicated to accept another optional parameter of type Action and execute it from inside of container builder lambda, passed to routeBuilder.MapODataServiceRoute in RestierRouteBuilderExtensions.MapRestier.

Thank you.

robertmclaws commented 3 years ago

Hey @vvdev! Actually, you can already do this without needing access to the ContainerBuilder. Check out this sample: if you look closely, you're adding services to the container for that particular route.

Note that if you have more than one API registered in your app, you'll need to register you service replacement in both containers.

LMK if you need any help with this. Thanks!

vvdev commented 3 years ago

Hello @robertmclaws ,

Thanks for your response.

Yes, i looked into these methods during my investigation, maybe i missed something, but for me it looks like that no one of them affects ODataMessageReaderSettings.MessageQuotas values. RestierBatchHandler simply resolves ODataMessageReaderSettings from container and then uses to read/deserialize batch body. And as soon as we do not have API to set these Microsoft.OData defaults, then default value of MessageQuotas is always used. Right today i spent several hours trying to find a way to set ODataMessageQuotas.MaxOperationsPerChangeset - default 1000 was not enough for couple of our calls.

As i wrote, i found quite hacky workaround, but i'm really sure, that access to Microsoft.OData.IContainerBuilder of routeBuilder.MapODataServiceRoute would make life much easier and also future-proof: we always will have access to the whole underlying Microsoft.OData configuration API without a need to introduce additional methods/services to RESTier.

Thank you again.

vvdev commented 3 years ago

...Please note, API i was missing is this call: .AddServicePrototype(new ODataMessageReaderSettings { MessageQuotas = new ODataMessageQuotas { MaxPartsPerBatch = 256 } }) ( https://github.com/OData/WebApi/issues/2163#issuecomment-632512766 )

There is no way to register ServicePrototype without access to Microsoft.OData.IContainerBuilder.

I tried to AddService, funny thing my factory was executed, but somehow not for every batch request, our huge batch requests were failing regardless my registration.

Only way to get stable result was:

  1. Find existing (default) service registration of ODataMessageReaderSettings
  2. Remove this registration
  3. Register my own factory.

Also, to preserve possible configuration of ODataMessageReaderSettings done by other code, i also preserve service registration i found on step 1 and in my factory i first resolve instance from this registration and then apply changes i need.

Again, my point is: it is possible to workaround, but i assume it would be much more error-proof and future-proof to use OData ServicePrototype.

PS: great thanks for your commitment to RESTier, it is simply saver for those, who remember WcfDataServices and do not like to write all this boilerplate code manually for every single entity type :)

robertmclaws commented 3 years ago

@vvdev, thanks so much for those kind words, I really appreciate it.

I'm not sure why ServicePrototypes exist. I'm sure there is a good reason, I just can't find one. Can you share the code you used with me? I'll create an extension method on IServiceCollection that can accept an ODataMessageReaderSettings object and replace the built-in instance with a new one. I can use reflection to create the ServicePrototype internally.

Thanks!

vvdev commented 3 years ago

@robertmclaws , ok, i maybe got too focused on missing ServicePrototype registration feature.

I will refactor my code to make it more generic and clear, retest it and will post it here.

Actually i'm not a big expert in WebAPI/OData internal APIs, so i will relay on your decision, but please think again, don't we better need an access to Microsoft.OData.IContainerBuilder, considering that it is not really complex implementation.

Thank you again.

robertmclaws commented 3 years ago

No, sorry if that came across wrong. That wasn't on you. I was passing on that I honestly don't know what they were thinking with the need for that class. It doesn't make a lot of sense. BUT you're not the only person that's gonna need to do this, so I'd like to give you a way to make it easier.

robertmclaws commented 3 years ago

Hey @vvdev, just an FYI, in OData 8.0 I was able to change the architecture to move all service registration calls on top of ServiceCollection instead, and make ContainerBuilders a totally internal implementation, with the ultimate goal of removing them entirely from the platform moving forward.

Unfortunately, you still have to deal with them in Restier at the moment, but we're hoping to change that soon.

I'm working with the OData team to identify what ServicePrototypes are and have them eliminated from the architecture as well. In the meantime, I'm going to provide an extension method to replace the ODataReader and Writer settings the same way you did. Hopefully that will help ease the situation a bit.