diff --git a/src/JsonApiDotNetCore.Annotations/Resources/Annotations/AttrAttribute.cs b/src/JsonApiDotNetCore.Annotations/Resources/Annotations/AttrAttribute.cs index 7a6cbd960f..26a660775a 100644 --- a/src/JsonApiDotNetCore.Annotations/Resources/Annotations/AttrAttribute.cs +++ b/src/JsonApiDotNetCore.Annotations/Resources/Annotations/AttrAttribute.cs @@ -31,6 +31,7 @@ public AttrCapabilities Capabilities set => _capabilities = value; } + /// public override bool Equals(object? obj) { if (ReferenceEquals(this, obj)) @@ -48,6 +49,7 @@ public override bool Equals(object? obj) return Capabilities == other.Capabilities && base.Equals(other); } + /// public override int GetHashCode() { return HashCode.Combine(Capabilities, base.GetHashCode()); diff --git a/src/JsonApiDotNetCore.Annotations/Resources/Annotations/HasManyAttribute.cs b/src/JsonApiDotNetCore.Annotations/Resources/Annotations/HasManyAttribute.cs index 5792744d5c..d310028ae6 100644 --- a/src/JsonApiDotNetCore.Annotations/Resources/Annotations/HasManyAttribute.cs +++ b/src/JsonApiDotNetCore.Annotations/Resources/Annotations/HasManyAttribute.cs @@ -1,3 +1,4 @@ +using System.Collections; using JetBrains.Annotations; // ReSharper disable NonReadonlyMemberInGetHashCode @@ -65,6 +66,34 @@ private bool EvaluateIsManyToMany() return false; } + /// + public override void SetValue(object resource, object? newValue) + { + ArgumentGuard.NotNull(newValue); + AssertIsIdentifiableCollection(newValue); + + base.SetValue(resource, newValue); + } + + private void AssertIsIdentifiableCollection(object newValue) + { + if (newValue is not IEnumerable enumerable) + { + throw new InvalidOperationException($"Resource of type '{newValue.GetType()}' must be a collection."); + } + + foreach (object? element in enumerable) + { + if (element == null) + { + throw new InvalidOperationException("Resource collection must not contain null values."); + } + + AssertIsIdentifiable(element); + } + } + + /// public override bool Equals(object? obj) { if (ReferenceEquals(this, obj)) @@ -82,6 +111,7 @@ public override bool Equals(object? obj) return _capabilities == other._capabilities && base.Equals(other); } + /// public override int GetHashCode() { return HashCode.Combine(_capabilities, base.GetHashCode()); diff --git a/src/JsonApiDotNetCore.Annotations/Resources/Annotations/HasOneAttribute.cs b/src/JsonApiDotNetCore.Annotations/Resources/Annotations/HasOneAttribute.cs index c0416c92fb..72212c76f2 100644 --- a/src/JsonApiDotNetCore.Annotations/Resources/Annotations/HasOneAttribute.cs +++ b/src/JsonApiDotNetCore.Annotations/Resources/Annotations/HasOneAttribute.cs @@ -64,6 +64,14 @@ private bool EvaluateIsOneToOne() return false; } + /// + public override void SetValue(object resource, object? newValue) + { + AssertIsIdentifiable(newValue); + base.SetValue(resource, newValue); + } + + /// public override bool Equals(object? obj) { if (ReferenceEquals(this, obj)) @@ -81,6 +89,7 @@ public override bool Equals(object? obj) return _capabilities == other._capabilities && base.Equals(other); } + /// public override int GetHashCode() { return HashCode.Combine(_capabilities, base.GetHashCode()); diff --git a/src/JsonApiDotNetCore.Annotations/Resources/Annotations/RelationshipAttribute.cs b/src/JsonApiDotNetCore.Annotations/Resources/Annotations/RelationshipAttribute.cs index dd94bab221..0b4848ada1 100644 --- a/src/JsonApiDotNetCore.Annotations/Resources/Annotations/RelationshipAttribute.cs +++ b/src/JsonApiDotNetCore.Annotations/Resources/Annotations/RelationshipAttribute.cs @@ -86,6 +86,7 @@ public bool CanInclude set => _canInclude = value; } + /// public override bool Equals(object? obj) { if (ReferenceEquals(this, obj)) @@ -103,6 +104,7 @@ public override bool Equals(object? obj) return _rightType?.ClrType == other._rightType?.ClrType && Links == other.Links && base.Equals(other); } + /// public override int GetHashCode() { return HashCode.Combine(_rightType?.ClrType, Links, base.GetHashCode()); diff --git a/src/JsonApiDotNetCore.Annotations/Resources/Annotations/ResourceFieldAttribute.cs b/src/JsonApiDotNetCore.Annotations/Resources/Annotations/ResourceFieldAttribute.cs index e8e1d17aca..3a3707442c 100644 --- a/src/JsonApiDotNetCore.Annotations/Resources/Annotations/ResourceFieldAttribute.cs +++ b/src/JsonApiDotNetCore.Annotations/Resources/Annotations/ResourceFieldAttribute.cs @@ -68,6 +68,7 @@ internal set public object? GetValue(object resource) { ArgumentGuard.NotNull(resource); + AssertIsIdentifiable(resource); if (Property.GetMethod == null) { @@ -82,7 +83,7 @@ internal set { throw new InvalidOperationException( $"Unable to get property value of '{Property.DeclaringType!.Name}.{Property.Name}' on instance of type '{resource.GetType().Name}'.", - exception); + exception.InnerException ?? exception); } } @@ -90,9 +91,10 @@ internal set /// Sets the value of this field on the specified resource instance. Throws if the property is read-only or if the field does not belong to the specified /// resource instance. /// - public void SetValue(object resource, object? newValue) + public virtual void SetValue(object resource, object? newValue) { ArgumentGuard.NotNull(resource); + AssertIsIdentifiable(resource); if (Property.SetMethod == null) { @@ -107,15 +109,25 @@ public void SetValue(object resource, object? newValue) { throw new InvalidOperationException( $"Unable to set property value of '{Property.DeclaringType!.Name}.{Property.Name}' on instance of type '{resource.GetType().Name}'.", - exception); + exception.InnerException ?? exception); } } + protected void AssertIsIdentifiable(object? resource) + { + if (resource != null && resource is not IIdentifiable) + { + throw new InvalidOperationException($"Resource of type '{resource.GetType()}' does not implement {nameof(IIdentifiable)}."); + } + } + + /// public override string? ToString() { return _publicName ?? (_property != null ? _property.Name : base.ToString()); } + /// public override bool Equals(object? obj) { if (ReferenceEquals(this, obj)) @@ -133,6 +145,7 @@ public override bool Equals(object? obj) return _publicName == other._publicName && _property == other._property; } + /// public override int GetHashCode() { return HashCode.Combine(_publicName, _property); diff --git a/src/JsonApiDotNetCore/Queries/FieldSelectors.cs b/src/JsonApiDotNetCore/Queries/FieldSelectors.cs index ffd95c01bc..04b32b6499 100644 --- a/src/JsonApiDotNetCore/Queries/FieldSelectors.cs +++ b/src/JsonApiDotNetCore/Queries/FieldSelectors.cs @@ -52,9 +52,10 @@ public void IncludeAttributes(IEnumerable attributes) } } - public void IncludeRelationship(RelationshipAttribute relationship, QueryLayer? queryLayer) + public void IncludeRelationship(RelationshipAttribute relationship, QueryLayer queryLayer) { ArgumentGuard.NotNull(relationship); + ArgumentGuard.NotNull(queryLayer); this[relationship] = queryLayer; } diff --git a/src/JsonApiDotNetCore/Queries/Internal/Parsing/FilterParser.cs b/src/JsonApiDotNetCore/Queries/Internal/Parsing/FilterParser.cs index bdac0d8962..541b50a220 100644 --- a/src/JsonApiDotNetCore/Queries/Internal/Parsing/FilterParser.cs +++ b/src/JsonApiDotNetCore/Queries/Internal/Parsing/FilterParser.cs @@ -420,7 +420,7 @@ private object ConvertStringToType(string value, Type type) private Converter GetConstantValueConverterForAttribute(AttrAttribute attribute) { - return stringValue => attribute.Property.Name == nameof(IIdentifiable.Id) + return stringValue => attribute.Property.Name == nameof(Identifiable.Id) ? DeObfuscateStringId(attribute.Type.ClrType, stringValue) : ConvertStringToType(stringValue, attribute.Property.PropertyType); } diff --git a/src/JsonApiDotNetCore/Queries/Internal/QueryableBuilding/SelectClauseBuilder.cs b/src/JsonApiDotNetCore/Queries/Internal/QueryableBuilding/SelectClauseBuilder.cs index 6a4f43d7e1..c020653775 100644 --- a/src/JsonApiDotNetCore/Queries/Internal/QueryableBuilding/SelectClauseBuilder.cs +++ b/src/JsonApiDotNetCore/Queries/Internal/QueryableBuilding/SelectClauseBuilder.cs @@ -162,9 +162,9 @@ private void IncludeAllScalarProperties(Type elementType, Dictionary propertySelectors) { - foreach ((ResourceFieldAttribute resourceField, QueryLayer? queryLayer) in fieldSelectors) + foreach ((ResourceFieldAttribute resourceField, QueryLayer? nextLayer) in fieldSelectors) { - var propertySelector = new PropertySelector(resourceField.Property, queryLayer); + var propertySelector = new PropertySelector(resourceField.Property, nextLayer); IncludeWritableProperty(propertySelector, propertySelectors); } } diff --git a/src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs b/src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs index 89cfe76b87..6f9c0cbb0d 100644 --- a/src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs +++ b/src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs @@ -163,6 +163,8 @@ public virtual Task GetForCreateAsync(Type resourceClrType, TId id, C id }); + ArgumentGuard.NotNull(resourceClrType); + var resource = (TResource)_resourceFactory.CreateInstance(resourceClrType); resource.Id = id; diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/QueryStrings/SparseFieldSets/SparseFieldSetTests.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/QueryStrings/SparseFieldSets/SparseFieldSetTests.cs index 4036c62f13..60855a80f2 100644 --- a/test/JsonApiDotNetCoreTests/IntegrationTests/QueryStrings/SparseFieldSets/SparseFieldSetTests.cs +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/QueryStrings/SparseFieldSets/SparseFieldSetTests.cs @@ -791,7 +791,7 @@ public async Task Cannot_select_ToMany_relationship_with_blocked_capability() } [Fact] - public async Task Retrieves_all_properties_when_fieldset_contains_readonly_attribute() + public async Task Fetches_all_scalar_properties_when_fieldset_contains_readonly_attribute() { // Arrange var store = _testContext.Factory.Services.GetRequiredService(); diff --git a/test/JsonApiDotNetCoreTests/UnitTests/ResourceGraph/HasManyAttributeTests.cs b/test/JsonApiDotNetCoreTests/UnitTests/ResourceGraph/HasManyAttributeTests.cs new file mode 100644 index 0000000000..66b6dd0c81 --- /dev/null +++ b/test/JsonApiDotNetCoreTests/UnitTests/ResourceGraph/HasManyAttributeTests.cs @@ -0,0 +1,141 @@ +using FluentAssertions; +using JsonApiDotNetCore.Resources; +using JsonApiDotNetCore.Resources.Annotations; +using TestBuildingBlocks; +using Xunit; + +namespace JsonApiDotNetCoreTests.UnitTests.ResourceGraph; + +public sealed class HasManyAttributeTests +{ + [Fact] + public void Cannot_set_value_to_null() + { + // Arrange + var attribute = new HasManyAttribute + { + Property = typeof(TestResource).GetProperty(nameof(TestResource.Children))! + }; + + var resource = new TestResource(); + + // Act + Action action = () => attribute.SetValue(resource, null); + + // Assert + action.Should().ThrowExactly(); + } + + [Fact] + public void Cannot_set_value_to_primitive_type() + { + // Arrange + var attribute = new HasManyAttribute + { + Property = typeof(TestResource).GetProperty(nameof(TestResource.Children))! + }; + + var resource = new TestResource(); + + // Act + Action action = () => attribute.SetValue(resource, 1); + + // Assert + action.Should().ThrowExactly().WithMessage("Resource of type 'System.Int32' must be a collection."); + } + + [Fact] + public void Cannot_set_value_to_single_resource() + { + // Arrange + var attribute = new HasManyAttribute + { + Property = typeof(TestResource).GetProperty(nameof(TestResource.Children))! + }; + + var resource = new TestResource(); + + // Act + Action action = () => attribute.SetValue(resource, resource); + + // Assert + action.Should().ThrowExactly().WithMessage($"Resource of type '{typeof(TestResource).FullName}' must be a collection."); + } + + [Fact] + public void Can_set_value_to_collection_with_single_resource() + { + // Arrange + var attribute = new HasManyAttribute + { + Property = typeof(TestResource).GetProperty(nameof(TestResource.Children))! + }; + + var resource = new TestResource(); + + var children = new List + { + resource + }; + + // Act + attribute.SetValue(resource, children); + + // Assert + attribute.GetValue(resource).Should().BeOfType>().Subject.ShouldHaveCount(1); + } + + [Fact] + public void Cannot_set_value_to_collection_with_null_element() + { + // Arrange + var attribute = new HasManyAttribute + { + Property = typeof(TestResource).GetProperty(nameof(TestResource.Children))! + }; + + var resource = new TestResource(); + + var children = new List + { + resource, + null! + }; + + // Act + Action action = () => attribute.SetValue(resource, children); + + // Assert + action.Should().ThrowExactly().WithMessage("Resource collection must not contain null values."); + } + + [Fact] + public void Cannot_set_value_to_collection_with_primitive_element() + { + // Arrange + var attribute = new HasManyAttribute + { + Property = typeof(TestResource).GetProperty(nameof(TestResource.Children))! + }; + + var resource = new TestResource(); + + var children = new List + { + resource, + 1 + }; + + // Act + Action action = () => attribute.SetValue(resource, children); + + // Assert + action.Should().ThrowExactly().WithMessage("Resource of type 'System.Int32' does not implement IIdentifiable."); + } + + private sealed class TestResource : Identifiable + { + [HasMany] + public IEnumerable Children { get; set; } = new HashSet(); + } +} diff --git a/test/JsonApiDotNetCoreTests/UnitTests/ResourceGraph/HasOneAttributeTests.cs b/test/JsonApiDotNetCoreTests/UnitTests/ResourceGraph/HasOneAttributeTests.cs new file mode 100644 index 0000000000..bd2b44c418 --- /dev/null +++ b/test/JsonApiDotNetCoreTests/UnitTests/ResourceGraph/HasOneAttributeTests.cs @@ -0,0 +1,69 @@ +using FluentAssertions; +using JsonApiDotNetCore.Resources; +using JsonApiDotNetCore.Resources.Annotations; +using Xunit; + +namespace JsonApiDotNetCoreTests.UnitTests.ResourceGraph; + +public sealed class HasOneAttributeTests +{ + [Fact] + public void Can_set_value_to_null() + { + // Arrange + var attribute = new HasOneAttribute + { + Property = typeof(TestResource).GetProperty(nameof(TestResource.Parent))! + }; + + var resource = new TestResource(); + + // Act + attribute.SetValue(resource, null); + + // Assert + attribute.GetValue(resource).Should().BeNull(); + } + + [Fact] + public void Can_set_value_to_self() + { + // Arrange + var attribute = new HasOneAttribute + { + Property = typeof(TestResource).GetProperty(nameof(TestResource.Parent))! + }; + + var resource = new TestResource(); + + // Act + attribute.SetValue(resource, resource); + + // Assert + attribute.GetValue(resource).Should().Be(resource); + } + + [Fact] + public void Cannot_set_value_to_primitive_type() + { + // Arrange + var attribute = new HasOneAttribute + { + Property = typeof(TestResource).GetProperty(nameof(TestResource.Parent))! + }; + + var resource = new TestResource(); + + // Act + Action action = () => attribute.SetValue(resource, 1); + + // Assert + action.Should().ThrowExactly().WithMessage("Resource of type 'System.Int32' does not implement IIdentifiable."); + } + + private sealed class TestResource : Identifiable + { + [HasOne] + public TestResource? Parent { get; set; } + } +} diff --git a/test/JsonApiDotNetCoreTests/UnitTests/ResourceGraph/ResourceFieldAttributeTests.cs b/test/JsonApiDotNetCoreTests/UnitTests/ResourceGraph/ResourceFieldAttributeTests.cs new file mode 100644 index 0000000000..b7a028bd6e --- /dev/null +++ b/test/JsonApiDotNetCoreTests/UnitTests/ResourceGraph/ResourceFieldAttributeTests.cs @@ -0,0 +1,254 @@ +using System.Reflection; +using FluentAssertions; +using JsonApiDotNetCore.Resources; +using JsonApiDotNetCore.Resources.Annotations; +using Microsoft.AspNetCore.Http; +using Xunit; + +namespace JsonApiDotNetCoreTests.UnitTests.ResourceGraph; + +public sealed class ResourceFieldAttributeTests +{ + [Fact] + public void Cannot_set_public_name_to_null() + { + // Arrange + var attribute = new AttrAttribute(); + + // Act + Action action = () => attribute.PublicName = null!; + + // Assert + action.Should().ThrowExactly().WithMessage("Exposed name cannot be null, empty or contain only whitespace. (Parameter 'value')"); + } + + [Fact] + public void Cannot_set_public_name_to_empty() + { + // Arrange + var attribute = new AttrAttribute(); + + // Act + Action action = () => attribute.PublicName = string.Empty; + + // Assert + action.Should().ThrowExactly().WithMessage("Exposed name cannot be null, empty or contain only whitespace. (Parameter 'value')"); + } + + [Fact] + public void Cannot_set_public_name_to_whitespace() + { + // Arrange + var attribute = new AttrAttribute(); + + // Act + Action action = () => attribute.PublicName = " "; + + // Assert + action.Should().ThrowExactly().WithMessage("Exposed name cannot be null, empty or contain only whitespace. (Parameter 'value')"); + } + + [Fact] + public void Cannot_get_value_for_null() + { + // Arrange + var attribute = new AttrAttribute(); + + // Act + Action action = () => attribute.GetValue(null!); + + // Assert + action.Should().ThrowExactly(); + } + + [Fact] + public void Cannot_get_value_for_primitive_type() + { + // Arrange + var attribute = new AttrAttribute(); + + // Act + Action action = () => attribute.GetValue(1); + + // Assert + action.Should().ThrowExactly().WithMessage("Resource of type 'System.Int32' does not implement IIdentifiable."); + } + + [Fact] + public void Cannot_get_value_for_write_only_resource_property() + { + // Arrange + var attribute = new AttrAttribute + { + Property = typeof(TestResource).GetProperty(nameof(TestResource.WriteOnlyAttribute))! + }; + + var resource = new TestResource(); + + // Act + Action action = () => attribute.GetValue(resource); + + // Assert + action.Should().ThrowExactly().WithMessage("Property 'TestResource.WriteOnlyAttribute' is write-only."); + } + + [Fact] + public void Cannot_get_value_for_unknown_resource_property() + { + // Arrange + var attribute = new AttrAttribute + { + Property = typeof(IHttpContextAccessor).GetProperty(nameof(IHttpContextAccessor.HttpContext))! + }; + + var resource = new TestResource(); + + // Act + Action action = () => attribute.GetValue(resource); + + // Assert + action.Should().ThrowExactly() + .WithMessage("Unable to get property value of 'IHttpContextAccessor.HttpContext' on instance of type 'TestResource'.") + .WithInnerException(); + } + + [Fact] + public void Cannot_get_value_for_throwing_resource_property() + { + // Arrange + var attribute = new AttrAttribute + { + Property = typeof(TestResource).GetProperty(nameof(TestResource.ThrowOnGetAttribute))! + }; + + var resource = new TestResource(); + + // Act + Action action = () => attribute.GetValue(resource); + + // Assert + action.Should().ThrowExactly().WithInnerException().WithMessage("Getting value is not supported."); + } + + [Fact] + public void Cannot_set_value_for_null() + { + // Arrange + var attribute = new AttrAttribute(); + + // Act + Action action = () => attribute.SetValue(null!, "some"); + + // Assert + action.Should().ThrowExactly(); + } + + [Fact] + public void Cannot_set_value_for_primitive_type() + { + // Arrange + var attribute = new AttrAttribute(); + + // Act + Action action = () => attribute.SetValue(1, "some"); + + // Assert + action.Should().ThrowExactly().WithMessage("Resource of type 'System.Int32' does not implement IIdentifiable."); + } + + [Fact] + public void Cannot_set_value_for_read_only_resource_property() + { + // Arrange + var attribute = new AttrAttribute + { + Property = typeof(TestResource).GetProperty(nameof(TestResource.ReadOnlyAttribute))! + }; + + var resource = new TestResource(); + + // Act + Action action = () => attribute.SetValue(resource, true); + + // Assert + action.Should().ThrowExactly().WithMessage("Property 'TestResource.ReadOnlyAttribute' is read-only."); + } + + [Fact] + public void Cannot_set_value_for_unknown_resource_property() + { + // Arrange + var attribute = new AttrAttribute + { + Property = typeof(IHttpContextAccessor).GetProperty(nameof(IHttpContextAccessor.HttpContext))! + }; + + var resource = new TestResource(); + + // Act + Action action = () => attribute.SetValue(resource, "some"); + + // Assert + action.Should().ThrowExactly() + .WithMessage("Unable to set property value of 'IHttpContextAccessor.HttpContext' on instance of type 'TestResource'.") + .WithInnerException(); + } + + [Fact] + public void Cannot_set_value_for_throwing_resource_property() + { + // Arrange + var attribute = new AttrAttribute + { + Property = typeof(TestResource).GetProperty(nameof(TestResource.ThrowOnSetAttribute))! + }; + + var resource = new TestResource(); + + // Act + Action action = () => attribute.SetValue(resource, 1); + + // Assert + action.Should().ThrowExactly().WithInnerException().WithMessage("Setting value is not supported."); + } + + [Fact] + public void Cannot_set_value_to_incompatible_value() + { + // Arrange + var attribute = new AttrAttribute + { + Property = typeof(TestResource).GetProperty(nameof(TestResource.WriteOnlyAttribute))! + }; + + var resource = new TestResource(); + + // Act + Action action = () => attribute.SetValue(resource, DateTime.UtcNow); + + // Assert + action.Should().ThrowExactly().WithMessage("Object of type 'System.DateTime' cannot be converted to type 'System.Boolean'."); + } + + private sealed class TestResource : Identifiable + { + [Attr] + public bool ReadOnlyAttribute => true; + + [Attr] + public bool WriteOnlyAttribute + { + set => _ = value; + } + + [Attr] + public int ThrowOnGetAttribute => throw new NotSupportedException("Getting value is not supported."); + + [Attr] + public int ThrowOnSetAttribute + { + get => 1; + set => throw new NotSupportedException("Setting value is not supported."); + } + } +} diff --git a/test/TestBuildingBlocks/DbContextExtensions.cs b/test/TestBuildingBlocks/DbContextExtensions.cs index 8ce859f356..7f32073874 100644 --- a/test/TestBuildingBlocks/DbContextExtensions.cs +++ b/test/TestBuildingBlocks/DbContextExtensions.cs @@ -23,15 +23,15 @@ public static async Task ClearTablesAsync(this DbContext dbC await ClearTablesAsync(dbContext, typeof(TEntity1), typeof(TEntity2)); } - private static async Task ClearTablesAsync(this DbContext dbContext, params Type[] models) + private static async Task ClearTablesAsync(this DbContext dbContext, params Type[] modelTypes) { - foreach (Type model in models) + foreach (Type modelType in modelTypes) { - IEntityType? entityType = dbContext.Model.FindEntityType(model); + IEntityType? entityType = dbContext.Model.FindEntityType(modelType); if (entityType == null) { - throw new InvalidOperationException($"Table for '{model.Name}' not found."); + throw new InvalidOperationException($"Table for '{modelType.Name}' not found."); } string? tableName = entityType.GetTableName(); @@ -44,7 +44,7 @@ private static async Task ClearTablesAsync(this DbContext dbContext, params Type } else { - await dbContext.Database.ExecuteSqlRawAsync($"delete from \"{tableName}\""); + await dbContext.Database.ExecuteSqlRawAsync($"DELETE FROM \"{tableName}\""); } } }