Skip to content

Commit

Permalink
Libs(all): bump openapi generator from 5.2.0 to 7.9.0 (#1480)
Browse files Browse the repository at this point in the history
With the existing generator versions, we've had issues during codegen
for certain cases in the svix spec including:

- serialization of nullable fields
- enum (`oneOf`, `anyOf`) representations

Bumping the generators all the way up to the latest version (at time of
writing, `7.9.0`) seems to resolve both of these, at least partially.

Some refactoring of extant template overrides has been required to
"re-seat" our local patches on top of the newer templates.

There have also been cases of the generated symbol names using slightly
different schemes than in earlier generator versions. The high-level API
had to be updated to use the new names.

## Approach

Customization to the codegen comes in 2 flavors:
- edits to templates.
- manual edits where the codegen is checked-in to git and the paths are
recorded in an `.openapi-generator-ignore` file.

Templates are better because they don't hide the customization as
readily, but we've got a mix of these approaches throughout.

At a high-level, this change followed the following steps:
- replace templates entirely with the version tagged for 7.9.0.
- recreate our local patches _on top_ as a single rev.
- leave behind any customization made moot by upstream refactors.
- try to remove as many manual edits/ignores by migrating into templates
where needed or just omitting the change entirely.
- add a light/gentle test to check specific behaviors things we had
hacks in place for:
  - nullable containers (ie. Endpoint channels and filter types).
  - responses with empty bodies.

In addition to doing the "upstream merge" on template sources, I also
had to sometimes look at the dependencies a lib depends on, upgrading or
changing the package manifest, or build tooling.

When refering/comparing to the upstream templates:
-
https://github.com/OpenAPITools/openapi-generator/blob/v7.9.0/modules/openapi-generator/src/main/resources/
(the new generator version tag)
-
https://github.com/OpenAPITools/openapi-generator/blob/v5.2.0/modules/openapi-generator/src/main/resources/
(the older one)

## Status

Each lib came with its own challenges. Some I have higher or lower
confidence in.
I'll call out the ones that are weaker so reviewers can give more
scrutiny.

**Ruby**: ~~I wasn't able to get my local ruby setup in a healthy enough
state to follow along with the readme instructions for building/testing.
This one needs a so-called "kitchen sink" test like I added to the other
libs, and may need some dependencies bumped. Normally the dep updates
required become known during testing.~~

**Edit:** setting `BUNDLE_PATH=$HOME/.gems` unblocked me. Apparently
`bundle install` tries to put stuff on the "system" level by default
(i.e. requiring `sudo`, and not really what I wanted anyway). With this,
I was able to learn enough Ruby to get a test written, and also verify
that the deps are lined up as needed. Ruby seems to be in good shape!


**Python**: This was the last lib I worked on, and I ran out of time.
The "kitchen sink" test looks good, and we also have a decent set of
tests that give me _okay confidence_ but here's where I cut corners. I
left the templates, manual edits, dependencies and the rest all as-is in
the hopes that the boon of the tests will surface problems that stem
from all the surrounding code being updated around this stuff that stays
where it is.

The tests are passing, so maybe we're good, but we should loop back and
try to do proper updates to this one.

**C#**: manual edits for nullability were left in place for now. A
"kitchen sink" test was written, but has also been reverted because
however I wrote it ended up using language features not supported by
dotnet 5.0 (which our CI process is targeting).

The tests pass locally when I update the solution to target 8.0, so I
feel good about the lib generally. I was not able to figure out how to
install dotnet 5.0 on my system since it's beyond End of Life. We should
look to update our target to a currently supported dotnet like 8.0, at
which point we can un-revert the test!

**Kotlin**: had to update the version of kotlin we build with, but other
than that I think things look good. There's a fixme in the ApiClient
template about updating the retry loop to work for 2 flavors of request
sending; I'd like to look at that in a follow-up, later.

The rest all seemed okay and not very concerning to me.

## Verification

The so-called "kitchen sink" tests added to each lib follow a pattern of
auto-skipping by default and running only when the env vars `SVIX_TOKEN`
and `SVIX_SERVER_URL` are set. The test will then step through creating
an app and an endpoint, patching the endpoint, and finally deleting the
endpoint.

Currently these tests are not setup to run in CI, but they _could be_ if
we are able to automate raising the docker-compose stack, generating a
valid token, and setting the env vars such that they are available to
each "lint" or "test" workflow.

For now, these tests have been seen to work locally on my machine, and
hopefully yours as well.

## Notes for Reviewers

I can offer 2 bits of advice when reviewing this chungus of a diff.

1. I split this into many commits and it may be helpful to review one by
one.
2. Many files such as the templates themselves may be easier to read if
you "ignore whitespace" (at your discretion).


It would be good if someone knowledgeable in Ruby could actually build
the package and add a similar "kitchen sink" test before this merges,
but I don't know if @svix-gabriel is willing to dive into this rabbit
hole with me. Please and thank you, Gabriel!

Good luck!
  • Loading branch information
svix-onelson authored Oct 24, 2024
2 parents 54457a1 + e6301c5 commit 141d3cd
Show file tree
Hide file tree
Showing 266 changed files with 19,997 additions and 12,479 deletions.
5 changes: 5 additions & 0 deletions .github/workflows/ruby-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,8 @@ jobs:
run: |
cd ruby
bundler exec rake build
- name: Test
run: |
cd ruby
bundler exec rspec spec
1 change: 1 addition & 0 deletions .typos.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
extend-ignore-re = [
# Mock token
";iuani;ansd;ifgjbnai;sdjfgb",
"jakarta\\.ws\\.rs",
]

[files]
Expand Down
3 changes: 2 additions & 1 deletion csharp/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,5 @@ msbuild.wrn

# Svix OpenApi generated content
.openapi-generator/
/Svix/Generated/OpenApi/Svix/
/Svix/Generated/OpenApi/Svix/
/api/openapi.yaml
4 changes: 4 additions & 0 deletions csharp/.openapi-generator-ignore
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ Svix.sln

docs/
src/
# When bumping generator versions:
# - Comment these out to "re-up" the generator source.
# - Roll back any of our local changes that got wiped out in the process.
# - Uncomment them again to prevent the generator from wiping out the manual edits.
Svix/Generated/OpenApi/Svix/Model/ApplicationPatch.cs
Svix/Generated/OpenApi/Svix/Model/EndpointPatch.cs
Svix/Generated/OpenApi/Svix/Model/EventTypePatch.cs
91 changes: 16 additions & 75 deletions csharp/Svix/Generated/OpenApi/Svix/Model/ApplicationPatch.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ namespace Svix.Model
/// ApplicationPatch
/// </summary>
[DataContract(Name = "ApplicationPatch")]
public partial class ApplicationPatch : IEquatable<ApplicationPatch>, IValidatableObject
public partial class ApplicationPatch : IValidatableObject
{
/// <summary>
/// Initializes a new instance of the <see cref="ApplicationPatch" /> class.
Expand Down Expand Up @@ -71,6 +71,9 @@ public partial class ApplicationPatch : IEquatable<ApplicationPatch>, IValidatab
/// The app&#39;s UID
/// </summary>
/// <value>The app&#39;s UID</value>
/*
<example>unique-app-identifier</example>
*/
[DataMember(Name = "uid", EmitDefaultValue = false)]
public string Uid { get; set; }

Expand All @@ -80,7 +83,7 @@ public partial class ApplicationPatch : IEquatable<ApplicationPatch>, IValidatab
/// <returns>String presentation of the object</returns>
public override string ToString()
{
var sb = new StringBuilder();
StringBuilder sb = new StringBuilder();
sb.Append("class ApplicationPatch {\n");
sb.Append(" Metadata: ").Append(Metadata).Append("\n");
sb.Append(" Name: ").Append(Name).Append("\n");
Expand All @@ -99,101 +102,39 @@ public virtual string ToJson()
return Newtonsoft.Json.JsonConvert.SerializeObject(this, Newtonsoft.Json.Formatting.Indented);
}

/// <summary>
/// Returns true if objects are equal
/// </summary>
/// <param name="input">Object to be compared</param>
/// <returns>Boolean</returns>
public override bool Equals(object input)
{
return this.Equals(input as ApplicationPatch);
}

/// <summary>
/// Returns true if ApplicationPatch instances are equal
/// </summary>
/// <param name="input">Instance of ApplicationPatch to be compared</param>
/// <returns>Boolean</returns>
public bool Equals(ApplicationPatch input)
{
if (input == null)
return false;

return
(
this.Metadata == input.Metadata ||
this.Metadata != null &&
input.Metadata != null &&
this.Metadata.SequenceEqual(input.Metadata)
) &&
(
this.Name == input.Name ||
(this.Name != null &&
this.Name.Equals(input.Name))
) &&
(
this.RateLimit == input.RateLimit ||
(this.RateLimit != null &&
this.RateLimit.Equals(input.RateLimit))
) &&
(
this.Uid == input.Uid ||
(this.Uid != null &&
this.Uid.Equals(input.Uid))
);
}

/// <summary>
/// Gets the hash code
/// </summary>
/// <returns>Hash code</returns>
public override int GetHashCode()
{
unchecked // Overflow is fine, just wrap
{
int hashCode = 41;
if (this.Metadata != null)
hashCode = hashCode * 59 + this.Metadata.GetHashCode();
if (this.Name != null)
hashCode = hashCode * 59 + this.Name.GetHashCode();
if (this.RateLimit != null)
hashCode = hashCode * 59 + this.RateLimit.GetHashCode();
if (this.Uid != null)
hashCode = hashCode * 59 + this.Uid.GetHashCode();
return hashCode;
}
}

/// <summary>
/// To validate all properties of the instance
/// </summary>
/// <param name="validationContext">Validation context</param>
/// <returns>Validation Result</returns>
IEnumerable<System.ComponentModel.DataAnnotations.ValidationResult> IValidatableObject.Validate(ValidationContext validationContext)
IEnumerable<ValidationResult> IValidatableObject.Validate(ValidationContext validationContext)
{
// RateLimit (int?) minimum
if (this.RateLimit < (int?)0)
{
yield return new System.ComponentModel.DataAnnotations.ValidationResult("Invalid value for RateLimit, must be a value greater than or equal to 0.", new[] { "RateLimit" });
yield return new ValidationResult("Invalid value for RateLimit, must be a value greater than or equal to 0.", new[] { "RateLimit" });
}

// Uid (string) maxLength
if (this.Uid != null && this.Uid.Length > 256)
{
yield return new System.ComponentModel.DataAnnotations.ValidationResult("Invalid value for Uid, length must be less than 256.", new[] { "Uid" });
yield return new ValidationResult("Invalid value for Uid, length must be less than 256.", new[] { "Uid" });
}

// Uid (string) minLength
if (this.Uid != null && this.Uid.Length < 1)
{
yield return new System.ComponentModel.DataAnnotations.ValidationResult("Invalid value for Uid, length must be greater than 1.", new[] { "Uid" });
yield return new ValidationResult("Invalid value for Uid, length must be greater than 1.", new[] { "Uid" });
}

// Uid (string) pattern
Regex regexUid = new Regex(@"^[a-zA-Z0-9\\-_.]+$", RegexOptions.CultureInvariant);
if (false == regexUid.Match(this.Uid).Success)
if (this.Uid != null)
{
yield return new System.ComponentModel.DataAnnotations.ValidationResult("Invalid value for Uid, must match a pattern of " + regexUid, new[] { "Uid" });
// Uid (string) pattern
Regex regexUid = new Regex(@"^[a-zA-Z0-9\-_.]+$", RegexOptions.CultureInvariant);
if (!regexUid.Match(this.Uid).Success)
{
yield return new System.ComponentModel.DataAnnotations.ValidationResult("Invalid value for Uid, must match a pattern of " + regexUid, new[] { "Uid" });
}
}

yield break;
Expand Down
Loading

0 comments on commit 141d3cd

Please sign in to comment.