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

regresssion in 8.2.5 : ApplyTo method changes OrderBy property #1319

Open
hendrik-schmieder opened this issue Sep 25, 2024 · 7 comments
Open
Assignees
Labels
bug Something isn't working follow-up

Comments

@hendrik-schmieder
Copy link

Hello,

I have an object queryoptions of type ODataQueryOptions
where the OrderBy property is null.

For this object i call the method
ApplyTo(IQueryable, ODataQuerySettings)
where ODataQuerySettings is new ODataQuerySettings()

In 8.2.4 OrderBy property is still null,
after the return of the ApplyTo method

Begiining with 8.2.5 OrderBy property is NOT null anynore,
after the return of the ApplyTo method

I don't see any hints that the the behaviour of Applyo have changed.

tia

Hendrik

@hendrik-schmieder hendrik-schmieder added the bug Something isn't working label Sep 25, 2024
@habbes
Copy link
Contributor

habbes commented Oct 1, 2024

Hello @hendrik-schmieder What is the value of OrderBy in 8.2.5. My suspicion is that this could be due to changes we made to support $orderby and other advanced expressions in skip tokens: #1164. Do you have a repro project we could use to investigate? Let me look into it and get back to you.

@habbes
Copy link
Contributor

habbes commented Oct 1, 2024

@hendrik-schmieder another question, are you running into any issues as a result of the OrderBy property no longer being null after calling ApplyTo

@habbes
Copy link
Contributor

habbes commented Oct 1, 2024

@hendrik-schmieder I've not been able to reproduce the issue so far. Could you share a sample project that reproduces the issue?

@habbes habbes self-assigned this Oct 1, 2024
@hendrik-schmieder
Copy link
Author

@habbes

Sorry for the delay,
but I had an extended weekend.

It is not that easy to give a simple example.

Maybe the following information will help.

The url used for testing looks like

...?$skip=1

The ApplyTo method is called inside the following method

    public IEnumerable<T> ApplyQuery<T>(ODataQueryOptions<T> queryOptions, IEnumerable<T> cells)
    {
        var tmpCells = cells.ToList();

        var noOrder = queryOptions.OrderBy == null;

        if (queryOptions.SelectExpand == null && queryOptions.OrderBy == null)
    [
            var filtered = queryOptions.ApplyTo(tmpCells.AsQueryable(), new ODataQuerySettings()) as IEnumerable<T>;
            return filtered;
        }
        
    // additional code

   }

This method is called two times while processing the url and on the second call queryOptions.OrderBy is NOT null anymore.

The first call looks like

var fetchedCells = new ConcurrentBag();

// fill fetchedCells in parallel.

var tmpCells = _oDataQueryFilter.ApplyQuery(fetchCellExportDTO.QueryOptions, fetchedCells);

with T =

public class Cell
{
[AutoExpand]
public IDictionary<string, object> DynamicProperties { get; set; }

public virtual decimal? Value { get; set; }

public virtual string StringValue { get; set; }

public virtual string Path { get; set; }

[Key]
public virtual int Id { get; set; }

// contructors ...

}

It may be that fetchedCells) is not ordered by id,
but tmpCells is ordered by id and the entry with the lowest id is missing as desired.

In 8.2.4 OrderBy is still null, but in 8.2.5 I get for OrderBy

-queryOptions.OrderBy {Microsoft.AspNetCore.OData.Query.OrderByQueryOption}
Compute null Microsoft.AspNetCore.OData.Query.ComputeQueryOption

  • Context {Microsoft.AspNetCore.OData.Query.ODataQueryContext}
  • OrderByClause {Microsoft.OData.UriParser.OrderByClause}
  • OrderByNodes Count = 1 System.Collections.Generic.IList<Microsoft.AspNetCore.OData.Query.OrderByNode>
    RawValue "Id" string
    Validator {Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator}

I hope this helps.

1 similar comment
@hendrik-schmieder
Copy link
Author

@habbes

Sorry for the delay,
but I had an extended weekend.

It is not that easy to give a simple example.

Maybe the following information will help.

The url used for testing looks like

...?$skip=1

The ApplyTo method is called inside the following method

    public IEnumerable<T> ApplyQuery<T>(ODataQueryOptions<T> queryOptions, IEnumerable<T> cells)
    {
        var tmpCells = cells.ToList();

        var noOrder = queryOptions.OrderBy == null;

        if (queryOptions.SelectExpand == null && queryOptions.OrderBy == null)
    [
            var filtered = queryOptions.ApplyTo(tmpCells.AsQueryable(), new ODataQuerySettings()) as IEnumerable<T>;
            return filtered;
        }
        
    // additional code

   }

This method is called two times while processing the url and on the second call queryOptions.OrderBy is NOT null anymore.

The first call looks like

var fetchedCells = new ConcurrentBag();

// fill fetchedCells in parallel.

var tmpCells = _oDataQueryFilter.ApplyQuery(fetchCellExportDTO.QueryOptions, fetchedCells);

with T =

public class Cell
{
[AutoExpand]
public IDictionary<string, object> DynamicProperties { get; set; }

public virtual decimal? Value { get; set; }

public virtual string StringValue { get; set; }

public virtual string Path { get; set; }

[Key]
public virtual int Id { get; set; }

// contructors ...

}

It may be that fetchedCells) is not ordered by id,
but tmpCells is ordered by id and the entry with the lowest id is missing as desired.

In 8.2.4 OrderBy is still null, but in 8.2.5 I get for OrderBy

-queryOptions.OrderBy {Microsoft.AspNetCore.OData.Query.OrderByQueryOption}
Compute null Microsoft.AspNetCore.OData.Query.ComputeQueryOption

  • Context {Microsoft.AspNetCore.OData.Query.ODataQueryContext}
  • OrderByClause {Microsoft.OData.UriParser.OrderByClause}
  • OrderByNodes Count = 1 System.Collections.Generic.IList<Microsoft.AspNetCore.OData.Query.OrderByNode>
    RawValue "Id" string
    Validator {Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator}

I hope this helps.

@hendrik-schmieder
Copy link
Author

@habbes

ODataRoutingSample2.zip

I made a copy of the existing sample ODataRoutingSample and added some files

Make breakpoints in Line 44, 48, 52 of
Controllers/CellExportController.ss
and look at the values of noOrder
and stillNull

uses as test url
{{base_url}}/databases(23)/Cubes(33)/Cells?$skip=1
with {{base_url}}==http://localhost:5000

@hendrik-schmieder
Copy link
Author

Has anybody tried the testcase I provided in my last post ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working follow-up
Projects
None yet
Development

No branches or pull requests

2 participants