Skip to content

Support for thrift annotation auth.actor_uuidAnnotate - #2463

Open
oneporter wants to merge 6 commits into
yarpc:mainfrom
oneporter:annotate
Open

Support for thrift annotation auth.actor_uuidAnnotate#2463
oneporter wants to merge 6 commits into
yarpc:mainfrom
oneporter:annotate

Conversation

@oneporter

Copy link
Copy Markdown
Collaborator

What?
Add support for thrift annotation "auth.actor_uuid".

Why?
This supports validation for these fields.

RELEASE NOTES:
Added support for annotation auth.actor_uuid

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@CoolTomatos

Copy link
Copy Markdown
Collaborator

If my understanding is correct, this won't work with:

service TestService {
  string testMethod(1: string notInterested, 2: string interested (auth.actor_uuid = "true"))
}

@bananacocodrilo

bananacocodrilo commented Apr 20, 2026

Copy link
Copy Markdown
Collaborator

What? Add support for thrift annotation "auth.actor_uuid".

How's this intended to be consumed? By who?

Should we add something like this?

type ActorUUIDProvider interface { ActorUUID() string }

Otherwise consumers will need to define it themselves

for _, field := range ss.Fields {
if _, ok := field.Annotations[_UUIDAnnotationKey]; ok {
if prevValue, pres := annotatedTypes[ss]; pres {
return map[*compile.StructSpec]string{}, fmt.Errorf("multiple fields annotated with %s for type %s. Prev %s; Duplicate at %s",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

generated code assumes *string but somthing like required string UserID generates UserID string, so uuid != nil is always true and *uuid won't compile.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

interesting, good catch!

// and re-compile it. Plugins do not actually have access to the original
// thrift file's module, and this is how we gain access to it, since this
// generator writes extra methods to exception types defined in the Thrift
// file.

@bananacocodrilo bananacocodrilo Apr 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

copied from exception.go?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

yes

@@ -0,0 +1,6 @@

@bananacocodrilo bananacocodrilo Apr 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

empty line on each new .thrift

}

templateOptions := append(templateOptions,
plugin.TemplateFunc("getAllAnnotatedTypes", func() map[*compile.StructSpec]string {

@bananacocodrilo bananacocodrilo Apr 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can this lead to a data race if this and the exceptions generator call try to write?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

they're executed sequentially, can you expand on the concern here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants