Prevent reinstalling same version of existing module with load or reinstall

CHANGELOG.md

@@ -5,7 +5,7 @@ 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).
 
-## [0.10.5] - Unreleased
+## [0.11.0] - Unreleased
 
 ### Added
 - #938 Added flag -export-python-deps to package command
@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - #885: Always synchronously load dependencies and let each module do multi-threading as needed
 to load using multicompile instead of trying to do own multi-threading of item load which causes
 lock contention by bypassing IRIS compiler.
+- #889: The `load` and `install` commands can no longer be used to reinstall the same version as a currently installed module. Instead, use the `-force` flag, the `reinstall` command, or include the `PermitReinstall` default parameter in module.xml
 
 ### Removed
 - #938 Removed secret flag NewVersion handling in %Publish()

src/cls/IPM/Main.cls

@@ -2342,6 +2342,8 @@ ClassMethod Reinstall(ByRef pCommandInfo) [ Internal ]
     // Overriding defaults in the "data" array:
     // For now this is just the "UpdateSnapshots" flag but it could be more eventually.
     set tData("UpdateSnapshots") = 1
+    // Set Reinstall flag to allow same-version installation
+    set tData("Reinstall") = 1
 
     // Note that pCommandInfo("data") may override the default of UpdateSnapshots=1.
     merge tData = pCommandInfo("data")

src/cls/IPM/Utils/Module.cls

@@ -1075,6 +1075,16 @@ ClassMethod LoadNewModule(
                 $$$ThrowStatus($$$ERROR($$$GeneralError, msg))
             }
             set cmd = $get(params("cmd"))
+            // Check for same-version install/load
+            if ((cmd = "install") || (cmd = "load")) && (fromVersionString = toVersionString) {
+                set permitReinstall = $get(params("PermitReinstall"), 0)
+                set forceFlag = $get(params("Force"), 0)
+                set isReinstall = $get(params("Reinstall"), 0)
+                if 'forceFlag && 'permitReinstall && 'isReinstall {
+                    set msg = $$$FormatText("%1 version %2 is already installed.", newModuleObj.Name, fromVersionString)
+                    $$$ThrowStatus($$$ERROR($$$GeneralError, msg))
+                }
+            }
             if ((cmd = "install") || (cmd = "load")) && '$get(params("Force"),0) && '$get(params("DeveloperMode"),0) && (toVersion.Follows(fromVersion)) && (newModuleObj.UpdatePackage '= "") {
                 set msg = $$$FormatText("Cannot perform an update of %1 from version %2 to %3 using the 'install' or 'load' commands. Please use the 'update' command instead. If you are intentionally trying to bypass running update steps, please use the -force flag for 'install' or 'load'.", newModuleObj.Name, fromVersionString, toVersionString)
                 $$$ThrowStatus($$$ERROR($$$GeneralError, msg))

tests/integration_tests/Test/PM/Integration/FileBinaryTar.cls

@@ -5,36 +5,36 @@ Class Test.PM.Integration.FileBinaryTar Extends %UnitTest.TestCase
 
 Method TestPackageAndExtract()
 {
-  Set tSC = $$$OK
-  Try {
-    Set tTestRoot = ##class(%File).NormalizeDirectory($Get(^UnitTestRoot))
-
-    Set tModuleDir = ##class(%File).NormalizeDirectory(##class(%File).GetDirectory(tTestRoot)_"/_data/simple-module/")
-    Set tSC = ##class(%IPM.Main).Shell("load "_tModuleDir)
-    Do $$$AssertStatusOK(tSC,"Loaded SimpleModule module successfully.")
-
-    Set tempDir = ##class(%Library.File).TempFilename()_"dir"
-    Set tSC = ##class(%IPM.Main).Shell("simplemodule package -only -DPath="_tempDir)
-    Do $$$AssertStatusOK(tSC,"Packaged SimpleModule successfully.")
-
-    Set outFile = ##class(%Library.File).TempFilename()
-    Set outDir = ##class(%Library.File).NormalizeDirectory(##class(%Library.File).TempFilename()_"dir-out")
-    Do ##class(%Library.File).CreateDirectoryChain(outDir)
-    Do $$$AssertEquals($zf(-100,"/STDOUT="""_outFile_"""/STDERR="""_outFile_"""","tar","-xvf",tempDir_".tgz","-C",outDir),0)
-
-    Do $$$AssertNotTrue(##class(%File).DirectoryExists(outDir_"src/cls/Test/Test"))
-    Do $$$AssertNotTrue(##class(%File).DirectoryExists(outDir_"src/src"))
-    Do $$$AssertNotTrue(##class(%File).DirectoryExists(outDir_"simplemodule"))
-    Do $$$AssertTrue(##class(%File).Exists(outDir_"src/cls/Test/Test.cls"))
-    Do $$$AssertTrue(##class(%File).Exists(outDir_"module.xml"))
-
-    Set tSC = ##class(%IPM.Main).Shell("load "_tempDir_".tgz")
-    Do $$$AssertStatusOK(tSC,"Loaded SimpleModule module successfully from .tgz file.")
-
-    Set tSC = ##class(%IPM.Main).Shell("load "_outDir)
-    Do $$$AssertStatusOK(tSC,"Loaded SimpleModule module successfully from package directory.")
-  } Catch e {
-    Do $$$AssertStatusOK(e.AsStatus(),"An exception occurred.")
+  set tSC = $$$OK
+  try {
+    set tTestRoot = ##class(%File).NormalizeDirectory($get(^UnitTestRoot))
+
+    set tModuleDir = ##class(%File).NormalizeDirectory(##class(%File).GetDirectory(tTestRoot)_"/_data/simple-module/")
+    set tSC = ##class(%IPM.Main).Shell("load "_tModuleDir)
+    do $$$AssertStatusOK(tSC,"Loaded SimpleModule module successfully.")
+
+    set tempDir = ##class(%Library.File).TempFilename()_"dir"
+    set tSC = ##class(%IPM.Main).Shell("simplemodule package -only -DPath="_tempDir)
+    do $$$AssertStatusOK(tSC,"Packaged SimpleModule successfully.")
+
+    set outFile = ##class(%Library.File).TempFilename()
+    set outDir = ##class(%Library.File).NormalizeDirectory(##class(%Library.File).TempFilename()_"dir-out")
+    do ##class(%Library.File).CreateDirectoryChain(outDir)
+    do $$$AssertEquals($zf(-100,"/STDOUT="""_outFile_"""/STDERR="""_outFile_"""","tar","-xvf",tempDir_".tgz","-C",outDir),0)
+
+    do $$$AssertNotTrue(##class(%File).DirectoryExists(outDir_"src/cls/Test/Test"))
+    do $$$AssertNotTrue(##class(%File).DirectoryExists(outDir_"src/src"))
+    do $$$AssertNotTrue(##class(%File).DirectoryExists(outDir_"simplemodule"))
+    do $$$AssertTrue(##class(%File).Exists(outDir_"src/cls/Test/Test.cls"))
+    do $$$AssertTrue(##class(%File).Exists(outDir_"module.xml"))
+
+    set tSC = ##class(%IPM.Main).Shell("load -force "_tempDir_".tgz")
+    do $$$AssertStatusOK(tSC,"Loaded SimpleModule module successfully from .tgz file.")
+
+    set tSC = ##class(%IPM.Main).Shell("load -force "_outDir)
+    do $$$AssertStatusOK(tSC,"Loaded SimpleModule module successfully from package directory.")
+  } catch e {
+    do $$$AssertStatusOK(e.AsStatus(),"An exception occurred.")
   }
 }
 

tests/integration_tests/Test/PM/Integration/FileCopy.cls

@@ -9,6 +9,13 @@ Method OnBeforeAllTests() As %Status
     return status
 }
 
+Method OnAfterOneTest() As %Status
+{
+    // Uninstall modules
+    set status = ##class(%IPM.Main).Shell("uninstall -all")
+    return status
+}
+
 Method TestModuleNoDependency()
 {
     set status = $$$OK

tests/integration_tests/Test/PM/Integration/Health.cls

@@ -5,7 +5,14 @@ Parameter CommonPathPrefix As STRING = "health-test";
 
 Method OnBeforeAllTests() As %Status
 {
-  Quit ##class(%Library.EnsembleMgr).EnableNamespace($Namespace)
+  quit ##class(%Library.EnsembleMgr).EnableNamespace($namespace)
+}
+
+Method OnAfterOneTest() As %Status
+{
+    // Uninstall modules
+    set status = ##class(%IPM.Main).Shell("uninstall -all")
+    return status
 }
 
 Method TestHealthTrue()

tests/integration_tests/Test/PM/Integration/InstallModule.cls

@@ -164,6 +164,73 @@ Method TestDevMode()
     $$$ThrowOnError(##class(%IPM.Main).Shell("uninstall update-simple"))
 }
 
+/// Verify that attempting to install/load the same version fails with informational message
+/// unless -force flag is used, and that reinstall command always works
+Method TestSameVersionInstallPrevention()
+{
+    set sc = ##class(%IPM.Main).Shell("install -v update-simple 2.1.1")
+    do $$$AssertStatusOK(sc,"Installed update-simple 2.1.1")
+
+    // Test install command with same version - should fail with informational message
+    set sc = ##class(%IPM.Main).Shell("install -v update-simple 2.1.1")
+    do $$$AssertStatusNotOK(sc,"Trying to install same version of update-simple without -force flag fails")
+    do $$$AssertTrue($system.Status.GetOneErrorText(sc) [ "update-simple version 2.1.1 is already installed.", "Error message indicates version is already installed")
+
+    // Test install command with same version and -force flag - should succeed
+    set sc = ##class(%IPM.Main).Shell("install -v -force update-simple 2.1.1")
+    do $$$AssertStatusOK(sc,"Trying to install same version of update-simple with -force flag succeeds")
+
+    // Test reinstall command - should always work
+    set sc = ##class(%IPM.Main).Shell("reinstall update-simple")
+    do $$$AssertStatusOK(sc,"Reinstalling update-simple with reinstall command succeeds")
+
+    // Clean up the module after test is completed
+    $$$ThrowOnError(##class(%IPM.Main).Shell("uninstall update-simple"))
+}
+
+/// Verify that load command also prevents same-version installation with a message
+Method TestSameVersionLoadPrevention()
+{
+    // Get test root and prepare module directory
+    set tTestRoot = ##class(%File).NormalizeDirectory($get(^UnitTestRoot))
+    set tModuleDir = ##class(%File).NormalizeDirectory(##class(%File).GetDirectory(tTestRoot)_"/_data/simple-module/")
+
+    // First load
+    set sc = ##class(%IPM.Main).Shell("load "_tModuleDir)
+    do $$$AssertStatusOK(sc,"Loaded SimpleModule from directory")
+
+    // Try to load same version again - should fail with informational message
+    set sc = ##class(%IPM.Main).Shell("load "_tModuleDir)
+    do $$$AssertStatusNotOK(sc,"Trying to load same version without -force flag fails")
+    do $$$AssertTrue($system.Status.GetOneErrorText(sc) [ "simplemodule version 0.0.1+snapshot is already installed.", "Error message indicates version is already installed")
+
+    // Try to load with -force flag - should succeed
+    set sc = ##class(%IPM.Main).Shell("load -force "_tModuleDir)
+    do $$$AssertStatusOK(sc,"Trying to load same version with -force flag succeeds")
+
+    // Clean up the module after test is completed
+    $$$ThrowOnError(##class(%IPM.Main).Shell("uninstall SimpleModule"))
+}
+
+/// Verify that PermitReinstall parameter allows same-version installation without -force flag
+Method TestPermitReinstallParameter()
+{
+    // Get test root and prepare module directory
+    set tTestRoot = ##class(%File).NormalizeDirectory($get(^UnitTestRoot))
+    set tModuleDir = ##class(%File).NormalizeDirectory(##class(%File).GetDirectory(tTestRoot)_"/_data/permit-reinstall/")
+
+    // First load
+    set sc = ##class(%IPM.Main).Shell("load "_tModuleDir)
+    do $$$AssertStatusOK(sc,"Loaded PermitReinstall module")
+
+    // Try to load same version again - should succeed because PermitReinstall=1 in module.xml
+    set sc = ##class(%IPM.Main).Shell("load "_tModuleDir)
+    do $$$AssertStatusOK(sc,"Loading same version with PermitReinstall=1 parameter succeeds")
+
+    // Clean up the module after test is completed
+    $$$ThrowOnError(##class(%IPM.Main).Shell("uninstall PermitReinstall"))
+}
+
 Method ConfirmUpdateStepsExist(
 	updateStepsToSeed As %DynamicArray,
 	doesExist As %Boolean)

tests/integration_tests/Test/PM/Integration/LoadModuleWithDeps.cls

@@ -3,6 +3,13 @@ Class Test.PM.Integration.LoadModuleWithDeps Extends Test.PM.Integration.Base
 
 Parameter ModuleFolder = "load-export";
 
+Method OnAfterOneTest() As %Status
+{
+    // Uninstall modules
+    set status = ##class(%IPM.Main).Shell("uninstall -all")
+    return status
+}
+
 Method TestLoadModuleWithDependencies() As %Status
 {
     /// Load from folder

tests/integration_tests/Test/PM/Integration/ModuleVersion.cls

@@ -71,6 +71,8 @@ Method TestModuleVersion()
         set sc = ##class(%IPM.Main).Shell("modver mv " _ args)
         if expected = "" {
             do $$$AssertStatusNotOK(sc, "Module-version failed as expected")
+            set sc = ##class(%IPM.Main).Shell("uninstall mv")
+            do $$$AssertStatusOK(sc, "Module 'mv' successfully uninstalled between test cases.")
             continue
         }
         do $$$AssertStatusOK(sc, "Module-version executed successfully")

tests/integration_tests/Test/PM/Integration/ProcessPythonWheel.cls

@@ -15,6 +15,13 @@ Parameter PackageWithPythonDepsLocation = "package-with-python-deps";
 
 Parameter ExportPackageWithPythonDepsLocation = "export-package-with-python-deps";
 
+Method OnAfterOneTest() As %Status
+{
+    // Uninstall modules
+    set status = ##class(%IPM.Main).Shell("uninstall -all")
+    return status
+}
+
 /// The PythonWheel resource processor expects to install wheels from the dist directory, as opposed to source code
 ClassMethod GenerateWheel()
 {

tests/integration_tests/Test/PM/Integration/_data/permit-reinstall/module.xml

@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<Export generator="Cache" version="25">
+  <Document name="permitreinstall.ZPM">
+    <Module>
+      <Name>PermitReinstall</Name>
+      <Version>1.0.0</Version>
+      <Packaging>module</Packaging>
+      <SourcesRoot>src</SourcesRoot>
+      <Resource Directory="cls" Name="PermitReinstall.Test.cls"/>
+      <Defaults>
+        <Parameter Name="PermitReinstall">1</Parameter>
+      </Defaults>
+    </Module>
+  </Document>
+</Export>

tests/integration_tests/Test/PM/Integration/_data/permit-reinstall/src/cls/PermitReinstall/Test.cls

@@ -0,0 +1,4 @@
+Class PermitReinstall.Test
+{
+
+}

tests/unit_tests/Test/PM/Unit/Oras.cls

@@ -54,11 +54,13 @@ Method TestOras()
     do ..RunCommand("repo -publish 1 -n oci")
 
     // Install from default
+    do ..RunCommand("uninstall " _ Name)
     do ..AssertNoException("install " _ Name)
 
     // Clean up
     do ..RunCommand("repo -delete-all")
     do ..RunCommand("repo -reset-defaults")
+    do ..RunCommand("uninstall " _ Name)
 }
 
 Method RunCommand(pCommand As %String)