Skip to content

Commit 0290ae4

Browse files
maureimaurei
committed
fix: separation and merge
2 parents db5bf23 + 30765c3 commit 0290ae4

File tree

11 files changed

+327
-29
lines changed

11 files changed

+327
-29
lines changed

‎src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs‎

Lines changed: 37 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -174,21 +174,36 @@ public virtual async Task<TEntity> CreateAsync(TEntity entity)
174174
/// </summary>
175175
private void LoadInverseRelationships(object trackedRelationshipValue, RelationshipAttribute relationshipAttr)
176176
{
177-
if (relationshipAttr.InverseNavigation == null) return;
177+
if (relationshipAttr.InverseNavigation == null || trackedRelationshipValue == null) return;
178178
if (relationshipAttr is HasOneAttribute hasOneAttr)
179179
{
180-
_context.Entry((IIdentifiable)trackedRelationshipValue).Reference(hasOneAttr.InverseNavigation).Load();
180+
var relationEntry = _context.Entry((IIdentifiable)trackedRelationshipValue);
181+
if (IsHasOneRelationship(hasOneAttr.InverseNavigation, trackedRelationshipValue.GetType()))
182+
{
183+
relationEntry.Reference(hasOneAttr.InverseNavigation).Load();
184+
} else
185+
{
186+
relationEntry.Collection(hasOneAttr.InverseNavigation).Load();
187+
}
181188
}
182-
else if (relationshipAttr is HasManyAttribute hasManyAttr)
189+
else if (relationshipAttr is HasManyAttribute hasManyAttr && !(relationshipAttr is HasManyThroughAttribute))
183190
{
184191
foreach (IIdentifiable relationshipValue in (IList)trackedRelationshipValue)
185192
{
186-
_context.Entry((IIdentifiable)trackedRelationshipValue).Reference(hasManyAttr.InverseNavigation).Load();
193+
_context.Entry(relationshipValue).Reference(hasManyAttr.InverseNavigation).Load();
187194
}
188195
}
189196
}
190197

191198

199+
private bool IsHasOneRelationship(string internalRelationshipName, Type type)
200+
{
201+
var relationshipAttr = _jsonApiContext.ResourceGraph.GetContextEntity(type).Relationships.Single(r => r.InternalRelationshipName == internalRelationshipName);
202+
if (relationshipAttr is HasOneAttribute) return true;
203+
return false;
204+
}
205+
206+
192207
/// <inheritdoc />
193208
public void DetachRelationshipPointers(TEntity entity)
194209
{
@@ -223,14 +238,16 @@ public void DetachRelationshipPointers(TEntity entity)
223238
}
224239
}
225240

226-
/// <inheritdoc />
241+
[Obsolete("Use overload UpdateAsync(TEntity updatedEntity): providing parameter ID does no longer add anything relevant")]
227242
public virtual async Task<TEntity> UpdateAsync(TId id, TEntity updatedEntity)
228243
{
229-
/// WHY is parameter "entity" even passed along to this method??
230-
/// It does nothing!
231-
232-
var oldEntity = await GetAsync(id);
244+
return await UpdateAsync(updatedEntity);
245+
}
233246

247+
/// <inheritdoc />
248+
public virtual async Task<TEntity> UpdateAsync(TEntity updatedEntity)
249+
{
250+
var oldEntity = await GetAsync(updatedEntity.Id);
234251
if (oldEntity == null)
235252
return null;
236253

@@ -244,6 +261,7 @@ public virtual async Task<TEntity> UpdateAsync(TId id, TEntity updatedEntity)
244261
LoadInverseRelationships(trackedRelationshipValue, relationshipAttr);
245262
AssignRelationshipValue(oldEntity, trackedRelationshipValue, relationshipAttr);
246263
}
264+
247265
await _context.SaveChangesAsync();
248266
return oldEntity;
249267
}
@@ -375,7 +393,9 @@ public virtual async Task<IEnumerable<TEntity>> PageAsync(IQueryable<TEntity> en
375393
{
376394
if (pageNumber >= 0)
377395
{
378-
return await entities.PageForward(pageSize, pageNumber).ToListAsync();
396+
// the IQueryable returned from the hook executor is sometimes consumed here.
397+
// In this case, it does not support .ToListAsync(), so we use the method below.
398+
return await this.ToListAsync(entities.PageForward(pageSize, pageNumber));
379399
}
380400

381401
// since EntityFramework does not support IQueryable.Reverse(), we need to know the number of queried entities
@@ -384,11 +404,10 @@ public virtual async Task<IEnumerable<TEntity>> PageAsync(IQueryable<TEntity> en
384404
// may be negative
385405
int virtualFirstIndex = numberOfEntities - pageSize * Math.Abs(pageNumber);
386406
int numberOfElementsInPage = Math.Min(pageSize, virtualFirstIndex + pageSize);
387-
388-
return await entities
407+
408+
return await ToListAsync(entities
389409
.Skip(virtualFirstIndex)
390-
.Take(numberOfElementsInPage)
391-
.ToListAsync();
410+
.Take(numberOfElementsInPage));
392411
}
393412

394413
/// <inheritdoc />
@@ -441,7 +460,7 @@ protected void LoadCurrentRelationships(TEntity oldEntity, RelationshipAttribute
441460
/// retrieve from the context WHICH relationships to update, but the actual
442461
/// values should not come from the context.
443462
/// </summary>
444-
protected void AssignRelationshipValue(TEntity oldEntity, object relationshipValue, RelationshipAttribute relationshipAttribute)
463+
private void AssignRelationshipValue(TEntity oldEntity, object relationshipValue, RelationshipAttribute relationshipAttribute)
445464
{
446465
if (relationshipAttribute is HasManyThroughAttribute throughAttribute)
447466
{
@@ -479,7 +498,7 @@ private void AssignHasManyThrough(TEntity entity, HasManyThroughAttribute hasMan
479498
/// A helper method that gets the relationship value in the case of
480499
/// entity resource separation.
481500
/// </summary>
482-
IIdentifiable GetEntityResourceSeparationValue(TEntity entity, HasOneAttribute attribute)
501+
private IIdentifiable GetEntityResourceSeparationValue(TEntity entity, HasOneAttribute attribute)
483502
{
484503
if (attribute.EntityPropertyName == null)
485504
{
@@ -492,7 +511,7 @@ IIdentifiable GetEntityResourceSeparationValue(TEntity entity, HasOneAttribute a
492511
/// A helper method that gets the relationship value in the case of
493512
/// entity resource separation.
494513
/// </summary>
495-
IEnumerable<IIdentifiable> GetEntityResourceSeparationValue(TEntity entity, HasManyAttribute attribute)
514+
private IEnumerable<IIdentifiable> GetEntityResourceSeparationValue(TEntity entity, HasManyAttribute attribute)
496515
{
497516
if (attribute.EntityPropertyName == null)
498517
{
@@ -508,7 +527,7 @@ IEnumerable<IIdentifiable> GetEntityResourceSeparationValue(TEntity entity, HasM
508527
///
509528
/// useful article: https://stackoverflow.com/questions/30987806/dbset-attachentity-vs-dbcontext-entryentity-state-entitystate-modified
510529
/// </summary>
511-
IIdentifiable AttachOrGetTracked(IIdentifiable relationshipValue)
530+
private IIdentifiable AttachOrGetTracked(IIdentifiable relationshipValue)
512531
{
513532
var trackedEntity = _context.GetTrackedEntity(relationshipValue);
514533

‎src/JsonApiDotNetCore/Data/IEntityWriteRepository.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ public interface IEntityWriteRepository<TEntity, in TId>
1515
{
1616
Task<TEntity> CreateAsync(TEntity entity);
1717

18+
Task<TEntity> UpdateAsync(TEntity entity);
19+
20+
[Obsolete("Use overload UpdateAsync(TEntity updatedEntity): providing parameter ID does no longer add anything relevant")]
1821
Task<TEntity> UpdateAsync(TId id, TEntity entity);
1922

2023
Task UpdateRelationshipsAsync(object parent, RelationshipAttribute relationship, IEnumerable<string> relationshipIds);

‎src/JsonApiDotNetCore/Formatters/JsonApiReader.cs‎

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
using System;
2+
using System.Collections;
23
using System.IO;
34
using System.Threading.Tasks;
45
using JsonApiDotNetCore.Internal;
6+
using JsonApiDotNetCore.Models;
57
using JsonApiDotNetCore.Serialization;
68
using JsonApiDotNetCore.Services;
79
using Microsoft.AspNetCore.Mvc.Formatters;
@@ -37,7 +39,7 @@ public Task<InputFormatterResult> ReadAsync(InputFormatterContext context)
3739
{
3840
var body = GetRequestBody(context.HttpContext.Request.Body);
3941

40-
object model =null;
42+
object model = null;
4143

4244
if (_jsonApiContext.IsRelationshipPath)
4345
{
@@ -48,10 +50,29 @@ public Task<InputFormatterResult> ReadAsync(InputFormatterContext context)
4850
model = _deSerializer.Deserialize(body);
4951
}
5052

53+
5154
if (model == null)
5255
{
5356
_logger?.LogError("An error occurred while de-serializing the payload");
5457
}
58+
59+
if (context.HttpContext.Request.Method == "PATCH")
60+
{
61+
bool idMissing;
62+
if (model is IList list)
63+
{
64+
idMissing = CheckForId(list);
65+
}
66+
else
67+
{
68+
idMissing = CheckForId(model);
69+
}
70+
if (idMissing)
71+
{
72+
_logger?.LogError("Payload must include id attribute");
73+
throw new JsonApiException(400, "Payload must include id attribute");
74+
}
75+
}
5576
return InputFormatterResult.SuccessAsync(model);
5677
}
5778
catch (Exception ex)
@@ -62,6 +83,38 @@ public Task<InputFormatterResult> ReadAsync(InputFormatterContext context)
6283
}
6384
}
6485

86+
private bool CheckForId(object model)
87+
{
88+
if (model == null) return false;
89+
if (model is ResourceObject ro)
90+
{
91+
if (string.IsNullOrEmpty(ro.Id)) return true;
92+
}
93+
else if (model is IIdentifiable identifiable)
94+
{
95+
if (string.IsNullOrEmpty(identifiable.StringId)) return true;
96+
}
97+
return false;
98+
}
99+
100+
private bool CheckForId(IList modelList)
101+
{
102+
foreach (var model in modelList)
103+
{
104+
if (model == null) continue;
105+
if (model is ResourceObject ro)
106+
{
107+
if (string.IsNullOrEmpty(ro.Id)) return true;
108+
}
109+
else if (model is IIdentifiable identifiable)
110+
{
111+
if (string.IsNullOrEmpty(identifiable.StringId)) return true;
112+
}
113+
}
114+
return false;
115+
116+
}
117+
65118
private string GetRequestBody(Stream body)
66119
{
67120
using (var reader = new StreamReader(body))

‎src/JsonApiDotNetCore/Services/EntityResourceService.cs‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -212,12 +212,9 @@ public virtual async Task<object> GetRelationshipAsync(TId id, string relationsh
212212
public virtual async Task<TResource> UpdateAsync(TId id, TResource resource)
213213
{
214214
var entity = MapIn(resource);
215-
// why is entity passed along to UpdateAsync if it is never used internally??
216-
// I think we should just set the id on the request-parsed entity and pass that along to the repo.
217-
entity.Id = id;
218215

219216
entity = IsNull(_hookExecutor) ? entity : _hookExecutor.BeforeUpdate(AsList(entity), ResourcePipeline.Patch).SingleOrDefault();
220-
entity = await _entities.UpdateAsync(id, entity);
217+
entity = await _entities.UpdateAsync(entity);
221218
if (!IsNull(_hookExecutor, entity))
222219
{
223220
_hookExecutor.AfterUpdate(AsList(entity), ResourcePipeline.Patch);

‎test/JsonApiDotNetCoreExampleTests/Acceptance/CamelCasedModelsControllerTests.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ public async Task Can_Patch_CamelCasedModels()
143143
data = new
144144
{
145145
type = "camelCasedModels",
146+
id = model.Id,
146147
attributes = new Dictionary<string, object>()
147148
{
148149
{ "compoundAttr", newModel.CompoundAttr }

‎test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/CreatingDataTests.cs‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
using Microsoft.EntityFrameworkCore;
1818
using Newtonsoft.Json;
1919
using Xunit;
20+
using Person = JsonApiDotNetCoreExample.Models.Person;
2021

2122
namespace JsonApiDotNetCoreExampleTests.Acceptance.Spec
2223
{
@@ -26,6 +27,7 @@ public class CreatingDataTests
2627
private TestFixture<TestStartup> _fixture;
2728
private IJsonApiContext _jsonApiContext;
2829
private Faker<TodoItem> _todoItemFaker;
30+
private Faker<Person> _personFaker;
2931

3032
public CreatingDataTests(TestFixture<TestStartup> fixture)
3133
{
@@ -35,6 +37,10 @@ public CreatingDataTests(TestFixture<TestStartup> fixture)
3537
.RuleFor(t => t.Description, f => f.Lorem.Sentence())
3638
.RuleFor(t => t.Ordinal, f => f.Random.Number())
3739
.RuleFor(t => t.CreatedDate, f => f.Date.Past());
40+
_personFaker = new Faker<Person>()
41+
.RuleFor(t => t.FirstName, f => f.Name.FirstName())
42+
.RuleFor(t => t.LastName, f => f.Name.LastName());
43+
3844
}
3945

4046
[Fact]
@@ -586,5 +592,113 @@ public async Task Respond_409_ToIncorrectEntityType()
586592
// assert
587593
Assert.Equal(HttpStatusCode.Conflict, response.StatusCode);
588594
}
595+
596+
[Fact]
597+
public async Task Create_With_ToOne_Relationship_With_Implicit_Remove()
598+
{
599+
// Arrange
600+
var context = _fixture.GetService<AppDbContext>();
601+
var passport = new Passport();
602+
var person1 = _personFaker.Generate();
603+
person1.Passport = passport;
604+
context.People.AddRange(new List<Person>() { person1 });
605+
await context.SaveChangesAsync();
606+
var passportId = person1.PassportId;
607+
var content = new
608+
{
609+
data = new
610+
{
611+
type = "people",
612+
attributes = new Dictionary<string, string>() { { "first-name", "Joe" } },
613+
relationships = new Dictionary<string, object>
614+
{
615+
{ "passport", new
616+
{
617+
data = new { type = "passports", id = $"{passportId}" }
618+
}
619+
}
620+
}
621+
}
622+
};
623+
624+
var httpMethod = new HttpMethod("POST");
625+
var route = $"/api/v1/people";
626+
var request = new HttpRequestMessage(httpMethod, route);
627+
628+
string serializedContent = JsonConvert.SerializeObject(content);
629+
request.Content = new StringContent(serializedContent);
630+
request.Content.Headers.ContentType = new MediaTypeHeaderValue("application/vnd.api+json");
631+
632+
// Act
633+
var response = await _fixture.Client.SendAsync(request);
634+
var body = await response.Content.ReadAsStringAsync();
635+
var personResult = _fixture.GetService<IJsonApiDeSerializer>().Deserialize<Person>(body);
636+
637+
// Assert
638+
639+
Assert.True(HttpStatusCode.Created == response.StatusCode, $"{route} returned {response.StatusCode} status code with payload: {body}");
640+
var dbPerson = context.People.AsNoTracking().Where(p => p.Id == personResult.Id).Include("Passport").FirstOrDefault();
641+
Assert.Equal(passportId, dbPerson.Passport.Id);
642+
}
643+
644+
[Fact]
645+
public async Task Create_With_ToMany_Relationship_With_Implicit_Remove()
646+
{
647+
// Arrange
648+
var context = _fixture.GetService<AppDbContext>();
649+
var person1 = _personFaker.Generate();
650+
person1.TodoItems = _todoItemFaker.Generate(3).ToList();
651+
context.People.AddRange(new List<Person>() { person1 });
652+
await context.SaveChangesAsync();
653+
var todoItem1Id = person1.TodoItems[0].Id;
654+
var todoItem2Id = person1.TodoItems[1].Id;
655+
656+
var content = new
657+
{
658+
data = new
659+
{
660+
type = "people",
661+
attributes = new Dictionary<string, string>() { { "first-name", "Joe" } },
662+
relationships = new Dictionary<string, object>
663+
{
664+
{ "todo-items", new
665+
{
666+
data = new List<object>
667+
{
668+
new {
669+
type = "todo-items",
670+
id = $"{todoItem1Id}"
671+
},
672+
new {
673+
type = "todo-items",
674+
id = $"{todoItem2Id}"
675+
}
676+
}
677+
}
678+
}
679+
}
680+
}
681+
};
682+
683+
var httpMethod = new HttpMethod("POST");
684+
var route = $"/api/v1/people";
685+
var request = new HttpRequestMessage(httpMethod, route);
686+
687+
string serializedContent = JsonConvert.SerializeObject(content);
688+
request.Content = new StringContent(serializedContent);
689+
request.Content.Headers.ContentType = new MediaTypeHeaderValue("application/vnd.api+json");
690+
691+
// Act
692+
var response = await _fixture.Client.SendAsync(request);
693+
var body = await response.Content.ReadAsStringAsync();
694+
var personResult = _fixture.GetService<IJsonApiDeSerializer>().Deserialize<Person>(body);
695+
696+
// Assert
697+
Assert.True(HttpStatusCode.Created == response.StatusCode, $"{route} returned {response.StatusCode} status code with payload: {body}");
698+
var dbPerson = context.People.AsNoTracking().Where(p => p.Id == personResult.Id).Include("TodoItems").FirstOrDefault();
699+
Assert.Equal(2, dbPerson.TodoItems.Count);
700+
Assert.NotNull(dbPerson.TodoItems.SingleOrDefault(ti => ti.Id == todoItem1Id));
701+
Assert.NotNull(dbPerson.TodoItems.SingleOrDefault(ti => ti.Id == todoItem2Id));
702+
}
589703
}
590704
}

0 commit comments

Comments
 (0)