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

Querying with explicit property specification (without attributes) #4

Closed
tflanitzer opened this issue Feb 9, 2018 · 6 comments
Closed

Comments

@tflanitzer
Copy link

tflanitzer commented Feb 9, 2018

I would like to be able to (optionally) explicitly specify which properties are used for the queries instead of using the Sieve attribute. Ideally, this "specification" would also allow the definition of a mapping between a query term to an actual property on the model class.

My arguments supporting this request:

  1. Different APIs for the same underlying model may support filtering or sorting on different properties.
  2. By directly using names of properties in my model classes, the domain is coupled to the query endpoint. This e.g. means that it is not possible to rename a property on a model class without a breaking change in the API.
  3. I don't want to "pollute" my domain classes with attributes that are only relevant for querying them via a REST API.

Here is an example of how the changed call to SieveProcessor could look like:

` var result = _dbContext.Posts.AsNoTracking();

        var sieveProperties = new[]
        {
            SieveProperty<Post>.For(_ => _.Title, Allow.Filter, "name"), 
            SieveProperty<Post>.For(_ => _.CommentCount, Allow.SortAndFilter), 
            SieveProperty<Post>.For(_ => _.LikeCount, Allow.Sort), 
            SieveProperty<Post>.For(_ => _.DateCreated, Allow.SortAndFilter), 
        };

        result = _sieveProcessor.ApplyAll(sieveModel, result, sieveProperties);

        return Json(result.ToList());

I have implemented a draft of the necessary changes and will open a PR. Feedback is welcome!

BTW: Great work so far!

@Biarity
Copy link
Owner

Biarity commented Feb 10, 2018

For 2, this is already supported using Name, so for example:

[Sieve(CanFilter = true, CanSort = true, Name= "custom_title_query_term_here")]
public string Title { get; set; }

I completely agree with 1 & 3 so I'll get started working on this. Will also have a look at your PR in a sec. Glad you liked Sieve!

Biarity added a commit that referenced this issue Feb 10, 2018
@Biarity
Copy link
Owner

Biarity commented Feb 10, 2018

Had a look at your PR, thanks a lot for taking the time to write that! Seems like a reasonable way of doing it, but I don't like that it uses arguments to pass the array of SieveProperty. IMO this would make it harder to share a mapping across an entire project (or even within a controller) since the mapping has to be passed in every time an Apply* method is called.

Instead, I propose having a centralized location where these mappings could be defined. One way would be to allow overriding a method of SieveProcessor, allowing defining mappings there, then using (ie. injecting) the new subclass instead. I've implemented a version of this with a Fluent API in 0bd38b8.

Here's how to use it:

public class ApplicationSieveProcessor : SieveProcessor
{
    public ApplicationSieveProcessor(
        IOptions<SieveOptions> options, 
        ISieveCustomSortMethods customSortMethods, 
        ISieveCustomFilterMethods customFilterMethods) 
        : base(options, customSortMethods, customFilterMethods)
    {
    }

    protected override SievePropertyMapper MapProperties(SievePropertyMapper mapper)
    {
        mapper.Property<Post>(p => p.Title)
            .CanFilter()
            .HasName("a_different_query_name_here");

        mapper.Property<Post>(p => p.CommentCount)
            .CanSort();

        mapper.Property<Post>(p => p.DateCreated)
            .CanSort()
            .CanFilter()
            .HasName("created_on");

        return mapper;
    }
}

Now you should inject the new class instead:

services.AddScoped<ISieveProcessor, ApplicationSieveProcessor>();

This should be familiar to developers since a similar pattern is used to extend DbContext and customize it's ModelBuilder. Let me know if you think I missed something, I'd be happy to do more work on this feature. PS. I used some of your code from #5 in my implementation.

@tflanitzer
Copy link
Author

Thank you, works for me!

@dinanrm
Copy link

dinanrm commented Apr 7, 2020

I'm sorry but after I added this code to my startup.cs :
services.AddScoped<ISieveProcessor, ApplicationSieveProcessor>();

Then I found an error message like this :

AggregateException: Some services are not able to be constructed (Error while validating the service descriptor 'ServiceType: Sieve.Services.ISieveProcessor Lifetime: Scoped ImplementationType: SkyDentalBE.Services.ApplicationSieveProcessor': Unable to resolve service for type 'Sieve.Services.ISieveCustomSortMethods' while attempting to activate 'SkyDentalBE.Services.ApplicationSieveProcessor'.)

Can anyone help me? Thank you

@dpoblacion
Copy link

Probably, you don't use custom sort/filter methods. Try to activate ApplicationSieveProcessor just using SieveOptions parameter.

public ApplicationSieveProcessor( IOptions<SieveOptions> options) : base(options) { }

@RyhneKazempour
Copy link

Hello I used this code but it doesn't work for nested class I add
mapper.Property(p => p.Contract.LoadTime)
.HasName("Contract.LoadTime")
.CanFilter();
but it is not filtered.

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

5 participants