Skip to content

Commit d05d85f

Browse files
ldanilekConvex, Inc.
authored andcommitted
use CanonicalizedUdfPath everywhere (#28010)
this fixes a bug where actions can't always look up a Reference because the reference always holds a UdfPath that lacks a `.js` extension, while some ways of constructing the same UdfPath end up with a `.js` extension. See regression test `test_schedule_by_string`. To do this, we should make `Reference` always hold the canonical version of each UdfPath, i.e. CanonicalizedUdfPath. An alternative would be to make a new type `CanonicalizedReference` but I don't think that's worth it. Once Reference held canonical paths, Resource needed to change as well. And then I went down the rabbit hole, ending up with the following conclusions: - Developers can refer to `basic:insertObject` and `basic.js:insertObject` and they mean exactly the same thing. - We already always store canonical paths in system tables like `_modules` and `_scheduled_jobs` and `_cron_job_logs` etc. - If the developer sends a module path `basic` or `basic.js` into Convex, they expect them to be stored the same and have the same meaning, because that's almost true today (it's true if we end up storing it). - Almost all internal code should use canonical UDF paths. - If we want to display without the `.js` extension in error messages or virtual tables, we can use `.strip()` to remove it from the canonical representation. We should not choose to display it based on user input. - I fixed the error message `Couldn't resolve api.engine.events.js.batchRegisteredMutations` to remove the `.js` regardless of user input. GitOrigin-RevId: 606712e7a2426781bba9f517c9345eb578caf633
1 parent 58eb733 commit d05d85f

File tree

31 files changed

+184
-152
lines changed

31 files changed

+184
-152
lines changed

crates/application/src/api.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use async_trait::async_trait;
77
use bytes::Bytes;
88
use common::{
99
components::{
10-
ComponentFunctionPath,
10+
CanonicalizedComponentFunctionPath,
1111
ComponentId,
1212
},
1313
pause::PauseClient,
@@ -99,7 +99,7 @@ pub trait ApplicationApi: Send + Sync {
9999
host: &str,
100100
request_id: RequestId,
101101
identity: Identity,
102-
path: ComponentFunctionPath,
102+
path: CanonicalizedComponentFunctionPath,
103103
args: Vec<JsonValue>,
104104
caller: FunctionCaller,
105105
ts: ExecuteQueryTimestamp,
@@ -111,7 +111,7 @@ pub trait ApplicationApi: Send + Sync {
111111
host: &str,
112112
request_id: RequestId,
113113
identity: Identity,
114-
path: ComponentFunctionPath,
114+
path: CanonicalizedComponentFunctionPath,
115115
args: Vec<JsonValue>,
116116
caller: FunctionCaller,
117117
// Identifier used to make this mutation idempotent.
@@ -123,7 +123,7 @@ pub trait ApplicationApi: Send + Sync {
123123
host: &str,
124124
request_id: RequestId,
125125
identity: Identity,
126-
path: ComponentFunctionPath,
126+
path: CanonicalizedComponentFunctionPath,
127127
args: Vec<JsonValue>,
128128
caller: FunctionCaller,
129129
) -> anyhow::Result<Result<RedactedActionReturn, RedactedActionError>>;
@@ -133,7 +133,7 @@ pub trait ApplicationApi: Send + Sync {
133133
host: &str,
134134
request_id: RequestId,
135135
identity: Identity,
136-
path: ComponentFunctionPath,
136+
path: CanonicalizedComponentFunctionPath,
137137
args: Vec<JsonValue>,
138138
caller: FunctionCaller,
139139
) -> anyhow::Result<Result<FunctionReturn, FunctionError>>;
@@ -218,7 +218,7 @@ impl<RT: Runtime> ApplicationApi for Application<RT> {
218218
_host: &str,
219219
request_id: RequestId,
220220
identity: Identity,
221-
path: ComponentFunctionPath,
221+
path: CanonicalizedComponentFunctionPath,
222222
args: Vec<JsonValue>,
223223
caller: FunctionCaller,
224224
ts: ExecuteQueryTimestamp,
@@ -242,7 +242,7 @@ impl<RT: Runtime> ApplicationApi for Application<RT> {
242242
_host: &str,
243243
request_id: RequestId,
244244
identity: Identity,
245-
path: ComponentFunctionPath,
245+
path: CanonicalizedComponentFunctionPath,
246246
args: Vec<JsonValue>,
247247
caller: FunctionCaller,
248248
// Identifier used to make this mutation idempotent.
@@ -270,7 +270,7 @@ impl<RT: Runtime> ApplicationApi for Application<RT> {
270270
_host: &str,
271271
request_id: RequestId,
272272
identity: Identity,
273-
path: ComponentFunctionPath,
273+
path: CanonicalizedComponentFunctionPath,
274274
args: Vec<JsonValue>,
275275
caller: FunctionCaller,
276276
) -> anyhow::Result<Result<RedactedActionReturn, RedactedActionError>> {
@@ -288,7 +288,7 @@ impl<RT: Runtime> ApplicationApi for Application<RT> {
288288
_host: &str,
289289
request_id: RequestId,
290290
identity: Identity,
291-
path: ComponentFunctionPath,
291+
path: CanonicalizedComponentFunctionPath,
292292
args: Vec<JsonValue>,
293293
caller: FunctionCaller,
294294
) -> anyhow::Result<Result<FunctionReturn, FunctionError>> {

crates/application/src/application_function_runner/mod.rs

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ use common::{
2020
components::{
2121
CanonicalizedComponentFunctionPath,
2222
ComponentDefinitionPath,
23-
ComponentFunctionPath,
2423
ComponentId,
2524
ComponentPath,
2625
},
@@ -737,7 +736,7 @@ impl<RT: Runtime> ApplicationFunctionRunner<RT> {
737736
pub async fn retry_mutation(
738737
&self,
739738
request_id: RequestId,
740-
path: ComponentFunctionPath,
739+
path: CanonicalizedComponentFunctionPath,
741740
arguments: Vec<JsonValue>,
742741
identity: Identity,
743742
mutation_identifier: Option<SessionRequestIdentifier>,
@@ -768,7 +767,7 @@ impl<RT: Runtime> ApplicationFunctionRunner<RT> {
768767
async fn _retry_mutation(
769768
&self,
770769
request_id: RequestId,
771-
path: ComponentFunctionPath,
770+
path: CanonicalizedComponentFunctionPath,
772771
arguments: Vec<JsonValue>,
773772
identity: Identity,
774773
mutation_identifier: Option<SessionRequestIdentifier>,
@@ -787,7 +786,6 @@ impl<RT: Runtime> ApplicationFunctionRunner<RT> {
787786
}))
788787
},
789788
};
790-
let path = path.canonicalize();
791789
let udf_path_string = (!path.udf_path.is_system()).then_some(path.udf_path.to_string());
792790

793791
let mut backoff = Backoff::new(
@@ -1047,7 +1045,7 @@ impl<RT: Runtime> ApplicationFunctionRunner<RT> {
10471045
pub async fn run_action(
10481046
&self,
10491047
request_id: RequestId,
1050-
path: ComponentFunctionPath,
1048+
path: CanonicalizedComponentFunctionPath,
10511049
arguments: Vec<JsonValue>,
10521050
identity: Identity,
10531051
caller: FunctionCaller,
@@ -1065,12 +1063,11 @@ impl<RT: Runtime> ApplicationFunctionRunner<RT> {
10651063
},
10661064
};
10671065
let context = ExecutionContext::new(request_id.clone(), &caller);
1068-
let canonicalized_path = path.canonicalize();
10691066
let usage_tracking = FunctionUsageTracker::new();
10701067
let start = self.runtime.monotonic_now();
10711068
let completion_result = self
10721069
.run_action_no_udf_log(
1073-
canonicalized_path.clone(),
1070+
path.clone(),
10741071
arguments.clone(),
10751072
identity.clone(),
10761073
caller.clone(),
@@ -1083,7 +1080,7 @@ impl<RT: Runtime> ApplicationFunctionRunner<RT> {
10831080
Err(e) => {
10841081
self.function_log.log_action_system_error(
10851082
&e,
1086-
canonicalized_path,
1083+
path,
10871084
arguments,
10881085
identity.into(),
10891086
start,
@@ -1866,7 +1863,7 @@ impl<RT: Runtime> ApplicationFunctionRunner<RT> {
18661863
pub async fn run_query_at_ts(
18671864
&self,
18681865
request_id: RequestId,
1869-
path: ComponentFunctionPath,
1866+
path: CanonicalizedComponentFunctionPath,
18701867
args: Vec<JsonValue>,
18711868
identity: Identity,
18721869
ts: Timestamp,
@@ -1899,7 +1896,7 @@ impl<RT: Runtime> ApplicationFunctionRunner<RT> {
18991896
async fn run_query_at_ts_inner(
19001897
&self,
19011898
request_id: RequestId,
1902-
path: ComponentFunctionPath,
1899+
path: CanonicalizedComponentFunctionPath,
19031900
args: Vec<JsonValue>,
19041901
identity: Identity,
19051902
ts: Timestamp,
@@ -1920,13 +1917,12 @@ impl<RT: Runtime> ApplicationFunctionRunner<RT> {
19201917
});
19211918
},
19221919
};
1923-
let canonicalized = path.canonicalize();
19241920
let usage_tracker = FunctionUsageTracker::new();
19251921
let result = self
19261922
.cache_manager
19271923
.get(
19281924
request_id,
1929-
canonicalized,
1925+
path,
19301926
args,
19311927
identity.clone(),
19321928
ts,
@@ -2000,7 +1996,7 @@ impl<RT: Runtime> ActionCallbacks for ApplicationFunctionRunner<RT> {
20001996
async fn execute_query(
20011997
&self,
20021998
identity: Identity,
2003-
path: ComponentFunctionPath,
1999+
path: CanonicalizedComponentFunctionPath,
20042000
args: Vec<JsonValue>,
20052001
context: ExecutionContext,
20062002
) -> anyhow::Result<FunctionResult> {
@@ -2026,7 +2022,7 @@ impl<RT: Runtime> ActionCallbacks for ApplicationFunctionRunner<RT> {
20262022
async fn execute_mutation(
20272023
&self,
20282024
identity: Identity,
2029-
path: ComponentFunctionPath,
2025+
path: CanonicalizedComponentFunctionPath,
20302026
args: Vec<JsonValue>,
20312027
context: ExecutionContext,
20322028
) -> anyhow::Result<FunctionResult> {
@@ -2054,7 +2050,7 @@ impl<RT: Runtime> ActionCallbacks for ApplicationFunctionRunner<RT> {
20542050
async fn execute_action(
20552051
&self,
20562052
identity: Identity,
2057-
path: ComponentFunctionPath,
2053+
path: CanonicalizedComponentFunctionPath,
20582054
args: Vec<JsonValue>,
20592055
context: ExecutionContext,
20602056
) -> anyhow::Result<FunctionResult> {
@@ -2160,7 +2156,7 @@ impl<RT: Runtime> ActionCallbacks for ApplicationFunctionRunner<RT> {
21602156
&self,
21612157
identity: Identity,
21622158
scheduling_component: ComponentId,
2163-
scheduled_path: ComponentFunctionPath,
2159+
scheduled_path: CanonicalizedComponentFunctionPath,
21642160
udf_args: Vec<JsonValue>,
21652161
scheduled_ts: UnixTimestamp,
21662162
context: ExecutionContext,

crates/application/src/lib.rs

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ use common::{
4343
CanonicalizedComponentFunctionPath,
4444
CanonicalizedComponentModulePath,
4545
ComponentDefinitionPath,
46-
ComponentFunctionPath,
4746
ComponentId,
4847
ComponentPath,
4948
},
@@ -848,7 +847,7 @@ impl<RT: Runtime> Application<RT> {
848847
pub async fn read_only_udf(
849848
&self,
850849
request_id: RequestId,
851-
path: ComponentFunctionPath,
850+
path: CanonicalizedComponentFunctionPath,
852851
args: Vec<JsonValue>,
853852
identity: Identity,
854853
caller: FunctionCaller,
@@ -862,7 +861,7 @@ impl<RT: Runtime> Application<RT> {
862861
pub async fn read_only_udf_at_ts(
863862
&self,
864863
request_id: RequestId,
865-
path: ComponentFunctionPath,
864+
path: CanonicalizedComponentFunctionPath,
866865
args: Vec<JsonValue>,
867866
identity: Identity,
868867
ts: Timestamp,
@@ -934,7 +933,7 @@ impl<RT: Runtime> Application<RT> {
934933
pub async fn mutation_udf(
935934
&self,
936935
request_id: RequestId,
937-
path: ComponentFunctionPath,
936+
path: CanonicalizedComponentFunctionPath,
938937
args: Vec<JsonValue>,
939938
identity: Identity,
940939
// Identifier used to make this mutation idempotent.
@@ -1000,7 +999,7 @@ impl<RT: Runtime> Application<RT> {
1000999
pub async fn action_udf(
10011000
&self,
10021001
request_id: RequestId,
1003-
name: ComponentFunctionPath,
1002+
name: CanonicalizedComponentFunctionPath,
10041003
args: Vec<JsonValue>,
10051004
identity: Identity,
10061005
caller: FunctionCaller,
@@ -1126,7 +1125,7 @@ impl<RT: Runtime> Application<RT> {
11261125
pub async fn any_udf(
11271126
&self,
11281127
request_id: RequestId,
1129-
path: ComponentFunctionPath,
1128+
path: CanonicalizedComponentFunctionPath,
11301129
args: Vec<JsonValue>,
11311130
identity: Identity,
11321131
caller: FunctionCaller,
@@ -1147,7 +1146,7 @@ impl<RT: Runtime> Application<RT> {
11471146
// rare enough to disregard it.
11481147
let mut tx_type = self.begin(identity.clone()).await?;
11491148

1150-
let canonicalized_path = path.clone().canonicalize();
1149+
let canonicalized_path = path.clone();
11511150
let Some(analyzed_function) = ModuleModel::new(&mut tx_type)
11521151
.get_analyzed_function(&canonicalized_path)
11531152
.await?
@@ -2025,7 +2024,7 @@ impl<RT: Runtime> Application<RT> {
20252024
component: ComponentPath::TODO(),
20262025
udf_path: CanonicalizedUdfPath::new(module_path, function_name),
20272026
};
2028-
let arguments = parse_udf_args(&path.clone().into(), args)?;
2027+
let arguments = parse_udf_args(&path, args)?;
20292028
let (result, log_lines) = match analyzed_function.udf_type {
20302029
UdfType::Query => self
20312030
.runner

crates/application/src/tests/components.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use common::{
1414
ComponentType,
1515
},
1616
components::{
17-
ComponentFunctionPath,
17+
CanonicalizedComponentFunctionPath,
1818
ComponentId,
1919
ComponentPath,
2020
Reference,
@@ -209,7 +209,7 @@ bar.invokeAction = async (requestId, argsStr) => {
209209
let action_return = application
210210
.action_udf(
211211
RequestId::new(),
212-
ComponentFunctionPath {
212+
CanonicalizedComponentFunctionPath {
213213
component: ComponentPath::test_user(),
214214
udf_path: "foo:bar".parse()?,
215215
},

crates/application/src/tests/cron_jobs.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use std::{
66

77
use common::{
88
components::{
9-
ComponentFunctionPath,
9+
CanonicalizedComponentFunctionPath,
1010
ComponentId,
1111
ComponentPath,
1212
},
@@ -72,12 +72,12 @@ async fn create_cron_job(
7272
"key".to_string(),
7373
serde_json::Value::String("value".to_string()),
7474
);
75-
let path = ComponentFunctionPath {
75+
let path = CanonicalizedComponentFunctionPath {
7676
component: ComponentPath::test_user(),
7777
udf_path: "basic:insertObject".parse()?,
7878
};
7979
let cron_spec = CronSpec {
80-
udf_path: path.udf_path.clone().canonicalize(),
80+
udf_path: path.udf_path.clone(),
8181
udf_args: parse_udf_args(&path, vec![JsonValue::Object(map)])?,
8282
cron_schedule: CronSchedule::Interval { seconds: 60 },
8383
};

crates/application/src/tests/mutation.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use anyhow::Context;
22
use common::{
33
components::{
4-
ComponentFunctionPath,
4+
CanonicalizedComponentFunctionPath,
55
ComponentPath,
66
},
77
knobs::UDF_EXECUTOR_OCC_MAX_RETRIES,
@@ -33,7 +33,7 @@ async fn insert_object(
3333
let result = application
3434
.mutation_udf(
3535
RequestId::new(),
36-
ComponentFunctionPath {
36+
CanonicalizedComponentFunctionPath {
3737
component: ComponentPath::test_user(),
3838
udf_path: "basic:insertObject".parse()?,
3939
},
@@ -57,7 +57,7 @@ async fn insert_and_count(
5757
let result = application
5858
.mutation_udf(
5959
RequestId::new(),
60-
ComponentFunctionPath {
60+
CanonicalizedComponentFunctionPath {
6161
component: ComponentPath::test_user(),
6262
udf_path: "basic:insertAndCount".parse()?,
6363
},

crates/application/src/tests/returns_validation.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use common::{
22
components::{
3-
ComponentFunctionPath,
3+
CanonicalizedComponentFunctionPath,
44
ComponentPath,
55
},
66
pause::PauseClient,
@@ -33,7 +33,7 @@ async fn run_zero_arg_mutation(
3333
application
3434
.mutation_udf(
3535
RequestId::new(),
36-
ComponentFunctionPath {
36+
CanonicalizedComponentFunctionPath {
3737
component: ComponentPath::test_user(),
3838
udf_path: name.parse()?,
3939
},
@@ -54,7 +54,7 @@ async fn run_zero_arg_query(
5454
application
5555
.read_only_udf(
5656
RequestId::new(),
57-
ComponentFunctionPath {
57+
CanonicalizedComponentFunctionPath {
5858
component: ComponentPath::test_user(),
5959
udf_path: name.parse()?,
6060
},
@@ -73,7 +73,7 @@ async fn run_zero_arg_action(
7373
application
7474
.action_udf(
7575
RequestId::new(),
76-
ComponentFunctionPath {
76+
CanonicalizedComponentFunctionPath {
7777
component: ComponentPath::test_user(),
7878
udf_path: name.parse()?,
7979
},

0 commit comments

Comments
 (0)