Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: limit the generated names of the $refs #47

Merged
merged 1 commit into from
Dec 11, 2023
Merged

Conversation

paulsturgess
Copy link
Contributor

If we use a generator to produce a Ruby client, and then try to build that gem, it's possible for the RubyGems build process to fail with: ERROR: While executing gem ... (Gem::Package::TooLongFileName) This can happen if the $ref ids we are using are too long.

This PR makes 3 changes which are applied during the generation of ids only if they too long:

  • error responses that can be "one of" many, will now no longer use all of the error names to generate the id
  • nested partial responses will no longer use all of the parent field names in the id generation
  • endpoint ids will resort to using the initials of the full path, when the id has a clash and the path is too long (this will be super rare).

this is a breaking change, but we're on 0.1.1 and not actually using any generated clients yet. So I'm not marking it as such.

closes: #46

If we use a generator to produce a Ruby client, and then try to build that gem, it's possible for the RubyGems build process to fail with:
`ERROR: While executing gem ... (Gem::Package::TooLongFileName)`
This can happen if the $ref ids we are using are too long.

This PR makes 3 changes which are applied during the generation of ids only if they too long:
- error responses that can be "one of" many, will now no longer use all of the error names to generate the id
- nested partial responses will no longer use all of the parent field names in the id generation
- endpoint ids will resort to using the initials of the full path, when the id has a clash and the path is too long (this will be super rare).

this is a breaking change, but we're on 0.1.1 and not actually using any generated clients yet. So I'm not marking it as such.

closes: #46
@paulsturgess paulsturgess self-assigned this Dec 11, 2023
@paulsturgess paulsturgess marked this pull request as ready for review December 11, 2023 17:00
@paulsturgess paulsturgess merged commit 8dec51f into main Dec 11, 2023
@paulsturgess paulsturgess deleted the 46-char-limits branch December 11, 2023 17:00
@replease replease bot mentioned this pull request Dec 11, 2023
@paulsturgess
Copy link
Contributor Author

paulsturgess commented Dec 11, 2023

@jimeh I went ahead and merged this without your review because I didn't want block anyone from trying out the generated clients. Also I figured it's better to get something published than have nothing. Given no-one is using this yet, I figure we can be quite bold at this point. If you hate what i've done, then we can easily revisit.

If we continue to have problems with the length of $ref ids and corresponding filenames that are generated. Then a more involved solution could be adapt Apia so that some kind of "short id" could be defined on the endpoint itself in the app that defines the actual API. Then our schema generator could use this "short id", instead of relying on the path of the endpoint. This would give the user the control to apply acronyms or simply leave out words as appropriate.

I don't think we could make this "short id" mandatory, so either way I think we'd need this automatic shortening logic (or some version of it, that i've implemented in this PR).

It's also worth pointing out that declaring a "short id" for an endpoint wouldn't make any difference for the way we need to name error responses, so it wouldn't solve everything anyway.

@jimeh
Copy link

jimeh commented Dec 11, 2023

@paulsturgess that all sounds reasonable to me. I've only had a very brief look at the code changes, but what you've described sounds good to me. It'll be nice to get all this stuff off the ground :)

And if you want, I'm happy to have a deeper dive with you into this next week when I'm back.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Limit the generated reference ids to 100 characters
2 participants