Repository: Senth/minecraft-mod-manager (python)

Issue:
Skip downloading already up to date files on update
Maybe I'm looking at this wrong, but it seems like when the "update" command is used, the latest version of installed mods are downloaded and replace the current installed versions regardless if the currently installed version is up to date already.

On my end this makes for the 'update' command to take a long time (about an extra 1.5 seconds per mod). This seems unnecessary. Is there a faster way this could be implemented to avoid downloading the whole mod every time (maybe comparing metadata from curse/modrinth is faster?)


Pull request:
Skip downloading up-to-date files on update
Checks the upload time of the latest version and our version and doesn't update the version if they are the same.

### Testing
- [X] Added unit tests
- [ ] Added integration tests
- [ ] ...Your own tests...

### Checklist
- [x] I have reviewed my own code

### Related Issues
Fixes #139 


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index d6a4cb6..78510a4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Now uses tealprint library which will automatically disable color and unicode
   when trying to output Unicode characters [#141](https://github.com/Senth/minecraft-mod-manager/issues/141)
 - Improved the searching algorithm where the search term `entityculling` would previously return 0 results [#56](https://github.com/Senth/minecraft-mod-manager/issues/56)
+- `update` now skips downloading files if we already have the latest version [#139](https://github.com/Senth/minecraft-mod-manager/issues/139)
 
 ### Fixed
 
diff --git a/minecraft_mod_manager/app/download/download.py b/minecraft_mod_manager/app/download/download.py
index 9b8c3ea..5f028de 100644
--- a/minecraft_mod_manager/app/download/download.py
+++ b/minecraft_mod_manager/app/download/download.py
@@ -37,11 +37,15 @@ class Download:
                 latest_version = LatestVersionFinder.find_latest_version(mod, versions, filter=True)
 
                 if latest_version:
-                    ok = self._download_latest_version(mod, latest_version)
-
-                    # Add possible dependencies to download queue
-                    if ok:
-                        download_queue.extend(self._get_dependencies(latest_version))
+                    # Different version
+                    if latest_version.upload_time != mod.upload_time:
+                        ok = self._download_latest_version(mod, latest_version)
+
+                        # Add possible dependencies to download queue
+                        if ok:
+                            download_queue.extend(self._get_dependencies(latest_version))
+                    else:
+                        self.on_no_change(mod)
                 else:
                     self.on_version_not_found(mod, versions)
 
@@ -66,7 +70,7 @@ class Download:
             downloaded_mod = self._download(mod, latest_version)
             self._update_mod_from_file(downloaded_mod)
             self._repo.update_mod(downloaded_mod)
-            self.on_version_found(mod, downloaded_mod)
+            self.on_new_version_downloaded(mod, downloaded_mod)
             return True
         except (DownloadFailed, MaxRetriesExceeded) as e:
             TealPrint.error(
@@ -119,7 +123,10 @@ class Download:
                 TealPrint.info(f"{mod.name}")
             TealPrint.pop_indent()
 
-    def on_version_found(self, old: Mod, new: Mod) -> None:
+    def on_new_version_downloaded(self, old: Mod, new: Mod) -> None:
+        raise NotImplementedError("Not implemented in subclass")
+
+    def on_no_change(self, mod: Mod) -> None:
         raise NotImplementedError("Not implemented in subclass")
 
     def on_version_not_found(self, mod: Mod, versions: List[VersionInfo]) -> None:
diff --git a/minecraft_mod_manager/app/install/install.py b/minecraft_mod_manager/app/install/install.py
index b76e07b..92b8ec9 100644
--- a/minecraft_mod_manager/app/install/install.py
+++ b/minecraft_mod_manager/app/install/install.py
@@ -70,12 +70,15 @@ class Install(Download):
 
         return loader_max
 
-    def on_version_found(self, old: Mod, new: Mod) -> None:
+    def on_new_version_downloaded(self, old: Mod, new: Mod) -> None:
         TealPrint.info(
             f"🟢 Installed version {new.version}",
             color=LogColors.add,
         )
 
+    def on_no_change(self, mod: Mod) -> None:
+        TealPrint.verbose("🔵 Already installed and up-to-date")
+
     def on_version_not_found(self, mod: Mod, versions: List[VersionInfo]) -> None:
         TealPrint.info("🟨 All versions were filtered out", color=LogColors.skip, push_indent=True)
 
diff --git a/minecraft_mod_manager/app/update/update.py b/minecraft_mod_manager/app/update/update.py
index a25f125..39ad455 100644
--- a/minecraft_mod_manager/app/update/update.py
+++ b/minecraft_mod_manager/app/update/update.py
@@ -30,7 +30,7 @@ class Update(Download):
 
         self.find_download_and_install(mods_to_update)
 
-    def on_version_found(self, old: Mod, new: Mod) -> None:
+    def on_new_version_downloaded(self, old: Mod, new: Mod) -> None:
         if new.file:
             if Update._has_downloaded_new_file(old, new):
                 if not config.pretend and old.file:
@@ -38,6 +38,9 @@ class Update(Download):
 
                 TealPrint.info(f"🟢 Updated {old.version} -> {new.version}", color=LogColors.updated)
 
+    def on_no_change(self, mod: Mod) -> None:
+        TealPrint.verbose("🔵 Already up-to-date")
+
     def on_version_not_found(self, mod: Mod, versions: List[VersionInfo]) -> None:
         TealPrint.verbose("🟨 No new version found", color=LogColors.skip)
 


Test patch:
diff --git a/minecraft_mod_manager/app/download/download_test.py b/minecraft_mod_manager/app/download/download_test.py
index 2f2f3db..bc00752 100644
--- a/minecraft_mod_manager/app/download/download_test.py
+++ b/minecraft_mod_manager/app/download/download_test.py
@@ -1,5 +1,5 @@
 from pathlib import Path
-from typing import Any
+from typing import Any, List
 
 import pytest
 from mockito import mock, unstub, verifyStubbedInvocationsAreUsed, when
@@ -25,27 +25,33 @@ class T:
         stability=Stabilities.release,
         mod_loaders=set([ModLoaders.fabric]),
         site=Sites.curse,
-        upload_time=0,
+        upload_time=100,
         minecraft_versions=[],
         download_url="",
     )
 
+    @staticmethod
+    def set_input(input: List[Mod]):
+        T.input = input
+
 
 @pytest.mark.parametrize(
     "name,prepare_function",
     [
         (
-            "Call on_version_found() when found",
+            "Call on_new_version_downloaded() when found",
             lambda: (
+                when(T.mock_repo).download(...).thenReturn(Path("mod.jar")),
                 when(T.mock_repo).get_versions(...).thenReturn([T.version_info]),
                 when(T.mock_repo).get_mod_from_file(...).thenReturn(Mod("found", "")),
                 when(T.mock_repo).update_mod(...),
-                when(T.download).on_version_found(...),
+                when(T.download).on_new_version_downloaded(...),
             ),
         ),
         (
             "Remove downloaded file when invalid mod file",
             lambda: (
+                when(T.mock_repo).download(...).thenReturn(Path("mod.jar")),
                 when(T.mock_repo).get_versions(...).thenReturn([T.version_info]),
                 when(T.mock_repo).get_mod_from_file(...).thenRaise(ModFileInvalid(Path("mod.jar"))),
                 when(T.mock_repo).remove_mod_file(...),
@@ -54,6 +60,7 @@ class T:
         (
             "Update mod_id after download",
             lambda: (
+                when(T.mock_repo).download(...).thenReturn(Path("mod.jar")),
                 when(T.mock_repo).get_versions(...).thenReturn([T.version_info]),
                 when(T.mock_repo).update_mod(
                     Mod(
@@ -62,15 +69,17 @@ class T:
                         sites={Sites.curse: Site(Sites.curse, "", "")},
                         version="1.0.0",
                         file="mod.jar",
+                        upload_time=100,
                     )
                 ),
                 when(T.mock_repo).get_mod_from_file("mod.jar").thenReturn(Mod("validid", "name", version="1.0.0")),
-                when(T.download).on_version_found(...),
+                when(T.download).on_new_version_downloaded(...),
             ),
         ),
         (
             "Download dependency",
             lambda: (
+                when(T.mock_repo).download(...).thenReturn(Path("mod.jar")),
                 when(T.mock_repo)
                 .get_versions(T.input[0])
                 .thenReturn(
@@ -79,7 +88,7 @@ class T:
                             stability=Stabilities.release,
                             mod_loaders=set([ModLoaders.fabric]),
                             site=Sites.curse,
-                            upload_time=0,
+                            upload_time=100,
                             minecraft_versions=[],
                             download_url="",
                             dependencies={Sites.curse: ["123", "456"]},
@@ -96,7 +105,26 @@ class T:
                 when(T.mock_repo).get_mod_from_file("mod.jar").thenReturn(Mod("123", "123 Name")),
                 when(T.mock_repo).get_mod_from_file("parent.jar").thenReturn(T.input[0]),
                 when(T.mock_repo).update_mod(...),
-                when(T.download).on_version_found(...),
+                when(T.download).on_new_version_downloaded(...),
+            ),
+        ),
+        (
+            "Skip downloading if the upload date is same as the current version",
+            lambda: (
+                T.set_input([Mod("123", "", upload_time=100)]),
+                when(T.mock_repo).get_versions(...).thenReturn([T.version_info]),
+                when(T.download).on_no_change(...),
+            ),
+        ),
+        (
+            "Downloading if the upload date older than the current version",
+            lambda: (
+                T.set_input([Mod("123", "", upload_time=200)]),
+                when(T.mock_repo).download(...).thenReturn(Path("mod.jar")),
+                when(T.mock_repo).get_versions(...).thenReturn([T.version_info]),
+                when(T.mock_repo).get_mod_from_file(...).thenReturn(Mod("found", "", upload_time=200)),
+                when(T.mock_repo).update_mod(...),
+                when(T.download).on_new_version_downloaded(...),
             ),
         ),
     ],
@@ -105,7 +133,6 @@ def test_find_download_and_install(name, prepare_function):
     print(name)
 
     T.mock_repo = mock(DownloadRepo)
-    when(T.mock_repo).download(...).thenReturn(Path("mod.jar"))
     T.mock_finder = mock(ModFinder)
     when(T.mock_finder).find_mod(...).thenReturn({Sites.curse: Site(Sites.curse, "", "")})
     T.download = Download(T.mock_repo, T.mock_finder)
diff --git a/minecraft_mod_manager/app/update/update_test.py b/minecraft_mod_manager/app/update/update_test.py
index 4333942..430cb08 100644
--- a/minecraft_mod_manager/app/update/update_test.py
+++ b/minecraft_mod_manager/app/update/update_test.py
@@ -116,7 +116,7 @@ def test_on_version_found(
     update = Update(mock_repo, mock_finder)
     old_mod = Mod("", "", file=old)
     new_mod = Mod("", "", file=new)
-    update.on_version_found(old_mod, new_mod)
+    update.on_new_version_downloaded(old_mod, new_mod)
 
     config.pretend = False
     verifyStubbedInvocationsAreUsed()