Skip to content

Imlementation of AtUri proposal#37

Merged
rudyfraser merged 7 commits intoblacksky-algorithms:mainfrom
afbase:afbase/using_aturi
Jan 21, 2025
Merged

Imlementation of AtUri proposal#37
rudyfraser merged 7 commits intoblacksky-algorithms:mainfrom
afbase:afbase/using_aturi

Conversation

@afbase
Copy link
Contributor

@afbase afbase commented Jan 16, 2025

closes #36

Overview

This PR implements the TryFrom trait for AtUri to handle string-to-AtUri conversions. This addresses multiple TODOs throughout the codebase where string manipulation was being used instead of proper AtUri types.

Changes

Files modified:

  • src/apis/com/atproto/admin/update_subject_status.rs
  • src/apis/com/atproto/repo/apply_writes.rs
  • src/apis/com/atproto/repo/create_record.rs
  • src/apis/com/atproto/repo/delete_record.rs
  • src/apis/com/atproto/repo/get_record.rs
  • src/apis/com/atproto/repo/list_records.rs
  • src/apis/com/atproto/repo/put_record.rs
  • src/repo/mod.rs
  • src/repo/record/mod.rs
  • src/repo/types.rs
  • src/syntax/aturi.rs

Implementation Details

The core change is implementing TryFrom for AtUri with three variants:

impl TryFrom<&str> for AtUri
impl TryFrom<String> for AtUri  
impl TryFrom<&String> for AtUri

This allows for ergonomic conversion of strings to AtUri types using the try_into() method, replacing manual string manipulation throughout the codebase, e.g.:

// Before
let uri_without_prefix = uri.replace("at://", "");
let parts = uri_without_prefix.split("/").collect::<Vec<&str>>();

// After
let at_uri: AtUri = uri.try_into()?;

Review Notes

  1. Please review the get_backlink_conflicts function in rsky-pds/src/repo/record/mod.rs - specifically the use of flat_map. I chose flat_map since we're dealing with Option<AtUri>, but I'm not 100% sure if this is the optimal approach vs using map. Would appreciate input here.

  2. TODO: AtUri validation still needs to be implemented. I plan to add validation logic that mirrors the TypeScript implementation found at: https://github.com/bluesky-social/atproto/blob/main/packages/syntax/src/aturi_validation.ts - I don't intend on implementing the validation in the branch unless you'd prefer it that way. It's already a fairly large PR ready to go.

Copy link
Member

@rudyfraser rudyfraser left a comment

Choose a reason for hiding this comment

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

Left a comment about updating the get_backlink_conflicts function but everything else LGTM, thank you

@rudyfraser rudyfraser merged commit 622089c into blacksky-algorithms:main Jan 21, 2025
3 of 4 checks passed
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.

Proposal: Utilizing AtUri in place of Strings

2 participants