Generate for spec changes #96

Nexushunter · 2023-08-09T03:56:54Z

Description

Currently the openapi-generator doesn't rerun when the oas spec changes. This prevents incremental changes to the oas spec, making it a requirement to make hacky changes. To alleviate this storing a copy of the oas spec in the build directory allowing for changes to be diffed. Another option is to use git. It should also not require flutter and should allow for dart only if the pubspec doesn't include the flutter SDK as a dependency.

Boyscouting:

added a bunch of tests to help provide improved coverage

Acceptance Criteria

Generator runs on changes to the oas spec
- Needs handle both JSON and yaml
Generator runs when a fresh build
Generator runs when a change to the config file occurs

… clean up duplicated dependency,

…tests

…sts.

Nexushunter · 2023-08-11T04:58:59Z

openapi-generator/test/method_test.dart

-      expect(
-          generator.getGeneratorNameFromEnum(n.Generator.dart), equals('dart'));
-    });
-  });
 }


Tested as part of the GeneratorArguments test suite

openapi-generator/lib/src/determine_flutter_project_status.dart

Nexushunter · 2023-08-11T05:00:44Z

openapi-generator-annotations/lib/src/openapi_generator_annotations_base.dart

Added new config options, added deprecations

Nexushunter · 2023-08-11T05:01:43Z

openapi-generator/test/generator_arguments_test.dart

+                '--type-mappings=package=type'
+              ]));
+    });
+    test('uses config', () async {


I'm not sure how best to test this. I need to build a constantreader

Test the jarargs? Just confirming it generates the right command should be enough

This is mostly to ensure that the expected values are injected correctly.

Nexushunter · 2023-08-11T05:02:23Z

openapi-generator/test/utils.dart

A bunch of refactoring

Nexushunter · 2023-08-11T05:03:09Z

openapi-generator/lib/src/models/generator_arguments.dart

Provides a typed api to interact with through out the builder

Nexushunter · 2023-08-11T05:03:23Z

openapi-generator/lib/src/gen_on_spec_changes.dart

Bulk of the next gen support

Nexushunter · 2023-08-11T05:04:38Z

openapi-generator/lib/src/openapi_generator_runner.dart

This required a lot of changes to clean up the processing of things, more work is to be done still

gibahjoe · 2023-08-12T15:49:19Z

Awesome work. You practically rewrote the entire library. Lol.

Will this support remote openapi specs? That is when a URL is provided as the input spec to the generator.

Also, I am unable to compile the code, getting an error

Nexushunter · 2023-08-12T16:01:55Z

Awesome work. You practically rewrote the entire library. Lol.

Will this support remote openapi specs? That is when a URL is provided as the input spec to the generator.

Also, I am unable to compile the code, getting an error

Thanks 😄😅 I hope that's ok. There were a lot of things that were repeated and somewhat hard to grok but I wanted to keep support for your original workflow to allow for a nice migration path. This should still support that as the value gets passed to the openapi jar.

I will add a test to keep that support. That also brings up a good case for me to test for the load spec and caching!

If you comment out that function and the use config test block it should build again.

Nexushunter · 2023-08-12T16:04:20Z

@gibahjoe Do you know an easy way to gather the reader? I want to ensure that the GeneratorArguments class parses the config as expected.

gibahjoe · 2023-08-12T17:00:23Z

Take a look at this test library which provides useful utility methods source_gen_test.

I guess, you can use this method to generate a library reader initializeLibraryReaderForDirectory and then get the constant reader from there var reader=libraryReader.annotatedWith(checker).first.annotation;

Have not tested it myself though.

…k the classes from annotations to have the toMap call, Look into why inline schema name mappings failing to produce first entry

Nexushunter · 2023-08-17T11:58:00Z

@gibahjoe 2 things:

Do I need to update the commit messages?
- I think that minor version bump will be fine as this leaves the original functionality intact while adding net new functionality that is optional.
It looks like there is a circular dependency in example where it needs the api/petstore as per the pubspec but it doesn't exist until the build_runner is built.
- I can adjust this to be a nested directory but that would change the example fundamentally.

gibahjoe · 2023-08-17T12:18:14Z

Do I need to update the commit messages?

You can leave that. I will let this pr go if all other checks pass.

It looks like there is a circular dependency in example where it needs the api/petstore as per the pubspec but it doesn't exist until the build_runner is built.

That file is meant to be in the repo. Must have been removed by mistake. You can see the unignore line here.

Please add it back.

!api/petstore_api/pubspec.yaml

…turns

Nexushunter · 2023-08-17T13:31:06Z

@gibahjoe The jar is unable to find the spec when the path exists. I've tried relative and absolute paths. Is this something you've run into? It seems like the jar is up to date

That file is meant to be in the repo. Must have been removed by mistake. You can see the unignore line here.

It's not something I removed but i believe it should get generated and I will add it back once I can get it generating again.

gibahjoe · 2023-08-17T14:22:47Z

@gibahjoe The jar is unable to find the spec when the path exists. I've tried relative and absolute paths. Is this something you've run into? It seems like the jar is up to date

@Nexushunter
No. What configuration are you using to run the generator?

Nexushunter · 2023-08-17T14:32:47Z

I'm running flutter pub run build_runner build --delete-conflicting-outputs within the root example directory. (I had to comment out the petstore dependency to be able to flutter pub get)

gibahjoe · 2023-08-17T19:22:25Z

Note: New lines only included in logged output for debugging purposes

[INFO] openapi_generator on lib/main.dart:
 - :::::::::::::::::::::::::::::::::::::::::::
 - ::      Openapi generator for dart       ::
 - :::::::::::::::::::::::::::::::::::::::::::
[INFO] openapi_generator on lib/main.dart:OpenapiGenerator :: [ generate
-o api/petstore_api
-i openapi-spec.yaml
-g dart-dio
--type-mappings=Pet=ExamplePet
--additional-properties=allowUnicodeIdentifiers=false,ensureUniqueParams=true,useEnumExtension=true,prependFormOrBodyParameters=false,pubAuthor=Johnny dep..,pubName=petstore_api,legacyDiscriminatorBehavior=true,sortModelPropertiesByRequiredFlag=true,sortParamsByRequiredFlag=true,wrapper=none ]
[SEVERE] openapi_generator on lib/main.dart:
 - :: Codegen Failed. Generator output: ::
[error] The spec file is not found:  openapi-spec.yaml
[error] Check the path of the OpenAPI spec and try again.

[INFO] Running build completed, took 9.0s

This is taken from the example directory at the root of the repo (which is what I'm using)

@Openapi(
    additionalProperties:
        DioProperties(pubName: 'petstore_api', pubAuthor: 'Johnny dep..'),
    inputSpecFile: 'openapi-spec.yaml',
    typeMappings: {'Pet': 'ExamplePet'},
    generatorName: Generator.dio,
    runSourceGenOnOutput: true,
    alwaysRun: true,
    outputDirectory: 'api/petstore_api')
class MyApp extends StatelessWidget {
/// Rest of MyApp

Sorry, just seeing this. Have you figured out the issue? these config look fine to me

Nexushunter · 2023-08-17T19:33:41Z

No worries!

I have not, I spent some time scouring the net and couldn't find an issue for this (outside of a docker issue, which isn't being used here). As far as I'm aware the jar is throwing the error. I also went and pulled down a new copy of the jar with no luck. I'm super confused 🤔 I will double check but I'm almost certain it's not a directory permission either.

Nexushunter · 2023-08-17T23:03:24Z

Ok, so it seems that it is definitely something happening during the processing as I ran java -jar lib/openapi-generator.jar generate -i https://raw.githubusercontent.com/openapitools/openapi-generator/master/modules/openapi-generator/src/test/resources/3_0/petstore.yaml -o api -g dart from within the openapi-generator-cli directory and it worked as expected

Nexushunter · 2023-08-17T23:35:02Z

I found a solution. It seems like the space was being added. So I swapped out the space for an = and it generated.

…he analyzer warnings, fmt

Nexushunter · 2023-08-18T17:01:06Z

@gibahjoe The checks should pass now. I was able to get it all building locally.

Nexushunter · 2023-08-18T19:13:21Z

@gibahjoe 🎉🎉 it's finally passing so excited for this! Thanks for being so patient and responsive with everything!

gibahjoe · 2023-08-18T19:43:51Z

Great work. I am running a few manual checks to ensure everything is great.

Thank you for this.

gibahjoe · 2023-08-18T20:36:11Z

@Nexushunter

Just a few more hurdles to jump through. Kindly check my reviews.

Again, I appreciate the effort.

Nexushunter · 2023-08-18T21:03:20Z

@gibahjoe I'm not seeing them. Are you referring to the comment above about testing the jarArgs?

gibahjoe · 2023-08-18T21:09:09Z

@gibahjoe I'm not seeing them. Are you referring to the comment above about testing the jarArgs?

Not that.
Just below this event #96 (comment). I left a couple of reviews

Nexushunter · 2023-08-18T21:24:25Z

@gibahjoe This is what I'm seeing. I did readd the pubspec as requested (which is why the flutter example passed) and the rest of the messages are about getting the config to run. Could you list out what is missing / wrong?

As an aside: I am currently looking into why there isn't as many logs coming through as I would expect.

gibahjoe

Please add some documentation that "useNextGen" flag would be removed in the future and next gen would be the default mode.
A modification to the annotation is still required for the sourcegen framework to trigger the openapi generator (which in turn then checks for diff between cache spec and local spec).
Is this the desired behavior?
For example:- If you run dart run build_runner build --delete-conflicting-outputs and the generator kicks in then make changes to your spec and run it again, the generator won't kick in again until changes are made to the annotation itself.
I made some changes to a spec but the generator still does not run ( I get No diff between versions, not running generator). Perhaps a regular JSON string comparison will be more thorough when checking for diffs? The specific change I made was to add a model under component schema

openapi-generator-annotations/lib/src/openapi_generator_annotations_base.dart

openapi-generator/lib/src/gen_on_spec_changes.dart

example/lib/main.dart

gibahjoe · 2023-08-18T21:34:10Z

@gibahjoe This is what I'm seeing. I did readd the pubspec as requested (which is why the flutter example passed) and the rest of the messages are about getting the config to run. Could you list out what is missing / wrong?

As an aside: I am currently looking into why there isn't as many logs coming through as I would expect.

My bad. I forgot to submit the reviews

…run. Add documentation around the usage of useNextGen, add tests

Nexushunter · 2023-08-19T02:43:31Z

@gibahjoe I've added the work around and made the require compare and doc tweaks 🎉

gibahjoe · 2023-08-19T11:07:12Z

@gibahjoe I've added the work around and made the require compare and doc tweaks 🎉

Perfect. LGTM.
Looks like a check is failing. I will publish once its fixed.

You have done an awesome job.

Nexushunter · 2023-08-19T13:14:08Z

@gibahjoe All fixed!
🤦🏻 I should really stop using Dart 3 while working with this lmao.

Thanks! I appreciate that 😄

gibahjoe

Great.

AndreBlumenthal · 2023-08-21T09:47:15Z

Thank you for your changes. I see that you deprecated alwaysRun and overwriteExistingFiles. Do I understand this PR correctly, that we should remove them and put useNextGen to true?

Nexushunter · 2023-08-21T11:08:52Z

@AndreBlumenthal

Np!

Yes 😄 that is the recommendation. With useNextGen it opens up a path to use incremental updates to the oas spec. It does have some current limitations around remote specs (only public are supported) but I do have a WIP to address that issue.

AndreBlumenthal · 2023-08-21T11:55:39Z

@AndreBlumenthal

Np!

Yes 😄 that is the recommendation. With useNextGen it opens up a path to use incremental updates to the oas spec. It does have some current limitations around remote specs (only public are supported) but I do have a WIP to address that issue.

Thank you 🙏 🙌

Nexushunter added 2 commits August 8, 2023 19:22

Provide Yaml and Json version of test spec

39a7aa1

Refactor spec loading, comment out the dependency on the flutter sdk,…

d317ed9

… clean up duplicated dependency,

gibahjoe added the generator label Aug 9, 2023

Nexushunter added 3 commits August 9, 2023 09:41

Provide support for nested lists, reorganize some tests

49a96e2

Provide support for nested lists, reorganize some tests, refine some …

a17d7bb

…tests

(WIP) Large refactor to get things moving in a nicer way. Adds ~80 te…

0f833d7

…sts.

gibahjoe added the annotations label Aug 11, 2023

Nexushunter commented Aug 11, 2023

View reviewed changes

(WIP) FMT, fix up remaining tests for Generator Arguments. TODO: Twea…

985de8b

…k the classes from annotations to have the toMap call, Look into why inline schema name mappings failing to produce first entry

gibahjoe added the github actions label Aug 13, 2023

Nexushunter added 4 commits August 13, 2023 14:42

(WIP) Address failing original tests. Add tests for NextGen path.

b2f03f2

All tests passing

a619f4d

Add more tests for the annotations library

763f115

Add example builder annotation tests

f7d5737

Nexushunter marked this pull request as ready for review August 13, 2023 23:29

Nexushunter requested a review from gibahjoe August 16, 2023 22:14

Fix the dart ci not being run in the working dirs

4e1898d

doc: Add next gen documentation to readme

0df70e1

gibahjoe added the docs label Aug 17, 2023

fix: Tweak the processing of the builder to handle how process.run re…

90a1b90

…turns

Nexushunter added 4 commits August 17, 2023 19:43

fix: fix gitignore, readd api/petstore_api/pubspec

37f862c

fix: fix tests now that the generator is running correctly

4643a0c

fix: use bool.tryParse instead of casting

5003ea1

fix: Use API within SDK bounds (will change in future release), fix t…

c229767

…he analyzer warnings, fmt

fix: Last few tweaks

12c138a

gibahjoe self-assigned this Aug 18, 2023

gibahjoe reviewed Aug 18, 2023

View reviewed changes

openapi-generator-annotations/lib/src/openapi_generator_annotations_base.dart Show resolved Hide resolved

openapi-generator/lib/src/gen_on_spec_changes.dart Outdated Show resolved Hide resolved

example/lib/main.dart Show resolved Hide resolved

Nexushunter mentioned this pull request Aug 18, 2023

Add ability to always run dart-lang/build#3561

Closed

fix: Add work around to be able to always be able to regenerate when …

523f169

…run. Add documentation around the usage of useNextGen, add tests

fix: Use Dart 2 API

8d59613

gibahjoe approved these changes Aug 19, 2023

View reviewed changes

gibahjoe merged commit 52e6821 into gibahjoe:master Aug 19, 2023
5 of 6 checks passed

Generate for spec changes #96

Generate for spec changes #96

Conversation

Nexushunter commented Aug 9, 2023 • edited Loading

Description

Acceptance Criteria

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gibahjoe commented Aug 12, 2023

Nexushunter commented Aug 12, 2023

Nexushunter commented Aug 12, 2023

gibahjoe commented Aug 12, 2023

Nexushunter commented Aug 17, 2023

gibahjoe commented Aug 17, 2023

Nexushunter commented Aug 17, 2023 • edited Loading

gibahjoe commented Aug 17, 2023 • edited Loading

Nexushunter commented Aug 17, 2023

gibahjoe commented Aug 17, 2023

Nexushunter commented Aug 17, 2023 • edited Loading

Nexushunter commented Aug 17, 2023

Nexushunter commented Aug 17, 2023

Nexushunter commented Aug 18, 2023

Nexushunter commented Aug 18, 2023

gibahjoe commented Aug 18, 2023

gibahjoe commented Aug 18, 2023

Nexushunter commented Aug 18, 2023

gibahjoe commented Aug 18, 2023

Nexushunter commented Aug 18, 2023

gibahjoe left a comment

Choose a reason for hiding this comment

gibahjoe commented Aug 18, 2023

Nexushunter commented Aug 19, 2023

gibahjoe commented Aug 19, 2023

Nexushunter commented Aug 19, 2023

gibahjoe left a comment

Choose a reason for hiding this comment

AndreBlumenthal commented Aug 21, 2023

Nexushunter commented Aug 21, 2023

AndreBlumenthal commented Aug 21, 2023

Nexushunter commented Aug 9, 2023 •

edited

Loading

Nexushunter commented Aug 17, 2023 •

edited

Loading

gibahjoe commented Aug 17, 2023 •

edited

Loading

Nexushunter commented Aug 17, 2023 •

edited

Loading