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
controller_did: scope_proof.controller_did().map(|c| c.into_did()),
72
})
73
}
74
#[derive(Deserialize)]
75
#[allow(dead_code)]
76
pub struct CreateRecordInput {
···
102
pub async fn create_record(
103
State(state): State<AppState>,
104
auth: Auth<Active>,
105
-
Json(input): Json<CreateRecordInput>,
106
) -> Result<Response, crate::api::error::ApiError> {
107
let scope_proof = match auth.verify_repo_create(&input.collection) {
108
Ok(proof) => proof,
···
127
return Ok(ApiError::InvalidSwap(Some("Repo has been modified".into())).into_response());
128
}
129
130
let validation_status = if input.validate.should_skip() {
131
None
132
} else {
···
407
pub async fn put_record(
408
State(state): State<AppState>,
409
auth: Auth<Active>,
410
-
Json(input): Json<PutRecordInput>,
411
) -> Result<Response, crate::api::error::ApiError> {
412
let upsert_proof = match auth.verify_repo_upsert(&input.collection) {
413
Ok(proof) => proof,
···
450
};
451
let mst = Mst::load(Arc::new(tracking_store.clone()), commit.data, None);
452
let key = format!("{}/{}", input.collection, input.rkey);
453
let validation_status = if input.validate.should_skip() {
454
None
455
} else {
···
71
controller_did: scope_proof.controller_did().map(|c| c.into_did()),
72
})
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
+
89
#[derive(Deserialize)]
90
#[allow(dead_code)]
91
pub struct CreateRecordInput {
···
117
pub async fn create_record(
118
State(state): State<AppState>,
119
auth: Auth<Active>,
120
+
Json(mut input): Json<CreateRecordInput>,
121
) -> Result<Response, crate::api::error::ApiError> {
122
let scope_proof = match auth.verify_repo_create(&input.collection) {
123
Ok(proof) => proof,
···
142
return Ok(ApiError::InvalidSwap(Some("Repo has been modified".into())).into_response());
143
}
144
145
+
input.record = elide_record_type(input.record, &input.collection);
146
+
147
let validation_status = if input.validate.should_skip() {
148
None
149
} else {
···
424
pub async fn put_record(
425
State(state): State<AppState>,
426
auth: Auth<Active>,
427
+
Json(mut input): Json<PutRecordInput>,
428
) -> Result<Response, crate::api::error::ApiError> {
429
let upsert_proof = match auth.verify_repo_upsert(&input.collection) {
430
Ok(proof) => proof,
···
467
};
468
let mst = Mst::load(Arc::new(tracking_store.clone()), commit.data, None);
469
let key = format!("{}/{}", input.collection, input.rkey);
470
+
471
+
input.record = elide_record_type(input.record, &input.collection);
472
+
473
let validation_status = if input.validate.should_skip() {
474
None
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