From 42030523c54790f63eda1ed973cf0b13fa4911a2 Mon Sep 17 00:00:00 2001 From: Pravin Barton <9560941+isc-pbarton@users.noreply.github.com> Date: Wed, 2 Sep 2026 14:15:59 -0400 Subject: [PATCH] fix: import all no longer deletes mapped items --- CHANGELOG.md | 3 + cls/SourceControl/Git/Utils.cls | 16 ++-- .../UnitTest/SourceControl/Git/MappedItem.cls | 84 +++++++++++++++++++ 3 files changed, 97 insertions(+), 6 deletions(-) create mode 100644 test/UnitTest/SourceControl/Git/MappedItem.cls diff --git a/CHANGELOG.md b/CHANGELOG.md index 3dd5aba0..f531668d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Record map classes and their generated data classes are now automatically added to source control (#955) +### Fixed +- Import All no longer deletes items that are mapped in from another database, which could delete Embedded Git itself where it is mapped instance-wide (#997) + ## [2.17.1] - 2026-08-18 ### Fixed diff --git a/cls/SourceControl/Git/Utils.cls b/cls/SourceControl/Git/Utils.cls index 240cfbad..d2bb11f3 100644 --- a/cls/SourceControl/Git/Utils.cls +++ b/cls/SourceControl/Git/Utils.cls @@ -1752,12 +1752,16 @@ ClassMethod ImportRoutines(force As %Boolean = 0, pullEventClass As %String) As set externalName = ..ExternalName(item) set fullExternalName = ..FullExternalName(item) if '##class(%File).Exists(fullExternalName) { - write !,fullExternalName," does not exist - deleting ",item - set modification = ##class(SourceControl.Git.Modification).%New() - set modification.changeType = "D" - set modification.internalName = item - set modification.externalName = externalName - set files($increment(files)) = modification + if ##class(%Library.RoutineMgr).IsMapped(item) { + write !,fullExternalName," does not exist, but ",item," is mapped in from another database - not deleting it" + } else { + write !,fullExternalName," does not exist - deleting ",item + set modification = ##class(SourceControl.Git.Modification).%New() + set modification.changeType = "D" + set modification.internalName = item + set modification.externalName = externalName + set files($increment(files)) = modification + } } } diff --git a/test/UnitTest/SourceControl/Git/MappedItem.cls b/test/UnitTest/SourceControl/Git/MappedItem.cls new file mode 100644 index 00000000..d3b072a1 --- /dev/null +++ b/test/UnitTest/SourceControl/Git/MappedItem.cls @@ -0,0 +1,84 @@ +/// Tests that items mapped in from another database are left alone by Import All. +/// Deleting such an item removes it from the database it is mapped from, and so from every namespace that +/// maps it in - that is how Embedded Git, mapped instance-wide, deleted itself. +/// See https://github.com/intersystems/git-source-control/issues/997 . +Class UnitTest.SourceControl.Git.MappedItem Extends UnitTest.SourceControl.Git.AbstractTest +{ + +/// Package mapped in from the %SYS namespace's routine database for the duration of the test. +Parameter MappedPackage As STRING = "gitunittestmapped"; + +Parameter MappedClass As STRING = "gitunittestmapped.TestClass"; + +Property PackageMappingCreated As %Boolean [ InitialExpression = 0 ]; + +Method %OnNew(initvalue) As %Status +{ + $$$QuitOnError(##super(initvalue)) + // mapped items are read-only by default, which keeps them out of source control entirely; + // this test needs the mapped class tracked so that Import All considers deleting it + set settings = ##class(SourceControl.Git.Settings).%New() + set settings.mappedItemsReadOnly = 0 + quit settings.%Save() +} + +Method %OnClose() As %Status [ Private, ServerOnly = 1 ] +{ + // the class lives in the mapped database, so it has to go before the mapping that reaches it + if $$$defClassDefined(..#MappedClass) { + do $system.OBJ.Delete(..#MappedClass) + } + if ..PackageMappingCreated { + do ..DeletePackageMapping(..#MappedPackage) + } + quit ##super() +} + +Method TestMappedClassNotDeleted() +{ + set internalName = ..#MappedClass_".CLS" + + $$$ThrowOnError(..CreatePackageMapping(..#MappedPackage)) + set ..PackageMappingCreated = 1 + + set classDef = ##class(%Dictionary.ClassDefinition).%New() + set classDef.Name = ..#MappedClass + $$$ThrowOnError(classDef.%Save()) + $$$ThrowOnError($system.OBJ.Compile(..#MappedClass, "ck")) + + // the class is stored in the %SYS namespace's database rather than this namespace's own + do $$$AssertTrue(##class(%Library.RoutineMgr).IsMapped(internalName), internalName_" is mapped in from another database") + + do $$$AssertStatusOK(##class(SourceControl.Git.Utils).AddToSourceControl(internalName)) + set fullExternalName = ##class(SourceControl.Git.Utils).FullExternalName(internalName) + do $$$AssertTrue(##class(%File).Exists(fullExternalName), "mapped class exported to "_fullExternalName) + + // the situation that triggered #997: the item is in source control, but has no file on disk + do $$$AssertTrue(##class(%File).Delete(fullExternalName), "deleted "_fullExternalName) + do $$$AssertStatusOK(##class(SourceControl.Git.API).ImportAll(1)) + + do $$$AssertTrue($$$defClassDefined(..#MappedClass), ..#MappedClass_" still exists after Import All") +} + +/// Creates a package mapping in %SYS so that classes in package are stored in the database +/// the %SYS namespace keeps its routines in, rather than this namespace's own. +Method CreatePackageMapping(package As %String) As %Status +{ + set ns = $namespace + new $namespace + set $namespace = "%SYS" + $$$QuitOnError(##class(Config.Namespaces).Get("%SYS", .namespaceProps)) + set props("Database") = namespaceProps("Routines") + quit ##class(Config.MapPackages).Create(ns, package, .props) +} + +/// Deletes the package mapping for package. +ClassMethod DeletePackageMapping(package As %String) As %Status +{ + set ns = $namespace + new $namespace + set $namespace = "%SYS" + quit ##class(Config.MapPackages).Delete(ns, package) +} + +}