tranquil.farm
Some clients (anisota) seem to miss the $type here. It's quite inconvenient if they are not allowed to write their records - and we can easily elide the $type anyway by what collection the record is going to!
+22
-2
crates/tranquil-pds/src/api/repo/record/write.rs
+22
-2
crates/tranquil-pds/src/api/repo/record/write.rs
···
71
71
controller_did: scope_proof.controller_did().map(|c| c.into_did()),
72
72
})
73
73
}
74
+
75
+
pub fn elide_record_type(mut record: serde_json::Value, collection: &Nsid) -> serde_json::Value {
76
+
match record {
77
+
serde_json::Value::Object(ref mut value) => {
78
+
if value.contains_key("$type") {
79
+
record
80
+
} else {
81
+
value.insert("$type".to_owned(), collection.as_str().into());
82
+
record
83
+
}
84
+
},
85
+
_ => record
86
+
}
87
+
}
88
+
74
89
#[derive(Deserialize)]
75
90
#[allow(dead_code)]
76
91
pub struct CreateRecordInput {
···
102
117
pub async fn create_record(
103
118
State(state): State<AppState>,
104
119
auth: Auth<Active>,
105
-
Json(input): Json<CreateRecordInput>,
120
+
Json(mut input): Json<CreateRecordInput>,
106
121
) -> Result<Response, crate::api::error::ApiError> {
107
122
let scope_proof = match auth.verify_repo_create(&input.collection) {
108
123
Ok(proof) => proof,
···
127
142
return Ok(ApiError::InvalidSwap(Some("Repo has been modified".into())).into_response());
128
143
}
129
144
145
+
input.record = elide_record_type(input.record, &input.collection);
146
+
130
147
let validation_status = if input.validate.should_skip() {
131
148
None
132
149
} else {
···
407
424
pub async fn put_record(
408
425
State(state): State<AppState>,
409
426
auth: Auth<Active>,
410
-
Json(input): Json<PutRecordInput>,
427
+
Json(mut input): Json<PutRecordInput>,
411
428
) -> Result<Response, crate::api::error::ApiError> {
412
429
let upsert_proof = match auth.verify_repo_upsert(&input.collection) {
413
430
Ok(proof) => proof,
···
450
467
};
451
468
let mst = Mst::load(Arc::new(tracking_store.clone()), commit.data, None);
452
469
let key = format!("{}/{}", input.collection, input.rkey);
470
+
471
+
input.record = elide_record_type(input.record, &input.collection);
472
+
453
473
let validation_status = if input.validate.should_skip() {
454
474
None
455
475
} else {
History
1 round
5 comments
a.starrysky.fyi
submitted
#0
1 commit
expand
collapse
feat(write): allow records with missing $type
Some clients (anisota) seem to miss the $type here. It's quite
inconvenient if they are not allowed to write their records - and we can
easily elide the $type anyway by what collection the record is going to!
merge conflicts detected
expand
collapse
expand
collapse
- crates/tranquil-pds/src/api/proxy.rs:40
expand 5 comments
im a bit on the fence in some ways about this yea but honestly think its less controversial than the former 馃槄. records without $type fields are objectively invalid types but also that field is required to match the collection anyways ...
main two arguments i see against this are
- dont accept invalid objects. theyre invalid.
- enforcing $type causes just a bit of extra friction against accidentally writing invalid records the first argument has some value and is what makes me hesitant but the second argument gets shot down by one: lexicon validation is there and more comprehensive and two: just like for the first reason to not do this. buggy clients arent our issue.
not sure quite yet honestly. i think we see if anyone has any strong very strong arguments for before we merge. otherwise like the former: try and get a hold of dame? this is arguably an anisota and reference impl bug not an tranquil one
try and get a hold of dame
I've already sent a message in bluesky DMs, I'll try keep this thread updated with any responses. Anisota is definitely broken here (and in a few other places) - I just would also like to be able to use broken clients that work with bsky... (actually specifically anisota. I just really like moth bluesky :))
Just checked - I actually got a response shortly before I sent this
Anisota seems to be interested in fixing this, I'm offering to give them an account on my PDS for debugging...
nice nice! im sure we can set them up with an account on tranquil.farm too if needed.
the main logic here i we want to keep one-off exceptions like this to a minimum. the expectation is that eventually either spec will be updated to align with ref impl or ref impl will be updated to align with spec. so we stick to spec where we can rn. having to account for all the various ways smaller clients are broken because they have buggy implementations gets messy really quickly. its already messy having to do that for bluesky to work at all. so we're using the official bluesky client + spec as the baseline to keep this thing at all maintainable long term 馃槄
So, this is necessarily pretty controversial and I'm not really happy with it. I'm certain it must break some tests somewhere too ... but I needed the patch for myself and I wanted to start some conversation
I stacked this ontop of the other patch so hopefully it should be possible to merge only that if (as I think is plausible) we decide we want that but not this