Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat(146): add oas-specific guidance on mutually exclusive fields #274

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 80 additions & 25 deletions aep/general/0146/aep.md.j2
Original file line number Diff line number Diff line change
@@ -1,28 +1,29 @@
# Generic fields
# Mutually exclusive fields
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: generic fields covers much more than mutually exclusive fields. It refers to structs and the Any type at minimum, and we may want to cover more things like sets.

I'd suggest creating a sub-section for mutually exclusive fields, or perhaps splitting it into it's own AEP (150? since aip.dev/149 exists).


Most fields in any API, whether in a request, a resource, or a custom response,
have a specific type or schema. This schema is part of the contract that
developers write their code against.

However, occasionally it is appropriate to have a generic or polymorphic field
of some kind that can conform to multiple schemata, or even be entirely
free-form.
However, occasionally it is appropriate to have a mutually exclusive or
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I won't leave a comment everywhere, but I think it'll be important to revert the nomenclature in this PR to continue to refer to the larger design area of generic fields.

polymorphic field of some kind that can conform to multiple schemata, or even
be entirely free-form.

## Guidance

While generic fields are generally rare, a API **may** introduce generic field
where necessary. There are several approaches to this depending on how generic
the field needs to be; in general, APIs **should** attempt to introduce the
"least generic" approach that is able to satisfy the use case.
While mutually exclusive fields are generally rare, a API **may** introduce
mutually exclusive field where necessary. There are several approaches to this
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
mutually exclusive field where necessary. There are several approaches to this
mutually exclusive fields where necessary. There are several approaches,

depending on how mutually exclusive the field needs to be; in general, APIs
**should** attempt to introduce the "least mutually exclusive" approach that is
able to satisfy the use case.

For example, an API **should not** use a completely generic field (such as
`google.protobuf.Struct` in protobuf APIs) when the value of the field must
correspond to one of a known number of schemas. Instead, the API **should** use
a [`oneof`](./oneof) to represent the known schemas.
For example, an API **should not** use a completely mutually exclusive field
(such as `google.protobuf.Struct` in protobuf APIs) when the value of the field
must correspond to one of a known number of schemas. Instead, the API
**should** use a [`oneof`](./oneof) to represent the known schemas.
Comment on lines +19 to +22
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think most users would use a protobuf struct to define a mutually exclusive set of fields - perhaps it's better to start with OneOf?

We can always call out bad patterns below, in a new section like Rationale: https://aep.dev/8/#document-structure


### Generic fields in protobuf APIs
{% tab proto %}

#### Oneof
#### Type Union
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#### Type Union
#### Union Type

I think most descriptions of types describe this as "Union Types":

https://docs.python.org/3/library/typing.html#typing.Union
https://www.typescriptlang.org/docs/handbook/declaration-files/do-s-and-don-ts.html#use-union-types

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I don't think protobuf oneofs are union types really - they are a set of fields that are mutually exclusive.

A union type is "field foo can be X or Y" - in this case it's "field foo of type X, or bar of type Y can be set". So really I'd say "mutually exclusive fields" is a better term.


A `oneof` **may** be used to introduce a type union: the user or API is able to
specify one of the fields inside the `oneof`. Additionally, a `oneof` **may**
Expand All @@ -33,11 +34,11 @@ Because the individual fields in the `oneof` have different keys, a developer
can programmatically determine which (if any) of the fields is populated.

A `oneof` preserves the largest degree of type safety and semantic meaning for
each option, and APIs **should** generally prefer them over other generic or
polymorphic options when feasible. However, the `oneof` construct is ill-suited
when there is a large (or unlimited) number of potential options, or when there
is a large resource structure that would require a long series of "cascading
oneofs".
each option, and APIs **should** generally prefer them over other mutually
exclusive or polymorphic options when feasible. However, the `oneof` construct
is ill-suited when there is a large (or unlimited) number of potential options,
or when there is a large resource structure that would require a long series of
"cascading oneofs".

**Note:** Adding additional possible fields to an existing `oneof` is a
non-breaking change, but moving existing fields into or out of a `oneof` is
Expand All @@ -48,9 +49,9 @@ breaking (it creates a backwards-incompatible change in Go protobuf stubs).
Maps **may** be used in situations where many values _of the same type_ are
needed, but the keys are unknown or user-determined.

Maps are usually not appropriate for generic fields because the map values all
share a type, but occasionally they are useful. In particular, a map can
sometimes be suited to a situation where many objects of the same type are
Maps are usually not appropriate for mutually exclusive fields because the map
values all share a type, but occasionally they are useful. In particular, a map
can sometimes be suited to a situation where many objects of the same type are
needed, with different behavior based on the names of their keys (for example,
using keys as environment names).

Expand Down Expand Up @@ -86,9 +87,63 @@ which is an often-unfamiliar process.
Because of this, `Any` **should not** be used unless other options are
infeasible.

### Generic fields in OAS APIs

**Note:** OAS-specific guidance not yet written.
{% tab oas %}

#### Type Union

Under the definition of the OpenAPI Specification, [JSONSchema][] keywords for
schema composition (`allOf`, `anyOf`, `oneOf`, `not`) **may** be used to
introduce a type union. The type union produced is equivalent to that of using
`oneof` in protobuf APIs. Due to the schema's complexity, validation for this
schema **should** be handled server-side, and it is fine to not document the
schema explicitly.
Comment on lines +97 to +99
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`oneof` in protobuf APIs. Due to the schema's complexity, validation for this
schema **should** be handled server-side, and it is fine to not document the
schema explicitly.
`oneof` in protobuf APIs. Due to the schema's complexity, validation for this
schema **should** be handled server-side, and OpenAPI schemas do not need to document the
schema explicitly.

"it is fine" feels a bit colloquial here - I'd say something along the lines of "do not need" or "are not required".


```JSONSchema
{
"$schema": "https://json-schema.org/draft-04/schema",
"title": "Book",
"type": "object",
"allOf": [
{
"properties": {
"title": {
"type": "string"
}
}
},
{
"oneOf": [
{
"properties": {
"isbn_number": {
"type": "string"
}
},
"required": ["isbn_number"],
},
{
"properties": {
"doi": {
"type": "string"
}
},
"required": ["doi"],
},
{
"not": {
"anyOf": [
{ "required": ["isbn_number"] },
{ "required": ["doi"] },
]
}
}
]
}
]
}
```

{% endtabs %}

<!-- prettier-ignore-start -->
[any]: https://github.com/protocolbuffers/protobuf/tree/master/src/google/protobuf/any.proto
Expand Down
3 changes: 2 additions & 1 deletion aep/general/0146/aep.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
---
id: 146
state: approved
slug: generic-fields
slug: mutually-exclusive-fields
created: 2024-07-02
updated: 2025-01-23
placement:
category: fields