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

Issue:
Sometimes permanently deletes mods
### Describe the bug 
Sometimes the tool deletes mods when it definitely should not. Over time this effect can delete almost an entire mods folder.

### Steps to reproduce
Steps to reproduce the behavior:
1. Download the sample mods folder from the following link. There should be 40 mods in the archive. [Sample Mods](https://drive.google.com/file/d/1qoBmoHcNow0gNDMlgTlDx1ib7HbrxJMB/view?usp=sharing)
2. Extract to a folder (doesn't need to be minecraft's actual mods folder, just somewhere the tool can run)
3. Run `minecraft-mod-manager list` and ensure the tool can see all 40 mods
4. Run `minecraft-mod-manager update`
5. Repeat step 4 until something seems off (should only take 2 or 3 runs to become rather obvious that some mods are gone)

### Expected Behavior
After step 4, all 40 mods should remain intact

### Info
 - OS: Ubuntu Linux 18.04
 - Python --version: 3.8.5
 - minecraft-mod-manager --version: 1.2.2


Pull request:
Don't remove mods that are up-to-date
### What changed?
- Only remove the old file if a new one was downloaded
- Don't show "Updated" message if already up-to-date

### Testing
- [X] Added unit tests
- [X] Tested running `update` manually

### Related Issues
Fixes #81 


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7306187..7e3c12c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [1.2.3] - 2021-06-16
+
+### Fixed
+
+- Don't remove mods that are up-to-date [#81](https://github.com/Senth/minecraft-mod-manager/issues/81)
+
 ## [1.2.2] - 2021-06-15
 
 ### Fixed
diff --git a/minecraft_mod_manager/app/update/update.py b/minecraft_mod_manager/app/update/update.py
index 652c2d0..c68c0f1 100644
--- a/minecraft_mod_manager/app/update/update.py
+++ b/minecraft_mod_manager/app/update/update.py
@@ -28,14 +28,29 @@ class Update(Download):
         self.find_download_and_install(mods_to_update)
 
     def on_version_found(self, download_info: DownloadInfo) -> None:
-        if download_info.mod.file and not config.pretend:
-            self._update_repo.remove_mod_file(download_info.mod.file)
-        # TODO #32 improve message
-        Logger.info(
-            f"🟢 Updated -> {download_info.version_info.filename}",
-            LogColors.green,
-            indent=1,
-        )
+        if download_info.mod.file:
+            if Update._has_downloaded_new_file(download_info):
+                if not config.pretend:
+                    self._update_repo.remove_mod_file(download_info.mod.file)
+
+                # TODO #32 improve message
+                Logger.info(
+                    f"🟢 Updated {download_info.mod.version} -> {download_info.version_info.filename}",
+                    LogColors.green,
+                    indent=1,
+                )
 
     def on_version_not_found(self, mod: Mod, versions: List[VersionInfo]) -> None:
         Logger.verbose("🟨 No new version found", LogColors.skip, indent=1)
+
+    @staticmethod
+    def _has_downloaded_new_file(download_info: DownloadInfo) -> bool:
+        mod = download_info.mod
+        version_info = download_info.version_info
+
+        if mod.file:
+            if len(version_info.filename) > 0:
+                if mod.file != version_info.filename:
+                    return True
+
+        return False


Test patch:
diff --git a/minecraft_mod_manager/app/update/update_test.py b/minecraft_mod_manager/app/update/update_test.py
index 9f31117..d24479e 100644
--- a/minecraft_mod_manager/app/update/update_test.py
+++ b/minecraft_mod_manager/app/update/update_test.py
@@ -1,9 +1,13 @@
-from typing import List
+from typing import List, Union
 
 import pytest
+from minecraft_mod_manager.app.download.download import DownloadInfo
 from mockito import mock, unstub, verifyStubbedInvocationsAreUsed, when
 
+from ...config import config
 from ...core.entities.mod import Mod
+from ...core.entities.sites import Sites
+from ...core.entities.version_info import Stabilities, VersionInfo
 from .update import Update
 from .update_repo import UpdateRepo
 
@@ -34,3 +38,75 @@ def test_call_find_download_and_install(mock_repo):
 
     verifyStubbedInvocationsAreUsed()
     unstub()
+
+
+def version_info(filename: str) -> VersionInfo:
+    return VersionInfo(
+        stability=Stabilities.release,
+        mod_loaders=set(),
+        site=Sites.curse,
+        upload_time=1,
+        minecraft_versions=[],
+        download_url="",
+        filename=filename,
+    )
+
+
+def download_info(old: Union[str, None], new: str) -> DownloadInfo:
+    return DownloadInfo(Mod("", "", file=old), version_info(new))
+
+
+@pytest.mark.parametrize(
+    "name,input,pretend,expected",
+    [
+        (
+            "Remove file when new file has been downloaded",
+            download_info("old", "new"),
+            False,
+            True,
+        ),
+        (
+            "Keep file when no new file has been downloaded",
+            download_info("old", "old"),
+            False,
+            False,
+        ),
+        (
+            "Keep old file when new filename is empty",
+            download_info("old", ""),
+            False,
+            False,
+        ),
+        (
+            "Don't remove old file when it doesn't exist",
+            download_info(None, "new"),
+            False,
+            False,
+        ),
+        (
+            "Don't remove old file when it's empty",
+            download_info("", "new"),
+            False,
+            False,
+        ),
+        (
+            "Don't remove old file when --pretend is on",
+            download_info("old", "new"),
+            True,
+            False,
+        ),
+    ],
+)
+def test_on_version_found(name: str, input: DownloadInfo, pretend: bool, expected: bool, mock_repo):
+    print(name)
+
+    config.pretend = pretend
+    if expected:
+        when(mock_repo).remove_mod_file(input.mod.file)
+
+    update = Update(mock_repo)
+    update.on_version_found(input)
+
+    config.pretend = False
+    verifyStubbedInvocationsAreUsed()
+    unstub()