Generate for spec changes

Nexushunter commented 1 year ago

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

gibahjoe commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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.

Nexushunter commented 1 year ago

@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 commented 1 year ago

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

Nexushunter commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

There should be a console output of the generator command that is going to be passed to the library. Please share. also share your annotation config

it should look like generate -i ....

Nexushunter commented 1 year ago

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

gibahjoe commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

Nexushunter commented 1 year ago

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

Nexushunter commented 1 year ago

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

gibahjoe commented 1 year ago

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

Thank you for this.

gibahjoe commented 1 year ago

@Nexushunter

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

Again, I appreciate the effort.

Nexushunter commented 1 year ago

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

gibahjoe commented 1 year ago

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

Not that. Just below this event https://github.com/gibahjoe/openapi-generator-dart/pull/96#event-10113149245. I left a couple of reviews

Nexushunter commented 1 year ago

@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 commented 1 year ago

@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

Nexushunter commented 1 year ago

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

gibahjoe commented 1 year ago

@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 commented 1 year ago

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

Thanks! I appreciate that 😄

AndreBlumenthal commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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 🙏 🙌

gibahjoe / openapi-generator-dart

Generate for spec changes #96

Description

Acceptance Criteria