Skip to content

Filter included relations #474

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
medokin opened this issue Feb 8, 2019 · 4 comments
Closed

Filter included relations #474

medokin opened this issue Feb 8, 2019 · 4 comments

Comments

@medokin
Copy link
Contributor

medokin commented Feb 8, 2019

Description

Let's assume I have two entities.

A Person that has many Transaction. I've implemented a TransactionService to hide some transactions from the user.

Works great when querying Transaction, but when I query a Person with include=transactions, it includes the hidden transactions.

Am I missing something, or do I need to reimplement the filter in every relation of Transaction?

Thanks for this great piece of software!

Environment

  • JsonApiDotNetCore Version: 3.0.0
  • Other Relevant Package Versions: Core 2.2
@jaredcnance
Copy link
Contributor

@medokin there are two issues here. The first is due to a design flaw in JADNC that we still need to resolve. Currently, when you include resources, this is done from the target resource service.

  • /personsPersonService
  • /persons?include=transactionsPersonService
  • /transactions?include=personTransactionService

Any logic you've defined in the TransactionService won't be invoked unless transaction is the target resource.

One option you have is to run your same filtering logic in the PersonService if the included relationship is transactions. So, instead of:

protected virtual IQueryable<TEntity> IncludeRelationships(IQueryable<TEntity> entities, List<string> relationships)
{
_jsonApiContext.IncludedRelationships = relationships;
foreach (var r in relationships)
entities = _entities.Include(entities, r);
return entities;

You might try:

foreach (var r in relationships) {
  entities = _entities.Include(entities, r);
  if(r?.Equals("transactions")) {
    // add filtering logic here
  }
}

The second issue (depending on what you're trying to do) is described here: aspnet/EntityFrameworkCore#1833 Support filtered Include. One alternative is to load transactions into memory and the perform the filtering, but this will not scale well.

@wisepotato
Copy link
Contributor

I mean doesnt this call for a more generic Access Control system? Something thats tied to the model and therefore gets implemented in every layer? This can stand on its own I guess. Just spitballin' here. Might look into this.

@wisepotato
Copy link
Contributor

Running the same logic /repeating yourself is of course not the proper way

@maurei
Copy link
Member

maurei commented Oct 15, 2019

This has been solved with the introduction of resource hooks in v4. The proper way would now be to implement the OnReturn or AfterRead for the Transaction resource.

public class TransactionResource : ResourceDefinition<Transaction>
{
    public TransactionResource(IResourceGraph graph) : base(graph) { }

    public override IEnumerable<Transaction> OnReturn(HashSet<Transaction> entities, ResourcePipeline pipeline)
    {
        return entities.Where(t => !t.ShouldHide);
    }
}

This hook will be fired in both cases:

  • /persons?include=transactions
  • /transactions

For more information, see the docs for resource hooks. Docs are a little out of date, see #552 about that

@maurei maurei closed this as completed Oct 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants