Skip to content

Allow PATCH objects with nullable attribute (e.g. DateTime?) #96

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.

Sign up for GitHub

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 7 commits into from Apr 26, 2017
Merged

Allow PATCH objects with nullable attribute (e.g. DateTime?) #96

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ea3a076
Merge pull request #83 from Research-Institute/develop
jaredcnance Mar 31, 2017
219ccfc
Merge pull request #92 from Research-Institute/develop
jaredcnance Apr 13, 2017
b1ecb9a
Allow PATCH objects with nullable attribute (e.g. DateTime?)
Apr 25, 2017
244b91f
Merge branch 'develop' into patch-nullable-attributes
jaredcnance Apr 25, 2017
38f4818
refactor(serialization): use TypeHelper.ConvertType
Apr 26, 2017
aeb41af
fix(format): convert tabs to spaces
Apr 26, 2017
d89dd23
refactor(type-helper): remove reflection dep and simplify null check
jaredcnance Apr 26, 2017
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
13 changes: 9 additions & 4 deletions src/JsonApiDotNetCore/Models/AttrAttribute.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,15 @@ public void SetValue(object entity, object newValue)
var propertyInfo = entity
.GetType()
.GetProperty(InternalAttributeName);

var convertedValue = Convert.ChangeType(newValue, propertyInfo.PropertyType);
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the PR!

We do have a method called TypeHelper.ConvertType which is where type conversions should be processed, but looks like I missed this here. Can you try using that method instead and see if your tests still pass?

Copy link
Author

Choose a reason for hiding this comment

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

Hi @jaredcnance, I tested with TypeHelper.ConvertType and all tests are green!

While reading this method, I was wondering if it was not a good idea to replace

if (type.GetTypeInfo().IsGenericType && type.GetGenericTypeDefinition().Equals(typeof(Nullable<>)))
                type = Nullable.GetUnderlyingType(type);

by

type = Nullable.GetUnderlyingType(type) ?? type;

It changes nothing about the result (I think), but it is more easy to read (maybe subjective), WDYT?

Thanks a lot for review!

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah that looks fine to me and I agree the intent is clearer. Thanks again for the PR!


propertyInfo.SetValue(entity, convertedValue);

if (propertyInfo != null)
{
Type t = Nullable.GetUnderlyingType(propertyInfo.PropertyType) ?? propertyInfo.PropertyType;

var convertedValue = (newValue == null) ? null : Convert.ChangeType(newValue, t);

propertyInfo.SetValue(entity, convertedValue, null);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

some funkiness with your tab spacing throughout...not a big deal just a note

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for review! I fixed it 😄

}
}
}
8 changes: 6 additions & 2 deletions src/JsonApiDotNetCoreExample/Data/AppDbContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using JsonApiDotNetCoreExample.Models;
using JsonApiDotNetCoreExample.Models;
using Microsoft.EntityFrameworkCore;
using System;

namespace JsonApiDotNetCoreExample.Data
{
Expand All @@ -11,7 +12,10 @@ public AppDbContext(DbContextOptions<AppDbContext> options)

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<TodoItem>()
modelBuilder.Entity<TodoItem>()
.Property(t => t.CreatedDate).HasDefaultValueSql("CURRENT_TIMESTAMP").IsRequired();

modelBuilder.Entity<TodoItem>()
.HasOne(t => t.Assignee)
.WithMany(p => p.AssignedTodoItems)
.HasForeignKey(t => t.AssigneeId);
Expand Down
108 changes: 108 additions & 0 deletions ...JsonApiDotNetCoreExample/Migrations/20170424180950_AddCreatesAndAchievedDates.Designer.cs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

34 changes: 34 additions & 0 deletions src/JsonApiDotNetCoreExample/Migrations/20170424180950_AddCreatesAndAchievedDates.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
using System;
using System.Collections.Generic;
using Microsoft.EntityFrameworkCore.Migrations;

namespace JsonApiDotNetCoreExample.Migrations
{
public partial class AddCreatesAndAchievedDates : Migration
{
protected override void Up(MigrationBuilder migrationBuilder)
{
migrationBuilder.AddColumn<DateTime>(
name: "AchievedDate",
table: "TodoItems",
nullable: true);

migrationBuilder.AddColumn<DateTime>(
name: "CreatedDate",
table: "TodoItems",
nullable: false,
defaultValueSql: "CURRENT_TIMESTAMP");
}

protected override void Down(MigrationBuilder migrationBuilder)
{
migrationBuilder.DropColumn(
name: "AchievedDate",
table: "TodoItems");

migrationBuilder.DropColumn(
name: "CreatedDate",
table: "TodoItems");
}
}
}
6 changes: 6 additions & 0 deletions src/JsonApiDotNetCoreExample/Migrations/AppDbContextModelSnapshot.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,16 @@ protected override void BuildModel(ModelBuilder modelBuilder)
b.Property<int>("Id")
.ValueGeneratedOnAdd();

b.Property<DateTime?>("AchievedDate");

b.Property<int?>("AssigneeId");

b.Property<Guid?>("CollectionId");

b.Property<DateTime>("CreatedDate")
.ValueGeneratedOnAdd()
.HasDefaultValueSql("CURRENT_TIMESTAMP");

b.Property<string>("Description");

b.Property<Guid>("GuidProperty");
Expand Down
8 changes: 7 additions & 1 deletion src/JsonApiDotNetCoreExample/Models/TodoItem.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using JsonApiDotNetCore.Models;

namespace JsonApiDotNetCoreExample.Models
Expand All @@ -18,6 +18,12 @@ public TodoItem()

[Attr("guid-property")]
public Guid GuidProperty { get; set; }

[Attr("created-date")]
public DateTime CreatedDate { get; set; }

[Attr("achieved-date")]
public DateTime? AchievedDate { get; set; }

public int? OwnerId { get; set; }
public int? AssigneeId { get; set; }
Expand Down
5 changes: 3 additions & 2 deletions test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/AttributeFilterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Net;
using System.Net;
using System.Net.Http;
using System.Threading.Tasks;
using DotNetCoreDocs;
Expand Down Expand Up @@ -30,7 +30,8 @@ public AttributeFilterTests(DocsFixture<Startup, JsonDocWriter> fixture)
_fixture = fixture;
_todoItemFaker = new Faker<TodoItem>()
.RuleFor(t => t.Description, f => f.Lorem.Sentence())
.RuleFor(t => t.Ordinal, f => f.Random.Number());
.RuleFor(t => t.Ordinal, f => f.Random.Number())
.RuleFor(t => t.CreatedDate, f => f.Date.Past());

_personFaker = new Faker<Person>()
.RuleFor(p => p.FirstName, f => f.Name.FirstName())
Expand Down
17 changes: 11 additions & 6 deletions test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/CreatingDataTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Net;
using System.Net;
using System.Net.Http;
using System.Net.Http.Headers;
using System.Threading.Tasks;
Expand Down Expand Up @@ -35,7 +35,8 @@ public CreatingDataTests(DocsFixture<Startup, JsonDocWriter> fixture)
_jsonApiContext = fixture.GetService<IJsonApiContext>();
_todoItemFaker = new Faker<TodoItem>()
.RuleFor(t => t.Description, f => f.Lorem.Sentence())
.RuleFor(t => t.Ordinal, f => f.Random.Number());
.RuleFor(t => t.Ordinal, f => f.Random.Number())
.RuleFor(t => t.CreatedDate, f => f.Date.Past());
}

[Fact]
Expand Down Expand Up @@ -107,7 +108,8 @@ public async Task Cannot_Create_Entity_With_Client_Generate_Id()
attributes = new
{
description = todoItem.Description,
ordinal = todoItem.Ordinal
ordinal = todoItem.Ordinal,
createdDate = DateTime.Now
}
}
};
Expand Down Expand Up @@ -145,7 +147,8 @@ public async Task Can_Create_Entity_With_Client_Defined_Id_If_Configured()
attributes = new
{
description = todoItem.Description,
ordinal = todoItem.Ordinal
ordinal = todoItem.Ordinal,
createdDate = DateTime.Now
}
}
};
Expand Down Expand Up @@ -302,7 +305,8 @@ public async Task ShouldReceiveLocationHeader_InResponse()
attributes = new
{
description = todoItem.Description,
ordinal = todoItem.Ordinal
ordinal = todoItem.Ordinal,
createdDate = DateTime.Now
}
}
};
Expand Down Expand Up @@ -339,7 +343,8 @@ public async Task Respond_409_ToIncorrectEntityType()
attributes = new
{
description = todoItem.Description,
ordinal = todoItem.Ordinal
ordinal = todoItem.Ordinal,
createdDate = DateTime.Now
}
}
};
Expand Down
5 changes: 3 additions & 2 deletions test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/DeletingDataTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Linq;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Threading.Tasks;
Expand Down Expand Up @@ -27,7 +27,8 @@ public DeletingDataTests(DocsFixture<Startup, JsonDocWriter> fixture)
_context = fixture.GetService<AppDbContext>();
_todoItemFaker = new Faker<TodoItem>()
.RuleFor(t => t.Description, f => f.Lorem.Sentence())
.RuleFor(t => t.Ordinal, f => f.Random.Number());
.RuleFor(t => t.Ordinal, f => f.Random.Number())
.RuleFor(t => t.CreatedDate, f => f.Date.Past());
}

[Fact]
Expand Down
5 changes: 3 additions & 2 deletions test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/DocumentTests/Included.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Net;
using System.Net;
using System.Net.Http;
using System.Threading.Tasks;
using DotNetCoreDocs;
Expand Down Expand Up @@ -36,7 +36,8 @@ public Included(DocsFixture<Startup, JsonDocWriter> fixture)

_todoItemFaker = new Faker<TodoItem>()
.RuleFor(t => t.Description, f => f.Lorem.Sentence())
.RuleFor(t => t.Ordinal, f => f.Random.Number());
.RuleFor(t => t.Ordinal, f => f.Random.Number())
.RuleFor(t => t.CreatedDate, f => f.Date.Past());

_todoItemCollectionFaker = new Faker<TodoItemCollection>()
.RuleFor(t => t.Name, f => f.Company.CatchPhrase());
Expand Down
5 changes: 3 additions & 2 deletions test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/DocumentTests/PagingTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Net;
using System.Net;
using System.Net.Http;
using System.Threading.Tasks;
using DotNetCoreDocs;
Expand Down Expand Up @@ -36,7 +36,8 @@ public PagingTests(DocsFixture<Startup, JsonDocWriter> fixture)

_todoItemFaker = new Faker<TodoItem>()
.RuleFor(t => t.Description, f => f.Lorem.Sentence())
.RuleFor(t => t.Ordinal, f => f.Random.Number());
.RuleFor(t => t.Ordinal, f => f.Random.Number())
.RuleFor(t => t.CreatedDate, f => f.Date.Past());

_todoItemCollectionFaker = new Faker<TodoItemCollection>()
.RuleFor(t => t.Name, f => f.Company.CatchPhrase());
Expand Down
5 changes: 3 additions & 2 deletions test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/DocumentTests/Relationships.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Net;
using System.Net;
using System.Net.Http;
using System.Threading.Tasks;
using DotNetCoreDocs;
Expand Down Expand Up @@ -29,7 +29,8 @@ public Relationships(DocsFixture<Startup, JsonDocWriter> fixture)
_context = fixture.GetService<AppDbContext>();
_todoItemFaker = new Faker<TodoItem>()
.RuleFor(t => t.Description, f => f.Lorem.Sentence())
.RuleFor(t => t.Ordinal, f => f.Random.Number());
.RuleFor(t => t.Ordinal, f => f.Random.Number())
.RuleFor(t => t.CreatedDate, f => f.Date.Past());
}

[Fact]
Expand Down
5 changes: 3 additions & 2 deletions test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/FetchingDataTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Collections.Generic;
using System.Collections.Generic;
using System.Net;
using System.Net.Http;
using System.Threading.Tasks;
Expand Down Expand Up @@ -33,7 +33,8 @@ public FetchingDataTests(DocsFixture<Startup, JsonDocWriter> fixture)
_jsonApiContext = fixture.GetService<IJsonApiContext>();
_todoItemFaker = new Faker<TodoItem>()
.RuleFor(t => t.Description, f => f.Lorem.Sentence())
.RuleFor(t => t.Ordinal, f => f.Random.Number());
.RuleFor(t => t.Ordinal, f => f.Random.Number())
.RuleFor(t => t.CreatedDate, f => f.Date.Past());
_personFaker = new Faker<Person>()
.RuleFor(p => p.FirstName, f => f.Name.FirstName())
.RuleFor(p => p.LastName, f => f.Name.LastName());
Expand Down
5 changes: 3 additions & 2 deletions test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/FetchingRelationshipsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Linq;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Threading.Tasks;
Expand Down Expand Up @@ -27,7 +27,8 @@ public FetchingRelationshipsTests(DocsFixture<Startup, JsonDocWriter> fixture)
_jsonApiContext = fixture.GetService<IJsonApiContext>();
_todoItemFaker = new Faker<TodoItem>()
.RuleFor(t => t.Description, f => f.Lorem.Sentence())
.RuleFor(t => t.Ordinal, f => f.Random.Number());
.RuleFor(t => t.Ordinal, f => f.Random.Number())
.RuleFor(t => t.CreatedDate, f => f.Date.Past());
}

[Fact]
Expand Down
Loading