githubplugin: rework path and naming schemes, add error handling

Author: Morg42

githubplugin/__init__.py

@@ -39,6 +39,10 @@
 from git import Repo
 
 
+#
+# this is NOT the plugin class...
+#
+
 class GitHubHelper(object):
     """ Helper class for handling the GitHub API """
 
@@ -268,14 +272,19 @@ def get_plugins_from(self, fork=None, owner='', branch='', fetch=False) -> list:
         return sorted(plugins)
 
 
+#
+# this IS the plugin class :)
+#
+
+
 class GithubPlugin(SmartPlugin):
     """
     This class supports testing foreign plugins by letting the user select a
     shng plugins fork and branch, and then setting up a local repo containing
     that fork. Additionally, the specified plugin will be soft-linked into the
     "live" plugins repo worktree as a private plugin.
     """
-    PLUGIN_VERSION = '1.0.0'
+    PLUGIN_VERSION = '1.0.1'
     REPO_DIR = 'priv_repos'
 
     def loggerr(self, msg):
@@ -321,6 +330,9 @@ def __init__(self, sh):
             os.mkdir(self.repo_path)
 
         self.gh_apikey = self.get_parameter_value('app_token')
+        self.supermode = self.get_parameter_value('supermode') == "'I KNOW WHAT I'M DOING!'"
+        if self.supermode:
+            self.logger.warning('supermode active, be very careful...')
         self.gh = GitHubHelper(self._sh.shtime, apikey=self.gh_apikey, logger=self.logger)
 
         self.init_webinterface(WebInterface)
@@ -338,10 +350,10 @@ def read_repos_from_dir(self, exc=False):
             return
 
         self.logger.debug('checking plugin links')
-        pathlist = Path(self.plg_path).glob('priv_*')
+        pathlist = Path(self.plg_path).glob('*')
         for item in pathlist:
             if not item.is_symlink():
-                self.logger.debug(f'ignoring {item}, is not symlink')
+                # self.logger.debug(f'ignoring {item}, is not symlink')
                 continue
             target = os.path.join(self.plg_path, os.readlink(str(item)))
             if not os.path.isdir(target):
@@ -373,8 +385,7 @@ def read_repos_from_dir(self, exc=False):
             repo_path = os.path.join(self.repo_path, owner)
             wt_path = os.path.join(self.repo_path, f'{owner}_wt_{branch}')
 
-            # use part of link name after ".../plugins/priv_"
-            name = str(item)[len(self.plg_path) + 6:]
+            name = str(item)[len(self.plg_path) + 1:]
 
             self.repos[name] = {
                 'plugin': plugin,
@@ -391,6 +402,7 @@ def read_repos_from_dir(self, exc=False):
                 'repo': repo,
             }
             self.repos[name]['clean'] = self.is_repo_clean(name, exc)
+            self.logger.info(f'added plugin {plugin} with name {name} in {item}')
 
     def check_for_repo_name(self, name) -> bool:
         """ check if name exists in repos or link exists """
@@ -403,13 +415,22 @@ def check_for_repo_name(self, name) -> bool:
     def create_repo(self, name, owner, plugin, branch=None, rename=False) -> bool:
         """ create repo from given parameters """
 
-        if not rename:
-            try:
-                self.check_for_repo_name(name)
-            except Exception as e:
-                self.loggerr(e)
+        if any(x in name for x in ['/', '..']) or name == self.REPO_DIR:
+            self.loggerr(f'Invalid characters in name {name} (no dirs, not "{self.REPO_DIR}")')
+            return False
+
+        if not self.supermode:
+            if not name.startswith('priv_'):
+                self.loggerr(f'Invalid name, must start with "priv_"')
                 return False
 
+            if not rename:
+                try:
+                    self.check_for_repo_name(name)
+                except Exception as e:
+                    self.loggerr(e)
+                    return False
+
         if not owner or not plugin:
             self.loggerr(f'Insufficient parameters, github user {owner} or plugin {plugin} empty, unable to fetch repo, aborting.')
             return False
@@ -439,7 +460,7 @@ def create_repo(self, name, owner, plugin, branch=None, rename=False) -> bool:
         repo['rel_wt_path'] = os.path.join('..', f'{owner}_wt_{branch}')
 
         # set link location from plugin name
-        repo['link'] = os.path.join(self.plg_path, f'priv_{name}')
+        repo['link'] = os.path.join(self.plg_path, name)
         repo['rel_link_path'] = os.path.join(self.REPO_DIR, f'{owner}_wt_{branch}', plugin)
 
         # make plugins/priv_repos if not present
@@ -472,6 +493,7 @@ def create_repo(self, name, owner, plugin, branch=None, rename=False) -> bool:
                 repo['repo'] = Repo.clone_from(repo['url'], repo['repo_path'])
             except Exception as e:
                 self.loggerr(f'error while cloning: {e}')
+                return False
 
         # fetch repo data
         self.logger.debug('fetching from origin...')
@@ -515,24 +537,44 @@ def create_repo(self, name, owner, plugin, branch=None, rename=False) -> bool:
 
         repo['clean'] = True
 
-        if rename:
-            self.logger.debug(f'renaming old link priv_{name}')
-            if not self._move_old_link(name):
-                self.loggerr(f'unable to move old link priv_{name}, installation needs to be repaired manually')
+        # try to rename if requested or in supermode
+        if rename or self.supermode:
+            self.logger.debug(f'renaming old link {repo["link"]}')
+            if not self._move_old_link(repo['link']):
+                # moving not possible...
+                if not self.supermode:
+                    # quit in normal mode
+                    self.loggerr(f'unable to move old link {repo["link"]}, installation needs to be repaired manually')
+                    return False
+                else:
+                    # delete in supermode
+                    self.logger.warning(f'unable to move old link {repo["link"]}, deleting it')
+                    if os.path.isdir(repo['link']):
+                        self._rmtree(repo['link'])
+                    else:
+                        os.path.delete(repo['link'])
+                    if os.path.exists(repo['link']):
+                        self.loggerr(f'error removing old link/dir {repo["link"]}')
+                        return False
 
         self.logger.debug(f'creating link {repo["link"]} to {repo["rel_link_path"]}...')
         try:
             os.symlink(repo['rel_link_path'], repo['link'])
         except FileExistsError:
             self.loggerr(f'plugin link {repo["link"]} was created by someone else while we were setting up repo. Not overwriting, check link file manually')
+            return False
 
         self.repos[name] = repo
 
         return True
 
     def _move_old_link(self, name) -> bool:
+        if not self.supermode and not os.path.basename(name).startswith('priv_'):
+            self.loggerr(f'unable to move plugin with illegal name {name}')
+            return False
+
         """ rename old plugin link or folder and repo entry """
-        link = os.path.join(self.plg_path, f'priv_{name}')
+        link = os.path.join(self.plg_path, name)
         if not os.path.exists(link):
             self.logger.debug(f'old link/folder not found: {link}')
             return True
@@ -553,7 +595,7 @@ def _move_old_link(self, name) -> bool:
             os.rename(link, newlink)
             self.logger.debug(f'renamed {link} to {newlink}')
             try:
-                # try to move repo entry to new name
+                # try to move repo entry to new name (if repo exists)
                 # ignore if repo name is not existent
                 name_new = f'{name}_{ver}'
                 self.repos[name_new] = self.repos[name]

githubplugin/plugin.yaml

@@ -12,7 +12,7 @@ plugin:
 #    documentation: ''              # An url to optional plugin doc - NOT the url to user_doc!!!
 #    support: https://knx-user-forum.de/forum/supportforen/smarthome-py
 
-    version: 1.0.0                  # Plugin version (must match the version specified in __init__.py)
+    version: 1.0.1                  # Plugin version (must match the version specified in __init__.py)
 
     # these min/max-versions MUST be given in quotes, or e.g. 3.10 will be interpreted as 3.1 (3.1 < 3.9 < 3.10)
     sh_minversion: '1.10'           # minimum shNG version to use this plugin
@@ -34,6 +34,14 @@ parameters:
             de: 'App-Token zum Zugriff auf GitHub (optional)'
             en: 'App token for accessing GitHub (optional)'
 
+    # allow arbitrary targets to be set and overwritten. Do not use this.
+    supermode:
+        type: str
+        default: ''
+        description:
+            de: 'Nur zu Entwicklungszwecken. Nicht verwenden.'
+            en: 'Only for development, do not use.'
+
 
 item_attributes: NONE
 

githubplugin/webif/__init__.py

@@ -216,6 +216,9 @@ def getNameSuggestion(self):
             json = cherrypy.request.json
             plugin = json.get('plugin')
 
+            if self.plugin.supermode:
+                return {"operation": "request", "result": "success", "name": plugin}
+
             count = ''
             while os.path.exists(os.path.join(self.plugin.plg_path, f'priv_{plugin}{count}')) and int('0' + count) < 20:
                 count = str(int('0' + count) + 1)

githubplugin/webif/templates/index.html

@@ -145,8 +145,8 @@
 								</select>
 								<button id='btn-ref-plugin' type="button" class="btn btn-shng btn-sm" onclick="javascript:refetch('plugin');">&#8635;</i></button>
 							</td>
-							<td style='padding-left: 15px;'>Pluginname: priv_</td>
-							<td><input id='name' disabled /></td>
+							<td style='padding-left: 15px;'>Pluginname: </td>
+							<td><input id='name' disabled/></td>
 							<td style='padding-left: 15px;'>
 								<button id='btn-plugin' disabled type="button" class="btn btn-shng btn-sm" onclick="javascript:selectPlugin(this);">Auswählen</button>					
 							</td>
@@ -168,6 +168,9 @@
 {{ super() }}
 <script type="text/javascript">
 
+	var supermode = {% if p.supermode %}true{% else %}false{% endif %};
+	var plg_prefix = "{% if not p.supermode %}priv_{% endif %}";
+	
 	var rateInterval = null;
 
 	function handleUpdatedData(response, dataSet='overview') {
@@ -456,15 +459,17 @@
 		var plugin = document.getElementById('plugin').value;
 
 		if (plugin != '') {
-			document.getElementById('name').value = plugin;
-			sendData("getNameSuggestion", {"plugin": plugin},
-				function(response) {},
-				function(response) {
-					if (response["name"] != undefined && response["name"] != "") {
-						document.getElementById('name').value = response["name"];;
+			document.getElementById('name').value = plg_prefix + plugin;
+			if (!supermode) {
+				sendData("getNameSuggestion", {"plugin": plugin},
+					function(response) {},
+					function(response) {
+						if (response["name"] != undefined && response["name"] != "") {
+							document.getElementById('name').value = plg_prefix + response["name"];
+						}
 					}
-				}
-			);
+				);
+			};
 			document.getElementById('name').disabled = false;
 			document.getElementById('btn-plugin').disabled = false;
 		}