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

remove create > invalid user settable id > too short test because min allowed is 1 #280

Conversation

Yoshiji
Copy link
Contributor

@Yoshiji Yoshiji commented Nov 22, 2024

TL;DR: The Create/invalid_user_settable_id/too_short generated test is not following AIP-122 because single-letter ID is valid.


https://google.aip.dev/122#resource-id-segments mentions the regex to validate the user-provided ID:

^[a-z]([a-z0-9-]{0,61}[a-z0-9])?$

Which follows RFC-1034 rule for labels. Quoting https://www.rfc-editor.org/rfc/rfc1034 (ctrl+f "ARPANET host names")

The labels must follow the rules for ARPANET host names. They must
start with a letter, end with a letter or digit, and have as interior
characters only letters, digits, and hyphen. There are also some
restrictions on the length. Labels must be 63 characters or less.

A valid label can be a single letter. The generated test too short for user settable ID is not feasible because trying to test with an ID with a length equal to 0 = empty = the server will fall back to system-generated ID:

An API may allow the {resource}_id field have the field_behavior OPTIONAL, and generate a system-generated ID if one is not specified.

Source: https://google.aip.dev/133#user-specified-ids

@Yoshiji Yoshiji requested a review from a team as a code owner November 22, 2024 04:18
@Yoshiji Yoshiji changed the title remove create > invalid user settable id > too short test because min is 1 remove create > invalid user settable id > too short test because min allowed is 1 Nov 24, 2024
@ericwenn
Copy link
Member

Nice catch @Yoshiji! ⭐ LGTM

We follow conventional commits in this repository, so you'll have to update the commit message.

…because min allowed is 1

The generated test Create/invalid_user_settable_id/too_short is asserting the create call fails
with a value of "foo" (3 runes), but AIP states the minimum length allowed is 1.

Source: https://google.aip.dev/122#resource-id-segments
@Yoshiji Yoshiji force-pushed the remove-create-with-user-settable-id-too-short-test branch from 9c4ad3f to 7fb49c9 Compare November 25, 2024 14:23
@Yoshiji
Copy link
Contributor Author

Yoshiji commented Nov 25, 2024

@ericwenn I reworded the commit message.

@ericwenn ericwenn merged commit 1d476d2 into einride:master Nov 26, 2024
1 check passed
@Yoshiji Yoshiji deleted the remove-create-with-user-settable-id-too-short-test branch November 26, 2024 03:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants