OData / AspNetCoreOData

ASP.NET Core OData: A server library built upon ODataLib and ASP.NET Core
Other
455 stars 157 forks source link

Odata EDM models causes slow startup #956

Closed danielpastoor closed 1 year ago

danielpastoor commented 1 year ago

When I use a list to models in the ODataConventionModelBuilder and then I try to start the application it sometimes takes 1 minute to start. Do you know how I can optimize this because it can be irritating for the development of the project?

Information I use 5 entity models with 10 properties each. With some relations between each other

julealgon commented 1 year ago

Can you elaborate on this part here?

When I use a list to models in the ODataConventionModelBuilder...

It is not clear exactly what you mean there.

1 minute seems very excessive so I wonder if there is something wrong somewhere. Maybe if you could share a sample it would be even better.

Having said that, this is an interesting topic for the OData team to consider: the EDM could be generated at build time using a source generator. Is this something the team has explored?

danielpastoor commented 1 year ago

Thank you for responding.

I do have my method code to create the EDM model. But I will create a sample project tomorrow:

The GetEdmModels method get's all my odata controllers based on a base controller. Then it will get the name and the model type.

public static Dictionary<string, IEdmModel> GetEdmModels(Assembly asembly, string defaultVersion = "v1")
{
    Dictionary<string, ODataConventionModelBuilder> odataConventionBuilders = new();

    foreach (var item in asembly.GetTypes().Where(x => typeof(ODataController).IsAssignableFrom(x)))
    {
        var baseType = item.BaseType;

        var routeAttribute = item.GetCustomAttributes(typeof(GraphVersionAttribute)).FirstOrDefault();
        var routeVersion = routeAttribute != null ? ((GraphVersionAttribute)routeAttribute).VersionNumber : defaultVersion;

        if (string.IsNullOrEmpty(routeVersion))
            routeVersion = "none";

        if (!odataConventionBuilders.ContainsKey(routeVersion))
            odataConventionBuilders.Add(routeVersion, new ODataConventionModelBuilder());
    }

    return odataConventionBuilders.ToDictionary(x => x.Key, x => x.Value.GetEdmModel());
}

And then it adds the edm models with the AddRouteComponents method :

builder.Services.AddControllers()
    .AddOData(opt =>
    {
        foreach (var item in EdmModelGenerator.GetEdmModels(controllerAssembly, defaultVersion))
        {
            opt.AddRouteComponents(string.IsNullOrEmpty(item.Key) || item.Key == "none" ? routePrefix : routePrefix + "/" + item.Key, item.Value);
        }

        opt.EnableQueryFeatures(maxTopValue: null);

        opt.RouteOptions.EnableNonParenthesisForEmptyParameterFunction = true;
    });

I did do some profile testing because first I thought my reflection was the problem but that is really fast. But when I add the EDMModel to it with the opt.AddRouteComponents() method then it takes a long time.

julealgon commented 1 year ago

Are you attempting to do some sort of manual versioning @danielpastoor ? If so, have you considered using AspVersioning instead? It should have full OData support now:

How many EDM models is your logic currently returning?

EDIT: Maybe @commonsensesoftware can provide some guidance here in regards to versioning with multiple EDMs.

commonsensesoftware commented 1 year ago

A repro would certainly help confirm things.

The OData startup process generates the completed route templates for each prefix. Since you are versioning by URL segment (which isn't RESTful), this will lead to many different prefixes and route template generation passes for each prefix.

API Versioning avoids this issue by using a route template parameter. Whereas the above approach will call AddRouteComponents for each version, API Versioning would call it just once using the prefix {version:apiVersion}. The default route constraint name is apiVersion, but you can configure something else if you really want to. version is typically the route parameter name used, but you can use any name you like. You will still end up with an EDM per API version, but far fewer route templates are generated. An EDM-per-version is the preferred way to keep different configurations separate.

You're free to implement your own versioning code, but, as @julealgon mentioned above (which is the package you want), ASP.NET API Versioning is the established approach to solve that problem for you.

gathogojr commented 1 year ago

@danielpastoor Thank you for reporting this issue. As the other contributors said, it'd help if you could share a repro or a sample project that can help us dissect/troubleshoot the issue. It'd also help to the stats you're observing from your profiling.

gathogojr commented 1 year ago

@danielpastoor I'll go ahead and close this issue since there isn't sufficient details to help us investigate. Feel free to reopen with more details that can help dissect the issue.