Provide ability to handle JsonApi parameters in WebApi action itself manually

[sergey-litvinov-work]

Hi Jouke,

Here is pull request for issue #120 . In our case we don't use EntityFramework, but we want to consume already parsed parameters (include\filters\page\sort), so then we can handle them manually in our WebApi controllers\actions. Current issue is that if i specify AllowsQueryAttribute, then Saule will parse request and will try to apply it automatically that isn't what we need. I tried to follow your comments in that ticket by making QueryContext as publicly accessible, but with internal setters, so for external callers it will be immutable

This pull request tries to handle such use cases:

1. Provide immutable QueryContext to WebApi action, so action can handle it manually. There is a code in AllowsManuallyHandledQueryAttribute that passes current QueryContext to action if action has it as parameter. For example in case of Include, we can check what properties were specified in QueryContext and only then we would load them from our persistent storage. Otherwise we won't load them. Currently Saule does't allow to us to do it and we need to load the whole entity with DisableDefaultIncluded attribute, and then Saule will include specified properties. But not all consumer needs all properties.
2. Disable internal Saule handling for sort\page\filter
3. Allow user to use fields in filter that aren't present in actual model. For example we have entity and it can have different IsDeleted state, and we want to specify it like &filter[include-ancient-records]=true and right now Saule will fail here as it requires that IncludeAncientRecords should be present in entity itself
4. Allow automatic WebApi binding for filters. As filter properties can be specified in kebab case - WebApi won't map them, so i added JsonApiQueryValueProvider that gets parameters from query, and then converts them to PascalCase, so WebApi can bind them. Use case here is that we can have own PersonFilter like in added tests, where i can specify custom properties. I'm a bit afraid that it will affect something else, but it should be used after default QueryStringValueProvider so it will have lower priority in model binding

I added 19 unit tests that should validate these cases. And also i verified that existing unit tests are still valid and passes.
I also have some doubts about naming and not sure that ManuallyHandledQuery is good description for that, so i'm totally open for any other suggestions.

I haven't updated any docs, as i'm sure you will have some comments about code and it might need more changes. but if you want, i can add some documenation to docs/content folder about this feature

Thanks!

[joukevandermaas]

Sorry for the delayed response. Due to the holidays I didn't have time to review this. I will take a look as soon as possible.

Thanks for contributing!

[joukevandermaas]

This looks really great. 👍

Since we are now making them public API, perhaps we should rename the contexts to something nicer, e.g. FilterContext, SortContext, IncludeContext. Same goes for IncludeProperty, SortDirection etc.

Other than that I had a few questions/suggestions but nothing major. Awesome work!

[joukevandermaas]

I wonder if this attribute is needed at all? Perhaps it's enough to detect a QueryContext parameter? I'm not sure if that's possible and if that would still cover the same use cases.

In any case maybe HandlesQuery or ManuallyHanldesQuery might be a nicer name?

[sergey-litvinov-work]

i renamed it to HandlesQuery. We can detect this parameter, but we still would need to initialize Sort\Filter\Include like we do in AllowsQueryAttribute. One way would be to move that detection to AllowsQueryAttribute, and then it will process them automatically or won't do anything depending on QueryContext parameter in action. But AllowsQueryAttribute might be a bit confusing as for me.

There is another point, that HandlesQuery attribute gives a bit more flexiblity, so developer can use QueryContext in action, and Saule still can do automatic IQueryable processing if needed. So it's up to developer, but yeah, additional attribute confusing it a bit. One point for future that might makes sense is to combine AllowsQueryAttribute and HandlesQueryAttribute and also add such enum to attribute constructor:

public enum AutomaticProcessing
{
	None = 0,
	Sort = 1,
	Pagination = 2,
	Include = 4,
	Filter = 8,
	All = Sort | Pagination | Include | Filter
}

So we can specify it in such way:

public IHttpActionResult Get()
{
	
}

and it will initialize all query context properties from request, but will apply only filtering on action result. And all other actions won't be applied. So it would have more precise way to configure what needs to be processed and what not. And then in my case i can specify AllowsQuery(AutomaticProcissing.None) that will be equivalent to current HandlesQueryAttribute and then handle QueryContext in action itself.

If you like this idea, i can update the code.

[joukevandermaas]

I do like it, but I also feel that the current approach is probably simpler. If we ever do a Saule 2.0 I'll consider cleaning up the attribute situation.

[joukevandermaas]

Does DisableDefaultIncluded make sense in combination with ManuallyHandlesQuery? At first glance it does not and maybe it should error (?). If that's hard then IMO ManuallyHandlesQuery should take precedence and all other query-related attributes should be ignored.

[sergey-litvinov-work]

That makes sense. I've added validation to HandlesQueryAttribute, so it will return an error when any\both DisableDefaultIncluded \ AllowsQuery are specified.

[sergey-litvinov-work]

Sorry, for long answer. pushed change with such renames\changes:

1. QueryContext.Filtering\Sorting\Including to Filter\Sort\Include
2. Including\Filtering\Sorting properties to Include\Filter\Sort property
3. Also renamed Filtering\Sorting intercepter to Filter\Sort interceptor
4. AllowsManuallyHandledQueryAttribute to HandlesQueryAttribute
5. renamed two test classes for interceptors from FilteringInterceptorTests\Sorting to FilterInterceptorTests\Sort
6. added validation in HandlesQueryAttribute for other attributes

Also added answers to your comments

[joukevandermaas]

Great! I will publish a new pre-release with these changes in it. Thanks for the hard work! 🎉

[sergey-litvinov-work]

Thanks!