googleapis / api-linter

A linter for APIs defined in protocol buffers.
https://linter.aip.dev/
Apache License 2.0
599 stars 144 forks source link

AIP-123: Alternating collection identifiers and singletons #1398

Open liveFreeOrCode opened 5 months ago

liveFreeOrCode commented 5 months ago

I believe that this PR introduced a bug: https://github.com/googleapis/api-linter/pull/1151

As stated in AIP-122:

Resource name components should usually alternate between collection identifiers (example: publishers, books, users) and resource IDs (example: 123, les-miserables, vhugo1802).

Note the word "usually"

As stated in AIP-156:

Singleton resources may parent other resources.

And...

Singleton resources must not have a user-provided or system-generated ID; their [resource name][aip-122] includes the name of their parent followed by one static-segment.

While, this is not explicit in the documentation, I would expect the singleton pattern, with the ability to be a parent of other resources, to take precedent and put "usually" to the test.

(very contrived) Example of the problem:

// Each user can only have a single library
message Library {
  // The pattern indicates that a user is the canonical parent for library
  option (google.api.resource) = {
    type: "library.googleapis.com/Library"
    pattern: "users/{user}/library"
    singular: "library"
    plural: "libraries"
  };

  // The resource name for the user's library
  // Format: "users/{user}/library"
  string name = 1 [(google.api.field_behavior) = IDENTIFIER];

  // Resource names for books in the library
  // Format: "users/{user}/library/books/{book}
  repeated string books = 2  [
    (google.api.resource_reference) = {type: "library.googleapis.com/Book"},
    (google.api.field_behavior) = REQUIRED
  ];
}

// Each user's library can have many books
// (-- api-linter: core::0123::resource-name-components-alternate=disabled
//     aip.dev/not-precedent: sad panda --)
message Book {
  // The pattern indicates that a library is the canonical parent for book
  option (google.api.resource) = {
    type: "library.googleapis.com/Book"
    pattern: "users/{user}/library/books/{book}"
    singular: "book"
    plural: "books"
  };

  // The resource name for the book in a user's library
  // Format: "users/{user}/library/books/{book}"
  string name = 1 [(google.api.field_behavior) = IDENTIFIER]
}

Curious to hear your thoughts?

noahdietz commented 3 months ago

Yes I think you are right - a child resource of a singleton would have sequential static segments.

What we could probably do is first lop off the last two segments e.g. /books/{book} and see if the remaining parent pattern exists as a resource i.e. a Singleton, and allow that case.

@liveFreeOrCode do you have any interest in working on this?

(sorry for the late reply!)

liveFreeOrCode commented 3 months ago

@noahdietz yeah, I don't mind working on it, but I'm taking some PTO the next couple of weeks, so it wouldn't be until early/mid September I can look at it.