Skip to content

Commit 426fa9a

Browse files
lannlann
committed
manifest: Generate nicer IDs in normalization
Signed-off-by: Lann Martin <[email protected]>
1 parent 47fbba0 commit 426fa9a

File tree

6 files changed

+178
-33
lines changed

6 files changed

+178
-33
lines changed

‎crates/loader/src/local.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,7 @@ impl LocalLoader {
6060

6161
// Load the given manifest into a LockedApp, ready for execution.
6262
async fn load_manifest(&self, mut manifest: AppManifest) -> Result<LockedApp> {
63-
spin_manifest::normalize::normalize_inline_components(&mut manifest);
64-
spin_manifest::normalize::normalize_trigger_ids(&mut manifest.triggers);
63+
spin_manifest::normalize::normalize_manifest(&mut manifest);
6564

6665
let AppManifest {
6766
spin_manifest_version: _,

‎crates/manifest/src/normalize.rs‎

Lines changed: 57 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,35 +2,59 @@
22
33
use std::collections::HashSet;
44

5-
use crate::schema::v2::{self, AppManifest, ComponentSpec, KebabId};
5+
use crate::schema::v2::{AppManifest, ComponentSpec, KebabId};
66

7-
/// Extracts each `ComponentSpec::Inline` into `AppManifest::component`s,
8-
/// replacing it with a `ComponentSpec::Reference`.
9-
pub fn normalize_inline_components(manifest: &mut AppManifest) {
7+
/// Normalizes some optional [`AppManifest`] features into a canonical form:
8+
/// - Inline components in trigger configs are moved into top-level
9+
/// components and replaced with a reference.
10+
/// - Any triggers without an ID are assigned a generated ID.
11+
pub fn normalize_manifest(manifest: &mut AppManifest) {
12+
normalize_trigger_ids(manifest);
13+
normalize_inline_components(manifest);
14+
}
15+
16+
fn normalize_inline_components(manifest: &mut AppManifest) {
17+
// Normalize inline components
1018
let components = &mut manifest.components;
11-
for trigger in &mut manifest.triggers.values_mut().flatten() {
19+
20+
for trigger in manifest.triggers.values_mut().flatten() {
1221
let trigger_id = &trigger.id;
13-
let component_specs = trigger.component.iter_mut().chain(
14-
trigger
15-
.components
16-
.values_mut()
17-
.flat_map(|specs| specs.0.iter_mut()),
18-
);
22+
23+
let component_specs = trigger
24+
.component
25+
.iter_mut()
26+
.chain(
27+
trigger
28+
.components
29+
.values_mut()
30+
.flat_map(|specs| specs.0.iter_mut()),
31+
)
32+
.collect::<Vec<_>>();
33+
let multiple_components = component_specs.len() > 1;
1934

2035
let mut counter = 1;
2136
for spec in component_specs {
2237
if !matches!(spec, ComponentSpec::Inline(_)) {
2338
continue;
2439
};
2540

26-
// Generate an unused component ID
27-
let inline_id = loop {
28-
let id =
29-
KebabId::try_from(format!("{trigger_id}-inline-component{counter}")).unwrap();
30-
if !components.contains_key(&id) {
31-
break id;
41+
let inline_id = {
42+
// Try a "natural" component ID...
43+
let mut id = KebabId::try_from(format!("{trigger_id}-component"));
44+
// ...falling back to a counter-based component ID
45+
if multiple_components
46+
|| id.is_err()
47+
|| components.contains_key(id.as_ref().unwrap())
48+
{
49+
id = Ok(loop {
50+
let id = KebabId::try_from(format!("inline-component{counter}")).unwrap();
51+
if !components.contains_key(&id) {
52+
break id;
53+
}
54+
counter += 1;
55+
});
3256
}
33-
counter += 1;
57+
id.unwrap()
3458
};
3559

3660
// Replace the inline component with a reference...
@@ -44,23 +68,32 @@ pub fn normalize_inline_components(manifest: &mut AppManifest) {
4468
}
4569
}
4670

47-
/// Generates IDs for any [`Trigger`]s without one.
48-
pub fn normalize_trigger_ids(typed_triggers: &mut v2::Map<String, Vec<v2::Trigger>>) {
49-
let mut trigger_ids = typed_triggers
71+
fn normalize_trigger_ids(manifest: &mut AppManifest) {
72+
let mut trigger_ids = manifest
73+
.triggers
5074
.values()
5175
.flatten()
5276
.cloned()
5377
.map(|t| t.id)
5478
.collect::<HashSet<_>>();
55-
for (trigger_type, triggers) in typed_triggers {
79+
for (trigger_type, triggers) in &mut manifest.triggers {
5680
let mut counter = 1;
5781
for trigger in triggers {
5882
if !trigger.id.is_empty() {
5983
continue;
6084
}
61-
// Generate an unused trigger ID
85+
// Try to assign a "natural" ID to this trigger
86+
if let Some(ComponentSpec::Reference(component_id)) = &trigger.component {
87+
let candidate_id = format!("{component_id}-{trigger_type}-trigger");
88+
if !trigger_ids.contains(&candidate_id) {
89+
trigger.id = candidate_id.clone();
90+
trigger_ids.insert(candidate_id);
91+
continue;
92+
}
93+
}
94+
// Fall back to assigning a counter-based trigger ID
6295
trigger.id = loop {
63-
let id = format!("{trigger_type}-{counter}");
96+
let id = format!("{trigger_type}-trigger{counter}");
6497
if !trigger_ids.contains(&id) {
6598
trigger_ids.insert(id.clone());
6699
break id;

‎crates/manifest/tests/ui.rs‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
use std::path::Path;
22

3-
use ui_testing::{Failed, Normalizer, UiTestsRunner};
3+
use spin_manifest::normalize::normalize_manifest;
4+
use ui_testing::{Failed, UiTestsRunner};
45

56
fn main() -> anyhow::Result<()> {
67
let mut runner = UiTestsRunner::default();
78
for entry in glob::glob("tests/ui/**/*.toml")? {
89
let path = entry?.canonicalize()?;
910
let test_name = format!("ui::{}", path.file_stem().unwrap().to_string_lossy());
1011
let snapshot_path = path.with_extension("json");
11-
runner.add_test(test_name, snapshot_path, move |n| run_test(&path, n));
12+
runner.add_test(test_name, snapshot_path, move |_| run_test(&path));
1213
}
1314
for entry in glob::glob("tests/ui/v1/*.toml")? {
1415
let path = entry?.canonicalize()?;
@@ -17,24 +18,33 @@ fn main() -> anyhow::Result<()> {
1718
path.file_stem().unwrap().to_string_lossy()
1819
);
1920
let snapshot_path = path.with_extension("toml.v2");
20-
runner.add_test(test_name, snapshot_path, move |n| {
21-
run_v1_to_v2_test(&path, n)
22-
});
21+
runner.add_test(test_name, snapshot_path, move |_| run_v1_to_v2_test(&path));
2322
}
23+
runner.add_test(
24+
"ui::normalization".into(),
25+
"tests/ui/normalization.toml.norm",
26+
|_| run_normalization_test("tests/ui/normalization.toml"),
27+
);
2428

2529
runner.run_tests()
2630
}
2731

28-
fn run_test(input: &Path, _normalizer: &mut Normalizer) -> Result<String, Failed> {
32+
fn run_test(input: &Path) -> Result<String, Failed> {
2933
let manifest = spin_manifest::manifest_from_file(input)?;
3034
Ok(serde_json::to_string_pretty(&manifest).expect("serialization should work"))
3135
}
3236

33-
fn run_v1_to_v2_test(input: &Path, _normalizer: &mut Normalizer) -> Result<String, Failed> {
37+
fn run_v1_to_v2_test(input: &Path) -> Result<String, Failed> {
3438
let manifest_str =
3539
std::fs::read_to_string(input).unwrap_or_else(|err| panic!("reading {input:?}: {err:?}"));
3640
let v1_manifest = toml::from_str(&manifest_str)
3741
.unwrap_or_else(|err| panic!("parsing v1 manifest {input:?}: {err:?}"));
3842
let v2_manifest = spin_manifest::compat::v1_to_v2_app(v1_manifest)?;
3943
Ok(toml::to_string(&v2_manifest).expect("serialization should work"))
4044
}
45+
46+
fn run_normalization_test(input: impl AsRef<Path>) -> Result<String, Failed> {
47+
let mut manifest = spin_manifest::manifest_from_file(input)?;
48+
normalize_manifest(&mut manifest);
49+
Ok(toml::to_string(&manifest).expect("serialization should work"))
50+
}

‎crates/manifest/tests/ui/normalization.json‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
{
2+
"spin_manifest_version": 2,
3+
"application": {
4+
"name": "minimal-v2"
5+
},
6+
"trigger": {
7+
"http": [
8+
{
9+
"component": "hello"
10+
},
11+
{
12+
"id": "admin-handler",
13+
"component": {
14+
"source": "admin.wasm"
15+
}
16+
},
17+
{
18+
"component": {
19+
"source": "other.wasm"
20+
}
21+
}
22+
],
23+
"example": [
24+
{
25+
"components": {
26+
"hello": "hello",
27+
"inline1": {
28+
"source": "inline1.wasm"
29+
},
30+
"inline2": {
31+
"source": "inline2.wasm"
32+
}
33+
}
34+
}
35+
]
36+
},
37+
"component": {
38+
"hello": {
39+
"source": "hello.wasm"
40+
}
41+
}
42+
}

‎crates/manifest/tests/ui/normalization.toml‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
spin_manifest_version = 2
2+
3+
[application]
4+
name = "minimal-v2"
5+
6+
[[trigger.http]]
7+
component = "hello"
8+
9+
[[trigger.http]]
10+
id = "admin-handler"
11+
component = { source = "admin.wasm" }
12+
13+
[[trigger.http]]
14+
component = { source = "other.wasm" }
15+
16+
[[trigger.example]]
17+
components.hello = "hello"
18+
components.inline1 = { source = "inline1.wasm" }
19+
components.inline2 = { source = "inline2.wasm" }
20+
21+
[component.hello]
22+
source = "hello.wasm"

‎crates/manifest/tests/ui/normalization.toml.norm‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
spin_manifest_version = 2
2+
3+
[application]
4+
name = "minimal-v2"
5+
6+
[[trigger.http]]
7+
id = "hello-http-trigger"
8+
component = "hello"
9+
10+
[[trigger.http]]
11+
id = "admin-handler"
12+
component = "admin-handler-component"
13+
14+
[[trigger.http]]
15+
id = "http-trigger1"
16+
component = "http-trigger1-component"
17+
18+
[[trigger.example]]
19+
id = "example-trigger1"
20+
21+
[trigger.example.components]
22+
hello = "hello"
23+
inline1 = "inline-component1"
24+
inline2 = "inline-component2"
25+
26+
[component.hello]
27+
source = "hello.wasm"
28+
29+
[component.admin-handler-component]
30+
source = "admin.wasm"
31+
32+
[component.http-trigger1-component]
33+
source = "other.wasm"
34+
35+
[component.inline-component1]
36+
source = "inline1.wasm"
37+
38+
[component.inline-component2]
39+
source = "inline2.wasm"

0 commit comments

Comments
 (0)