Skip to content

Add odata metadata properties to ODataValue #513

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

Closed
crackalak opened this issue Jul 8, 2019 · 8 comments
Closed

Add odata metadata properties to ODataValue #513

crackalak opened this issue Jul 8, 2019 · 8 comments
Assignees

Comments

@crackalak
Copy link

Can metadata be added to ODataValue to support using count?

When using the following attribute parameters
[ProducesResponseType( typeof( ODataValue<IEnumerable<Color>> ), Status200OK )]

It currently produces this as the model:

{
    "value": []
}

Instead of what is really returned:

{
    "@odata.context":"http://localhost/$metadata#Collection(NS.Color)",
    "@odata.count": 3,  
    "@odata.nextLink":"http://localhost/Customers(5)/Colors?$count=true&$skip=2",
    "value": []
}

I have tried creating a class for it myself, however, the swagger docs produced then don't have most of the odata parameters like $filter, $orderby etc.

@commonsensesoftware commonsensesoftware self-assigned this Jul 8, 2019
@commonsensesoftware
Copy link
Collaborator

When you say that you "tried creating a class for it myself...", do you mean that you extended ODataValue<T> or you provided your own stand-in class?

These particular attributes are called annotations; specifically instance annotations. The @odata prefix is reserved for annotations produced by OData itself; however, it is possible to have your own annotations.

Your request isn't without merit, but there are some interesting challenges to supporting it. These particular annotations are driven by the media type parameter odata.metadata, which can have a value of full, minimal, or none. When the value is none, no annotations are present and the response looks like vanilla JSON. For example, if the media type is application/json; odata.metadata=none.

To complicate things further, the possible annotations can depend on supported query options. For example, if $count is not allowed, then you'll never get the @odata.count annotation is responses.

It's worth taking a look at this more in-depth, but I wanted to provide some context with regard to the complexities involved. Subclassing ODataValue<T> should be the bridge for this capability in the meantime.

Thanks.

@crackalak
Copy link
Author

I provided my own stand-in class, I'll extend ODataValue and see how I get on.

I had a feeling it wouldn't be simple with the number of OData options!

Thanks for responding so quick 😃

@crackalak
Copy link
Author

Extending ODataValue also affects the response model, it ignores changes to the entity type configuration so something like order.Ignore(o => o.Description); does nothing and the Description property is still listed in the model shown in the swagger doc.

@commonsensesoftware
Copy link
Collaborator

You're not by chance trying to actually return ODataValue<T> in your responses are you? If you are, you shouldn't be. ODataValue<T> is purely a stand-in type to respresent OData's output. The API Explorer and Swagger generators work off of the Reflection API. OData writes a protocol-specific output that is not formally captured as a type. This makes it impossible for Swagger generators to document the output without formal knowledge of OData. By using ODataValue<T> in the [ProducesResponseType] attribute, you can provide a surrogate type that is in the correct shape of the output. If you use ODataValue<T> as an actual action response type, it will almost certainly come out with the wrong result and it look rather strange. That would also explain why other EDM configurations are not being honored.

Ultimately, to get the correct output, the API Explorer unwraps T in ODataValue<T>, performs any required substituations according to the EDM, and then wraps it back up into the original ODataValue<T> as a new type. That should work even with a custom or extended implementation (but I honestly have not ever tried it).

If you can share what your extended type looks like and how you've applied it to an action, I can see if I can't reproduce the behavior and/or come up with some other recommendations.

@crackalak
Copy link
Author

No, I'm returning IQueryable<T>. Here's what I have:

[ODataRoute]
[ProducesResponseType(typeof(ODataValueWithCount<IEnumerable<CustomerOutputModel>>), Status200OK)]
[EnableQuery(AllowedQueryOptions = Select | Skip | Top | Count | OrderBy | Filter, 
                AllowedOrderByProperties = "accountNumber,company")]
public IQueryable<CustomerOutputModel> Get()
{
    var customers = _customerService.GetCustomers();
    return customers;
}
public class ODataValueWithCount<T> : ODataValue<T>
{
    [JsonProperty("@odata.context")]
    public string ODataContext { get; set; }
    [JsonProperty("@odata.count")]
    public int ODataCount { get; set; }
    [JsonProperty("@odata.nextLink")]
    public string ODataNextLink { get; set; }
}
public void Apply(ODataModelBuilder builder, ApiVersion apiVersion)
{
    var account = builder.EntitySet<CustomerOutputModel>("Customers").EntityType;

    account.Ignore(p => p.AccountIndex);
    account.HasKey(k => new { k.AccountNumber });
}

And this is how swagger looks:

image

image

@crackalak
Copy link
Author

@commonsensesoftware Have you got everything you need on this or can I help further?

@commonsensesoftware
Copy link
Collaborator

Everything you have is correct. The issue that the IsODataValue extension method isn't correct. As written, it will only allow types of ODataValue<T> (no inheritance), but that wasn't the intent. This is a pretty straight forward change. The fix will go out in the next patch. Stay tuned.

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

No branches or pull requests

2 participants