diff --git a/src/lib.rs b/src/lib.rs index 55e1611d5..984dd6ae6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -226,15 +226,19 @@ impl std::fmt::Debug for Group<'_> { /// `x-fern-ignore` does not stop its aliases. An empty array makes no /// method, an element that is not an object is ignored, and a value that /// is not an array is an error. Aliases do not nest: an `x-forge-aliases` -/// in an alias is ignored, as are its keys that are not extensions. +/// in an alias is ignored, as are its keys that are neither extensions nor +/// `operationId`. /// /// An alias says, as an operation would, the method's -/// [`group`](Method::group), [`name`](Method::name), +/// [`group`](Method::group) and [`name`](Method::name), with the two Fern +/// extensions or its own `operationId`, its /// [`availability`](Method::availability), whether it is /// [hidden](Method::is_hidden), its [`confirmation`](Method::confirmation) /// and its [`extensions`](Method::extensions), and whether it is ignored /// with `x-fern-ignore`. None of these come from the operation: an alias -/// without an extension does not have the operation's. The rest, from the +/// without one of these keys does not have the operation's, so an alias +/// without the Fern extensions or an `operationId` of its own makes no +/// method, whatever the operation's `operationId`. The rest, from the /// HTTP method and path to the responses, is the operation's, and every /// alias has it. Each alias is a method of its own group, so a model one /// uses is placed as though each alias were a separate operation. @@ -250,11 +254,24 @@ pub struct Method<'f> { impl<'f> Method<'f> { /// Returns the name of the method, which is unique within its group. /// - /// Corresponds to the `x-fern-sdk-method-name` extension of the - /// operation, or of the alias the method comes from. An operation or - /// alias without one, or with an empty one, is not a method, and one - /// with no letters or digits is an error. Two methods in a group with - /// equal names are an error, whether they are aliases or not. + /// An operation, or an alias the method comes from, is named in one of + /// two ways: + /// + /// 1. With both the `x-fern-sdk-group-name` and the + /// `x-fern-sdk-method-name` extensions, neither empty. The name is + /// the `x-fern-sdk-method-name`. + /// 2. Without both, with an `operationId` of the form + /// `.`, split at the last dot. The name is + /// the part after it: `pets.list` is the method `list` in the group + /// `pets`, and `accounts.members.list` is the method `list` in the + /// group `members` inside `accounts`. + /// + /// An operation or alias named in neither way, such as one whose + /// `operationId` has no dot, like `listPets`, is not a method, and is + /// not an error. A name with no letters or digits, as in the + /// `operationId` `pets.`, is an error. Two methods in a group with + /// equal names are an error, however they are named, and whether they + /// are aliases or not. pub fn name(self) -> &'f Name { &self.data().name } @@ -262,11 +279,17 @@ impl<'f> Method<'f> { /// Returns the group the method is in. /// /// Corresponds to the `x-fern-sdk-group-name` extension of the - /// operation, or of the alias the method comes from: a string of group - /// names separated by dots, such as `accounts.members`, or an array - /// with one group name per element, top level first. An operation or - /// alias without one, with an empty one, or with `x-fern-ignore: true` - /// is not a method. A group name with no letters or digits is an error. + /// operation, or of the alias the method comes from, when the method + /// is named by the Fern extensions, as [`name`](Method::name) says: a + /// string of group names separated by dots, such as + /// `accounts.members`, or an array with one group name per element, + /// top level first. Otherwise it is the part of the `operationId` + /// before the last dot, with group names separated by dots the same + /// way. + /// + /// An operation or alias with `x-fern-ignore: true` is not a method, + /// however it is named. A group name with no letters or digits, as in + /// the `operationId` `a..b.list` or `.list`, is an error. pub fn group(self) -> Group<'f> { self.forge.group(self.data().group) } diff --git a/src/link.rs b/src/link.rs index 0255688c2..8c67a2ee1 100644 --- a/src/link.rs +++ b/src/link.rs @@ -41,7 +41,10 @@ use openapi::{ /// /// The methods come from the operations under `paths`, including those in /// `additionalOperations` and in path items reached by `$ref`, and from -/// their aliases, as [`Method`](crate::Method) says. Webhooks, callbacks, +/// their aliases, as [`Method`](crate::Method) says. Each is named by its +/// `x-fern-sdk-group-name` and `x-fern-sdk-method-name`, or, without both, +/// by an `operationId` such as `pets.list`, as +/// [`Method::name`](crate::Method::name) says. Webhooks, callbacks, /// response headers, cookie parameters, security, and extensions these /// docs do not mention are ignored, except that those of an operation or /// alias are kept for [`Method::extensions`](crate::Method::extensions). @@ -566,23 +569,33 @@ impl<'d> Builder<'d> { if definition.ignore { return; } - if definition.method_name.is_empty() { - return; - } - let group_names = doc.list(definition.group_names); - if group_names.is_empty() { - return; - } - let group_names: Vec = group_names.iter().map(|&name| Name::new(doc.str(name))).collect(); - for name in &group_names { - if name.is_empty() { - self.error("x-fern-sdk-group-name has a group name with no letters or digits"); + // Both Fern extensions name the method, or else an `operationId` + // with a dot does, as the group path and the name. + let fern_groups = doc.list(definition.group_names); + let (group_names, name, group_error, name_error): (Vec, Name, &str, &str) = + if !definition.method_name.is_empty() && !fern_groups.is_empty() { + ( + fern_groups.iter().map(|&name| Name::new(doc.str(name))).collect(), + Name::new(doc.str(definition.method_name)), + "x-fern-sdk-group-name has a group name with no letters or digits", + "x-fern-sdk-method-name has no letters or digits", + ) + } else if let Some((groups, name)) = doc.str(definition.operation_id).rsplit_once('.') { + ( + groups.split('.').map(Name::new).collect(), + Name::new(name), + "operationId has a group name with no letters or digits", + "operationId has a method name with no letters or digits", + ) + } else { return; - } + }; + if group_names.iter().any(Name::is_empty) { + self.error(group_error); + return; } - let name = Name::new(doc.str(definition.method_name)); if name.is_empty() { - self.error("x-fern-sdk-method-name has no letters or digits"); + self.error(name_error); return; } let mut availability = None; diff --git a/src/link/openapi.rs b/src/link/openapi.rs index 51f1b3767..adb6f82f5 100644 --- a/src/link/openapi.rs +++ b/src/link/openapi.rs @@ -236,7 +236,7 @@ pub(super) struct Operation { pub(super) responses: Span<(Str, Id)>, } -/// The extensions that make an Operation Object, or an alias of one in its +/// The keys that make an Operation Object, or an alias of one in its /// `x-forge-aliases`, a method. #[derive(Default)] pub(super) struct Definition { @@ -245,6 +245,9 @@ pub(super) struct Definition { /// The `x-fern-sdk-group-name`: the group path, top level first, or /// empty for none. pub(super) group_names: Span, + /// The `operationId`, or empty for none. It names the method when the + /// two Fern extensions do not. + pub(super) operation_id: Str, /// `x-fern-ignore`. pub(super) ignore: bool, /// `x-forge-hidden`. @@ -1184,6 +1187,7 @@ keys! { DefinitionKey { MethodName = "x-fern-sdk-method-name", GroupNames = "x-fern-sdk-group-name", + OperationId = "operationId", Ignore = "x-fern-ignore", Hidden = "x-forge-hidden", Confirmation = "x-forge-require-confirmation", @@ -1202,6 +1206,7 @@ impl Definition { match key { DefinitionKey::MethodName => self.method_name = map.next_value_seed(seed::(doc))?, DefinitionKey::GroupNames => self.group_names = map.next_value_seed(seed::(doc))?, + DefinitionKey::OperationId => self.operation_id = map.next_value_seed(seed::(doc))?, DefinitionKey::Ignore => self.ignore = map.next_value()?, DefinitionKey::Hidden => self.hidden = map.next_value()?, DefinitionKey::Confirmation => { @@ -1217,8 +1222,8 @@ impl Definition { } /// Reads an alias in `x-forge-aliases`: an object of the keys of a -/// definition and other extensions. Its keys that are not extensions are -/// skipped, and so is an `x-forge-aliases` in it, since aliases do not +/// definition and other extensions. Its other keys that are not extensions +/// are skipped, and so is an `x-forge-aliases` in it, since aliases do not /// nest. struct Alias; diff --git a/tests/parse/aliases.rs b/tests/parse/aliases.rs index eb299ef5f..0843ab416 100644 --- a/tests/parse/aliases.rs +++ b/tests/parse/aliases.rs @@ -340,3 +340,101 @@ fn model_used_by_aliases_in_one_group_is_in_that_group() { assert_eq!(model(&forge, "M").group(), Some(group(&forge, "a"))); } + +#[test] +fn alias_is_named_by_its_operation_id() { + let forge = parse(&aliased( + r#"[{"operationId": "a.pause"}, {"operationId": "b.c.resume", "x-a": 1}]"#, + "", + )); + + assert_eq!(methods(&forge), ["a.pause", "b.c.resume"]); + let resume = method(&forge, "b.c.resume"); + let names: Vec<&str> = resume.extensions().map(|(name, _)| name).collect(); + assert_eq!(names, ["x-a"]); +} + +#[test] +fn alias_fern_extensions_win_over_its_operation_id() { + let forge = parse(&aliased( + r#"[{"x-fern-sdk-group-name": "a", "x-fern-sdk-method-name": "pause", "operationId": "b.resume"}]"#, + "", + )); + + assert_eq!(methods(&forge), ["a.pause"]); +} + +#[test] +fn alias_with_one_fern_extension_falls_back_to_its_operation_id() { + let forge = parse(&aliased( + r#"[{"x-fern-sdk-group-name": "a", "operationId": "b.resume"}]"#, + "", + )); + + assert_eq!(methods(&forge), ["b.resume"]); +} + +#[test] +fn alias_does_not_take_the_operation_id_of_the_operation() { + let forge = parse(&aliased( + r#"[{"x-fern-sdk-group-name": "a"}, {}, {"x-fern-sdk-group-name": "a", "x-fern-sdk-method-name": "pause"}]"#, + r#""operationId": "b.resume","#, + )); + + assert_eq!(methods(&forge), ["a.pause"]); +} + +#[test] +fn alias_operation_id_without_a_dot_is_not_a_method() { + let forge = parse(&aliased(r#"[{"operationId": "pause"}]"#, "")); + + assert_eq!(methods(&forge), Vec::::new()); +} + +#[test] +fn ignored_alias_with_an_operation_id_is_not_a_method() { + let forge = parse(&aliased( + r#"[{"operationId": "a.pause", "x-fern-ignore": true}, {"operationId": "a.resume"}]"#, + "", + )); + + assert_eq!(methods(&forge), ["a.resume"]); +} + +#[test] +fn alias_operation_id_name_without_letters_is_an_error_naming_the_alias() { + let err = parse_err(&aliased(r#"[{"operationId": "a."}, {"operationId": "a..pause"}]"#, "")); + + assert!( + err.contains( + "PATCH /a/{id}/status: x-forge-aliases[0]: operationId has a method name with no letters or digits" + ), + "{err}" + ); + assert!( + err.contains( + "PATCH /a/{id}/status: x-forge-aliases[1]: operationId has a group name with no letters or digits" + ), + "{err}" + ); +} + +#[test] +fn alias_operation_id_and_fern_alias_with_equal_names_are_an_error() { + let err = parse_err(&aliased( + r#"[{"x-fern-sdk-group-name": "a", "x-fern-sdk-method-name": "pause"}, {"operationId": "a.pause"}]"#, + "", + )); + + assert!( + err.contains(r#"x-forge-aliases[1]: method "pause" is also PATCH"#), + "{err}" + ); +} + +#[test] +fn alias_operation_id_that_is_not_a_string_is_an_error() { + let err = parse_err(&aliased(r#"[{"operationId": ["a", "pause"]}]"#, "")); + + assert!(err.contains("expected a string"), "{err}"); +} diff --git a/tests/parse/methods.rs b/tests/parse/methods.rs index 5daea4ea9..9807aa8ef 100644 --- a/tests/parse/methods.rs +++ b/tests/parse/methods.rs @@ -353,3 +353,124 @@ fn availability_and_deprecation_are_apart() { assert!(post.is_deprecated()); assert_eq!(post.availability(), None); } + +/// Returns a document with one operation, `GET /a`, with `fields`, which +/// end with a comma, and no other keys that name it. +fn named_by(fields: &str) -> String { + format!( + r#"{{ + "openapi": "3.0.3", "info": {{"title": "t", "version": "1"}}, + "paths": {{"/a": {{"get": {{{fields} + "responses": {{"204": {{"description": "ok"}}}}}}}}}} + }}"# + ) +} + +#[test] +fn dotted_operation_id_is_the_group_and_method() { + let forge = parse(&named_by(r#""operationId": "pets.list","#)); + + let list = method(&forge, "pets.list"); + assert_eq!(list.path(), "/a"); + assert_eq!(forge.groups().count(), 1); +} + +#[test] +fn operation_id_splits_at_the_last_dot_into_a_group_path() { + let forge = parse(&named_by(r#""operationId": "accounts.members.list","#)); + + assert_eq!(method(&forge, "accounts.members.list").path(), "/a"); + assert_eq!(group(&forge, "accounts").methods().len(), 0); +} + +#[test] +fn operation_id_names_are_names() { + let forge = parse(&named_by(r#""operationId": "userAccounts.listAll","#)); + + assert_eq!(method(&forge, "user_accounts.list_all").path(), "/a"); +} + +#[test] +fn operation_id_without_a_dot_is_not_a_method() { + let forge = parse(&named_by(r#""operationId": "listPets","#)); + + assert_eq!(forge.groups().count(), 0); +} + +#[test] +fn fern_extensions_win_over_operation_id() { + let forge = parse(&operation_with(r#""operationId": "pets.list","#)); + + assert_eq!(method(&forge, "a.get").path(), "/a"); + assert_eq!(forge.groups().count(), 1); +} + +#[test] +fn one_fern_extension_falls_back_to_operation_id() { + for fields in [ + r#""x-fern-sdk-group-name": "a", "operationId": "pets.list","#, + r#""x-fern-sdk-method-name": "get", "operationId": "pets.list","#, + r#""x-fern-sdk-group-name": "", "x-fern-sdk-method-name": "get", "operationId": "pets.list","#, + r#""x-fern-sdk-group-name": "a", "x-fern-sdk-method-name": "", "operationId": "pets.list","#, + ] { + let forge = parse(&named_by(fields)); + + assert_eq!(method(&forge, "pets.list").path(), "/a", "{fields}"); + assert_eq!(forge.groups().count(), 1, "{fields}"); + } +} + +#[test] +fn ignored_operation_with_an_operation_id_is_not_a_method() { + let forge = parse(&named_by(r#""operationId": "pets.list", "x-fern-ignore": true,"#)); + + assert_eq!(forge.groups().count(), 0); +} + +#[test] +fn operation_id_group_name_without_letters_is_an_error_naming_the_operation() { + for operation_id in ["a..b.list", ".list", "!.list"] { + let err = parse_err(&named_by(&format!(r#""operationId": "{operation_id}","#))); + + assert!( + err.contains("GET /a: operationId has a group name with no letters or digits"), + "{operation_id}: {err}" + ); + } +} + +#[test] +fn operation_id_method_name_without_letters_is_an_error_naming_the_operation() { + for operation_id in ["pets.", "pets.-"] { + let err = parse_err(&named_by(&format!(r#""operationId": "{operation_id}","#))); + + assert!( + err.contains("GET /a: operationId has a method name with no letters or digits"), + "{operation_id}: {err}" + ); + } +} + +#[test] +fn operation_id_that_is_not_a_string_is_an_error() { + let err = parse_err(&named_by(r#""operationId": 1,"#)); + + assert!(err.contains("expected a string"), "{err}"); +} + +#[test] +fn fern_and_operation_id_methods_with_equal_names_are_an_error() { + let err = parse_err( + r#"{ + "openapi": "3.0.3", "info": {"title": "t", "version": "1"}, + "paths": { + "/a": {"get": {"x-fern-sdk-group-name": "pets", "x-fern-sdk-method-name": "list", + "responses": {"204": {"description": "ok"}}}}, + "/b": {"get": {"operationId": "pets.list", + "responses": {"204": {"description": "ok"}}}} + } + }"#, + ); + + assert!(err.contains(r#"GET /b: method "list" is also GET /a"#), "{err}"); +}