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

[feature] Ability to generate mutable models #247

Conversation

rockgecko-development
Copy link

@rockgecko-development rockgecko-development commented Sep 22, 2021

I needed the ability to generate mutable, non-final models, so I went ahead and added it.
Offering this PR if you'd like the option as well.

Usage:
new option models_immutable, default is false. When true, the final keyword is omitted from model properties.

I also restored (#204 ) the functionality of the use_path_for_request_names option, because it wasn't working and I needed it. Default is now true. When false, the swagger operationId is used as the method name (but converted to lowerCamelCase). The path-generated method names looked particularly ugly for me, whereas in my C# Azure Function I have declared the operationId as the same as the Azure Function (& C# method) name, so this lines up perfectly for me. (I'm using this project in the backend: https://github.com/Azure/azure-functions-openapi-extension )

I also added swagger method description fields as comments, in addition to the existing summary. The azure-functions-openapi-extension only seems to generate descriptions.

Finally, I added unit tests for the codegen output with models_immutable both true and false.

@codecov
Copy link

codecov bot commented Sep 22, 2021

Codecov Report

Merging #247 (c406ce3) into master (1b14fcc) will decrease coverage by 0.36%.
The diff coverage is 68.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #247      +/-   ##
==========================================
- Coverage   77.23%   76.86%   -0.37%     
==========================================
  Files           9        9              
  Lines         751      765      +14     
==========================================
+ Hits          580      588       +8     
- Misses        171      177       +6     
Impacted Files Coverage Δ
...c/code_generators/swagger_additions_generator.dart 100.00% <ø> (ø)
lib/src/models/generator_options.dart 25.00% <ø> (ø)
lib/src/models/generator_options.g2.dart 0.00% <0.00%> (ø)
...rc/code_generators/swagger_requests_generator.dart 98.10% <91.66%> (-0.51%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b14fcc...c406ce3. Read the comment docs.

@fryette
Copy link
Contributor

fryette commented Sep 22, 2021

Why do you need mutable models?
We introduced copyWith specially for that case

Also using data models in UI is strongly not recommended. Better to have Domain models for that puprpuse.

@rockgecko-development
Copy link
Author

Yes I'm aware of copyWith, certainly I had to update that part of the codegen after removing the search for 'final '.
But that's quite awkward to use if you need to change a single deeply nested field in a parameter of a POST body. This can happen eg if you have a filter API that uses a shared filter model on the frontend and backend (just as an example).

Sure, not recommended for certain situations, quite useful for other situations. A codegen library can give you the option, it doesn't have to dictate. (The openapi-generator project using the dart generator creates mutable models, the dart-dio-next generator creates immutable).

@rockgecko-development
Copy link
Author

I actually found this project via this thread in the openapi-generator-dart project, and was inspired by this relevant feature request: gibahjoe/openapi-generator-dart#41 (comment)

As nice as immutability is a nice feature, it's a massive pain when passing and manipulating any graph deeper than one level; I think I'm about to cry. The rewriting everything to extract, rebuild, repackage, from all the prior dart generator contracts... it's not only tedious, it's noisy and feels prone to error if the reassignment isn't done in the proper order, etc.

@Vovanella95
Copy link
Collaborator

Hi @rockgecko-development , It's very good idea to have mutable objects. Unfortunately it contradicts our main idea. Feel free to fork out library and modify it, but we're not going to support mutable models.

@Vovanella95 Vovanella95 closed this Oct 5, 2021
@rockgecko-development
Copy link
Author

What about the other 2 features in this PR?

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.

4 participants