Skip to content

Parameters Serialization - Style doesn't respect the default value #858

Closed

Description

@safepage

This is similar to the issue in Pull Request 544.

There is now an inconsistency as Style is not set to a default value (but Explode is):
“Default values (based on value of in): for query - form; for path - simple; for header - simple; for cookie - form.”

I assume the fix would be similar?

Although the serialization code for Explode also looks wrong:

writer.WriteProperty(OpenApiConstants.Explode, Explode, Style.HasValue && Style.Value == ParameterStyle.Form);

Maybe it should be:

writer.WriteProperty(OpenApiConstants.Explode, _explode);

Then the serialized model would reflect the actual values read in (as _explode is a nullable bool?).

Activity

  1. added this to the NET:1.4 milestone on Jul 4, 2022
  2. darrelmiller commented on Jul 4, 2022

    @darrelmiller
    Member

    Thanks for reporting this. We will need to have a local field to track any explicitly set style value and if that is not set then return a style based on the In value. This does raise the question of why is In nullable if it is a required field. We can't put the In value in a constructor as that would make deserializing hard. We could set a default value in the class. I'm just not sure if that is a breaking change or not.

    Can you tell me more about the scenario where you ran into this problem? Thanks.

  3. safepage commented on Aug 25, 2022

    @safepage
    Author

    Sorry for the delay...

    I had to write the following workaround, as the model can be saying parameter.Explode == false if it is not explicitly set in the OpenAPI document, but if you work it out from the rules then Explode may in fact need to be true:

    `internal static bool GetExplodeValue(OpenApiParameter parameter)
    {
    bool explodeArray = true;

    // can't trust parameter.Explode if we don't have a Style, so work it out from defaults
    
    if (parameter.Style != null)
        explodeArray = parameter.Explode;
    else if (parameter.In == ParameterLocation.Query || parameter.In == ParameterLocation.Cookie)
        explodeArray = true;
    else
        explodeArray = false;
    
    return explodeArray;
    

    }`

    So I think it may be better if Explode was set to null if it is not explicitly set in the OpenAPI as it is misleading, or actually work it out from the rules (but then the model doesn't reflect the document). The same applies to style.

  4. darrelmiller commented on Aug 30, 2022

    @darrelmiller
    Member

    Let's not set a default for the In parameter because that would prevent us from knowing if a user has explicitly set it in the future.
    Let's create a backing property for style to know if the user has explicitly set it. If they have not set style then we should return a default value of style based on the In parameter. If the In parameter is null then the Style should return as Simple, as if the In was set to Path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

type:bugA broken experience

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions