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

Introduce dedicated types for more Lexicon string formats #106

Merged
merged 2 commits into from
Feb 19, 2024

Conversation

str4d
Copy link
Contributor

@str4d str4d commented Feb 19, 2024

  • datetime
  • nsid
  • language

@str4d
Copy link
Contributor Author

str4d commented Feb 19, 2024

There are three remaining Lexicon string formats that don't have dedicated types:

  • at-uri
    • I add a RecordKey type in this PR, but after reading the AT URI Scheme spec again I'm no longer sure whether at-uri is specifically the Restricted AT URI Syntax (which I can implement with regex), or if it needs to be parsed as the Full AT URI Syntax (which is a bunch of extra work, similar to uri).
  • cid
    • This is a CID in string format. This should be straightforward, but I haven't implemented it because the cid crate is currently behind the dag-cbor feature flag, so I want to check what your intentions are here before implementing anything. If you want this to remain configurable, then we'd need a fallback Cid(String) implementation (and would not be validating CIDs during parsing in that case).
  • uri
    • This definitely cannot be implemented with the url crate, and requires a bunch of extra work.

@sugyan
Copy link
Owner

sugyan commented Feb 19, 2024

Thanks a lot!
I'm not sure about at-uri. Looking at the original atproto repository, I see that both ensureValidAtUri and ensureValidAtUriRegex, but it seems that only ensureValidAtUri is actually used.

I see, we need to use cid crate to validate the cid string format. Currently, I only add it to dependencies when the dag-cbor feature is enabled, but if it is needed for validation, you can change it so that it is added regardless of the feature.

In the future, it might be nice to add a feature like no-validation and thereby reduce the number of crate dependencies that are used only for validation.

@sugyan sugyan merged commit f5d505f into sugyan:main Feb 19, 2024
4 checks passed
@str4d str4d deleted the lexicon-strings branch February 19, 2024 13:29
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