Skip to content

Add ValidateInput method to atproto/lexicon #1075

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

wdantuma
Copy link

@wdantuma wdantuma commented May 6, 2025

This PR adds a ValidateInput ( counterpart of ValidateRecord ) to validate the request body and parameters against a lexicon for use in a pds implementation.

This PR also changes the term field to property in the error messages ( to align it with the terms used in the lexicon )

@bnewbold bnewbold self-requested a review May 7, 2025 22:47
@bnewbold bnewbold self-assigned this May 7, 2025
// 'parameters' are the HTTP request parameters to be validated
// 'ref' is a reference to the schema type, as an NSID with optional fragment. For records, the '$type' must match 'ref'
// 'flags' are parameters tweaking Lexicon validation rules. Zero value is default.
func ValidateInput(cat Catalog, inputData any, parameters url.Values, ref string, flags ValidateFlags) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't love "Input" as the term here, but need to think more about what I would use instead. "Request"? Should this be broken out by "ValidateQuery" / "ValidateProcedure" for the two API request types?

Copy link
Author

Choose a reason for hiding this comment

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

I already renamed it to validateRequest, but the more i am thinking about it i think you're right, separate calls for ValidateQuery and ValidateProcedure are more aligned with the lexicon structure ( input is not a main type ). I shall make the changes and add test coverage.

@bnewbold
Copy link
Collaborator

bnewbold commented May 7, 2025

Thanks for sending this over! We do need this to help implement services of many types. Relatedly, we have some early work on SDK support for server-side auth validation coming along.

I probably will not be able to give this a real review in the next two weeks. Will also want to have some test coverage.

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