progenitor requires operationId to be set

[upachler]

In OAS 3, there is no such requirement.

See operationId field in the table in the spec here: https://swagger.io/specification/v3/#fixed-fields-8.

Progenitor requiring opIds makes a lot of OAS3 files unsuitable for code generation unnecessarily. There should be an easy way to generate opIds from path and http method.

[upachler]

I'm preparing a fix for this:

https://github.com/upachler/progenitor/tree/1105-progenitor-requires-operationid-to-be-set

[ahl]

It's true! And this is by design: we didn't want to try to figure out names for operations. It seemed hard to come up with operation names that would result in an ergonomic SDK. It also seems hard to come up with names that are stable i.e. adding or removing an operation in an OAS document shouldn't change the name of an existing SDK method.

Instead, we suggest using JSON patch or something similar to prepare the original OAS document. In the future, we intend to rely on this idea of patching the original document rather than trying to apply heuristics e.g. for resolving naming conflicts.

That said, we would certainly take a look at a patch, but please understand that we're fairly cautious to adopt new functionality that is achievable externally, not aligned with our own use cases, potentially burdens future functionality, or some combination thereof. You might want to consider augmentation of the OAS document or generation of a json patch file as a separate program (that would certainly benefit generation in other languages!).

[upachler]

If I understand you correctly you are suggesting to externally patch fully compliant OAS 3.0 documents in order to make them work with a non-standards compliant OAS 3.0 code generator ;-)

I understand that forcing some documents to have pre-set operation_ids can be a good thing, this way you force document authors to consider and add them everywhere. But not everyone has this requirement. On the contrary, I'd guess most people haven't, and this is also why it's not in the spec. I've used openapigenerator a fair bit and it doesn't have that requirement either. Also, it generates fairly usable method names without operation_ids being set in the document (even though it will use them if they are present).
But while I see the use case for enforcing operation_ids, I'd rather have something like this to be opt-in (e.g as part of a 'strict' mode), and progenitor generating useful operation_ids (and from there method names etc.) automatically by default.

Also, the name generation algorithm I'm proposing derives the names from path and method - so operation_ids would remain stable under normal circumstances and resemble the actual paths that you are calling. For instance, GET /foo/bar becomes foo_bar_get. Yes, there are edge cases that can cause collisions, there always will be, but those can be handled reasonably by the operation_id generator itself (and it's already in my draft branch).

The algorithm is documented here:
https://github.com/upachler/progenitor/blob/1105-progenitor-requires-operationid-to-be-set/progenitor-impl/src/opid.rs#L79

[kika]

In my case, I have 567 operations, writing a patch for that would be a weeklong effort (and that's an external vendor API, I don't control it and I can't force them to add an optional field). Thanks @upachler I'll try to implement your algorithm in the build.rs to patch the json in memory.

[ahl]

If I understand you correctly you are suggesting to externally patch fully compliant OAS 3.0 documents in order to make them work with a non-standards compliant OAS 3.0 code generator ;-)

Precisely! Although, I would observe that no OAS 3.0 compliant generator exists (i.e. one can build a construction on which it will fail or generate divergent code).

But while I see the use case for enforcing operation_ids, I'd rather have something like this to be opt-in (e.g as part of a 'strict' mode), and progenitor generating useful operation_ids (and from there method names etc.) automatically by default.

I agree that it's a matter of preference. We'll be happy to take a look at a PR, but I can't promise that we'd accept it.

Also, the name generation algorithm I'm proposing derives the names from path and method - so operation_ids would remain stable under normal circumstances and resemble the actual paths that you are calling. For instance, GET /foo/bar becomes foo_bar_get. Yes, there are edge cases that can cause collisions, there always will be, but those can be handled reasonably by the operation_id generator itself (and it's already in my draft branch).

That's great!

[ahl]

In my case, I have 567 operations, writing a patch for that would be a weeklong effort (and that's an external vendor API, I don't control it and I can't force them to add an optional field). Thanks @upachler I'll try to implement your algorithm in the build.rs to patch the json in memory.

My proposal is to have a program that generates the JSON patch to fill in operationIds, not to consider 567 operations by hand!

[kika]

to patch the json in memory.

Little naive kid. OpenAPI crate generates mostly readonly structure, good luck patching it. Wonderful world of open source: fork this, f*k that 🤣
@upachler I stole your code for my build.rs and added the generation of _by_ suffixes, so

"/path", "get" -> path_get
"/path/{id}", "get" -> path_get_by_id

So if you end up doing the PR, I can contribute that.

[upachler]

@kika: I'd rather treat path parameters like path components. Your approach seems to assume that the path param is always the last component (that's then a _by_<param> looks fine), but there are other cases where this becomes strange.

Consider:
GET /path/{id} -> path_id_get
GET /bank/{bankId}/accounts/{accountNo}/ledger -> bank_bankid_accounts_accountno_ledger_get

That's also what openapigenerator does, BTW.

And no, long method names are not beautiful, but if you want shorter ones, set operationId :)

[kika]

My "algorithm" collects all path params, removes them from path and then appends with _by_ prefix. So it will become bank_accounts_ledger_by_bankid_by_accountno. Works ok for my use case, but I admit it's absolutely not universal. The only "universal" approach I can think of is to employ LLM to read the doc, spec and then come up with names. lol (or not lol).

[ahl]

The only "universal" approach I can think of is to employ LLM to read the doc, spec and then come up with names. lol (or not lol).

I think that's good guidance: if you're missing operationIds ask an LLM to synthesize good names based on documentation, paths, and context.

[upachler]

I created a PR that won't need an LLM :D

#1110

[hjr3]

It seems unlikely that #1110 will be accepted because it introduces opinions on operationId naming and progenitor explicitly does not want to do this. Per @ahl

And this is by design: we didn't want to try to figure out names for operations. It seemed hard to come up with operation names that would result in an ergonomic SDK. It also seems hard to come up with names that are stable i.e. adding or removing an operation in an OAS document shouldn't change the name of an existing SDK method.

I created https://crates.io/crates/openapi-ops-mut so people can solve this problem however they see fit. Add the following to your build.rs:

diff --git a/openapi-client/build.rs b/openapi-client/build.rs
index f907a92264..0cb4ea301f 100644
--- a/openapi-client/build.rs
+++ b/openapi-client/build.rs
@@ -3,6 +3,12 @@
     println!("cargo:rerun-if-changed={}", src);
     let file = std::fs::File::open(src).unwrap();
     let mut spec = serde_json::from_reader(file).unwrap();
+
+    let mut oa_ext = openapi_ops_mut::OpenAPIExt::new(&mut spec);
+    for (path, method, op) in oa_ext.operations_mut() {
+        op.operation_id = Some(format!("{}_{}", path, method)); // <--- do whatever you want here
+    }
+
     let mut generator = progenitor::Generator::default();

     let tokens = generator.generate_tokens(&spec).unwrap();

This provides a function, operations_mut, that allows you to iterate over the operations and provide an operation id based on your requirements. My example above is very simple. If you want the extra features that @upachler introduced (e.g. proper naming for /, collision support, etc) then you can add those too!

The operations_mut seems useful enough to upstream, so I opened glademiller/openapiv3#87 in hopes that we can deprecate my utility crate and make life even easier for people to modify an OpenAPI v3 spec.

[ahl]

This is great; thanks for sharing.