Merge pull request #4383 from fufesou/feat/plugin_framework_uninstall

plugin_framework, uninstall

flutter/lib/plugin/manager.dart

@@ -158,19 +158,6 @@ class PluginInfo with ChangeNotifier {
   bool get installed => installedVersion.isNotEmpty;
   bool get needUpdate => installed && installedVersion != meta.version;
 
-  void update(PluginInfo plugin) {
-    assert(plugin.meta.id == meta.id, 'Plugin id not match');
-    if (plugin.meta.id != meta.id) {
-      // log error
-      return;
-    }
-    sourceInfo = plugin.sourceInfo;
-    meta = plugin.meta;
-    installedVersion = plugin.installedVersion;
-    invalidReason = plugin.invalidReason;
-    notifyListeners();
-  }
-
   void setInstall(String msg) {
     if (msg == "finished") {
       msg = '';
@@ -213,8 +200,6 @@ class PluginManager with ChangeNotifier {
   void handleEvent(Map<String, dynamic> evt) {
     if (evt['plugin_list'] != null) {
       _handlePluginList(evt['plugin_list']);
-    } else if (evt['plugin_update'] != null) {
-      _handlePluginUpdate(evt['plugin_update']);
     } else if (evt['plugin_install'] != null && evt['id'] != null) {
       _handlePluginInstall(evt['id'], evt['plugin_install']);
     } else if (evt['plugin_uninstall'] != null && evt['id'] != null) {
@@ -236,21 +221,6 @@ class PluginManager with ChangeNotifier {
     });
   }
 
-  void _handlePluginUpdate(Map<String, dynamic> evt) {
-    final plugin = _getPluginFromEvent(evt);
-    if (plugin == null) {
-      return;
-    }
-    for (var i = 0; i < _plugins.length; i++) {
-      if (_plugins[i].meta.id == plugin.meta.id) {
-        _plugins[i].update(plugin);
-        _sortPlugins();
-        notifyListeners();
-        return;
-      }
-    }
-  }
-
   void _handlePluginList(String pluginList) {
     _plugins.clear();
     try {
@@ -269,6 +239,7 @@ class PluginManager with ChangeNotifier {
   }
 
   void _handlePluginInstall(String id, String msg) {
+    debugPrint('Plugin \'$id\' install msg $msg');
     for (var i = 0; i < _plugins.length; i++) {
       if (_plugins[i].meta.id == id) {
         _plugins[i].setInstall(msg);
@@ -280,6 +251,7 @@ class PluginManager with ChangeNotifier {
   }
 
   void _handlePluginUninstall(String id, String msg) {
+    debugPrint('Plugin \'$id\' uninstall msg $msg');
     for (var i = 0; i < _plugins.length; i++) {
       if (_plugins[i].meta.id == id) {
         _plugins[i].setUninstall(msg);

src/core_main.rs

@@ -1,10 +1,10 @@
 #[cfg(not(debug_assertions))]
 #[cfg(not(any(target_os = "android", target_os = "ios")))]
 use crate::platform::breakdown_callback;
-use hbb_common::log;
 #[cfg(not(debug_assertions))]
 #[cfg(not(any(target_os = "android", target_os = "ios")))]
 use hbb_common::platform::register_breakdown_handler;
+use hbb_common::{allow_err, log};
 
 /// shared by flutter and sciter main function
 ///
@@ -106,15 +106,7 @@ pub fn core_main() -> Option<Vec<String>> {
     }
     #[cfg(all(feature = "flutter", feature = "plugin_framework"))]
     #[cfg(not(any(target_os = "android", target_os = "ios")))]
-    if args.is_empty() || "--server" == (&args[0] as &str) {
+    init_plugins(&args);
-        #[cfg(debug_assertions)]
-        let load_plugins = true;
-        #[cfg(not(debug_assertions))]
-        let load_plugins = crate::platform::is_installed();
-        if load_plugins {
-            crate::plugin::init();
-        }
-    }
     if args.is_empty() {
         std::thread::spawn(move || crate::start_server(false));
     } else {
@@ -244,10 +236,17 @@ pub fn core_main() -> Option<Vec<String>> {
             #[cfg(all(feature = "flutter", feature = "plugin_framework"))]
             #[cfg(not(any(target_os = "android", target_os = "ios")))]
             if args[0] == "--plugin-install" {
-                if args.len() == 3 {
+                if args.len() == 2 {
+                    crate::plugin::change_uninstall_plugin(&args[1], false);
+                } else if args.len() == 3 {
                     crate::plugin::install_plugin_with_url(&args[1], &args[2]);
                 }
                 return None;
+            } else if args[0] == "--plugin-uninstall" {
+                if args.len() == 2 {
+                    crate::plugin::change_uninstall_plugin(&args[1], true);
+                }
+                return None;
             }
         }
     }
@@ -258,6 +257,23 @@ pub fn core_main() -> Option<Vec<String>> {
     return Some(args);
 }
 
+#[inline]
+#[cfg(all(feature = "flutter", feature = "plugin_framework"))]
+#[cfg(not(any(target_os = "android", target_os = "ios")))]
+fn init_plugins(args: &Vec<String>) {
+    if args.is_empty() || "--server" == (&args[0] as &str) {
+        #[cfg(debug_assertions)]
+        let load_plugins = true;
+        #[cfg(not(debug_assertions))]
+        let load_plugins = crate::platform::is_installed();
+        if load_plugins {
+            crate::plugin::init();
+        }
+    } else if "--service" == (&args[0] as &str) {
+        allow_err!(crate::plugin::remove_uninstalled());
+    }
+}
+
 fn import_config(path: &str) {
     use hbb_common::{config::*, get_exe_time, get_modified_time};
     let path2 = path.replace(".toml", "2.toml");

src/plugin/config.rs

@@ -217,7 +217,6 @@ impl PeerConfig {
 #[derive(Debug, Serialize, Deserialize)]
 pub struct PluginStatus {
     pub enabled: bool,
-    pub uninstalled: bool,
 }
 
 const MANAGER_VERSION: &str = "0.1.0";
@@ -268,9 +267,16 @@ impl ManagerConfig {
     #[inline]
     pub fn get_plugin_option(id: &str, key: &str) -> Option<String> {
         let lock = CONFIG_MANAGER.lock().unwrap();
-        let status = lock.plugins.get(id)?;
         match key {
-            "enabled" => Some(status.enabled.to_string()),
+            "enabled" => {
+                let enabled = lock
+                    .plugins
+                    .get(id)
+                    .map(|status| status.enabled.to_owned())
+                    .unwrap_or(true.to_owned())
+                    .to_string();
+                Some(enabled)
+            }
             _ => None,
         }
     }
@@ -280,13 +286,7 @@ impl ManagerConfig {
         if let Some(status) = lock.plugins.get_mut(id) {
             status.enabled = enabled;
         } else {
-            lock.plugins.insert(
+            lock.plugins.insert(id.to_owned(), PluginStatus { enabled });
-                id.to_owned(),
-                PluginStatus {
-                    enabled,
-                    uninstalled: false,
-                },
-            );
         }
         hbb_common::config::store_path(Self::path(), &*lock)
     }
@@ -309,13 +309,8 @@ impl ManagerConfig {
     #[inline]
     pub fn add_plugin(id: &str) -> ResultType<()> {
         let mut lock = CONFIG_MANAGER.lock().unwrap();
-        lock.plugins.insert(
+        lock.plugins
-            id.to_owned(),
+            .insert(id.to_owned(), PluginStatus { enabled: true });
-            PluginStatus {
-                enabled: true,
-                uninstalled: false,
-            },
-        );
         hbb_common::config::store_path(Self::path(), &*lock)
     }
 
@@ -325,35 +320,6 @@ impl ManagerConfig {
         lock.plugins.remove(id);
         hbb_common::config::store_path(Self::path(), &*lock)
     }
-
-    #[inline]
-    pub fn is_uninstalled(id: &str) -> bool {
-        CONFIG_MANAGER
-            .lock()
-            .unwrap()
-            .plugins
-            .get(id)
-            .map(|p| p.uninstalled)
-            .unwrap_or(false)
-    }
-
-    #[inline]
-    pub fn set_uninstall(id: &str, uninstall: bool) -> ResultType<()> {
-        let mut lock = CONFIG_MANAGER.lock().unwrap();
-        if let Some(status) = lock.plugins.get_mut(id) {
-            status.uninstalled = uninstall;
-        } else {
-            lock.plugins.insert(
-                id.to_owned(),
-                PluginStatus {
-                    enabled: true,
-                    uninstalled: uninstall,
-                },
-            );
-        }
-        hbb_common::config::store_path(Self::path(), &*lock)?;
-        Ok(())
-    }
 }
 
 pub(super) extern "C" fn cb_get_local_peer_id() -> *const c_char {

src/plugin/desc.rs

@@ -4,7 +4,7 @@ use serde_json;
 use std::collections::HashMap;
 use std::ffi::{c_char, CStr};
 
-#[derive(Debug, Serialize, Deserialize)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct UiButton {
     key: String,
     text: String,
@@ -13,7 +13,7 @@ pub struct UiButton {
     action: String, // The action to be triggered when the button is clicked.
 }
 
-#[derive(Debug, Serialize, Deserialize)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct UiCheckbox {
     key: String,
     text: String,
@@ -21,14 +21,14 @@ pub struct UiCheckbox {
     action: String, // The action to be triggered when the checkbox is checked or unchecked.
 }
 
-#[derive(Debug, Serialize, Deserialize)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
 #[serde(tag = "t", content = "c")]
 pub enum UiType {
     Button(UiButton),
     Checkbox(UiCheckbox),
 }
 
-#[derive(Debug, Serialize, Deserialize)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct Location {
     pub ui: HashMap<String, Vec<UiType>>,
 }
@@ -67,7 +67,7 @@ pub struct Meta {
     pub publish_info: PublishInfo,
 }
 
-#[derive(Debug, Serialize, Deserialize)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct Desc {
     meta: Meta,
     need_reboot: bool,

src/plugin/ipc.rs

@@ -217,7 +217,6 @@ pub async fn handle_plugin(plugin: Plugin, stream: &mut Connection) {
             }
         },
         Plugin::Load(id) => {
-            allow_err!(super::config::ManagerConfig::set_uninstall(&id, false));
             allow_err!(super::load_plugin(&id));
         }
         Plugin::Reload(id) => {

src/plugin/manager.rs

@@ -7,13 +7,13 @@ use hbb_common::{allow_err, bail, log, tokio, toml};
 use serde_derive::{Deserialize, Serialize};
 use serde_json;
 use std::{
-    collections::HashMap,
+    collections::{HashMap, HashSet},
-    fs,
+    fs::{read_to_string, remove_dir_all, OpenOptions},
+    io::Write,
     sync::{Arc, Mutex},
 };
 
 const MSG_TO_UI_PLUGIN_MANAGER_LIST: &str = "plugin_list";
-const MSG_TO_UI_PLUGIN_MANAGER_UPDATE: &str = "plugin_update";
 const MSG_TO_UI_PLUGIN_MANAGER_INSTALL: &str = "plugin_install";
 const MSG_TO_UI_PLUGIN_MANAGER_UNINSTALL: &str = "plugin_uninstall";
 
@@ -186,6 +186,18 @@ fn elevate_install(
     crate::platform::elevate(args)
 }
 
+#[inline]
+#[cfg(target_os = "windows")]
+fn elevate_uninstall(plugin_id: &str) -> ResultType<bool> {
+    crate::platform::elevate(&format!("--plugin-uninstall {}", plugin_id))
+}
+
+#[inline]
+#[cfg(any(target_os = "linux", target_os = "macos"))]
+fn elevate_install(plugin_id: &str) -> ResultType<bool> {
+    crate::platform::elevate(vec!["--plugin-uninstall", plugin_id])
+}
+
 pub fn install_plugin(id: &str) -> ResultType<()> {
     match PLUGIN_INFO.lock().unwrap().get(id) {
         Some(plugin) => {
@@ -218,7 +230,7 @@ pub fn install_plugin(id: &str) -> ResultType<()> {
     }
 }
 
-fn get_uninstalled_plugins() -> ResultType<Vec<String>> {
+fn get_uninstalled_plugins(uninstalled_plugin_set: &HashSet<String>) -> ResultType<Vec<String>> {
     let plugins_dir = super::get_plugins_dir()?;
     let mut plugins = Vec::new();
     if plugins_dir.exists() {
@@ -228,7 +240,7 @@ fn get_uninstalled_plugins() -> ResultType<Vec<String>> {
                     let plugin_dir = entry.path();
                     if plugin_dir.is_dir() {
                         if let Some(id) = plugin_dir.file_name().and_then(|n| n.to_str()) {
-                            if super::config::ManagerConfig::is_uninstalled(id) {
+                            if uninstalled_plugin_set.contains(id) {
                                 plugins.push(id.to_string());
                             }
                         }
@@ -243,19 +255,24 @@ fn get_uninstalled_plugins() -> ResultType<Vec<String>> {
     Ok(plugins)
 }
 
-pub(super) fn remove_plugins() -> ResultType<()> {
+pub fn remove_uninstalled() -> ResultType<()> {
-    for id in get_uninstalled_plugins()?.iter() {
+    let mut uninstalled_plugin_set = get_uninstall_id_set()?;
+    for id in get_uninstalled_plugins(&uninstalled_plugin_set)?.iter() {
         super::config::remove(id as _);
         if let Ok(dir) = super::get_plugin_dir(id as _) {
-            allow_err!(fs::remove_dir_all(dir));
+            allow_err!(remove_dir_all(dir.clone()));
+            if !dir.exists() {
+                uninstalled_plugin_set.remove(id);
             }
         }
+    }
+    allow_err!(update_uninstall_id_set(uninstalled_plugin_set));
     Ok(())
 }
 
 pub fn uninstall_plugin(id: &str, called_by_ui: bool) {
     if called_by_ui {
-        match crate::platform::check_super_user_permission() {
+        match elevate_uninstall(id) {
             Ok(true) => {
                 if let Err(e) = super::ipc::uninstall_plugin(id) {
                     log::error!("Failed to uninstall plugin '{}': {}", id, e);
@@ -284,8 +301,6 @@ pub fn uninstall_plugin(id: &str, called_by_ui: bool) {
 
     if is_server() {
         super::plugins::unload_plugin(&id);
-        // allow_err is Ok here.
-        allow_err!(super::config::ManagerConfig::set_uninstall(&id, true));
     }
 }
 
@@ -379,6 +394,28 @@ pub async fn start_ipc() {
     }
 }
 
+pub(super) fn get_uninstall_id_set() -> ResultType<HashSet<String>> {
+    let uninstall_file_path = super::get_uninstall_file_path()?;
+    if !uninstall_file_path.exists() {
+        std::fs::create_dir_all(&super::get_plugins_dir()?)?;
+        return Ok(HashSet::new());
+    }
+    let s = read_to_string(uninstall_file_path)?;
+    Ok(serde_json::from_str::<HashSet<String>>(&s)?)
+}
+
+fn update_uninstall_id_set(set: HashSet<String>) -> ResultType<()> {
+    let content = serde_json::to_string(&set)?;
+    let file = OpenOptions::new()
+        .write(true)
+        .truncate(true)
+        .create(true)
+        .open(super::get_uninstall_file_path()?)?;
+    let mut writer = std::io::BufWriter::new(file);
+    writer.write_all(content.as_bytes())?;
+    Ok(())
+}
+
 // install process
 pub(super) mod install {
     use super::IPC_PLUGIN_POSTFIX;
@@ -463,7 +500,27 @@ pub(super) mod install {
         Ok(())
     }
 
+    pub fn change_uninstall_plugin(id: &str, add: bool) {
+        match super::get_uninstall_id_set() {
+            Ok(mut set) => {
+                if add {
+                    set.insert(id.to_string());
+                } else {
+                    set.remove(id);
+                }
+                if let Err(e) = super::update_uninstall_id_set(set) {
+                    log::error!("Failed to write uninstall list, {}", e);
+                }
+            }
+            Err(e) => log::error!(
+                "Failed to get plugins dir, unable to read uninstall list, {}",
+                e
+            ),
+        }
+    }
+
     pub fn install_plugin_with_url(id: &str, url: &str) {
+        log::info!("Installing plugin '{}', url: {}", id, url);
         let plugin_dir = match super::super::get_plugin_dir(id) {
             Ok(d) => d,
             Err(e) => {
@@ -480,12 +537,14 @@ pub(super) mod install {
             }
         }
 
-        let filename = plugin_dir.join(format!("{}.zip", id));
+        let filename = match url.rsplit('/').next() {
-
+            Some(filename) => plugin_dir.join(filename),
-        // download
+            None => {
-        if !download_file(id, url, &filename) {
+                send_install_status(id, InstallStatus::FailedDownloading);
+                log::error!("Failed to download plugin file, invalid url: {}", url);
                 return;
             }
+        };
 
         let filename_to_remove = filename.clone();
         let _call_on_ret = crate::common::SimpleCallOnReturn {
@@ -497,6 +556,11 @@ pub(super) mod install {
             }),
         };
 
+        // download
+        if !download_file(id, url, &filename) {
+            return;
+        }
+
         // install
         send_install_status(id, InstallStatus::Installing);
         if let Err(e) = do_install_file(&filename, &plugin_dir) {

src/plugin/mod.rs

@@ -20,7 +20,8 @@ mod plog;
 mod plugins;
 
 pub use manager::{
-    install::install_plugin_with_url, install_plugin, load_plugin_list, uninstall_plugin,
+    install::{change_uninstall_plugin, install_plugin_with_url},
+    install_plugin, load_plugin_list, remove_uninstalled, uninstall_plugin,
 };
 pub use plugins::{
     handle_client_event, handle_listen_event, handle_server_event, handle_ui_event, load_plugin,
@@ -92,14 +93,21 @@ pub fn init() {
     if !is_server() {
         std::thread::spawn(move || manager::start_ipc());
     } else {
-        if let Err(e) = manager::remove_plugins() {
+        if let Err(e) = remove_uninstalled() {
             log::error!("Failed to remove plugins: {}", e);
         }
     }
-    if let Err(e) = plugins::load_plugins() {
+    match manager::get_uninstall_id_set() {
+        Ok(ids) => {
+            if let Err(e) = plugins::load_plugins(&ids) {
                 log::error!("Failed to load plugins: {}", e);
             }
         }
+        Err(e) => {
+            log::error!("Failed to load plugins: {}", e);
+        }
+    }
+}
 
 #[inline]
 #[cfg(target_os = "windows")]
@@ -131,6 +139,11 @@ fn get_plugin_dir(id: &str) -> ResultType<PathBuf> {
     Ok(get_plugins_dir()?.join(id))
 }
 
+#[inline]
+fn get_uninstall_file_path() -> ResultType<PathBuf> {
+    Ok(get_plugins_dir()?.join("uninstall_list"))
+}
+
 #[inline]
 fn cstr_to_string(cstr: *const c_char) -> ResultType<String> {
     Ok(String::from_utf8(unsafe {

src/plugin/native_handlers/session.rs

@@ -8,9 +8,8 @@ use std::{
 use flutter_rust_bridge::StreamSink;
 
 use crate::{
-    define_method_prefix,
+    define_method_prefix, flutter::FlutterHandler, flutter_ffi::EventToUI,
-    flutter::{FlutterHandler},
+    plugin::MSG_TO_UI_TYPE_PLUGIN_EVENT, ui_session_interface::Session,
-    ui_session_interface::Session, plugin::MSG_TO_UI_TYPE_PLUGIN_EVENT, flutter_ffi::EventToUI,
 };
 
 const MSG_TO_UI_TYPE_SESSION_CREATED: &str = "session_created";
@@ -124,7 +123,7 @@ impl PluginNativeHandler for PluginNativeSessionHandler {
 impl PluginNativeSessionHandler {
     fn create_session(&self, session_id: String) -> String {
         let session =
-            crate::flutter::session_add(&session_id, false, false, "", false, "".to_owned());
+            crate::flutter::session_add(&session_id, false, false, false, "", false, "".to_owned());
         if let Ok(session) = session {
             let mut sessions = self.sessions.write().unwrap();
             sessions.push(session);
@@ -132,7 +131,10 @@ impl PluginNativeSessionHandler {
             let mut m = HashMap::new();
             m.insert("name", MSG_TO_UI_TYPE_SESSION_CREATED);
             m.insert("session_id", &session_id);
-            crate::flutter::push_global_event(crate::flutter::APP_TYPE_DESKTOP_REMOTE, serde_json::to_string(&m).unwrap_or("".to_string()));
+            crate::flutter::push_global_event(
+                crate::flutter::APP_TYPE_DESKTOP_REMOTE,
+                serde_json::to_string(&m).unwrap_or("".to_string()),
+            );
             return session_id;
         } else {
             return "".to_string();

src/plugin/plugins.rs

@@ -11,7 +11,7 @@ use hbb_common::{
 };
 use serde_derive::Serialize;
 use std::{
-    collections::HashMap,
+    collections::{HashMap, HashSet},
     ffi::{c_char, c_void},
     path::PathBuf,
     sync::{Arc, RwLock},
@@ -263,7 +263,7 @@ const DYLIB_SUFFIX: &str = ".so";
 #[cfg(target_os = "macos")]
 const DYLIB_SUFFIX: &str = ".dylib";
 
-pub(super) fn load_plugins() -> ResultType<()> {
+pub(super) fn load_plugins(uninstalled_ids: &HashSet<String>) -> ResultType<()> {
     let plugins_dir = super::get_plugins_dir()?;
     if !plugins_dir.exists() {
         std::fs::create_dir_all(&plugins_dir)?;
@@ -273,9 +273,18 @@ pub(super) fn load_plugins() -> ResultType<()> {
                 Ok(entry) => {
                     let plugin_dir = entry.path();
                     if plugin_dir.is_dir() {
+                        if let Some(plugin_id) = plugin_dir.file_name().and_then(|f| f.to_str()) {
+                            if uninstalled_ids.contains(plugin_id) {
+                                log::debug!(
+                                    "Ignore loading '{}' as it should be uninstalled",
+                                    plugin_id
+                                );
+                                continue;
+                            }
                             load_plugin_dir(&plugin_dir);
                         }
                     }
+                }
                 Err(e) => {
                     log::error!("Failed to read plugins dir entry, {}", e);
                 }
@@ -345,6 +354,14 @@ fn load_plugin_path(path: &str) -> ResultType<()> {
     // to-do validate plugin
     // to-do check the plugin id (make sure it does not use another plugin's id)
 
+    let id = desc.meta().id.clone();
+    let plugin_info = PluginInfo {
+        path: path.to_string(),
+        uninstalled: false,
+        desc: desc.clone(),
+    };
+    PLUGIN_INFO.write().unwrap().insert(id.clone(), plugin_info);
+
     let init_info = serde_json::to_string(&InitInfo {
         is_server: crate::common::is_server(),
     })?;
@@ -359,7 +376,10 @@ fn load_plugin_path(path: &str) -> ResultType<()> {
             native: super::native::cb_native_data,
         },
     };
-    plugin.init(&init_data, path)?;
+    // If do not load the plugin when init failed, the ui will not show the installed plugin.
+    if let Err(e) = plugin.init(&init_data, path) {
+        log::error!("Failed to init plugin '{}', {}", desc.meta().id, e);
+    }
 
     if is_server() {
         super::config::ManagerConfig::add_plugin(&desc.meta().id)?;
@@ -370,13 +390,6 @@ fn load_plugin_path(path: &str) -> ResultType<()> {
     reload_ui(&desc, None);
 
     // add plugins
-    let id = desc.meta().id.clone();
-    let plugin_info = PluginInfo {
-        path: path.to_string(),
-        uninstalled: false,
-        desc,
-    };
-    PLUGIN_INFO.write().unwrap().insert(id.clone(), plugin_info);
     PLUGINS.write().unwrap().insert(id.clone(), plugin);
 
     log::info!("Plugin {} loaded", id);