Skip to content

Conversation

@JoachimL
Copy link

@JoachimL JoachimL commented Jul 9, 2021

See #170

The current implementation, which changes the casing of enum values, is confusing our API clients.

@ghost
Copy link

ghost commented Jul 9, 2021

CLA assistant check
All CLA requirements met.

@joostvanhassel
Copy link
Contributor

Hi @JoachimL, This should already be solved by #159 (which is currently unreleased)

@JoachimL
Copy link
Author

JoachimL commented Jul 9, 2021

No, I don't think it is. The current main branch still accepts a NamingStrategy parameter, which is hard-coded to Camel Case. Check out https://github.com/healthfitnessnordic/azure-functions-openapi-extension/commits/feature/enum-names which I mentioned in my issue - it is based on main (including #159), and has the problem I describe.

@joostvanhassel
Copy link
Contributor

In EnumExtensionsTests.cs it's validated an enum value starting with a Capital letter will be returned as such (here). Please note de optional parameter here defaults to DefaultNamingStrategy (set here). This namingstrategy does not apply changes to the value. From the documentation: The default naming strategy. Property names and dictionary keys are unchanged.

@JoachimL
Copy link
Author

JoachimL commented Jul 9, 2021

I am aware of that. Problem is, the naming strategy is hard coded to be Camel Case further up type chain, so it doesn’t matter what the default is. If you have a look at the example I referenced, you will see how this looks in a real life scenario.

This is actually a very good example of when unit tests aren’t enough.

@joostvanhassel
Copy link
Contributor

In the PR I mentioned, the call of the method does not contain a NamingStrategy: line chaned

I had the exact same problem. Using the code from the PR, I no longer have the problem. But please note it has not yet been released; verion 0.7.2-preview still has the issue.

@JoachimL
Copy link
Author

JoachimL commented Jul 9, 2021

I’ll test when I’m back at a computer. As I’ve said a couple of times, current main branch still has the issue.

@JoachimL
Copy link
Author

JoachimL commented Jul 9, 2021

Had a thought: could it be that the previous PR only fixes inbound enums (eg as part of a POST/PUT payload) while mine fixes enums as part of a return payload?

@joostvanhassel
Copy link
Contributor

🤔 perhaps; will test in a minute

@joostvanhassel
Copy link
Contributor

Using latest main I get the proper enums in both request models and response models (screenshot below).

Screenshot 2021-07-09 at 15 33 36

For reference:

  public class TestModel
  {
    public DrinkSize DrinkSize { get; set; }
  }
  [JsonConverter(typeof(StringEnumConverter))]
  public enum DrinkSize
  {
    Small,
    Medium,
    Large
  }

@JoachimL JoachimL force-pushed the feature/leave-casing-unchanged-for-enum-values branch from 6bdae20 to e56f988 Compare July 9, 2021 17:36
@JoachimL
Copy link
Author

JoachimL commented Jul 9, 2021

You are absolutely right. It already works. Closing :-)

@JoachimL JoachimL closed this Jul 9, 2021
@JoachimL JoachimL deleted the feature/leave-casing-unchanged-for-enum-values branch July 9, 2021 17:54
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

Successfully merging this pull request may close these issues.

3 participants