From 74d84498c6a2ae03902788cd11499fa9a22805a9 Mon Sep 17 00:00:00 2001
From: Marcel van der Veldt <m.vanderveldt@outlook.com>
Date: Tue, 14 Mar 2023 00:37:01 +0100
Subject: [PATCH] Fix add provider logic (#527)

* fix default provider generation

* detect beta version in regex
---
 .github/workflows/docker-build.yml           |  2 +-
 music_assistant/server/controllers/config.py | 38 ++++++++++----------
 music_assistant/server/server.py             |  4 +--
 3 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/.github/workflows/docker-build.yml b/.github/workflows/docker-build.yml
index 8e6d25e1..241688fc 100644
--- a/.github/workflows/docker-build.yml
+++ b/.github/workflows/docker-build.yml
@@ -41,7 +41,7 @@ jobs:
           # If the VERSION looks like a version number, assume that
           # this is the most recent version of the image and also
           # tag it 'latest'.
-          if [[ $VERSION =~ ^[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}$ ]]; then
+          if [[ $VERSION =~ ^\d+\.\d+\.\d+[[:space:]]?(b[[:space:]]?\d+)? ]]; then
             TAGS="$TAGS,${DOCKER_IMAGE}:latest"
           fi
 
diff --git a/music_assistant/server/controllers/config.py b/music_assistant/server/controllers/config.py
index f0d34a51..bb83a043 100644
--- a/music_assistant/server/controllers/config.py
+++ b/music_assistant/server/controllers/config.py
@@ -173,16 +173,11 @@ class ConfigController:
             for prov in self.mass.get_available_providers():
                 if prov.domain != raw_conf["domain"]:
                     continue
-                return ProviderConfig.parse(
-                    prov.config_entries,
-                    raw_conf,
-                )
+                return ProviderConfig.parse(prov.config_entries, raw_conf, allow_none=True)
         raise KeyError(f"No config found for provider id {instance_id}")
 
     @api_command("config/providers/update")
-    def update_provider_config(
-        self, instance_id: str, update: ConfigUpdate, skip_reload: bool = False
-    ) -> None:
+    def update_provider_config(self, instance_id: str, update: ConfigUpdate) -> None:
         """Update ProviderConfig."""
         config = self.get_provider_config(instance_id)
         changed_keys = config.update(update)
@@ -193,10 +188,8 @@ class ConfigController:
 
         conf_key = f"{CONF_PROVIDERS}/{instance_id}"
         self.set(conf_key, config.to_raw())
-        # (re)load provider
-        if not skip_reload:
-            updated_config = self.get_provider_config(config.instance_id)
-            self.mass.create_task(self.mass.load_provider(updated_config))
+        updated_config = self.get_provider_config(config.instance_id)
+        self.mass.create_task(self.mass.load_provider(updated_config))
 
     @api_command("config/providers/create")
     def create_provider_config(self, provider_domain: str) -> ProviderConfig:
@@ -215,30 +208,39 @@ class ConfigController:
             raise KeyError(f"Unknown provider domain: {provider_domain}")
 
         # determine instance id based on previous configs
-        existing = self.get_provider_configs(provider_domain=provider_domain)
+        existing = {
+            x.instance_id for x in self.get_provider_configs(provider_domain=provider_domain)
+        }
+
         if existing and not manifest.multi_instance:
             raise ValueError(f"Provider {manifest.name} does not support multiple instances")
 
-        count = len(existing)
-        if count == 0:
+        if len(existing) == 0:
             instance_id = provider_domain
             name = manifest.name
         else:
-            instance_id = f"{provider_domain}{count+1}"
-            name = f"{manifest.name} {count+1}"
+            instance_id = f"{provider_domain}{len(existing)+1}"
+            name = f"{manifest.name} {len(existing)+1}"
 
-        return ProviderConfig.parse(
+        # all checks passed, return a default config
+        default_config = ProviderConfig.parse(
             prov.config_entries,
             {
                 "type": manifest.type.value,
                 "domain": manifest.domain,
                 "instance_id": instance_id,
                 "name": name,
-                "values": dict(),
+                "values": {},
             },
             allow_none=True,
         )
 
+        # config provided and checks passed, storeconfig
+        conf_key = f"{CONF_PROVIDERS}/{instance_id}"
+        self.set(conf_key, default_config.to_raw())
+
+        return default_config
+
     @api_command("config/providers/remove")
     async def remove_provider_config(self, instance_id: str) -> None:
         """Remove ProviderConfig."""
diff --git a/music_assistant/server/server.py b/music_assistant/server/server.py
index 0879be98..1bb083a2 100644
--- a/music_assistant/server/server.py
+++ b/music_assistant/server/server.py
@@ -417,9 +417,7 @@ class MusicAssistant:
             existing = any(x for x in provider_configs if x.domain == prov_manifest.domain)
             if existing:
                 continue
-            default_conf = self.config.create_provider_config(prov_manifest.domain)
-            # skip_reload to prevent race condition
-            self.config.update_provider_config(default_conf, skip_reload=True)
+            self.config.create_provider_config(prov_manifest.domain)
 
         # load all configured (and enabled) providers
         for allow_depends_on in (False, True):
-- 
2.34.1