AIP-133 parent detection shouldn't trigger for top-level resources

[bMacSwigg]

https://linter.aip.dev/133/request-parent-required will show a warning whenever there is no parent field. AIP-133 specifically mentions: A parent field must be included unless the resource being created is a top-level resource (since top-level resources don't have a parent, by definition).

It would be better if the linter could try to detect probable top-level resources. It would also be better if the language of the linter warning specifically called out the possibility that this is a top-level resource, and gave guidance on what to do (e.g. standard language for disabling the linter warning for that resource).

[Catgroove]

Any updates on this? Top level rules relating to 133 seems broken in general.

This should be perfectly fine according to the specification:

service SessionService {
  rpc CreateSession(CreateSessionRequest) returns (Session) {
    option (google.api.http) = {
      post: "/v1/sessions"
      body: "session"
    };
    option (google.api.method_signature) = "session";
  }
}

message CreateSessionRequest {
  Session session = 1 [(google.api.field_behavior) = REQUIRED];
}

But it complains about core::0133::method-signature, core::0133::http-uri-parent and core::0133::request-id-field, mainly because a parent field and annotation is always expected, along with a session_id.

[noahdietz]

core::0133::method-signature, core::0133::http-uri-parent

Hey @Catgroove these have been poked at a few times. It was actually supposed to have been fixed by #1439 (in v1.67.4). There are a number of issues in this repo and this one should've been closed by that. Are you using a sufficiently new version of api-linter?

If using a sufficiently recent release, if you don't mind sharing the definition of Session resource from your example, I could see if there is a gap in the logic.

core::0133::request-id-field

This one is unrelated to where it the resource to be created is top-level or not. It was introduced some time ago to fulfill requirements in AIP-133:

If this is guidance that doesn't apply to your usage of api-linter, please disable the rule.

[Catgroove]

Thanks for the reply. I'm using the latest version, v1.68.0 and it's still causing the same issue.

I do not think that the shape of session matters much but you can reproduce it with:

message Session {
  string name = 1 [(google.api.field_behavior) = IDENTIFIER];
}

core::0133::request-id-field

I think this still matters as there's some interplay with this rule and the top level rules. If I enable it, I get the following:

create methods should contain a singular string session_id field.

But that's not part of the method signature of the create request, as demonstrated by your library example here.

But even if I add that to the request message and ignore the library example, I ge the following:

Message "CreateSessionRequest" has no parent field

But since it's a top level resource, why do I need a parent? From AIP-133:

A parent field must be included unless the resource being created is a top-level resource.

[noahdietz]

I do not think that the shape of session matters much but you can reproduce it with:

Sorry, I should've been more specific: If you don't mind, please share the google.api.resource annotation on your Session resource. If there is not one, that might be part of the issue. I found some faulty logic that would be triggered by not having a google.api.resource with patterns defined. Pretty sure this is the root cause.

core::0133::request-id-field

I think this still matters as there's some interplay with this rule and the top level rules. If I enable it, I get the following:

create methods should contain a singular string session_id field.

There is no interplay, the rules are run independently from each other, and this specific rule is not influenced by existence of a parent or not. That said, they may share implementation utilities that result in the same behavior happening in multiple rules.

But that's not part of the method signature of the create request, as demonstrated by your library example here.

I would not consider that example authoritative, rather the contents of https://google.aip.dev is the authority. That said, the example there should be updated.

[noahdietz]

the google.api.resource annotation on your Session resource. If there is not one, that might be part of the issue. I found some faulty logic that would be triggered by not having a google.api.resource with patterns defined. Pretty sure this is the root cause.

I've got a fix/refactor open for this. Feel free to checkout the PR to build and test locally if you want.