Skip to content

Commit c30df6c

Browse files
authored
feat: sub extension can set 'platforms' now (#847)
Before this commit, sub extensions were not allowed to set their "platforms" field, this restriction is lifted in this commit. By allowing this, a group extension can have sub extensions for different platforms, here is an example (irrelavent fields are omitted for the sake of simplicity): ```json { "name": "Suspend my machine", "type": "Group", "platforms": ["macos", "windows"], "commands": [ { "name": "Suspend macOS": "platforms": ["macos"], "action": {...} }, { "name": "Suspend Windows": "platforms": ["windows"], "action": {...} } ] } ``` While loading or installing extensions, incompatible sub extensions will be filtered out by Coco, e.g., you won't see that "Suspend Windows" command if you are on macOS. An extra check is added in this commit to ensure a sub extensions won't support the platforms that are incompatible with its main extension. Even though main extensions and sub extensions can both have "platforms" specified, the semantics of this field, when not set, differs between them. For main extensions, it means this extension is compatible with all the platforms supported by Coco (null == all). For sub extensions, having it not set implicitly says that this field has the same value as the main extension's "platforms" field. The primary reason behind this design is that if we choose the semantics used by the main extension, treating null as all, all the extensions we currently have will become invalid, because they are all macOS-only, the main extensions's "platforms" field is "macos" and sub extensions' "platforms" is not set (null), they will be equivalent to: ```json { "name": "this is macOS-only", "type": "Group", "platforms": ["macos"], "commands": [ { "name": "How the fxxk can this support all the platforms!" "platforms": ["macos", "windows", "linux"], "type": "Command", "action": {...} } ] } ``` This hits exactly the check we mentioned earlier and will be rejected by Coco. If we have users installed them, the installed extensions will be treated invalid and rejected by future Coco release, boom, we break backward compatibility. Also, the current design actually makes sense. Nobody wants to repeatedly tell Coco that all the sub extensions support macOS if this can be said only once: ```json { "name": "this is macOS-only", "platforms": ["macos"], "commands": [ { "name": "This supports macOS" "platforms": ["macos"], }, { "name": "This supports macOS too" "platforms": ["macos"], }, { "name": "Guess what! this also supports macOS" "platforms": ["macos"], }, { "name": "Come on dude, do I really to say this platform=macos so many times" "platforms": ["macos"], } ] } ```
1 parent b833769 commit c30df6c

File tree

9 files changed

+441
-41
lines changed

9 files changed

+441
-41
lines changed

docs/content.en/docs/release-notes/_index.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ Information about release notes of Coco App is provided here.
1616
- feat: enhance ui for skipped version #834
1717
- feat: support installing local extensions #749
1818
- feat: support sending files in chat messages #764
19+
- feat: sub extension can set 'platforms' now #847
1920

2021
### 🐛 Bug fix
2122

src-tauri/Cargo.lock

Lines changed: 22 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src-tauri/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ tokio-stream = { version = "0.1.17", features = ["io-util"] }
106106
cfg-if = "1.0.1"
107107
sysinfo = "0.35.2"
108108
indexmap = { version = "2.10.0", features = ["serde"] }
109+
strum = { version = "0.27.2", features = ["derive"] }
109110

110111
[target."cfg(target_os = \"macos\")".dependencies]
111112
tauri-nspanel = { git = "https://github.com/ahkohd/tauri-nspanel", branch = "v2" }

src-tauri/src/extension/mod.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ fn default_true() -> bool {
2525
true
2626
}
2727

28-
#[derive(Debug, Deserialize, Serialize, Clone)]
28+
#[derive(Debug, Deserialize, Serialize, Clone, PartialEq)]
2929
pub struct Extension {
3030
/// Extension ID.
3131
///
@@ -275,13 +275,13 @@ impl Extension {
275275
}
276276
}
277277

278-
#[derive(Debug, Deserialize, Serialize, Clone)]
278+
#[derive(Debug, Deserialize, Serialize, Clone, PartialEq)]
279279
pub(crate) struct CommandAction {
280280
pub(crate) exec: String,
281281
pub(crate) args: Option<Vec<String>>,
282282
}
283283

284-
#[derive(Debug, Deserialize, Serialize, Clone)]
284+
#[derive(Debug, Deserialize, Serialize, Clone, PartialEq)]
285285
pub struct Quicklink {
286286
// NOTE that `struct QuicklinkLink` (not `struct Quicklink`) has its own
287287
// derived `Deserialize/Serialize` impl, which deserializes/serializes
@@ -328,7 +328,7 @@ pub(crate) fn quicklink_link_arguments(
328328
}
329329

330330
/// A quicklink consists of a sequence of components.
331-
#[derive(Debug, Clone, Serialize, Deserialize)]
331+
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
332332
pub(crate) struct QuicklinkLink {
333333
components: Vec<QuicklinkLinkComponent>,
334334
}
@@ -440,7 +440,7 @@ where
440440
/// The above link can be split into the following components:
441441
///
442442
/// [StaticStr("https://www.google.com/search?q="), DynamicPlaceholder { argument_name: "query", default: None }]
443-
#[derive(Debug, Clone, Serialize, Deserialize)]
443+
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
444444
pub(crate) enum QuicklinkLinkComponent {
445445
StaticStr(String),
446446
/// For the valid formats of dynamic placeholder, see the doc comments of `fn parse_dynamic_placeholder()`

src-tauri/src/extension/third_party/check.rs

Lines changed: 181 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,26 @@
1414
1515
use crate::extension::Extension;
1616
use crate::extension::ExtensionType;
17+
use crate::util::platform::Platform;
1718
use std::collections::HashSet;
1819

1920
pub(crate) fn general_check(extension: &Extension) -> Result<(), String> {
2021
// Check main extension
2122
check_main_extension_only(extension)?;
2223
check_main_extension_or_sub_extension(extension, &format!("extension [{}]", extension.id))?;
2324

25+
// `None` if `extension` is compatible with all the platforms. Otherwise `Some(limited_platforms)`
26+
let limited_supported_platforms = match extension.platforms.as_ref() {
27+
Some(platforms) => {
28+
if platforms.len() == Platform::num_of_supported_platforms() {
29+
None
30+
} else {
31+
Some(platforms)
32+
}
33+
}
34+
None => None,
35+
};
36+
2437
// Check sub extensions
2538
let commands = match extension.commands {
2639
Some(ref v) => v.as_slice(),
@@ -38,7 +51,7 @@ pub(crate) fn general_check(extension: &Extension) -> Result<(), String> {
3851
let mut sub_extension_ids = HashSet::new();
3952

4053
for sub_extension in sub_extensions.iter() {
41-
check_sub_extension_only(&extension.id, sub_extension)?;
54+
check_sub_extension_only(&extension.id, sub_extension, limited_supported_platforms)?;
4255
check_main_extension_or_sub_extension(
4356
extension,
4457
&format!("sub-extension [{}-{}]", extension.id, sub_extension.id),
@@ -94,7 +107,11 @@ fn check_main_extension_only(extension: &Extension) -> Result<(), String> {
94107
Ok(())
95108
}
96109

97-
fn check_sub_extension_only(extension_id: &str, sub_extension: &Extension) -> Result<(), String> {
110+
fn check_sub_extension_only(
111+
extension_id: &str,
112+
sub_extension: &Extension,
113+
limited_platforms: Option<&HashSet<Platform>>,
114+
) -> Result<(), String> {
98115
if sub_extension.r#type == ExtensionType::Group
99116
|| sub_extension.r#type == ExtensionType::Extension
100117
{
@@ -104,13 +121,6 @@ fn check_sub_extension_only(extension_id: &str, sub_extension: &Extension) -> Re
104121
));
105122
}
106123

107-
if sub_extension.platforms.is_some() {
108-
return Err(format!(
109-
"invalid sub-extension [{}-{}]: field [platforms] should not be set in sub-extensions",
110-
extension_id, sub_extension.id
111-
));
112-
}
113-
114124
if sub_extension.commands.is_some()
115125
|| sub_extension.scripts.is_some()
116126
|| sub_extension.quicklinks.is_some()
@@ -128,6 +138,29 @@ fn check_sub_extension_only(extension_id: &str, sub_extension: &Extension) -> Re
128138
));
129139
}
130140

141+
if let Some(platforms_supported_by_main_extension) = limited_platforms {
142+
match sub_extension.platforms {
143+
Some(ref platforms_supported_by_sub_extension) => {
144+
let diff = platforms_supported_by_sub_extension
145+
.difference(&platforms_supported_by_main_extension)
146+
.into_iter()
147+
.map(|p| p.to_string())
148+
.collect::<Vec<String>>();
149+
150+
if !diff.is_empty() {
151+
return Err(format!(
152+
"invalid sub-extension [{}-{}]: it supports platforms {:?} that are not supported by the main extension",
153+
extension_id, sub_extension.id, diff
154+
));
155+
}
156+
}
157+
None => {
158+
// if `sub_extension.platform` is None, it means it has the same value
159+
// as main extension's `platforms` field, so we don't need to check it.
160+
}
161+
}
162+
}
163+
131164
Ok(())
132165
}
133166

@@ -172,8 +205,6 @@ fn check_main_extension_or_sub_extension(
172205
mod tests {
173206
use super::*;
174207
use crate::extension::{CommandAction, Quicklink, QuicklinkLink, QuicklinkLinkComponent};
175-
use crate::util::platform::Platform;
176-
use std::collections::HashSet;
177208

178209
/// Helper function to create a basic valid extension
179210
fn create_basic_extension(id: &str, extension_type: ExtensionType) -> Extension {
@@ -368,27 +399,6 @@ mod tests {
368399
);
369400
}
370401

371-
#[test]
372-
fn test_sub_extension_cannot_have_platforms() {
373-
let mut extension = create_basic_extension("test-group", ExtensionType::Group);
374-
let mut sub_cmd = create_basic_extension("sub-cmd", ExtensionType::Command);
375-
sub_cmd.action = Some(create_command_action());
376-
377-
let mut platforms = HashSet::new();
378-
platforms.insert(Platform::Macos);
379-
sub_cmd.platforms = Some(platforms);
380-
381-
extension.commands = Some(vec![sub_cmd]);
382-
383-
let result = general_check(&extension);
384-
assert!(result.is_err());
385-
assert!(
386-
result
387-
.unwrap_err()
388-
.contains("field [platforms] should not be set in sub-extensions")
389-
);
390-
}
391-
392402
#[test]
393403
fn test_sub_extension_cannot_have_developer() {
394404
let mut extension = create_basic_extension("test-group", ExtensionType::Group);
@@ -542,4 +552,143 @@ mod tests {
542552

543553
assert!(general_check(&extension).is_ok());
544554
}
555+
556+
/*
557+
* Tests for check that sub extension cannot support extensions that are not
558+
* supported by the main extension
559+
*
560+
* Start here
561+
*/
562+
#[test]
563+
fn test_platform_validation_both_none() {
564+
// Case 1: main extension's platforms = None, sub extension's platforms = None
565+
// Should return Ok(())
566+
let mut main_extension = create_basic_extension("main-ext", ExtensionType::Group);
567+
main_extension.platforms = None;
568+
569+
let mut sub_cmd = create_basic_extension("sub-cmd", ExtensionType::Command);
570+
sub_cmd.action = Some(create_command_action());
571+
sub_cmd.platforms = None;
572+
573+
main_extension.commands = Some(vec![sub_cmd]);
574+
575+
let result = general_check(&main_extension);
576+
assert!(result.is_ok());
577+
}
578+
579+
#[test]
580+
fn test_platform_validation_main_all_sub_none() {
581+
// Case 2: main extension's platforms = Some(all platforms), sub extension's platforms = None
582+
// Should return Ok(())
583+
let mut main_extension = create_basic_extension("main-ext", ExtensionType::Group);
584+
main_extension.platforms = Some(Platform::all());
585+
586+
let mut sub_cmd = create_basic_extension("sub-cmd", ExtensionType::Command);
587+
sub_cmd.action = Some(create_command_action());
588+
sub_cmd.platforms = None;
589+
590+
main_extension.commands = Some(vec![sub_cmd]);
591+
592+
let result = general_check(&main_extension);
593+
assert!(result.is_ok());
594+
}
595+
596+
#[test]
597+
fn test_platform_validation_main_none_sub_some() {
598+
// Case 3: main extension's platforms = None, sub extension's platforms = Some([Platform::Macos])
599+
// Should return Ok(()) because None means supports all platforms
600+
let mut main_extension = create_basic_extension("main-ext", ExtensionType::Group);
601+
main_extension.platforms = None;
602+
603+
let mut sub_cmd = create_basic_extension("sub-cmd", ExtensionType::Command);
604+
sub_cmd.action = Some(create_command_action());
605+
sub_cmd.platforms = Some(HashSet::from([Platform::Macos]));
606+
607+
main_extension.commands = Some(vec![sub_cmd]);
608+
609+
let result = general_check(&main_extension);
610+
assert!(result.is_ok());
611+
}
612+
613+
#[test]
614+
fn test_platform_validation_main_all_sub_subset() {
615+
// Case 4: main extension's platforms = Some(all platforms), sub extension's platforms = Some([Platform::Macos])
616+
// Should return Ok(()) because sub extension supports a subset of main extension's platforms
617+
let mut main_extension = create_basic_extension("main-ext", ExtensionType::Group);
618+
main_extension.platforms = Some(Platform::all());
619+
620+
let mut sub_cmd = create_basic_extension("sub-cmd", ExtensionType::Command);
621+
sub_cmd.action = Some(create_command_action());
622+
sub_cmd.platforms = Some(HashSet::from([Platform::Macos]));
623+
624+
main_extension.commands = Some(vec![sub_cmd]);
625+
626+
let result = general_check(&main_extension);
627+
assert!(result.is_ok());
628+
}
629+
630+
#[test]
631+
fn test_platform_validation_main_limited_sub_unsupported() {
632+
// Case 5: main extension's platforms = Some([Platform::Macos]), sub extension's platforms = Some([Platform::Linux])
633+
// Should return Err because sub extension supports a platform not supported by main extension
634+
let mut main_extension = create_basic_extension("main-ext", ExtensionType::Group);
635+
main_extension.platforms = Some(HashSet::from([Platform::Macos]));
636+
637+
let mut sub_cmd = create_basic_extension("sub-cmd", ExtensionType::Command);
638+
sub_cmd.action = Some(create_command_action());
639+
sub_cmd.platforms = Some(HashSet::from([Platform::Linux]));
640+
641+
main_extension.commands = Some(vec![sub_cmd]);
642+
643+
let result = general_check(&main_extension);
644+
assert!(result.is_err());
645+
let error_msg = result.unwrap_err();
646+
assert!(error_msg.contains("it supports platforms"));
647+
assert!(error_msg.contains("that are not supported by the main extension"));
648+
assert!(error_msg.contains("Linux")); // Should mention the unsupported platform
649+
}
650+
651+
#[test]
652+
fn test_platform_validation_main_partial_sub_unsupported() {
653+
// Case 6: main extension's platforms = Some([Platform::Macos, Platform::Windows]), sub extension's platforms = Some([Platform::Linux])
654+
// Should return Err because sub extension supports a platform not supported by main extension
655+
let mut main_extension = create_basic_extension("main-ext", ExtensionType::Group);
656+
main_extension.platforms = Some(HashSet::from([Platform::Macos, Platform::Windows]));
657+
658+
let mut sub_cmd = create_basic_extension("sub-cmd", ExtensionType::Command);
659+
sub_cmd.action = Some(create_command_action());
660+
sub_cmd.platforms = Some(HashSet::from([Platform::Linux]));
661+
662+
main_extension.commands = Some(vec![sub_cmd]);
663+
664+
let result = general_check(&main_extension);
665+
assert!(result.is_err());
666+
let error_msg = result.unwrap_err();
667+
assert!(error_msg.contains("it supports platforms"));
668+
assert!(error_msg.contains("that are not supported by the main extension"));
669+
assert!(error_msg.contains("Linux")); // Should mention the unsupported platform
670+
}
671+
672+
#[test]
673+
fn test_platform_validation_main_limited_sub_none() {
674+
// Case 7: main extension's platforms = Some([Platform::Macos]), sub extension's platforms = None
675+
// Should return Ok(()) because when sub extension's platforms is None, it inherits main extension's platforms
676+
let mut main_extension = create_basic_extension("main-ext", ExtensionType::Group);
677+
main_extension.platforms = Some(HashSet::from([Platform::Macos]));
678+
679+
let mut sub_cmd = create_basic_extension("sub-cmd", ExtensionType::Command);
680+
sub_cmd.action = Some(create_command_action());
681+
sub_cmd.platforms = None;
682+
683+
main_extension.commands = Some(vec![sub_cmd]);
684+
685+
let result = general_check(&main_extension);
686+
assert!(result.is_ok());
687+
}
688+
/*
689+
* Tests for check that sub extension cannot support extensions that are not
690+
* supported by the main extension
691+
*
692+
* End here
693+
*/
545694
}

0 commit comments

Comments
 (0)