Skip to content

fix/#445 #446

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

Merged
merged 13 commits into from
Nov 29, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -234,3 +234,6 @@ _Pvt_Extensions

# FAKE - F# Make
.fake/

### Rider ###
.idea/
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using JsonApiDotNetCore.Models;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using JsonApiDotNetCoreExample.Models.Entities;

namespace JsonApiDotNetCoreExample.Models.Resources
{
Expand All @@ -17,7 +18,7 @@ public class CourseResource : Identifiable
[Attr("description")]
public string Description { get; set; }

[HasOne("department")]
[HasOne("department", mappedBy: "Department")]
public DepartmentResource Department { get; set; }
public int? DepartmentId { get; set; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ public class DepartmentResource : Identifiable
[Attr("name")]
public string Name { get; set; }

[HasMany("courses")]
[HasMany("courses", mappedBy: "Courses")]
public List<CourseResource> Courses { get; set; }
}
}
11 changes: 3 additions & 8 deletions src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ protected virtual List<RelationshipAttribute> GetRelationships(Type entityType)
attribute.Type = GetRelationshipType(attribute, prop);
attributes.Add(attribute);

if(attribute is HasManyThroughAttribute hasManyThroughAttribute) {
if (attribute is HasManyThroughAttribute hasManyThroughAttribute) {
var throughProperty = properties.SingleOrDefault(p => p.Name == hasManyThroughAttribute.InternalThroughName);
if(throughProperty == null)
throw new JsonApiSetupException($"Invalid '{nameof(HasManyThroughAttribute)}' on type '{entityType}'. Type does not contain a property named '{hasManyThroughAttribute.InternalThroughName}'.");
Expand Down Expand Up @@ -211,13 +211,8 @@ protected virtual List<RelationshipAttribute> GetRelationships(Type entityType)
return attributes;
}

protected virtual Type GetRelationshipType(RelationshipAttribute relation, PropertyInfo prop)
{
if (relation.IsHasMany)
return prop.PropertyType.GetGenericArguments()[0];
else
return prop.PropertyType;
}
protected virtual Type GetRelationshipType(RelationshipAttribute relation, PropertyInfo prop) =>
relation.IsHasMany ? prop.PropertyType.GetGenericArguments()[0] : prop.PropertyType;

private Type GetResourceDefinitionType(Type entityType) => typeof(ResourceDefinition<>).MakeGenericType(entityType);

Expand Down
74 changes: 62 additions & 12 deletions src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -155,24 +155,46 @@ public virtual async Task<TEntity> CreateAsync(TEntity entity)
protected virtual void AttachRelationships(TEntity entity = null)
{
AttachHasManyPointers(entity);
AttachHasOnePointers();
AttachHasOnePointers(entity);
}

/// <inheritdoc />
public void DetachRelationshipPointers(TEntity entity)
{
foreach (var hasOneRelationship in _jsonApiContext.HasOneRelationshipPointers.Get())
{
_context.Entry(hasOneRelationship.Value).State = EntityState.Detached;
var hasOne = (HasOneAttribute) hasOneRelationship.Key;
if (hasOne.EntityPropertyName != null)
{
var relatedEntity = entity.GetType().GetProperty(hasOne.EntityPropertyName)?.GetValue(entity);
if (relatedEntity != null)
_context.Entry(relatedEntity).State = EntityState.Detached;
}
else
{
_context.Entry(hasOneRelationship.Value).State = EntityState.Detached;
}
}

foreach (var hasManyRelationship in _jsonApiContext.HasManyRelationshipPointers.Get())
{
foreach (var pointer in hasManyRelationship.Value)
var hasMany = (HasManyAttribute) hasManyRelationship.Key;
if (hasMany.EntityPropertyName != null)
{
_context.Entry(pointer).State = EntityState.Detached;
var relatedList = (IList)entity.GetType().GetProperty(hasMany.EntityPropertyName)?.GetValue(entity);
foreach (var related in relatedList)
{
_context.Entry(related).State = EntityState.Detached;
}
}

else
{
foreach (var pointer in hasManyRelationship.Value)
{
_context.Entry(pointer).State = EntityState.Detached;
}
}

// HACK: detaching has many relationships doesn't appear to be sufficient
// the navigation property actually needs to be nulled out, otherwise
// EF adds duplicate instances to the collection
Expand All @@ -192,14 +214,27 @@ private void AttachHasManyPointers(TEntity entity)
if (relationship.Key is HasManyThroughAttribute hasManyThrough)
AttachHasManyThrough(entity, hasManyThrough, relationship.Value);
else
AttachHasMany(relationship.Key as HasManyAttribute, relationship.Value);
AttachHasMany(entity, relationship.Key as HasManyAttribute, relationship.Value);
}
}

private void AttachHasMany(HasManyAttribute relationship, IList pointers)
private void AttachHasMany(TEntity entity, HasManyAttribute relationship, IList pointers)
{
foreach (var pointer in pointers)
_context.Entry(pointer).State = EntityState.Unchanged;
if (relationship.EntityPropertyName != null)
{
var relatedList = (IList)entity.GetType().GetProperty(relationship.EntityPropertyName)?.GetValue(entity);
foreach (var related in relatedList)
{
_context.Entry(related).State = EntityState.Unchanged;
}
}
else
{
foreach (var pointer in pointers)
{
_context.Entry(pointer).State = EntityState.Unchanged;
}
}
}

private void AttachHasManyThrough(TEntity entity, HasManyThroughAttribute hasManyThrough, IList pointers)
Expand Down Expand Up @@ -227,12 +262,27 @@ private void AttachHasManyThrough(TEntity entity, HasManyThroughAttribute hasMan
/// This is used to allow creation of HasOne relationships when the
/// independent side of the relationship already exists.
/// </summary>
private void AttachHasOnePointers()
private void AttachHasOnePointers(TEntity entity)
{
var relationships = _jsonApiContext.HasOneRelationshipPointers.Get();
foreach (var relationship in relationships)
if (_context.Entry(relationship.Value).State == EntityState.Detached && _context.EntityIsTracked(relationship.Value) == false)
_context.Entry(relationship.Value).State = EntityState.Unchanged;
{
if (relationship.Key.GetType() != typeof(HasOneAttribute))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Under what circumstances does this happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is totally just a guard clause since the dictionary key is RelationshipAttribute.

continue;

var hasOne = (HasOneAttribute) relationship.Key;
if (hasOne.EntityPropertyName != null)
{
var relatedEntity = entity.GetType().GetProperty(hasOne.EntityPropertyName)?.GetValue(entity);
if (relatedEntity != null && _context.Entry(relatedEntity).State == EntityState.Detached && _context.EntityIsTracked((IIdentifiable)relatedEntity) == false)
_context.Entry(relatedEntity).State = EntityState.Unchanged;
}
else
{
if (_context.Entry(relationship.Value).State == EntityState.Detached && _context.EntityIsTracked(relationship.Value) == false)
_context.Entry(relationship.Value).State = EntityState.Unchanged;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's refactor this a bit so it's clearer and we don't have separate definitions for how the EF ChangeTracker entry state is modified. The only thing that needs to be in the conditional is how we determine the relatedEntity. so, maybe something like:

IIdentifiable relatedEntity = (hasOne.EntityPropertyName != null)
    ? entity.GetType().GetProperty(hasOne.EntityPropertyName)?.GetValue(entity) as IIdentifiable
    : relationship.Value;

if(relatedEntity != null 
    && _context.Entry(relatedEntity).State == EntityState.Detached 
    && _context.EntityIsTracked(relatedEntity) == false) {
        _context.Entry(relatedEntity).State = EntityState.Unchanged;
}

}
}
}

/// <inheritdoc />
Expand Down
2 changes: 1 addition & 1 deletion src/JsonApiDotNetCore/JsonApiDotNetCore.csproj
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<VersionPrefix>3.0.0</VersionPrefix>
<VersionPrefix>3.0.1</VersionPrefix>
<TargetFrameworks>$(NetStandardVersion)</TargetFrameworks>
<AssemblyName>JsonApiDotNetCore</AssemblyName>
<PackageId>JsonApiDotNetCore</PackageId>
Expand Down
5 changes: 3 additions & 2 deletions src/JsonApiDotNetCore/Models/HasManyAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ public class HasManyAttribute : RelationshipAttribute
/// <param name="publicName">The relationship name as exposed by the API</param>
/// <param name="documentLinks">Which links are available. Defaults to <see cref="Link.All"/></param>
/// <param name="canInclude">Whether or not this relationship can be included using the <c>?include=public-name</c> query string</param>
/// <param name="mappedBy">The name of the entity mapped property, defaults to null</param>
///
/// <example>
///
Expand All @@ -23,8 +24,8 @@ public class HasManyAttribute : RelationshipAttribute
/// </code>
///
/// </example>
public HasManyAttribute(string publicName = null, Link documentLinks = Link.All, bool canInclude = true)
: base(publicName, documentLinks, canInclude)
public HasManyAttribute(string publicName = null, Link documentLinks = Link.All, bool canInclude = true, string mappedBy = null)
: base(publicName, documentLinks, canInclude, mappedBy)
{ }

/// <summary>
Expand Down
13 changes: 8 additions & 5 deletions src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Reflection;
using System.Security;

namespace JsonApiDotNetCore.Models
{
Expand Down Expand Up @@ -30,14 +31,15 @@ public class HasManyThroughAttribute : HasManyAttribute
/// <param name="internalThroughName">The name of the navigation property that will be used to get the HasMany relationship</param>
/// <param name="documentLinks">Which links are available. Defaults to <see cref="Link.All"/></param>
/// <param name="canInclude">Whether or not this relationship can be included using the <c>?include=public-name</c> query string</param>
/// <param name="mappedBy">The name of the entity mapped property, defaults to null</param>
///
/// <example>
/// <code>
/// [HasManyThrough(nameof(ArticleTags), documentLinks: Link.All, canInclude: true)]
/// </code>
/// </example>
public HasManyThroughAttribute(string internalThroughName, Link documentLinks = Link.All, bool canInclude = true)
: base(null, documentLinks, canInclude)
public HasManyThroughAttribute(string internalThroughName, Link documentLinks = Link.All, bool canInclude = true, string mappedBy = null)
: base(null, documentLinks, canInclude, mappedBy)
{
InternalThroughName = internalThroughName;
}
Expand All @@ -50,14 +52,15 @@ public HasManyThroughAttribute(string internalThroughName, Link documentLinks =
/// <param name="internalThroughName">The name of the navigation property that will be used to get the HasMany relationship</param>
/// <param name="documentLinks">Which links are available. Defaults to <see cref="Link.All"/></param>
/// <param name="canInclude">Whether or not this relationship can be included using the <c>?include=public-name</c> query string</param>
/// <param name="mappedBy">The name of the entity mapped property, defaults to null</param>
///
/// <example>
/// <code>
/// [HasManyThrough("tags", nameof(ArticleTags), documentLinks: Link.All, canInclude: true)]
/// </code>
/// </example>
public HasManyThroughAttribute(string publicName, string internalThroughName, Link documentLinks = Link.All, bool canInclude = true)
: base(publicName, documentLinks, canInclude)
public HasManyThroughAttribute(string publicName, string internalThroughName, Link documentLinks = Link.All, bool canInclude = true, string mappedBy = null)
: base(publicName, documentLinks, canInclude, mappedBy)
{
InternalThroughName = internalThroughName;
}
Expand Down Expand Up @@ -161,4 +164,4 @@ public HasManyThroughAttribute(string publicName, string internalThroughName, Li
/// </example>
public override string RelationshipPath => $"{InternalThroughName}.{RightProperty.Name}";
}
}
}
9 changes: 5 additions & 4 deletions src/JsonApiDotNetCore/Models/HasOneAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,28 +13,29 @@ public class HasOneAttribute : RelationshipAttribute
/// <param name="documentLinks">Which links are available. Defaults to <see cref="Link.All"/></param>
/// <param name="canInclude">Whether or not this relationship can be included using the <c>?include=public-name</c> query string</param>
/// <param name="withForeignKey">The foreign key property name. Defaults to <c>"{RelationshipName}Id"</c></param>
/// <param name="mappedBy">The name of the entity mapped property, defaults to null</param>
///
/// <example>
/// Using an alternative foreign key:
///
/// <code>
/// public class Article : Identifiable
/// {
/// [HasOne("author", withForiegnKey: nameof(AuthorKey)]
/// [HasOne("author", withForeignKey: nameof(AuthorKey)]
/// public Author Author { get; set; }
/// public int AuthorKey { get; set; }
/// }
/// </code>
///
/// </example>
public HasOneAttribute(string publicName = null, Link documentLinks = Link.All, bool canInclude = true, string withForeignKey = null)
: base(publicName, documentLinks, canInclude)
public HasOneAttribute(string publicName = null, Link documentLinks = Link.All, bool canInclude = true, string withForeignKey = null, string mappedBy = null)
: base(publicName, documentLinks, canInclude, mappedBy)
{
_explicitIdentifiablePropertyName = withForeignKey;
}

private readonly string _explicitIdentifiablePropertyName;

/// <summary>
/// The independent resource identifier.
/// </summary>
Expand Down
12 changes: 7 additions & 5 deletions src/JsonApiDotNetCore/Models/RelationshipAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,16 @@ namespace JsonApiDotNetCore.Models
{
public abstract class RelationshipAttribute : Attribute
{
protected RelationshipAttribute(string publicName, Link documentLinks, bool canInclude)
protected RelationshipAttribute(string publicName, Link documentLinks, bool canInclude, string mappedBy)
{
PublicRelationshipName = publicName;
DocumentLinks = documentLinks;
CanInclude = canInclude;
EntityPropertyName = mappedBy;
}

public string PublicRelationshipName { get; internal set; }
public string InternalRelationshipName { get; internal set; }
public string InternalRelationshipName { get; internal set; }

/// <summary>
/// The related entity type. This does not necessarily match the navigation property type.
Expand All @@ -31,6 +32,7 @@ protected RelationshipAttribute(string publicName, Link documentLinks, bool canI
public bool IsHasOne => GetType() == typeof(HasOneAttribute);
public Link DocumentLinks { get; } = Link.All;
public bool CanInclude { get; }
public string EntityPropertyName { get; }

public bool TryGetHasOne(out HasOneAttribute result)
{
Expand All @@ -55,10 +57,10 @@ public bool TryGetHasMany(out HasManyAttribute result)
}

public abstract void SetValue(object entity, object newValue);

public object GetValue(object entity) => entity
?.GetType()
.GetProperty(InternalRelationshipName)
?.GetType()?
.GetProperty(InternalRelationshipName)?
.GetValue(entity);

public override string ToString()
Expand Down
Loading