Skip to content

Commit 3ec168d

Browse files
kistzjoshua-spacetimebfops
authored
[Procedures] Fix Identityand ConnectionId Regression (#5323)
# Description of Changes Closes #5250 #4636 introduced a regression where the ProcedureContext used to include the sender and connection_id from the caller while now it is always empty (which is wrong) Correct it. Also fully migrate to `database_identity` which was forgotten about so i deprecated it for procedures (since they are now stable) and just changed it for HttpHandlers (because they are still unstable) @gefjon since you did the lil woopsie (ugh pinging you again lol hope im not annoying haha) # API and ABI breaking changes Breaking the HttpHandler function which is unstable. Restoring behaviour of 2.3 which got lost with 2.4. <!-- If this is an API or ABI breaking change, please apply the corresponding GitHub label. --> # Expected complexity level and risk 1. Trivial refactoring <!-- How complicated do you think these changes are? Grade on a scale from 1 to 5, where 1 is a trivial change, and 5 is a deep-reaching and complex change. This complexity rating applies not only to the complexity apparent in the diff, but also to its interactions with existing and future code. If you answered more than a 2, explain what is complex about the PR, and what other components it interacts with in potentially concerning ways. --> # Testing <!-- Describe any testing you've done, and any testing you'd like your reviewers to do, so that you're confident that all the changes work as expected! --> - [x] The caller identity is there again for Procedures. --------- Signed-off-by: Zeke Foppa <196249+bfops@users.noreply.github.com> Co-authored-by: joshua-spacetime <josh@clockworklabs.io> Co-authored-by: Zeke Foppa <196249+bfops@users.noreply.github.com>
1 parent 5ea558e commit 3ec168d

4 files changed

Lines changed: 29 additions & 11 deletions

File tree

crates/bindings/src/http.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,18 +127,24 @@ impl HandlerContext {
127127
}
128128

129129
/// Read the current module's [`Identity`].
130+
#[deprecated(note = "Use `HandlerContext::database_identity` instead.")]
130131
pub fn identity(&self) -> Identity {
132+
self.database_identity()
133+
}
134+
135+
/// Read the current module's [`Identity`].
136+
pub fn database_identity(&self) -> Identity {
131137
Identity::from_byte_array(spacetimedb_bindings_sys::identity())
132138
}
133139

134140
/// Acquire a mutable transaction and execute `body` with read-write access.
135141
pub fn with_tx<T>(&mut self, body: impl Fn(&TxContext) -> T) -> T {
136-
with_tx(body)
142+
with_tx(body, Identity::ZERO, None)
137143
}
138144

139145
/// Acquire a mutable transaction and execute `body` with read-write access.
140146
pub fn try_with_tx<T, E>(&mut self, body: impl Fn(&TxContext) -> Result<T, E>) -> Result<T, E> {
141-
try_with_tx(body)
147+
try_with_tx(body, Identity::ZERO, None)
142148
}
143149

144150
/// Create a new random [`Uuid`] `v4` using the built-in RNG.

crates/bindings/src/lib.rs

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1180,7 +1180,14 @@ impl Deref for TxContext {
11801180
}
11811181
}
11821182

1183-
fn try_with_tx<T, E>(body: impl Fn(&TxContext) -> Result<T, E>) -> Result<T, E> {
1183+
/// We need to passthrough identity and connection_id because procedures can be invoked by users.
1184+
/// For [HttpContext] this is always anonymous ([Identity::ZERO]).
1185+
/// Construct the inner [ReducerContext] with the appropriate caller information.
1186+
fn try_with_tx<T, E>(
1187+
body: impl Fn(&TxContext) -> Result<T, E>,
1188+
identity: Identity,
1189+
connection_id: Option<ConnectionId>,
1190+
) -> Result<T, E> {
11841191
let abort = || {
11851192
crate::sys::procedure::procedure_abort_mut_tx()
11861193
.expect("should have a pending mutable anon tx as `procedure_start_mut_tx` preceded")
@@ -1191,8 +1198,7 @@ fn try_with_tx<T, E>(body: impl Fn(&TxContext) -> Result<T, E>) -> Result<T, E>
11911198
.expect("holding `&mut HandlerContext`, so should not be in a tx already; called manually elsewhere?");
11921199
let timestamp = Timestamp::from_micros_since_unix_epoch(timestamp);
11931200

1194-
// Use the internal auth context (no external caller identity).
1195-
let tx = ReducerContext::new(crate::Local {}, Identity::ZERO, None, timestamp);
1201+
let tx = ReducerContext::new(crate::Local {}, identity, connection_id, timestamp);
11961202
let tx = TxContext(tx);
11971203

11981204
struct DoOnDrop<F: Fn()>(F);
@@ -1225,9 +1231,9 @@ fn try_with_tx<T, E>(body: impl Fn(&TxContext) -> Result<T, E>) -> Result<T, E>
12251231
res
12261232
}
12271233

1228-
fn with_tx<T>(body: impl Fn(&TxContext) -> T) -> T {
1234+
fn with_tx<T>(body: impl Fn(&TxContext) -> T, identity: Identity, connection_id: Option<ConnectionId>) -> T {
12291235
use core::convert::Infallible;
1230-
match try_with_tx::<T, Infallible>(|tx| Ok(body(tx))) {
1236+
match try_with_tx::<T, Infallible>(|tx| Ok(body(tx)), identity, connection_id) {
12311237
Ok(v) => v,
12321238
Err(e) => match e {},
12331239
}
@@ -1293,7 +1299,13 @@ impl ProcedureContext {
12931299
}
12941300

12951301
/// Read the current module's [`Identity`].
1302+
#[deprecated(note = "Use `ProcedureContext::database_identity` instead.")]
12961303
pub fn identity(&self) -> Identity {
1304+
self.database_identity()
1305+
}
1306+
1307+
/// Read the current module's [`Identity`].
1308+
pub fn database_identity(&self) -> Identity {
12971309
// Hypothetically, we *could* read the module identity out of the system tables.
12981310
// However, this would be:
12991311
// - Onerous, because we have no tooling to inspect the system tables from module code.
@@ -1359,7 +1371,7 @@ impl ProcedureContext {
13591371
/// callers should avoid writing to any captured mutable state within `body`,
13601372
/// This includes interior mutability through types like [`std::cell::Cell`].
13611373
pub fn with_tx<T>(&mut self, body: impl Fn(&TxContext) -> T) -> T {
1362-
with_tx(body)
1374+
with_tx(body, self.sender(), self.connection_id())
13631375
}
13641376

13651377
/// Acquire a mutable transaction
@@ -1392,7 +1404,7 @@ impl ProcedureContext {
13921404
/// callers should avoid writing to any captured mutable state within `body`,
13931405
/// This includes interior mutability through types like [`std::cell::Cell`].
13941406
pub fn try_with_tx<T, E>(&mut self, body: impl Fn(&TxContext) -> Result<T, E>) -> Result<T, E> {
1395-
try_with_tx(body)
1407+
try_with_tx(body, self.sender(), self.connection_id())
13961408
}
13971409

13981410
/// Create a new random [`Uuid`] `v4` using the built-in RNG.

modules/module-test/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -543,7 +543,7 @@ fn with_tx(ctx: &mut ProcedureContext) {
543543
/// This is a silly thing to do, but an effective test of the procedure HTTP API.
544544
#[spacetimedb::procedure]
545545
fn get_my_schema_via_http(ctx: &mut ProcedureContext) -> String {
546-
let module_identity = ctx.identity();
546+
let module_identity = ctx.database_identity();
547547
match ctx.http.get(format!(
548548
"http://localhost:3000/v1/database/{module_identity}/schema?version=9"
549549
)) {

modules/sdk-test-procedure/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ fn will_panic(_ctx: &mut ProcedureContext) {
4242

4343
#[procedure]
4444
fn read_my_schema(ctx: &mut ProcedureContext, server_url: String) -> String {
45-
let module_identity = ctx.identity();
45+
let module_identity = ctx.database_identity();
4646
let server_url = server_url.trim_end_matches('/');
4747
match ctx
4848
.http

0 commit comments

Comments
 (0)