Improve behavior of updating aliases

jimporter · jimporter · commit d119286747a0 · 2021-03-24T12:02:16.000-07:00
This adds an important check for the `alias` subcommand to prevent setting an
alias on two different versions, allows for updating an alias via the `alias`
subcommand, and generally improves docs about aliases.
diff --git a/CHANGES.md b/CHANGES.md
@@ -13,6 +13,7 @@
 - Allow deploying docs to a subdirectory within the target branch via `--prefix`
 - Add support for custom templates with `mike set-default`
 - Read from `remote_branch` and `remote_name` if set in `mkdocs.yml`
+- Allow updating an existing alias with `mike alias -u`
 
 ### Breaking changes
 - Require Python 3.6+
@@ -21,6 +22,8 @@
 
 ### Bug fixes
 - Canonical URLs in generated documentation now point to the correct location
+- `mike alias` now checks for existing aliases to prevent erroneously setting an
+  alias for two different versions
 - Replace `packaging` dependency with `verspec` for future stability
 
 [material-mike]: https://squidfunk.github.io/mkdocs-material/setup/setting-up-versioning/#versioning
diff --git a/README.md b/README.md
@@ -78,18 +78,21 @@ particularly-relevant version of your docs somewhere special (e.g. `latest`):
 mike deploy [version] [alias]...
 ```
 
+If `[version]` already exists, this command will *also* update all of the
+pre-existing aliases for it. Normally, if an alias specified on the command line
+is already associated with another version, this will return an error. If you
+*do* want to move an alias from another version to this version (e.g. when
+releasing a new version and updating the `latest` alias to point to this new
+version), you can pass `-u`/`--update-aliases` to allow this.
+
 By default, aliases create a simple HTML redirect to the real version of the
 docs; to create a copy of the docs for each alias, you can pass `--no-redirect`.
 If you're using redirects, you can customize the redirect template with
 `-T`/`--template`; this takes a path to a [Jinja][jinja] template that accepts
 an `{{href}}` variable.
 
 If you'd like to specify a title for this version that doesn't match the version
-string, you can pass `-t TITLE`/`--title=TITLE` as well. If `version` already
-exists, this command will *also* update all of the pre-existing aliases for it.
-If you want to move an alias from a previous version to this version (e.g. when
-releasing a new version and updating a `latest` alias), you can pass
-`-u`/`--update-aliases` to allow this.
+string, you can pass `-t TITLE`/`--title=TITLE` as well.
 
 In addition, you can specify where to deploy your docs via `-b`/`--branch`,
 `-r`/`--remote`, and `--prefix`, specifying the branch, remote, and directory
@@ -185,6 +188,9 @@ your documentation. You can use the `alias` command for this:
 mike alias [version-or-alias] [alias]...
 ```
 
+As with `deploy`, you can pass `-u`/`--update-aliases` to change where an
+existing alias points to.
+
 Once again, you can specify `--branch`, `--push`, etc to control how the commit
 is handled.
 
diff --git a/mike/commands.py b/mike/commands.py
@@ -131,8 +131,8 @@ def delete(versions=None, all=False, *, branch='gh-pages', message=None,
             commit.add_file(versions_to_file_info(all_versions, prefix))
 
 
-def alias(cfg, version, aliases, redirect=True, template=None, *,
-          branch='gh-pages', message=None, prefix=''):
+def alias(cfg, version, aliases, update_aliases=False, redirect=True,
+          template=None, *, branch='gh-pages', message=None, prefix=''):
     all_versions = list_versions(branch, prefix)
     try:
         real_version = all_versions.find(version, strict=True)[0]
@@ -150,7 +150,8 @@ def alias(cfg, version, aliases, redirect=True, template=None, *,
             mike_version=app_version
         )
 
-    new_aliases = all_versions.update(real_version, aliases=aliases)
+    new_aliases = all_versions.update(real_version, aliases=aliases,
+                                      update_aliases=update_aliases)
     destdirs = [os.path.join(prefix, i) for i in new_aliases]
 
     if redirect and destdirs:
diff --git a/mike/driver.py b/mike/driver.py
@@ -141,9 +141,9 @@ def delete(args):
 def alias(args):
     cfg = load_mkdocs_config(args)
     check_remote_status(args, strict=True)
-    commands.alias(cfg, args.version, args.alias, args.redirect, args.template,
-                   branch=args.branch, message=args.message,
-                   prefix=args.prefix)
+    commands.alias(cfg, args.version, args.alias, args.update_aliases,
+                   args.redirect, args.template, branch=args.branch,
+                   message=args.message, prefix=args.prefix)
     if args.push:
         git_utils.push_branch(args.remote, args.branch, args.force)
 
@@ -223,7 +223,7 @@ def main():
     deploy_p.add_argument('-t', '--title',
                           help='short descriptive title for this version')
     deploy_p.add_argument('-u', '--update-aliases', action='store_true',
-                          help='allow aliases pointing to other versions')
+                          help='update aliases pointing to other versions')
     deploy_p.add_argument('--no-redirect', dest='redirect', default=True,
                           action='store_false',
                           help='make copies of docs for each alias')
@@ -249,6 +249,8 @@ def main():
         'alias', description=alias_desc, help='alias docs from a branch'
     )
     alias_p.set_defaults(func=alias)
+    alias_p.add_argument('-u', '--update-aliases', action='store_true',
+                         help='update aliases pointing to other versions')
     alias_p.add_argument('--no-redirect', dest='redirect', default=True,
                          action='store_false',
                          help='make copies of docs for each alias')
diff --git a/mike/versions.py b/mike/versions.py
@@ -82,30 +82,52 @@ def find(self, version, strict=False):
             raise KeyError(version)
         return None
 
-    def add(self, version, title=None, aliases=[], update_aliases=False):
-        v = _ensure_version(version)
+    def _ensure_unique_aliases(self, version, aliases, update_aliases=False):
         removed_aliases = []
         for i in aliases:
             key = self.find(i)
-            if key and key[0] != v:
-                if not update_aliases or len(key) == 1:
-                    raise ValueError('{!r} already exists'.format(i))
+            if key and key[0] != version:
+                if len(key) == 1:
+                    raise ValueError(
+                        'alias {!r} already specified as a version'.format(i)
+                    )
+                if not update_aliases:
+                    raise ValueError(
+                        'alias {!r} already exists for version {}'
+                        .format(i, str(key[0]))
+                    )
                 removed_aliases.append(key)
+        return removed_aliases
+
+    def add(self, version, title=None, aliases=[], update_aliases=False):
+        v = _ensure_version(version)
+        removed_aliases = self._ensure_unique_aliases(
+            v, aliases, update_aliases
+        )
 
         if v in self._data:
             self._data[v].update(title, aliases)
         else:
             if self.find(version):
-                raise ValueError('{!r} already exists'.format(version))
+                raise ValueError('version {} already exists'.format(version))
             self._data[v] = VersionInfo(version, title, aliases)
 
+        # Remove aliases from old versions that we've moved to this version.
         for i in removed_aliases:
             self._data[i[0]].aliases.remove(i[1])
 
         return self._data[v]
 
-    def update(self, version, title=None, aliases=[]):
+    def update(self, version, title=None, aliases=[], update_aliases=False):
         key = self.find(version, strict=True)
+        removed_aliases = self._ensure_unique_aliases(
+            key[0], aliases, update_aliases
+        )
+
+        # Remove aliases from old versions that we've moved to this version.
+        for i in removed_aliases:
+            self._data[i[0]].aliases.remove(i[1])
+
         return self._data[key[0]].update(title, aliases)
 
     def _remove_by_key(self, key):
diff --git a/test/integration/test_alias.py b/test/integration/test_alias.py
@@ -20,8 +20,9 @@ def _deploy(self, branch=None, versions=['1.0'], prefix=''):
             assertPopen(['mike', 'deploy', i] + extra_args)
 
     def _test_alias(self, expected_message=None,
-                    expected_versions=[versions.VersionInfo('1.0')],
-                    redirect=True, directory='.'):
+                    expected_versions=[
+                        versions.VersionInfo('1.0', aliases=['latest'])
+                    ], redirect=True, directory='.'):
         message = assertPopen(['git', 'log', '-1', '--pretty=%B']).rstrip()
         if expected_message:
             self.assertEqual(message, expected_message)
@@ -43,9 +44,8 @@ def _test_alias(self, expected_message=None,
         assertDirectory(directory, files, allow_extra=True)
 
         with open(os.path.join(directory, 'versions.json')) as f:
-            self.assertEqual(list(versions.Versions.loads(f.read())), [
-                versions.VersionInfo('1.0', aliases=['latest']),
-            ])
+            self.assertEqual(list(versions.Versions.loads(f.read())),
+                             expected_versions)
 
 
 class TestAlias(AliasTestCase):
@@ -65,6 +65,19 @@ def test_alias(self):
         with open('latest/index.html') as f:
             self.assertRegex(f.read(), match_redir('../1.0/'))
 
+    def test_update_aliases(self):
+        assertPopen(['mike', 'deploy', '1.0', 'latest'])
+        assertPopen(['mike', 'deploy', '2.0'])
+        assertPopen(['mike', 'alias', '2.0', 'latest', '-u'])
+        check_call_silent(['git', 'checkout', 'gh-pages'])
+        self._test_alias(expected_versions=[
+            versions.VersionInfo('2.0', aliases=['latest']),
+            versions.VersionInfo('1.0', ),
+        ])
+
+        with open('latest/index.html') as f:
+            self.assertRegex(f.read(), match_redir('../2.0/'))
+
     def test_alias_copy(self):
         self._deploy()
         assertPopen(['mike', 'alias', '1.0', 'latest', '--no-redirect'])
diff --git a/test/unit/test_commands.py b/test/unit/test_commands.py
@@ -88,6 +88,18 @@ def _test_deploy(self, expected_message=None,
 
         self._test_state(expected_message, expected_versions, **kwargs)
 
+    def _mock_commit(self):
+        with git_utils.Commit('gh-pages', 'add versions.json') as commit:
+            commit.add_file(git_utils.FileInfo(
+                'versions.json',
+                '[{"version": "1.0", "title": "1.0", "aliases": ["latest"]}]',
+            ))
+            commit.add_file(git_utils.FileInfo('1.0/page.html', ''))
+            commit.add_file(git_utils.FileInfo('1.0/file.txt', ''))
+            commit.add_file(git_utils.FileInfo('1.0/dir/index.html', ''))
+            commit.add_file(git_utils.FileInfo('latest/page.html', ''))
+            commit.add_file(git_utils.FileInfo('latest/dir/index.html', ''))
+
     def test_default(self):
         commands.deploy(self.cfg, '1.0')
         check_call_silent(['git', 'checkout', 'gh-pages'])
@@ -179,18 +191,24 @@ def test_overwrite_version(self):
             versions.VersionInfo('1.0', '1.0.1', ['latest', 'greatest'])
         ])
 
-    def test_overwrite_alias(self):
-        with git_utils.Commit('gh-pages', 'add versions.json') as commit:
-            commit.add_file(git_utils.FileInfo(
-                'versions.json',
-                '[{"version": "1.0", "title": "1.0", "aliases": ["latest"]}]',
-            ))
-            commit.add_file(git_utils.FileInfo('1.0/page.html', ''))
-            commit.add_file(git_utils.FileInfo('1.0/file.txt', ''))
-            commit.add_file(git_utils.FileInfo('1.0/dir/index.html', ''))
-            commit.add_file(git_utils.FileInfo('latest/page.html', ''))
-            commit.add_file(git_utils.FileInfo('latest/dir/index.html', ''))
+    def test_overwrite_same_alias(self):
+        self._mock_commit()
+        commands.deploy(self.cfg, '1.0', '1.0.1', ['latest'])
+        check_call_silent(['git', 'checkout', 'gh-pages'])
+        self._test_deploy(expected_versions=[
+            versions.VersionInfo('1.0', '1.0.1', ['latest'])
+        ])
 
+    def test_overwrite_include_same_alias(self):
+        self._mock_commit()
+        commands.deploy(self.cfg, '1.0', '1.0.1', ['latest', 'greatest'])
+        check_call_silent(['git', 'checkout', 'gh-pages'])
+        self._test_deploy(expected_versions=[
+            versions.VersionInfo('1.0', '1.0.1', ['latest', 'greatest'])
+        ])
+
+    def test_overwrite_alias_error(self):
+        self._mock_commit()
         with self.assertRaises(ValueError):
             commands.deploy(self.cfg, '2.0', '2.0.0', ['latest'])
         check_call_silent(['git', 'checkout', 'gh-pages'])
@@ -199,17 +217,7 @@ def test_overwrite_alias(self):
         ])
 
     def test_update_aliases(self):
-        with git_utils.Commit('gh-pages', 'add versions.json') as commit:
-            commit.add_file(git_utils.FileInfo(
-                'versions.json',
-                '[{"version": "1.0", "title": "1.0", "aliases": ["latest"]}]',
-            ))
-            commit.add_file(git_utils.FileInfo('1.0/page.html', ''))
-            commit.add_file(git_utils.FileInfo('1.0/file.txt', ''))
-            commit.add_file(git_utils.FileInfo('1.0/dir/index.html', ''))
-            commit.add_file(git_utils.FileInfo('latest/page.html', ''))
-            commit.add_file(git_utils.FileInfo('latest/dir/index.html', ''))
-
+        self._mock_commit()
         commands.deploy(self.cfg, '2.0', '2.0.0', ['latest'], True)
         check_call_silent(['git', 'checkout', 'gh-pages'])
         self._test_deploy('.*', [
@@ -341,7 +349,7 @@ def test_alias_from_alias(self):
         self._deploy()
         commands.alias(self.cfg, 'latest', ['greatest'])
         check_call_silent(['git', 'checkout', 'gh-pages'])
-        self._test_alias(expected_src='1.0')
+        self._test_alias()
 
     def test_alias_copy(self):
         self._deploy()
@@ -363,6 +371,32 @@ def test_alias_custom_redirect(self):
         with open('greatest/dir/index.html') as f:
             self.assertEqual(f.read(), '../../1.0/dir/')
 
+    def test_alias_overwrite_same(self):
+        self._deploy()
+        commands.alias(self.cfg, '1.0', ['latest'])
+        check_call_silent(['git', 'checkout', 'gh-pages'])
+        self._test_alias(expected_aliases=['latest'])
+
+    def test_alias_overwrite_include_same(self):
+        self._deploy()
+        commands.alias(self.cfg, '1.0', ['latest', 'greatest'])
+        check_call_silent(['git', 'checkout', 'gh-pages'])
+        self._test_alias(expected_aliases=['latest', 'greatest'])
+
+    def test_alias_overwrite_error(self):
+        self._deploy()
+        commands.deploy(self.cfg, '2.0')
+        with self.assertRaises(ValueError):
+            commands.alias(self.cfg, '2.0', ['latest'])
+        check_call_silent(['git', 'checkout', 'gh-pages'])
+        self._test_state(r'^Deployed \w+ to 2\.0', [
+            versions.VersionInfo('2.0', '2.0'),
+            versions.VersionInfo('1.0', '1.0', ['latest']),
+        ])
+
+    def test_alias_update(self):
+        pass
+
     def test_branch(self):
         self._deploy('branch')
         commands.alias(self.cfg, '1.0', ['greatest'], branch='branch')
@@ -381,7 +415,7 @@ def test_prefix(self):
         check_call_silent(['git', 'checkout', 'gh-pages'])
         self._test_alias(directory='prefix')
 
-    def test_alias_invalid(self):
+    def test_alias_invalid_version(self):
         self._deploy()
         self.assertRaises(ValueError, commands.alias, self.cfg, '2.0',
                           ['alias'])
diff --git a/test/unit/test_versions.py b/test/unit/test_versions.py
