From 12f0af4526f2dccce0a8abfae3d747d2c9903834 Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Thu, 12 Feb 2026 11:14:07 +0700 Subject: [PATCH 1/4] Prevent creating objects with duplicated GUIDs --- src/SIL.LCModel/LcmGenerate/factory.vm.cs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/SIL.LCModel/LcmGenerate/factory.vm.cs b/src/SIL.LCModel/LcmGenerate/factory.vm.cs index 21639ae4..c8b4f2be 100644 --- a/src/SIL.LCModel/LcmGenerate/factory.vm.cs +++ b/src/SIL.LCModel/LcmGenerate/factory.vm.cs @@ -64,7 +64,17 @@ internal partial class ${className}$classSfx : I${className}$classSfx, ILcmFacto if (m_cache.ServiceLocator.GetInstance().Singleton != null) throw new InvalidOperationException("Can not create more than one ${className}"); #end - if (guid == Guid.Empty) guid = Guid.NewGuid(); + if (guid == Guid.Empty) + { + guid = Guid.NewGuid(); + } + else + { + if (m_cache.ServiceLocator.GetInstance().TryGetObject(guid, out var _originalObject)) + { + throw new InvalidOperationException("Can not create more than one ${className} with identical GUIDs"); + } + } int hvo = m_cache.InternalServices.DataReader.GetNextRealHvo(); var newby = new $className(m_cache, hvo, guid); #if ( $ownerStatus != "required") From c4793dc77a3e6c58e5252073b63011257f90ebea Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Fri, 13 Feb 2026 15:49:33 +0700 Subject: [PATCH 2/4] Don't retrieve objects, just check if GUID exists TryGetObject would create and populate an object, but here we only want to check whether the GUID we're trying to use is already assigned. So we just use the IsValidObjectId() method, which only returns a bool. We just have to document that here, we want the method to return *false*. --- src/SIL.LCModel/LcmGenerate/factory.vm.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/SIL.LCModel/LcmGenerate/factory.vm.cs b/src/SIL.LCModel/LcmGenerate/factory.vm.cs index c8b4f2be..29a12e24 100644 --- a/src/SIL.LCModel/LcmGenerate/factory.vm.cs +++ b/src/SIL.LCModel/LcmGenerate/factory.vm.cs @@ -70,8 +70,10 @@ internal partial class ${className}$classSfx : I${className}$classSfx, ILcmFacto } else { - if (m_cache.ServiceLocator.GetInstance().TryGetObject(guid, out var _originalObject)) + if (m_cache.ServiceLocator.GetInstance().IsValidObjectId(guid)) { + // IsValidObjectId returns true if the GUID exists, i.e. you could call GetObject and get something. + // But here in Create(), it's an error if the GUID already exists: duplicate GUIDs are not allowed. throw new InvalidOperationException("Can not create more than one ${className} with identical GUIDs"); } } From a57ed16db08d43170a18e9a0ae4b430428095b56 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Fri, 13 Feb 2026 15:50:27 +0100 Subject: [PATCH 3/4] Add tests --- .../DomainImpl/FactoryAdditionsTests.cs | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/tests/SIL.LCModel.Tests/DomainImpl/FactoryAdditionsTests.cs b/tests/SIL.LCModel.Tests/DomainImpl/FactoryAdditionsTests.cs index 0a6edac1..22ab710b 100644 --- a/tests/SIL.LCModel.Tests/DomainImpl/FactoryAdditionsTests.cs +++ b/tests/SIL.LCModel.Tests/DomainImpl/FactoryAdditionsTests.cs @@ -1,7 +1,8 @@ -// Copyright (c) 2015 SIL International +// Copyright (c) 2015 SIL International // This software is licensed under the LGPL, version 2.1 or later // (http://www.gnu.org/licenses/lgpl-2.1.html) +using System; using System.Collections.Generic; using System.Linq; using NUnit.Framework; @@ -132,5 +133,28 @@ public void LexSenseFactoryCreate_NullGlossTss_DoesNotThrow() Assert.DoesNotThrow(() => sense = lexSenseFactory.Create(entry, msa, nullGloss)); Assert.AreEqual(0, sense.Gloss.StringCount); } + + [Test] + public void Create_WithDuplicateGuid_SameType_Throws() + { + var guid = Guid.NewGuid(); + var factory = Cache.ServiceLocator.GetInstance(); + factory.Create(guid); + + Assert.That(() => factory.Create(guid), + Throws.TypeOf().With.Message.Contains("identical GUIDs")); + } + + [Test] + public void Create_WithDuplicateGuid_DifferentType_Throws() + { + var guid = Guid.NewGuid(); + var entryFactory = Cache.ServiceLocator.GetInstance(); + entryFactory.Create(guid); + + var pictureFactory = Cache.ServiceLocator.GetInstance(); + Assert.That(() => pictureFactory.Create(guid), + Throws.TypeOf().With.Message.Contains("identical GUIDs")); + } } } From 6edd1515b59156a48d768fab08d2a84e44ee5d7e Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Fri, 13 Feb 2026 15:50:57 +0100 Subject: [PATCH 4/4] Fix exception message (the rule is global across all types) --- src/SIL.LCModel/LcmGenerate/factory.vm.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SIL.LCModel/LcmGenerate/factory.vm.cs b/src/SIL.LCModel/LcmGenerate/factory.vm.cs index 29a12e24..f169d1d5 100644 --- a/src/SIL.LCModel/LcmGenerate/factory.vm.cs +++ b/src/SIL.LCModel/LcmGenerate/factory.vm.cs @@ -74,7 +74,7 @@ internal partial class ${className}$classSfx : I${className}$classSfx, ILcmFacto { // IsValidObjectId returns true if the GUID exists, i.e. you could call GetObject and get something. // But here in Create(), it's an error if the GUID already exists: duplicate GUIDs are not allowed. - throw new InvalidOperationException("Can not create more than one ${className} with identical GUIDs"); + throw new InvalidOperationException("Can not create more than one object with identical GUIDs"); } } int hvo = m_cache.InternalServices.DataReader.GetNextRealHvo();