Skip to content
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

Dictionary values are always nullable. #2587

Closed
pms1969 opened this issue Jan 13, 2023 · 2 comments
Closed

Dictionary values are always nullable. #2587

pms1969 opened this issue Jan 13, 2023 · 2 comments

Comments

@pms1969
Copy link

pms1969 commented Jan 13, 2023

I was recently tasked with upgrading Swashbuckle in our codebase from 5.4.1, to what is now 6.5.0; tho I did most of my update with a build straight of of master due to the bug fixed in #2529.

The formatted swagger.json is some 110K, which is quite hefty, and the differences were extensive. In most cases, this was a stricter better change, but one class of change in particular hurt me. Dictionary's for projects that do not yet have implement the Nullability for reference types are producing schema's where the values are nullable. 5.4.1 would produce this:

          "instrumentIdentifiers": {
            "type": "object",
            "additionalProperties": {
              "type": "string",
            },
            "description": "The instrument ordered."
          },

and 6.x produces this

          "instrumentIdentifiers": {
            "type": "object",
            "additionalProperties": {
              "type": "string",
              "nullable": true
            },
            "description": "The instrument ordered."
          },

For me, this is breaking as we use other tools to generate SDK's based on the swagger.json. I have tried a few things to remove the "nullable": true, but have been completely bamboozaled. I am unable to enable nullable reference types in our solution.

Specifically, I've tried to specify what the schema should look like by providing a MapType definition for all the cases that are important to me; e.g.

...
            swaggerOptionsFilter.MapType<Dictionary<string, string>>(() => new OpenApiSchema
            {
                Type = "object",
                AdditionalProperties = new OpenApiSchema
                { 
                    Type = "string", 
                    Nullable = false,
                }, 
            });
...

But regardless of this override, I still get a nullable value. Not what I was expecting.

So I think this is a bug. If have a type map, I should get what I asked for. I have made a change to a local copy, and know where the change needs to take place. But before I submit a PR, I just wanted to ask and see if I was being a bit dippy, and there is in fact a way to get the behaviour I am looking for? Also, is this something you would consider taking as a PR, or not something you want? FWIW, it's not a big change;

diff --git a/src/Swashbuckle.AspNetCore.SwaggerGen/SchemaGenerator/SchemaGenerator.cs b/src/Swashbuckle.AspNetCore.SwaggerGen/SchemaGenerator/SchemaGenerator.cs
index 5ad1494..d0f7000 100644
--- a/src/Swashbuckle.AspNetCore.SwaggerGen/SchemaGenerator/SchemaGenerator.cs
+++ b/src/Swashbuckle.AspNetCore.SwaggerGen/SchemaGenerator/SchemaGenerator.cs
@@ -92,7 +92,10 @@ namespace Swashbuckle.AspNetCore.SwaggerGen
                 // NullableAttribute behaves diffrently for Dictionaries
                 if (schema.AdditionalPropertiesAllowed && modelType.IsGenericType && modelType.GetGenericTypeDefinition() == typeof(Dictionary<,>))
                 {
-                    schema.AdditionalProperties.Nullable = !memberInfo.IsDictionaryValueNonNullable();
+                    if (!HasCustomTypeMapping(modelType))
+                    {
+                        schema.AdditionalProperties.Nullable = !memberInfo.IsDictionaryValueNonNullable();
+                    }
                 }

                 schema.ApplyValidationAttributes(customAttributes);
@@ -264,7 +267,12 @@ namespace Swashbuckle.AspNetCore.SwaggerGen
         private bool TryGetCustomTypeMapping(Type modelType, out Func<OpenApiSchema> schemaFactory)
         {
             return _generatorOptions.CustomTypeMappings.TryGetValue(modelType, out schemaFactory)
-                || (modelType.IsConstructedGenericType && _generatorOptions.CustomTypeMappings.TryGetValue(modelType.GetGenericTypeDefinition(), out schemaFactory));
+                   || (modelType.IsConstructedGenericType && _generatorOptions.CustomTypeMappings.TryGetValue(modelType.GetGenericTypeDefinition(), out schemaFactory));
+        }
+
+        private bool HasCustomTypeMapping(Type modelType)
+        {
+            return _generatorOptions.CustomTypeMappings.ContainsKey(modelType);
         }

         private OpenApiSchema CreatePrimitiveSchema(DataContract dataContract)
@drod3763
Copy link

drod3763 commented Oct 5, 2023

I think I have the same problem. Were you able to find a workaround?

@martincostello
Copy link
Collaborator

To make issue tracking a bit less overwhelming for the new maintainers (see #2778), I've created a new tracking issue to roll-up various nullability issues here: #2793.

We'll refer back to this issue from there and include it as part of resolving that issue, but I'm going to close this one to help prune the backlog.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants