Fixed: Global options not respected in relationship links rendering (#946)

* Bugfix: relationship links were rendered for ToMany relationships, even when explicitly turned off in global options (and not configured on the relationship itself).

* Added tests and clarified documentation. Removed verification logic in value setters because this is a flags enum, so the checks were incorrect. For example, you are not allowed to pass `Links.Paging`, but are allowed to pass `Links.Related | Links.Paging` or `Links.All`, which all mean "include paging in the set of links".

Author: Bart Koelman

src/Examples/JsonApiDotNetCoreExample/Definitions/PassportHooksDefinition.cs

@@ -4,7 +4,6 @@
 using JsonApiDotNetCore.Configuration;
 using JsonApiDotNetCore.Errors;
 using JsonApiDotNetCore.Hooks.Internal.Execution;
-using JsonApiDotNetCore.Resources;
 using JsonApiDotNetCore.Serialization.Objects;
 using JsonApiDotNetCoreExample.Models;
 

src/JsonApiDotNetCore/Configuration/IJsonApiOptions.cs

@@ -56,41 +56,28 @@ public interface IJsonApiOptions
         bool UseRelativeLinks { get; }
 
         /// <summary>
-        /// Configures globally which links to show in the <see cref="Serialization.Objects.TopLevelLinks"/>
-        /// object for a requested resource. Setting can be overridden per resource by
-        /// adding a <see cref="ResourceLinksAttribute"/> to the class definition of that resource.
+        /// Configures which links to show in the <see cref="Serialization.Objects.TopLevelLinks"/>
+        /// object. Defaults to <see cref="LinkTypes.All"/>.
+        /// This setting can be overruled per resource type by
+        /// adding <see cref="ResourceLinksAttribute"/> on the class definition of a resource.
         /// </summary>
         LinkTypes TopLevelLinks { get; }
 
         /// <summary>
-        /// Configures globally which links to show in the <see cref="Serialization.Objects.ResourceLinks"/>
-        /// object for a requested resource. Setting can be overridden per resource by
-        /// adding a <see cref="ResourceLinksAttribute"/> to the class definition of that resource.
+        /// Configures which links to show in the <see cref="Serialization.Objects.ResourceLinks"/>
+        /// object. Defaults to <see cref="LinkTypes.All"/>.
+        /// This setting can be overruled per resource type by
+        /// adding <see cref="ResourceLinksAttribute"/> on the class definition of a resource.
         /// </summary>
         LinkTypes ResourceLinks { get; }
 
         /// <summary>
-        /// Configures globally which links to show in the <see cref="Serialization.Objects.RelationshipLinks"/>
-        /// object for a requested resource. Setting can be overridden per resource by
-        /// adding a <see cref="ResourceLinksAttribute"/> to the class definition of that resource.
-        /// This option can also be specified per relationship by using the associated links argument
-        /// in the constructor of <see cref="RelationshipAttribute"/>.
+        /// Configures which links to show in the <see cref="Serialization.Objects.RelationshipLinks"/>
+        /// object. Defaults to <see cref="LinkTypes.All"/>.
+        /// This setting can be overruled for all relationships per resource type by
+        /// adding <see cref="ResourceLinksAttribute"/> on the class definition of a resource.
+        /// This can be further overruled per relationship by setting <see cref="RelationshipAttribute.Links"/>.
         /// </summary>
-        /// <example>
-        /// <code>
-        /// options.RelationshipLinks = LinkTypes.None;
-        /// </code>
-        /// <code>
-        /// {
-        ///   "type": "articles",
-        ///   "id": "4309",
-        ///   "relationships": {
-        ///      "author": { "data": { "type": "people", "id": "1234" }
-        ///      }
-        ///   }
-        /// }
-        /// </code>
-        /// </example>
         LinkTypes RelationshipLinks { get; }
 
         /// <summary>
@@ -154,13 +141,13 @@ public interface IJsonApiOptions
         bool EnableLegacyFilterNotation { get; }
 
         /// <summary>
-        /// Determines whether the <see cref="JsonSerializerSettings.NullValueHandling"/> serialization setting can be overridden by using a query string parameter.
+        /// Determines whether the <see cref="JsonSerializerSettings.NullValueHandling"/> serialization setting can be controlled using a query string parameter.
         /// False by default.
         /// </summary>
         bool AllowQueryStringOverrideForSerializerNullValueHandling { get; }
 
         /// <summary>
-        /// Determines whether the <see cref="JsonSerializerSettings.DefaultValueHandling"/> serialization setting can be overridden by using a query string parameter.
+        /// Determines whether the <see cref="JsonSerializerSettings.DefaultValueHandling"/> serialization setting can be controlled using a query string parameter.
         /// False by default.
         /// </summary>
         bool AllowQueryStringOverrideForSerializerDefaultValueHandling { get; }

src/JsonApiDotNetCore/Configuration/ResourceContext.cs

@@ -52,27 +52,33 @@ public class ResourceContext
 
         /// <summary>
         /// Configures which links to show in the <see cref="Serialization.Objects.TopLevelLinks"/>
-        /// object for this resource. If set to <see cref="LinkTypes.NotConfigured"/>,
-        /// the configuration will be read from <see cref="IJsonApiOptions"/>.
-        /// Defaults to <see cref="LinkTypes.NotConfigured"/>.
+        /// object for this resource type.
+        /// Defaults to <see cref="LinkTypes.NotConfigured"/>, which falls back to <see cref="IJsonApiOptions.TopLevelLinks"/>.
         /// </summary>
+        /// <remarks>
+        /// In the process of building the resource graph, this value is set based on <see cref="ResourceLinksAttribute.TopLevelLinks"/> usage.
+        /// </remarks>
         public LinkTypes TopLevelLinks { get; internal set; } = LinkTypes.NotConfigured;
 
         /// <summary>
         /// Configures which links to show in the <see cref="Serialization.Objects.ResourceLinks"/>
-        /// object for this resource. If set to <see cref="LinkTypes.NotConfigured"/>,
-        /// the configuration will be read from <see cref="IJsonApiOptions"/>.
-        /// Defaults to <see cref="LinkTypes.NotConfigured"/>.
+        /// object for this resource type.
+        /// Defaults to <see cref="LinkTypes.NotConfigured"/>, which falls back to <see cref="IJsonApiOptions.ResourceLinks"/>.
         /// </summary>
+        /// <remarks>
+        /// In the process of building the resource graph, this value is set based on <see cref="ResourceLinksAttribute.ResourceLinks"/> usage.
+        /// </remarks>
         public LinkTypes ResourceLinks { get; internal set; } = LinkTypes.NotConfigured;
 
         /// <summary>
         /// Configures which links to show in the <see cref="Serialization.Objects.RelationshipLinks"/>
-        /// for all relationships of the resource for which this attribute was instantiated.
-        /// If set to <see cref="LinkTypes.NotConfigured"/>, the configuration will
-        /// be read from <see cref="RelationshipAttribute.Links"/>  or
-        /// <see cref="IJsonApiOptions"/>. Defaults to <see cref="LinkTypes.NotConfigured"/>.
+        /// object for all relationships of this resource type.
+        /// Defaults to <see cref="LinkTypes.NotConfigured"/>, which falls back to <see cref="IJsonApiOptions.RelationshipLinks"/>.
+        /// This can be overruled per relationship by setting <see cref="RelationshipAttribute.Links"/>.
         /// </summary>
+        /// <remarks>
+        /// In the process of building the resource graph, this value is set based on <see cref="ResourceLinksAttribute.RelationshipLinks"/> usage.
+        /// </remarks>
         public LinkTypes RelationshipLinks { get; internal set; } = LinkTypes.NotConfigured;
 
         public override string ToString()

src/JsonApiDotNetCore/Resources/Annotations/HasManyAttribute.cs

@@ -5,24 +5,17 @@ namespace JsonApiDotNetCore.Resources.Annotations
     /// <summary>
     /// Used to expose a property on a resource class as a JSON:API to-many relationship (https://jsonapi.org/format/#document-resource-object-relationships).
     /// </summary>
+    /// <example>
+    /// <code><![CDATA[
+    /// public class Author : Identifiable 
+    /// {
+    ///     [HasMany(PublicName = "articles")]
+    ///     public List<Article> Articles { get; set; }
+    /// }
+    /// ]]></code>
+    /// </example>
     [AttributeUsage(AttributeTargets.Property)]
     public class HasManyAttribute : RelationshipAttribute
     {
-        /// <summary>
-        /// Creates a HasMany relational link to another resource.
-        /// </summary>
-        /// <example>
-        /// <code><![CDATA[
-        /// public class Author : Identifiable 
-        /// {
-        ///     [HasMany(PublicName = "articles")]
-        ///     public List<Article> Articles { get; set; }
-        /// }
-        /// ]]></code>
-        /// </example>
-        public HasManyAttribute()
-        {
-            Links = LinkTypes.All;
-        }
     }
 }

src/JsonApiDotNetCore/Resources/Annotations/HasOneAttribute.cs

@@ -8,9 +8,5 @@ namespace JsonApiDotNetCore.Resources.Annotations
     [AttributeUsage(AttributeTargets.Property)]
     public sealed class HasOneAttribute : RelationshipAttribute
     {
-        public HasOneAttribute()
-        {
-            Links = LinkTypes.NotConfigured;
-        }
     }
 }

src/JsonApiDotNetCore/Resources/Annotations/RelationshipAttribute.cs

@@ -1,7 +1,6 @@
 using System;
 using System.Reflection;
 using JsonApiDotNetCore.Configuration;
-using JsonApiDotNetCore.Errors;
 
 namespace JsonApiDotNetCore.Resources.Annotations
 {
@@ -10,8 +9,6 @@ namespace JsonApiDotNetCore.Resources.Annotations
     /// </summary>
     public abstract class RelationshipAttribute : ResourceFieldAttribute
     {
-        private LinkTypes _links;
-
         /// <summary>
         /// The property name of the EF Core inverse navigation, which may or may not exist.
         /// Even if it exists, it may not be exposed as a JSON:API relationship.
@@ -58,27 +55,12 @@ public abstract class RelationshipAttribute : ResourceFieldAttribute
         public Type LeftType { get; internal set; }
 
         /// <summary>
-        /// Configures which links to show in the <see cref="Links"/> object for this relationship.
-        /// When not explicitly assigned, the default value depends on the relationship type (see remarks).
+        /// Configures which links to show in the <see cref="Serialization.Objects.RelationshipLinks"/>
+        /// object for this relationship.
+        /// Defaults to <see cref="LinkTypes.NotConfigured"/>, which falls back to <see cref="ResourceLinksAttribute.RelationshipLinks"/>
+        /// and then falls back to <see cref="IJsonApiOptions.RelationshipLinks"/>.
         /// </summary>
-        /// <remarks>
-        /// This defaults to <see cref="LinkTypes.All"/> for <see cref="HasManyAttribute"/> and <see cref="HasManyThroughAttribute"/> relationships.
-        /// This defaults to <see cref="LinkTypes.NotConfigured"/> for <see cref="HasOneAttribute"/> relationships, which means that
-        /// the configuration in <see cref="IJsonApiOptions"/> or <see cref="ResourceContext"/> is used.
-        /// </remarks>
-        public LinkTypes Links
-        {
-            get => _links;
-            set
-            {
-                if (value == LinkTypes.Paging)
-                {
-                    throw new InvalidConfigurationException($"{LinkTypes.Paging:g} not allowed for argument {nameof(value)}");
-                }
-
-                _links = value;
-            }
-        }
+        public LinkTypes Links { get; set; } = LinkTypes.NotConfigured;
 
         /// <summary>
         /// Whether or not this relationship can be included using the <c>?include=publicName</c> query string parameter.

src/JsonApiDotNetCore/Resources/Annotations/ResourceLinksAttribute.cs

@@ -1,75 +1,34 @@
 using System;
-using JsonApiDotNetCore.Errors;
+using JsonApiDotNetCore.Configuration;
 
 namespace JsonApiDotNetCore.Resources.Annotations
 {
-    // TODO: There are no tests for this.
-
     /// <summary>
     /// When put on a resource class, overrides global configuration for which links to render.
     /// </summary>
     [AttributeUsage(AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Interface)]
     public sealed class ResourceLinksAttribute : Attribute
     {
-        private LinkTypes _topLevelLinks = LinkTypes.NotConfigured;
-        private LinkTypes _resourceLinks = LinkTypes.NotConfigured;
-        private LinkTypes _relationshipLinks = LinkTypes.NotConfigured;
-
         /// <summary>
         /// Configures which links to show in the <see cref="Serialization.Objects.TopLevelLinks"/>
-        /// section for this resource.
-        /// Defaults to <see cref="LinkTypes.NotConfigured"/>.
+        /// object for this resource type.
+        /// Defaults to <see cref="LinkTypes.NotConfigured"/>, which falls back to <see cref="IJsonApiOptions.TopLevelLinks"/>.
         /// </summary>
-        public LinkTypes TopLevelLinks
-        {
-            get => _topLevelLinks;
-            set
-            {
-                if (value == LinkTypes.Related)
-                {
-                    throw new InvalidConfigurationException($"{LinkTypes.Related:g} not allowed for argument {nameof(value)}");
-                }
-
-                _topLevelLinks = value;
-            }
-        }
+        public LinkTypes TopLevelLinks { get; set; } = LinkTypes.NotConfigured;
 
         /// <summary>
         /// Configures which links to show in the <see cref="Serialization.Objects.ResourceLinks"/>
-        /// section for this resource.
-        /// Defaults to <see cref="LinkTypes.NotConfigured"/>.
+        /// object for this resource type.
+        /// Defaults to <see cref="LinkTypes.NotConfigured"/>, which falls back to <see cref="IJsonApiOptions.ResourceLinks"/>.
         /// </summary>
-        public LinkTypes ResourceLinks
-        {
-            get => _resourceLinks;
-            set
-            {
-                if (value == LinkTypes.Paging)
-                {
-                    throw new InvalidConfigurationException($"{LinkTypes.Paging:g} not allowed for argument {nameof(value)}");
-                }
-
-                _resourceLinks = value;
-            }
-        }
-
+        public LinkTypes ResourceLinks { get; set; } = LinkTypes.NotConfigured;
+        
         /// <summary>
         /// Configures which links to show in the <see cref="Serialization.Objects.RelationshipLinks"/>
-        /// for all relationships of the resource type on which this attribute was used.
-        /// Defaults to <see cref="LinkTypes.NotConfigured"/>.
+        /// object for all relationships of this resource type.
+        /// Defaults to <see cref="LinkTypes.NotConfigured"/>, which falls back to <see cref="IJsonApiOptions.RelationshipLinks"/>.
+        /// This can be overruled per relationship by setting <see cref="RelationshipAttribute.Links"/>.
         /// </summary>
-        public LinkTypes RelationshipLinks
-        {
-            get => _relationshipLinks;
-            set
-            {
-                if (value == LinkTypes.Paging)
-                {
-                    throw new InvalidConfigurationException($"{LinkTypes.Paging:g} not allowed for argument {nameof(value)}");
-                }
-
-                _relationshipLinks = value;
-            }
-        }
+        public LinkTypes RelationshipLinks { get; set; } = LinkTypes.NotConfigured;
     }
 }

test/JsonApiDotNetCoreExampleTests/IntegrationTests/Links/LinkInclusionTests.cs

@@ -0,0 +1,63 @@
+using System.Net;
+using System.Threading.Tasks;
+using FluentAssertions;
+using JsonApiDotNetCore.Serialization.Objects;
+using JsonApiDotNetCoreExampleTests.Startups;
+using TestBuildingBlocks;
+using Xunit;
+
+namespace JsonApiDotNetCoreExampleTests.IntegrationTests.Links
+{
+    public sealed class LinkInclusionTests
+        : IClassFixture<ExampleIntegrationTestContext<TestableStartup<LinksDbContext>, LinksDbContext>>
+    {
+        private readonly ExampleIntegrationTestContext<TestableStartup<LinksDbContext>, LinksDbContext> _testContext;
+        private readonly LinksFakers _fakers = new LinksFakers();
+
+        public LinkInclusionTests(ExampleIntegrationTestContext<TestableStartup<LinksDbContext>, LinksDbContext> testContext)
+        {
+            _testContext = testContext;
+        }
+
+        [Fact]
+        public async Task Get_primary_resource_with_include_applies_links_visibility_from_ResourceLinksAttribute()
+        {
+            // Arrange
+            var location = _fakers.PhotoLocation.Generate();
+            location.Photo = _fakers.Photo.Generate();
+            location.Album = _fakers.PhotoAlbum.Generate();
+
+            await _testContext.RunOnDatabaseAsync(async dbContext =>
+            {
+                dbContext.PhotoLocations.Add(location);
+                await dbContext.SaveChangesAsync();
+            });
+
+            var route = $"/photoLocations/{location.StringId}?include=photo,album";
+
+            // Act
+            var (httpResponse, responseDocument) = await _testContext.ExecuteGetAsync<Document>(route);
+
+            // Assert
+            httpResponse.Should().HaveStatusCode(HttpStatusCode.OK);
+
+            responseDocument.Links.Should().BeNull();
+
+            responseDocument.SingleData.Should().NotBeNull();
+            responseDocument.SingleData.Links.Should().BeNull();
+            responseDocument.SingleData.Relationships["photo"].Links.Self.Should().BeNull();
+            responseDocument.SingleData.Relationships["photo"].Links.Related.Should().NotBeNull();
+            responseDocument.SingleData.Relationships["album"].Links.Should().BeNull();
+
+            responseDocument.Included.Should().HaveCount(2);
+
+            responseDocument.Included[0].Links.Self.Should().NotBeNull();
+            responseDocument.Included[0].Relationships["location"].Links.Self.Should().NotBeNull();
+            responseDocument.Included[0].Relationships["location"].Links.Related.Should().NotBeNull();
+
+            responseDocument.Included[1].Links.Self.Should().NotBeNull();
+            responseDocument.Included[1].Relationships["photos"].Links.Self.Should().NotBeNull();
+            responseDocument.Included[1].Relationships["photos"].Links.Related.Should().NotBeNull();
+        }
+    }
+}

test/JsonApiDotNetCoreExampleTests/IntegrationTests/Links/LinksDbContext.cs

@@ -6,10 +6,19 @@ public sealed class LinksDbContext : DbContext
     {
         public DbSet<PhotoAlbum> PhotoAlbums { get; set; }
         public DbSet<Photo> Photos { get; set; }
+        public DbSet<PhotoLocation> PhotoLocations { get; set; }
 
         public LinksDbContext(DbContextOptions<LinksDbContext> options)
             : base(options)
         {
         }
+
+        protected override void OnModelCreating(ModelBuilder builder)
+        {
+            builder.Entity<Photo>()
+                .HasOne(photo => photo.Location)
+                .WithOne(location => location.Photo)
+                .HasForeignKey<Photo>("PhotoLocationKey");
+        }
     }
 }