Skip to content

Extend ResourceDefinition #477

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
wisepotato opened this issue Feb 20, 2019 · 7 comments
Closed

Extend ResourceDefinition #477

wisepotato opened this issue Feb 20, 2019 · 7 comments
Assignees

Comments

@wisepotato
Copy link
Contributor

wisepotato commented Feb 20, 2019

Description

To allow for greater flexibility and reliability using filters we need to change a few things:

We need to change the dependency on Include(string path) in

https://github.com/aspnet/EntityFrameworkCore/blob/473dafce1d2b2e2e2dfd49eb085b9c4bc220658a/src/EFCore/EntityFrameworkQueryableExtensions.cs#L2452-L2469

We then need to make sure that when we have the following cases:

users?include=posts
posts?include=users

and we have a ResourceDefinition on users, that on both urls the Users get filtered. An example of this would be a rule that the current user that is requesting the url cannot see user with an id of 4. Or that a user cannot patch a post with a certain type.

This will require a change in ResourceDefinition and the implementation thereof in the Service and Repository layer.

The repository will need to be changed:

public virtual IQueryable<TEntity> Include(IQueryable<TEntity> entities, string relationshipName)
{
if (string.IsNullOrWhiteSpace(relationshipName)) throw new JsonApiException(400, "Include parameter must not be empty if provided");
var relationshipChain = relationshipName.Split('.');
// variables mutated in recursive loop
// TODO: make recursive method
string internalRelationshipPath = null;
var entity = _jsonApiContext.RequestEntity;
for (var i = 0; i < relationshipChain.Length; i++)
{
var requestedRelationship = relationshipChain[i];
var relationship = entity.Relationships.FirstOrDefault(r => r.PublicRelationshipName == requestedRelationship);
if (relationship == null)
{
throw new JsonApiException(400, $"Invalid relationship {requestedRelationship} on {entity.EntityName}",
$"{entity.EntityName} does not have a relationship named {requestedRelationship}");
}
if (relationship.CanInclude == false)
{
throw new JsonApiException(400, $"Including the relationship {requestedRelationship} on {entity.EntityName} is not allowed");
}
internalRelationshipPath = (internalRelationshipPath == null)
? relationship.RelationshipPath
: $"{internalRelationshipPath}.{relationship.RelationshipPath}";
if(i < relationshipChain.Length)
entity = _jsonApiContext.ResourceGraph.GetContextEntity(relationship.Type);
}
return entities.Include(internalRelationshipPath);
}

Environment

  • JsonApiDotNetCore Version: v3.0.0
@wisepotato wisepotato mentioned this issue Feb 20, 2019
@wisepotato
Copy link
Contributor Author

@jaredcnance the one problem i'm facing at the moment is that there isnt a cache for all ResourceDefinitions. For this to work, we need to either register the ResourceDefinitions in the startup.cs (which would be .. suboptimal) or in the constructor of ResourceDefinition<TItem>.

I'd say both; but I'd like to know your opinion on the matter on how to approach. I made a very basic example of what I would like to achieve with OnList being the function called on the listing of a resource, other names would be OnPatch, OnResourcePatch or any combination, and maybe even a general function.

@wisepotato
Copy link
Contributor Author

Ok, took a detour to get the ResourceDefinition problem on track, but currently encountering the TreeBuilding, any suggestions on how to tackle? This would alleviate a lot of trouble with deeply nested problems. We could then recursively go down the tree and apply any rules that the user has laying around. ALlowing for maximum flexibility

@jaredcnance
Copy link
Contributor

We do not need a cache for ResourceDefinition they are cached by the IoC container in the request scope.

@wisepotato
Copy link
Contributor Author

How can I access them without injecting them all into the repository? I couldnt figure out how to grab all resourcedefinitions..

@jaredcnance
Copy link
Contributor

jaredcnance commented Feb 21, 2019

See my comment in the PR about GenericProcessorFactory.

@wisepotato wisepotato added this to the v3.2.0 milestone Apr 23, 2019
maurei added a commit to wisepotato/JsonApiDotNetCore that referenced this issue Apr 25, 2019
@maurei
Copy link
Member

maurei commented Apr 29, 2019

Most correspondence about this done in #478

@maurei maurei closed this as completed Apr 29, 2019
wisepotato pushed a commit to wisepotato/JsonApiDotNetCore that referenced this issue May 2, 2019
@maurei maurei reopened this May 2, 2019
wisepotato pushed a commit to wisepotato/JsonApiDotNetCore that referenced this issue May 8, 2019
@wisepotato wisepotato modified the milestones: v3.2.0, v4.0 May 17, 2019
maurei added a commit to wisepotato/JsonApiDotNetCore that referenced this issue Jun 4, 2019
maurei added a commit to wisepotato/JsonApiDotNetCore that referenced this issue Jun 12, 2019
maurei added a commit that referenced this issue Jun 17, 2019
@maurei
Copy link
Member

maurei commented Jun 20, 2019

Resource hooks have been merged and released in v4.0.0-alpha3

@maurei maurei closed this as completed Jun 20, 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

3 participants