Skip to content

Conversation

@agilob
Copy link
Contributor

@agilob agilob commented Oct 13, 2020

PR checklist

  1. Generate code from more complex schema
  2. Delete samples that were used before previous input schema (2 and 3 versions ago)

Current sample produces code that doesn't compile, but at least it gives us a target and exposes issues directly

Closes #7588

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • If contributing template-only or documentation-only changes which will change sample output, build the project beforehand.
  • Run the shell script ./bin/generate-samples.shto update all Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master. These must match the expectations made by your contribution. You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*. For Windows users, please run the script in Git BASH.
  • File the PR against the correct branch: master
  • Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

@agilob
Copy link
Contributor Author

agilob commented Oct 13, 2020

@noordawod the code generated in samples/client/petstore/dart2/petstore_client_lib_fake_endpoints/ is quite complex and it sets new target for dart-openapi compatibility. The code currently doesn't compile, so at least we know what's next to do in dart generator :)

@noordawod
Copy link
Contributor

@agilob let me know if you need some help ;)

@agilob
Copy link
Contributor Author

agilob commented Oct 13, 2020

circle ci doesnt use docker images?

Reading package lists... Done
W: An error occurred during the signature verification. The repository is not updated and the previous index files will be used. GPG error: https://packagecloud.io trusty InRelease: The following signatures couldn't be verified because the public key is not available: NO_PUBKEY 4E6910DFCB68C9CD

W: Failed to fetch https://packagecloud.io/circleci/trusty/ubuntu/dists/trusty/InRelease  

W: Some index files failed to download. They have been ignored, or old ones used instead.
Reading package lists... Done
Building dependency tree       
Reading state information... Done
The following NEW packages will be installed:
  dart
0 upgraded```

@noordawod
Copy link
Contributor

No idea...

@noordawod
Copy link
Contributor

@agilob
What I found from previous experience with CircleCI is that it barfs occasionally without any particular reason. So I suggest you to try rerunning the build. This is true also for other projects that I run on CircleCI.

@agilob agilob changed the title Dart2: generate from complex schema [Dart] generate from complex schema Oct 16, 2020
@wing328
Copy link
Member

wing328 commented Oct 27, 2020

I think you will need to move samples/client/petstore/dart2/petstore/pom.xml to the new location instead of deleting it.

@wing328
Copy link
Member

wing328 commented Oct 29, 2020

One more thing: please update pom.xml and replace the old dart petstore location with the new one.

@agilob
Copy link
Contributor Author

agilob commented Oct 29, 2020

done

@wing328
Copy link
Member

wing328 commented Nov 1, 2020

Looks like there are some issues with the dart2 petstore client as reported by the CirclecCI (partial result below):

  lib/api/another_fake_api.dart:28:30: Error: Expected '{' before this.
    Future<Response> 123test@$%SpecialTagsWithHttpInfo(Client client) async {
                               ^^^^^^^^^^^^^^^^^^^^^^^
  lib/api/another_fake_api.dart:28:29: Error: Operator '%' should have exactly one parameter.
    Future<Response> 123test@$%SpecialTagsWithHttpInfo(Client client) async {
                              ^
  lib/api/another_fake_api.dart:79:3: Error: A method declaration needs an explicit list of parameters.
  Try adding a parameter list to the method declaration.
    Future<Client> 123test@$%SpecialTags(Client client) async {
    ^^^^^^
01:32 +0 -56: loading test/tag_test.dart [E]                                                                                                                                                           
  Failed to load "test/tag_test.dart":
  Unable to spawn isolate: lib/api/another_fake_api.dart:28:3: Error: A method declaration needs an explicit list of parameters.
  Try adding a parameter list to the method declaration.
    Future<Response> 123test@$%SpecialTagsWithHttpInfo(Client client) async {
    ^^^^^^
  lib/api/another_fake_api.dart:28:20: Error: Expected '{' before this.
    Future<Response> 123test@$%SpecialTagsWithHttpInfo(Client client) async {
                     ^^^
  lib/api/another_fake_api.dart:28:20: Error: Expected a class member, but got '123'.
    Future<Response> 123test@$%SpecialTagsWithHttpInfo(Client client) async {
                     ^^^
  lib/api/another_fake_api.dart:28:23: Error: Variables must be declared using the keywords 'const', 'final', 'var' or a type name.
  Try adding the name of the type of the variable or the keyword 'var'.
    Future<Response> 123test@$%SpecialTagsWithHttpInfo(Client client) async {
                        ^^^^
  lib/api/another_fake_api.dart:28:23: Error: Expected ';' after this.
    Future<Response> 123test@$%SpecialTagsWithHttpInfo(Client client) async {
                        ^^^^
  lib/api/another_fake_api.dart:28:29: Error: Operator declarations must be preceded by the keyword 'operator'.
  Try adding the keyword 'operator'.
    Future<Response> 123test@$%SpecialTagsWithHttpInfo(Client client) async {
                              ^
  lib/api/another_fake_api.dart:28:29: Error: A method declaration needs an explicit list of parameters.
  Try adding a parameter list to the method declaration.
    Future<Response> 123test@$%SpecialTagsWithHttpInfo(Client client) async {
                              ^
  lib/api/another_fake_api.dart:28:30: Error: Expected '{' before this.
    Future<Response> 123test@$%SpecialTagsWithHttpInfo(Client client) async {
                               ^^^^^^^^^^^^^^^^^^^^^^^
  lib/api/another_fake_api.dart:28:29: Error: Operator '%' should have exactly one parameter.
    Future<Response> 123test@$%SpecialTagsWithHttpInfo(Client client) async {
                              ^
  lib/api/another_fake_api.dart:79:3: Error: A method declaration needs an explicit list of parameters.
  Try adding a parameter list to the method declaration.
    Future<Client> 123test@$%SpecialTags(Client client) async {
    ^^^^^^
01:32 +0 -56: Some tests failed.                                                                                                                                                                       
[ERROR] Failed to execute goal org.codehaus.mojo:exec-maven-plugin:1.2.1:exec (pub-test) on project Dart2PetstoreClientTests: Command execution failed. Process exited with an error: 1 (Exit value: 1) -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
[ERROR] 
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <goals> -rf :Dart2PetstoreClientTests

Ref: https://app.circleci.com/pipelines/github/OpenAPITools/openapi-generator/6672/workflows/43411d7e-0154-48ac-94c1-42c137e74f3e/jobs/20138

@agilob
Copy link
Contributor Author

agilob commented Nov 1, 2020

It required dart names to be sanitized as names cant start with number and contain special charts. Added code for that. Complex dart code still doesnt compile well as the spec contains classes that are dart-code classes like Map and List. I've handled List but run of time today to take care of Map<>

@agilob
Copy link
Contributor Author

agilob commented Nov 22, 2020

@wing328 pls

@agilob agilob changed the title [Dart] generate from complex schema [Dart] generate from complex schema, class and field name sanitation Nov 22, 2020
@wing328
Copy link
Member

wing328 commented Nov 23, 2020

Sorry we were busy with the v5.0.0-beta3 release before. Can you please ping me via Slack to further discuss this when you've time as I still see some issues with the Dart clients in the CI?

@kuhnroyal
Copy link
Contributor

I just saw this PR, I have some similar work done here https://github.com/kuhnroyal/openapi-generator/tree/feature/dart/advanced, trying to reuse specialCharReplacements from DefaultCodegen and also escaping variable names, operationIds and so on. Maybe you can take a look and see if we can merge efforts.

@agilob
Copy link
Contributor Author

agilob commented Dec 2, 2020

Maybe you can take a look and see if we can merge efforts.

Can you open a PR from my branch targeting my branch?

@kuhnroyal
Copy link
Contributor

It currently seems unrealistic to generate the full fake-api for both dart and dart-dio, there are just way to many compile errors. I have solved most but the branch just gets way too big.
Instead I took a step back and started with the simple petstore example and did some cleanup. I think this is a better baseline for complex schema support.
See #8085

@noordawod
Copy link
Contributor

Nice changes @agilob !

@agilob
Copy link
Contributor Author

agilob commented Dec 3, 2020

Hopefully all tests pass now and we can stop duplicating efforts on the same problems.

@kuhnroyal
Copy link
Contributor

I really doubt it, the whole dart package doesn't compile. Your integration tests fail with several compile errors.
besides that you have deleted all the existing integration tests from samples/client/petstore/dart2/petstore/test/... They were not empty but actually contained non-generated integration tests.

@agilob
Copy link
Contributor Author

agilob commented Dec 3, 2020

I really doubt it

You doubt that this will prevent people duplicating efforts or tests will (eventually) pass?

When the tests fail, it's a good place to start if you want to help pushing this forward. In this moment it's a positive sign when something new breaks, means previous problem has been fixed and we're heading toward another detail of specification to implement. I purposely chosen the most complex spec file so it can highlight all problems and instead caring about whitespaces or variable name in generated code, contributors can pick up the next failing thing.

If you don't want to help, we can always make a few incompatible versions of this generator. Of course everybody wins in that case.

@kuhnroyal
Copy link
Contributor

kuhnroyal commented Dec 3, 2020

You doubt that this will prevent people duplicating efforts or tests will (eventually) pass?

I doubt that the branch will go green in the next few weeks and even if so that anybody can actually review it. It just too much.

I have spent the better part of this and last week on getting this to work. I have a lot of it solved but the PR is gonna get so big that nobody can actually review it. I suggest to start small with the basic 3.0 petstore example. This already doesn't compile so its a good start to get that working and then iterate on.

Afterwards we can iterate on the fake-api without adding the integration tests to the master POM, as long as no existing tests for the simple petstore break this should be fine.

@agilob
Copy link
Contributor Author

agilob commented Dec 3, 2020

have spent the better part of this and last week on getting this to work

Why havent you committed any fixes?

@kuhnroyal
Copy link
Contributor

I linked you one of my branches here: https://github.com/kuhnroyal/openapi-generator/tree/feature/dart/advanced
It already fixes escaping, naming clashes, decimal support, int/double based enums, set support and there is still so much to go that I stopped and took a step back to start with the smaller version and then cherrypick small bits.

@agilob
Copy link
Contributor Author

agilob commented Dec 3, 2020

Cool, looks like you're ahead of this branch :)

@agilob agilob closed this Dec 3, 2020
@agilob agilob deleted the dart-more-complex-schema branch September 25, 2021 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Dart] Use resources/3_0/petstore.yaml in dart2 generators

4 participants