From 6f377d7446071bc04eeabfda39cd0b1e8ab2b763 Mon Sep 17 00:00:00 2001 From: Gabriel Ramos Date: Thu, 1 Oct 2026 19:34:31 -0300 Subject: [PATCH 1/3] Validate storage name size to comply with MongoDB --- cli/cli/App.cs | 9 + cli/cli/Commands/Project/NewStorageCommand.cs | 26 +++ cli/cli/Services/BeamoLocalSystem.cs | 7 + cli/cli/Services/CliRequester.cs | 15 ++ cli/cli/Services/DeploymentService.cs | 13 ++ cli/cli/Services/ServiceUploadUtil.cs | 11 +- cli/cli/Services/StorageNameValidator.cs | 107 +++++++++++ cli/tests/StorageNameValidatorTests.cs | 177 ++++++++++++++++++ .../StorageObjectConnectionProvider.cs | 55 +++++- .../microservice/StorageDatabaseNameTests.cs | 59 ++++++ 10 files changed, 470 insertions(+), 9 deletions(-) create mode 100644 cli/cli/Services/StorageNameValidator.cs create mode 100644 cli/tests/StorageNameValidatorTests.cs create mode 100644 microservice/microserviceTests/microservice/StorageDatabaseNameTests.cs diff --git a/cli/cli/App.cs b/cli/cli/App.cs index 935609f115..6642b788e0 100644 --- a/cli/cli/App.cs +++ b/cli/cli/App.cs @@ -1454,6 +1454,15 @@ protected virtual Parser GetProgram() Console.Error.WriteLine("\nLogs at\n " + Path.GetFullPath(logFile)); } } + + // console-only: a CliException's message can end up above a long stack trace (e.g. when wrapped in an + // AggregateException during manifest init), so repeat it last, in red, where it's seen without scrolling. + var cliError = ex as CliException ?? + (ex as AggregateException)?.Flatten().InnerExceptions.OfType().FirstOrDefault(); + if (cliError != null && !appContext.UsePipeOutput && !appContext.ShowRawOutput) + { + AnsiConsole.MarkupLine($"\n[red]{Markup.Escape(cliError.Message)}[/]"); + } }); diff --git a/cli/cli/Commands/Project/NewStorageCommand.cs b/cli/cli/Commands/Project/NewStorageCommand.cs index 862d9298a2..a5d93e2701 100644 --- a/cli/cli/Commands/Project/NewStorageCommand.cs +++ b/cli/cli/Commands/Project/NewStorageCommand.cs @@ -67,6 +67,8 @@ public override void Configure() public override async Task Handle(NewStorageCommandArgs args) { await args.CreateConfigIfNeeded(_initCommand); + StorageNameValidator.ValidateNewStorage(args.AppContext.Cid, await ResolveTargetScope(args), args.ProjectName, + args.IsZone); var newMicroserviceInfo = await args.ProjectService.CreateNewStorage(args); Log.Information( $"Registering local project... 'beam services register --id {args.ProjectName} --type EmbeddedMongoDb'"); @@ -118,6 +120,30 @@ public override async Task Handle(NewStorageCommandArgs args) await sequence; } + // The id the storage's database name will be scoped by: the pid for a realm storage, the zid for a zone storage. + // Null when it can't be resolved (e.g. not logged in), in which case the name check only warns. + private static async Task ResolveTargetScope(NewStorageCommandArgs args) + { + if (!args.IsZone) + { + return args.AppContext.Pid; + } + + var localZid = args.ConfigService.GetConfigString(ConfigService.CFG_JSON_FIELD_ZID); + try + { + return await ZoneResolver.ResolveZid(args.DependencyProvider, args.AppContext.Cid, args.AppContext.Pid, + localZid); + } + catch (Exception ex) + { + // a selected realm's zone binding is authoritative, so don't guess with the local zid; an unresolved + // zone leaves the name check warn-only. + Log.Debug($"Could not resolve the zone for the storage name check: {ex.Message}"); + return null; + } + } + private async Promise UpdateDependencyDockerFile(NewStorageCommandArgs args, BeamoServiceDefinition definition) { await args.BeamoLocalSystem.UpdateDockerFile(definition); diff --git a/cli/cli/Services/BeamoLocalSystem.cs b/cli/cli/Services/BeamoLocalSystem.cs index f922f726cc..b89b05820d 100644 --- a/cli/cli/Services/BeamoLocalSystem.cs +++ b/cli/cli/Services/BeamoLocalSystem.cs @@ -124,6 +124,13 @@ public async Task InitManifest(bool useManifestCache=true, bool fetchServerManif { Log.Warning(conflict); } + + // A storage name that makes its MongoDB database name too long breaks both local runs and deploys, even when + // it was renamed by hand rather than through the CLI. In-memory only: zone storages use the local zid, and + // are skipped (left to the deploy check and the runtime guard) when none is set. + var localZid = _configService.GetConfigString(ConfigService.CFG_JSON_FIELD_ZID); + StorageNameValidator.ThrowIfAny( + StorageNameValidator.FindTooLong(BeamoManifest.ServiceDefinitions, _ctx.Cid, _ctx.Pid, localZid)); } private static Uri GetLocalDockerEndpoint(ConfigService config) diff --git a/cli/cli/Services/CliRequester.cs b/cli/cli/Services/CliRequester.cs index 725e6e153a..ec32097bb7 100644 --- a/cli/cli/Services/CliRequester.cs +++ b/cli/cli/Services/CliRequester.cs @@ -78,6 +78,21 @@ private string GetEffectiveScope() return string.IsNullOrEmpty(pid) ? cid : $"{cid}.{pid}"; } + /// + /// The cid and scope id the next request targets: parsed from when set (a zone + /// deploy's {cid}.{zid} gives the zid), otherwise the current cid and pid. + /// + public (string cid, string scopeId) GetEffectiveScopeIds() + { + if (string.IsNullOrEmpty(BeamScopeOverride)) + { + return (AccessToken?.Cid ?? Cid, AccessToken?.Pid ?? Pid); + } + + var scopeParts = BeamScopeOverride.Split('.', 2); + return (scopeParts[0], scopeParts.Length > 1 ? scopeParts[1] : AccessToken?.Pid ?? Pid); + } + private sealed class ScopeResetter : IDisposable { private readonly CliRequester _requester; diff --git a/cli/cli/Services/DeploymentService.cs b/cli/cli/Services/DeploymentService.cs index c3110ad235..b2ce3dc871 100644 --- a/cli/cli/Services/DeploymentService.cs +++ b/cli/cli/Services/DeploymentService.cs @@ -1588,6 +1588,19 @@ public static bool IsJsonAManifest(IDictionary data) } else { + // A storage whose MongoDB database name doesn't fit the target would only fail once the manifest is + // posted or the service first touches it; stop before building anything. The target is the requester's + // scope: `{cid}.{zid}` for a zone deploy (resolved by ApplyDeployScopeAsync), else the realm's cid/pid. + { + var (targetCid, targetScopeId) = provider.GetService().GetEffectiveScopeIds(); + + // only this scope's storages are deployed, so the one scope id serves as both pid and zid. + var scopedDefinitions = beamo.BeamoManifest.ServiceDefinitions + .Where(d => d.IsZoneScoped == (args.Scope == DeployScope.Zone)); + StorageNameValidator.ThrowIfAny(StorageNameValidator.FindTooLong( + scopedDefinitions, targetCid, targetScopeId, targetScopeId, includeOnlyBeamoIds)); + } + // Docker is only needed to build service images. A filtered plan whose set contains no // local microservice (e.g. a portal-extension-only bundle) can run without the daemon. var needsServiceBuild = beamo.BeamoManifest.ServiceDefinitions.Any(d => diff --git a/cli/cli/Services/ServiceUploadUtil.cs b/cli/cli/Services/ServiceUploadUtil.cs index 2c146c5639..9974e4264a 100644 --- a/cli/cli/Services/ServiceUploadUtil.cs +++ b/cli/cli/Services/ServiceUploadUtil.cs @@ -191,14 +191,9 @@ void TrackProgress(float ratio) // `{cid}.{zid}`) and send the zone id in the SAME x-ks-projectid header — no new headers. The repo // itself is already the scoped one returned by /api/beamo/registry-uri for this scope. var registryRequester = provider.GetService(); - var registryClientId = ctx.Cid; - var registryProjectId = ctx.Pid; - if (registryRequester is CliRequester cliRequester && !string.IsNullOrEmpty(cliRequester.BeamScopeOverride)) - { - var scopeParts = cliRequester.BeamScopeOverride.Split('.', 2); - registryClientId = scopeParts[0]; - if (scopeParts.Length > 1) registryProjectId = scopeParts[1]; - } + var (registryClientId, registryProjectId) = registryRequester is CliRequester cliRequester + ? cliRequester.GetEffectiveScopeIds() + : (ctx.Cid, ctx.Pid); Log.Information($"[{beamoId}] registry auth headers: x-ks-clientid=[{registryClientId}] x-ks-projectid=[{registryProjectId}]"); client.DefaultRequestHeaders.Add("x-ks-clientid", registryClientId); diff --git a/cli/cli/Services/StorageNameValidator.cs b/cli/cli/Services/StorageNameValidator.cs new file mode 100644 index 0000000000..e443699b28 --- /dev/null +++ b/cli/cli/Services/StorageNameValidator.cs @@ -0,0 +1,107 @@ +using Beamable.Server; + +namespace cli.Services; + +/// +/// Validates that local storage names fit in a MongoDB database name. A storage maps to the database +/// {cid}{scope}_{storageName} (see ), where the scope is the pid for a realm +/// storage and the zid for a zone storage, so how long a storage name may be depends on the target cid and scope. +/// MongoDB rejects the name locally and remotely alike, so this guards both local runs and deploys. +/// +public static class StorageNameValidator +{ + // Worst-case ids are snowflake longs of up to 19 digits: cid (19), pid "DE_" + 19, zid "ZONE_" + 19. + private const int WorstCaseCidLength = 19; + public const int WorstCaseRealmStorageNameLength = StorageDatabaseName.MaxLength - WorstCaseCidLength - 22 - 1; + public const int WorstCaseZoneStorageNameLength = StorageDatabaseName.MaxLength - WorstCaseCidLength - 24 - 1; + + /// + /// Validates the name of a storage about to be created. With a known cid and scope (pid, or zid for a zone + /// storage) a name that doesn't fit throws; without them, the name is only compared against the worst-case + /// budget and a warning is logged, since the target isn't known yet. + /// + public static void ValidateNewStorage(string cid, string scope, string storageName, bool isZone) + { + if (IsNumericCid(cid) && !string.IsNullOrEmpty(scope)) + { + if (StorageDatabaseName.IsTooLong(cid, scope, storageName)) + { + ThrowIfAny(new List { StorageDatabaseName.DescribeTooLong(cid, scope, storageName) }); + } + + return; + } + + var worstCase = isZone ? WorstCaseZoneStorageNameLength : WorstCaseRealmStorageNameLength; + if (storageName.Length > worstCase) + { + Log.Warning($"Storage [{storageName}] is {storageName.Length} characters long. Its MongoDB database name " + + $"is {{cid}}{{{(isZone ? "zid" : "pid")}}}_{{storageName}} (at most {StorageDatabaseName.MaxLength} " + + $"characters), and the {(isZone ? "zone" : "realm")} isn't known yet, so it can't be checked. " + + $"Names up to {worstCase} characters always fit; a longer name may fail once you run or deploy it."); + } + } + + /// + /// Returns one message per local storage whose database name would be too long. Realm storages are checked + /// against and zone storages against ; a storage whose scope id is + /// unknown (null/empty) is skipped, as is everything when isn't a numeric customer id. + /// + /// The manifest's definitions; only local storages are checked. + /// When set, only these storages are checked. + public static List FindTooLong( + IEnumerable definitions, + string cid, + string pid, + string zid, + ICollection includeOnlyBeamoIds = null) + { + var errors = new List(); + if (!IsNumericCid(cid)) + { + return errors; + } + + foreach (var definition in definitions) + { + if (!definition.IsLocal || definition.Protocol != BeamoProtocolType.EmbeddedMongoDb) + continue; + if (includeOnlyBeamoIds != null && !includeOnlyBeamoIds.Contains(definition.BeamoId)) + continue; + + var scope = definition.IsZoneScoped ? zid : pid; + if (string.IsNullOrEmpty(scope)) + { + Log.Trace($"Skipping storage name length check for storage=[{definition.BeamoId}]; no " + + $"{(definition.IsZoneScoped ? "zid" : "pid")} is known."); + continue; + } + + if (StorageDatabaseName.IsTooLong(cid, scope, definition.BeamoId)) + { + errors.Add(StorageDatabaseName.DescribeTooLong(cid, scope, definition.BeamoId)); + } + } + + return errors; + } + + /// + /// Throws a single listing every storage in , if any. + /// + public static void ThrowIfAny(List errors) + { + if (errors.Count == 0) + { + return; + } + + var lines = string.Join(Environment.NewLine, errors.Select(e => " - " + e)); + throw new CliException( + $"Some storage names are too long for MongoDB:{Environment.NewLine}{lines}{Environment.NewLine}" + + "Rename these storages to fit: change the property (or the .csproj name when isn't set) " + + "and the matching [StorageObject] attribute name."); + } + + private static bool IsNumericCid(string cid) => !string.IsNullOrEmpty(cid) && cid.All(char.IsDigit); +} diff --git a/cli/tests/StorageNameValidatorTests.cs b/cli/tests/StorageNameValidatorTests.cs new file mode 100644 index 0000000000..09cb10da79 --- /dev/null +++ b/cli/tests/StorageNameValidatorTests.cs @@ -0,0 +1,177 @@ +using System.Collections.Generic; +using Beamable.Server; +using cli; +using cli.Services; +using NUnit.Framework; + +namespace tests; + +/// +/// A storage maps to the MongoDB database {cid}{scope}_{storageName} (scope = pid for realm storages, zid for +/// zone storages), and MongoDB rejects names longer than 63 characters, locally and remotely. The CLI must catch a +/// name that doesn't fit before running or deploying it. +/// +[TestFixture] +public class StorageNameValidatorTests +{ + const string Cid = "1706624984549280"; // 16 + const string Pid = "DE_1706624984549283"; // 19 -> realm budget 63 - 16 - 19 - 1 = 27 + const string Zid = "ZONE_100000000000000000"; // 23 -> zone budget 63 - 16 - 23 - 1 = 23 + + static BeamoServiceDefinition Storage(string beamoId, bool zone = false, bool local = true) => new() + { + BeamoId = beamoId, + Protocol = BeamoProtocolType.EmbeddedMongoDb, + ServiceScope = zone ? "zone" : null, + ProjectPath = local ? $"services/{beamoId}/{beamoId}.csproj" : null, + }; + + static BeamoServiceDefinition Service(string beamoId) => new() + { + BeamoId = beamoId, + Protocol = BeamoProtocolType.HttpMicroservice, + ProjectPath = $"services/{beamoId}/{beamoId}.csproj", + }; + + // ---- StorageDatabaseName ---- + + [Test] + public void Compose_ConcatenatesCidScopeAndName() + { + Assert.That(StorageDatabaseName.Compose(Cid, Pid, "Inv"), Is.EqualTo($"{Cid}{Pid}_Inv")); + } + + [Test] + public void MaxStorageNameLength_IsWhatIsLeftOfTheDatabaseName() + { + Assert.That(StorageDatabaseName.MaxStorageNameLength(Cid, Pid), Is.EqualTo(27)); + Assert.That(StorageDatabaseName.MaxStorageNameLength(Cid, Zid), Is.EqualTo(23)); + } + + [Test] + public void IsTooLong_At63Characters_Fits() + { + var name = new string('a', 27); + Assert.That(StorageDatabaseName.Compose(Cid, Pid, name).Length, Is.EqualTo(63)); + Assert.That(StorageDatabaseName.IsTooLong(Cid, Pid, name), Is.False); + } + + [Test] + public void IsTooLong_At64Characters_DoesNotFit() + { + Assert.That(StorageDatabaseName.IsTooLong(Cid, Pid, new string('a', 28)), Is.True); + } + + [Test] + public void DescribeTooLong_NamesTheStorageDatabaseAndBudget() + { + var name = new string('a', 24); + var message = StorageDatabaseName.DescribeTooLong(Cid, Zid, name); + + Assert.That(message, Does.Contain($"[{name}]")); + Assert.That(message, Does.Contain($"zone [{Zid}]")); + Assert.That(message, Does.Contain($"[{Cid}{Zid}_{name}]")); + Assert.That(message, Does.Contain("64 characters")); + Assert.That(message, Does.Contain("at most 23 characters")); + } + + // ---- FindTooLong ---- + + [Test] + public void FindTooLong_ChecksRealmStoragesAgainstPidAndZoneStoragesAgainstZid() + { + // 25 characters fits the realm (27) but not the zone (23). + var name = new string('a', 25); + var errors = StorageNameValidator.FindTooLong( + new[] { Storage(name), Storage("Z" + name, zone: true) }, Cid, Pid, Zid); + + Assert.That(errors, Has.Count.EqualTo(1)); + Assert.That(errors[0], Does.Contain($"[Z{name}]")); + } + + [Test] + public void FindTooLong_SkipsZoneStoragesWhenNoZidIsKnown() + { + var errors = StorageNameValidator.FindTooLong( + new[] { Storage(new string('a', 40), zone: true) }, Cid, Pid, zid: null); + + Assert.That(errors, Is.Empty); + } + + [Test] + public void FindTooLong_SkipsEverythingWithoutANumericCid() + { + var definitions = new[] { Storage(new string('a', 40)) }; + + Assert.That(StorageNameValidator.FindTooLong(definitions, null, Pid, Zid), Is.Empty); + Assert.That(StorageNameValidator.FindTooLong(definitions, "my-alias", Pid, Zid), Is.Empty); + } + + [Test] + public void FindTooLong_IgnoresServicesAndRemoteOnlyStorages() + { + var longName = new string('a', 40); + var errors = StorageNameValidator.FindTooLong( + new[] { Service(longName), Storage("R" + longName, local: false) }, Cid, Pid, Zid); + + Assert.That(errors, Is.Empty); + } + + [Test] + public void FindTooLong_OnlyChecksIncludedStorages() + { + var tooLong = new string('a', 30); + var definitions = new[] { Storage(tooLong), Storage("Other") }; + + Assert.That(StorageNameValidator.FindTooLong(definitions, Cid, Pid, Zid, new HashSet { "Other" }), + Is.Empty); + } + + [Test] + public void ThrowIfAny_ListsEveryOffenderInOneError() + { + var errors = StorageNameValidator.FindTooLong( + new[] { Storage(new string('a', 30)), Storage(new string('b', 30)) }, Cid, Pid, Zid); + + var ex = Assert.Throws(() => StorageNameValidator.ThrowIfAny(errors)); + Assert.That(ex!.Message, Does.Contain(new string('a', 30))); + Assert.That(ex.Message, Does.Contain(new string('b', 30))); + } + + [Test] + public void ThrowIfAny_WithNoErrors_DoesNothing() + { + Assert.DoesNotThrow(() => StorageNameValidator.ThrowIfAny(new List())); + } + + // ---- ValidateNewStorage ---- + + [Test] + public void ValidateNewStorage_WithKnownTarget_ThrowsWhenTooLong() + { + Assert.Throws(() => + StorageNameValidator.ValidateNewStorage(Cid, Zid, new string('a', 24), isZone: true)); + } + + [Test] + public void ValidateNewStorage_WithKnownTarget_PassesAtTheLimit() + { + Assert.DoesNotThrow(() => + StorageNameValidator.ValidateNewStorage(Cid, Zid, new string('a', 23), isZone: true)); + } + + [Test] + public void ValidateNewStorage_WithoutTarget_OnlyWarns() + { + Assert.DoesNotThrow(() => + StorageNameValidator.ValidateNewStorage(null, null, new string('a', 40), isZone: true)); + } + + [Test] + public void WorstCaseBudgets_MatchTheLongestPossibleIds() + { + // cid 19 digits, pid "DE_" + 19, zid "ZONE_" + 19. + Assert.That(StorageNameValidator.WorstCaseRealmStorageNameLength, Is.EqualTo(21)); + Assert.That(StorageNameValidator.WorstCaseZoneStorageNameLength, Is.EqualTo(19)); + } +} diff --git a/microservice/beamable.tooling.common/SharedRuntime/StorageObjectConnectionProvider.cs b/microservice/beamable.tooling.common/SharedRuntime/StorageObjectConnectionProvider.cs index 3a888f7bd9..a02e82536a 100644 --- a/microservice/beamable.tooling.common/SharedRuntime/StorageObjectConnectionProvider.cs +++ b/microservice/beamable.tooling.common/SharedRuntime/StorageObjectConnectionProvider.cs @@ -128,6 +128,12 @@ public async Promise> GetCollection GetDatabaseByStorageName(string storageName) { + // MongoDB would reject the name on the first call with a bare InvalidNamespace; fail early and explain why. + if (StorageDatabaseName.IsTooLong(_realmInfo.CustomerID, _realmInfo.ProjectName, storageName)) + { + throw new StorageNameTooLongException(_realmInfo.CustomerID, _realmInfo.ProjectName, storageName); + } + var connStr = await GetConnectionString(storageName); var clientSettings = MongoClientSettings.FromConnectionString(connStr); @@ -140,7 +146,7 @@ private async Promise GetDatabaseByStorageName(string storageNam }; var client = new MongoClient(clientSettings); - var db = client.GetDatabase($"{_realmInfo.CustomerID}{_realmInfo.ProjectName}_{storageName}"); + var db = client.GetDatabase(StorageDatabaseName.Compose(_realmInfo.CustomerID, _realmInfo.ProjectName, storageName)); return db; } @@ -178,5 +184,52 @@ public class ConnectionStringException : MicroserviceException { public ConnectionStringException(string storageName) : base(500, "InvalidConnectionString", $"Connection string for storage name '{storageName}' is null or empty.") { } } + + [Serializable] + public class StorageNameTooLongException : MicroserviceException + { + public StorageNameTooLongException(string cid, string scope, string storageName) : base(500, "StorageNameTooLong", StorageDatabaseName.DescribeTooLong(cid, scope, storageName)) { } + } + } + + /// + /// The MongoDB database name a storage object maps to: {cid}{scope}_{storageName}, where the scope is the + /// pid for a realm-scoped storage and the zid for a zone-scoped one. MongoDB rejects database names longer than + /// characters (locally and on Atlas), so a storage name only fits when the whole composed + /// name does. Shared by the microservice runtime and the CLI so both apply the same rule. + /// + public static class StorageDatabaseName + { + /// + /// The longest database name MongoDB accepts. + /// + public const int MaxLength = 63; + + public static string Compose(string cid, string scope, string storageName) => $"{cid}{scope}_{storageName}"; + + public static bool IsTooLong(string cid, string scope, string storageName) => + Compose(cid, scope, storageName).Length > MaxLength; + + /// + /// The longest storage name that still fits for the given cid and scope (pid or zid). + /// + public static int MaxStorageNameLength(string cid, string scope) => + Math.Max(0, MaxLength - (cid?.Length ?? 0) - (scope?.Length ?? 0) - 1); + + /// + /// A human-readable explanation of why the storage name doesn't fit, naming the composed database name, + /// its length, and the longest storage name allowed for this cid and scope. + /// + public static string DescribeTooLong(string cid, string scope, string storageName) + { + var databaseName = Compose(cid, scope, storageName); + var scopeKind = IsZoneId(scope) ? "zone" : "realm"; + return $"Storage [{storageName}] is too long for {scopeKind} [{scope}]: its MongoDB database name " + + $"[{databaseName}] would be {databaseName.Length} characters, but MongoDB allows at most {MaxLength}. " + + $"Storage names in this {scopeKind} can be at most {MaxStorageNameLength(cid, scope)} characters."; + } + + private static bool IsZoneId(string scope) => + scope != null && scope.StartsWith("ZONE_", StringComparison.OrdinalIgnoreCase); } } diff --git a/microservice/microserviceTests/microservice/StorageDatabaseNameTests.cs b/microservice/microserviceTests/microservice/StorageDatabaseNameTests.cs new file mode 100644 index 0000000000..4189f158e6 --- /dev/null +++ b/microservice/microserviceTests/microservice/StorageDatabaseNameTests.cs @@ -0,0 +1,59 @@ +using System; +using System.Threading.Tasks; +using Beamable.Server; +using NUnit.Framework; + +namespace microserviceTests.microservice +{ + /// + /// A storage's MongoDB database is {cid}{pid}_{storageName} (pid is the zid for zone services). MongoDB + /// rejects names over 63 characters with a bare InvalidNamespace on the first call, so the provider must fail + /// first with an error that names the storage. + /// + [TestFixture] + public class StorageDatabaseNameTests + { + const string Cid = "1706624984549280"; // 16 + const string Pid = "DE_1706624984549283"; // 19 -> storage names up to 27 characters fit + + class RealmInfo : IRealmInfo + { + public string CustomerID => Cid; + public string ProjectName => Pid; + } + +#pragma warning disable CS0618 // the by-name indexer is the direct path to the database lookup + static async Task GetDatabase(string storageName) => + await new StorageObjectConnectionProvider(new RealmInfo(), null)[storageName]; +#pragma warning restore CS0618 + + [Test] + public void GetDatabase_WhenNameIsTooLong_ThrowsStorageNameTooLong() + { + var storageName = new string('a', 28); + + var ex = Assert.ThrowsAsync( + () => GetDatabase(storageName)); + Assert.That(ex.Message, Does.Contain($"[{storageName}]")); + Assert.That(ex.Message, Does.Contain("at most 27 characters")); + } + + [Test] + public async Task GetDatabase_AtTheLimit_UsesTheComposedName() + { + var storageName = new string('b', 27); + var connStrVar = $"STORAGE_CONNSTR_{storageName}"; + Environment.SetEnvironmentVariable(connStrVar, "mongodb://localhost:27017"); + try + { + var db = await GetDatabase(storageName); + Assert.That(db.DatabaseNamespace.DatabaseName, Is.EqualTo($"{Cid}{Pid}_{storageName}")); + Assert.That(db.DatabaseNamespace.DatabaseName.Length, Is.EqualTo(StorageDatabaseName.MaxLength)); + } + finally + { + Environment.SetEnvironmentVariable(connStrVar, null); + } + } + } +} From 49a2fed1023be0f5864aede5891a1d62cc65ae71 Mon Sep 17 00:00:00 2001 From: Gabriel Ramos Date: Wed, 7 Oct 2026 17:27:19 -0300 Subject: [PATCH 2/3] pr fixes --- cli/cli/Commands/Project/NewStorageCommand.cs | 26 ---- cli/cli/Services/BeamoLocalSystem.cs | 4 +- cli/cli/Services/DeploymentService.cs | 2 +- cli/cli/Services/ProjectService.cs | 125 ++++++++++++++++++ cli/cli/Services/StorageNameValidator.cs | 107 --------------- 5 files changed, 128 insertions(+), 136 deletions(-) delete mode 100644 cli/cli/Services/StorageNameValidator.cs diff --git a/cli/cli/Commands/Project/NewStorageCommand.cs b/cli/cli/Commands/Project/NewStorageCommand.cs index a5d93e2701..862d9298a2 100644 --- a/cli/cli/Commands/Project/NewStorageCommand.cs +++ b/cli/cli/Commands/Project/NewStorageCommand.cs @@ -67,8 +67,6 @@ public override void Configure() public override async Task Handle(NewStorageCommandArgs args) { await args.CreateConfigIfNeeded(_initCommand); - StorageNameValidator.ValidateNewStorage(args.AppContext.Cid, await ResolveTargetScope(args), args.ProjectName, - args.IsZone); var newMicroserviceInfo = await args.ProjectService.CreateNewStorage(args); Log.Information( $"Registering local project... 'beam services register --id {args.ProjectName} --type EmbeddedMongoDb'"); @@ -120,30 +118,6 @@ public override async Task Handle(NewStorageCommandArgs args) await sequence; } - // The id the storage's database name will be scoped by: the pid for a realm storage, the zid for a zone storage. - // Null when it can't be resolved (e.g. not logged in), in which case the name check only warns. - private static async Task ResolveTargetScope(NewStorageCommandArgs args) - { - if (!args.IsZone) - { - return args.AppContext.Pid; - } - - var localZid = args.ConfigService.GetConfigString(ConfigService.CFG_JSON_FIELD_ZID); - try - { - return await ZoneResolver.ResolveZid(args.DependencyProvider, args.AppContext.Cid, args.AppContext.Pid, - localZid); - } - catch (Exception ex) - { - // a selected realm's zone binding is authoritative, so don't guess with the local zid; an unresolved - // zone leaves the name check warn-only. - Log.Debug($"Could not resolve the zone for the storage name check: {ex.Message}"); - return null; - } - } - private async Promise UpdateDependencyDockerFile(NewStorageCommandArgs args, BeamoServiceDefinition definition) { await args.BeamoLocalSystem.UpdateDockerFile(definition); diff --git a/cli/cli/Services/BeamoLocalSystem.cs b/cli/cli/Services/BeamoLocalSystem.cs index 8741eee59a..9bf72c5c3c 100644 --- a/cli/cli/Services/BeamoLocalSystem.cs +++ b/cli/cli/Services/BeamoLocalSystem.cs @@ -129,8 +129,8 @@ public async Task InitManifest(bool useManifestCache=true, bool fetchServerManif // it was renamed by hand rather than through the CLI. In-memory only: zone storages use the local zid, and // are skipped (left to the deploy check and the runtime guard) when none is set. var localZid = _configService.GetConfigString(ConfigService.CFG_JSON_FIELD_ZID); - StorageNameValidator.ThrowIfAny( - StorageNameValidator.FindTooLong(BeamoManifest.ServiceDefinitions, _ctx.Cid, _ctx.Pid, localZid)); + ProjectService.ThrowIfStorageNamesTooLong( + ProjectService.FindTooLongStorageNames(BeamoManifest.ServiceDefinitions, _ctx.Cid, _ctx.Pid, localZid)); } private static Uri GetLocalDockerEndpoint(ConfigService config) diff --git a/cli/cli/Services/DeploymentService.cs b/cli/cli/Services/DeploymentService.cs index b2ce3dc871..a6ec32dc7e 100644 --- a/cli/cli/Services/DeploymentService.cs +++ b/cli/cli/Services/DeploymentService.cs @@ -1597,7 +1597,7 @@ public static bool IsJsonAManifest(IDictionary data) // only this scope's storages are deployed, so the one scope id serves as both pid and zid. var scopedDefinitions = beamo.BeamoManifest.ServiceDefinitions .Where(d => d.IsZoneScoped == (args.Scope == DeployScope.Zone)); - StorageNameValidator.ThrowIfAny(StorageNameValidator.FindTooLong( + ProjectService.ThrowIfStorageNamesTooLong(ProjectService.FindTooLongStorageNames( scopedDefinitions, targetCid, targetScopeId, targetScopeId, includeOnlyBeamoIds)); } diff --git a/cli/cli/Services/ProjectService.cs b/cli/cli/Services/ProjectService.cs index 1eaae201d9..72fb757523 100644 --- a/cli/cli/Services/ProjectService.cs +++ b/cli/cli/Services/ProjectService.cs @@ -339,6 +339,8 @@ public async Task CreateNewPortalExtensionLib(NewPortalExtension public async Task CreateNewStorage(NewStorageCommandArgs args) { + ValidateNewStorageName(args.AppContext.Cid, await ResolveNewStorageScope(args), args.ProjectName, args.IsZone); + string usedVersion = VersionService.GetNugetPackagesForExecutingCliVersion().ToString(); var microserviceInfo = new NewServiceInfo(); // check that we have the templates available @@ -421,6 +423,129 @@ private static void SetZoneScopeOnCsproj(string csprojPath) File.WriteAllLines(csprojPath, lines.Skip(1).ToArray()); } + // ── Storage name length ───────────────────────────────────────────────────────────────────── + // A storage maps to the MongoDB database {cid}{scope}_{storageName} (see StorageDatabaseName), where the scope is + // the pid for a realm storage and the zid for a zone storage, so how long a storage name may be depends on the + // target cid and scope. MongoDB rejects the name locally and remotely alike, so these checks guard creation, + // manifest init (local runs) and deploys. + + // The id the new storage's database name will be scoped by: the pid for a realm storage, the zid for a zone + // storage. Null when it can't be resolved (e.g. not logged in), in which case the name check only warns. + private static async Task ResolveNewStorageScope(NewStorageCommandArgs args) + { + if (!args.IsZone) + { + return args.AppContext.Pid; + } + + var localZid = args.ConfigService.GetConfigString(ConfigService.CFG_JSON_FIELD_ZID); + try + { + return await ZoneResolver.ResolveZid(args.DependencyProvider, args.AppContext.Cid, args.AppContext.Pid, + localZid); + } + catch (Exception ex) + { + // a selected realm's zone binding is authoritative, so don't guess with the local zid; an unresolved + // zone leaves the name check warn-only. + Log.Debug($"Could not resolve the zone for the storage name check: {ex.Message}"); + return null; + } + } + + /// + /// Validates the name of a storage about to be created. With a known cid and scope (pid, or zid for a zone + /// storage) a name that doesn't fit throws; without them, the name is only compared against the worst-case + /// budget and a warning is logged, since the target isn't known yet. + /// + public static void ValidateNewStorageName(string cid, string scope, string storageName, bool isZone) + { + if (IsNumericCid(cid) && !string.IsNullOrEmpty(scope)) + { + if (StorageDatabaseName.IsTooLong(cid, scope, storageName)) + { + ThrowIfStorageNamesTooLong(new List { StorageDatabaseName.DescribeTooLong(cid, scope, storageName) }); + } + + return; + } + + // Worst-case ids are snowflake longs of up to 19 digits: cid (19), pid "DE_" + 19, zid "ZONE_" + 19. + const int worstCaseCidLength = 19; + var worstCaseScopeLength = isZone ? 24 : 22; + var worstCase = StorageDatabaseName.MaxLength - worstCaseCidLength - worstCaseScopeLength - 1; + if (storageName.Length > worstCase) + { + Log.Warning($"Storage [{storageName}] is {storageName.Length} characters long. Its MongoDB database name " + + $"is {{cid}}{{{(isZone ? "zid" : "pid")}}}_{{storageName}} (at most {StorageDatabaseName.MaxLength} " + + $"characters), and the {(isZone ? "zone" : "realm")} isn't known yet, so it can't be checked. " + + $"Names up to {worstCase} characters always fit; a longer name may fail once you run or deploy it."); + } + } + + /// + /// Returns one message per local storage whose database name would be too long. Realm storages are checked + /// against and zone storages against ; a storage whose scope id is + /// unknown (null/empty) is skipped, as is everything when isn't a numeric customer id. + /// + /// The manifest's definitions; only local storages are checked. + /// When set, only these storages are checked. + public static List FindTooLongStorageNames( + IEnumerable definitions, + string cid, + string pid, + string zid, + ICollection includeOnlyBeamoIds = null) + { + var errors = new List(); + if (!IsNumericCid(cid)) + { + return errors; + } + + foreach (var definition in definitions) + { + if (!definition.IsLocal || definition.Protocol != BeamoProtocolType.EmbeddedMongoDb) + continue; + if (includeOnlyBeamoIds != null && !includeOnlyBeamoIds.Contains(definition.BeamoId)) + continue; + + var scope = definition.IsZoneScoped ? zid : pid; + if (string.IsNullOrEmpty(scope)) + { + Log.Trace($"Skipping storage name length check for storage=[{definition.BeamoId}]; no " + + $"{(definition.IsZoneScoped ? "zid" : "pid")} is known."); + continue; + } + + if (StorageDatabaseName.IsTooLong(cid, scope, definition.BeamoId)) + { + errors.Add(StorageDatabaseName.DescribeTooLong(cid, scope, definition.BeamoId)); + } + } + + return errors; + } + + /// + /// Throws a single listing every storage in , if any. + /// + public static void ThrowIfStorageNamesTooLong(List errors) + { + if (errors.Count == 0) + { + return; + } + + var lines = string.Join(Environment.NewLine, errors.Select(e => " - " + e)); + throw new CliException( + $"Some storage names are too long for MongoDB:{Environment.NewLine}{lines}{Environment.NewLine}" + + "Rename these storages to fit: change the property (or the .csproj name when isn't set) " + + "and the matching [StorageObject] attribute name."); + } + + private static bool IsNumericCid(string cid) => !string.IsNullOrEmpty(cid) && cid.All(char.IsDigit); + public async Task CreateNewMicroservice(NewMicroserviceArgs args) { // check that we have the templates available diff --git a/cli/cli/Services/StorageNameValidator.cs b/cli/cli/Services/StorageNameValidator.cs deleted file mode 100644 index e443699b28..0000000000 --- a/cli/cli/Services/StorageNameValidator.cs +++ /dev/null @@ -1,107 +0,0 @@ -using Beamable.Server; - -namespace cli.Services; - -/// -/// Validates that local storage names fit in a MongoDB database name. A storage maps to the database -/// {cid}{scope}_{storageName} (see ), where the scope is the pid for a realm -/// storage and the zid for a zone storage, so how long a storage name may be depends on the target cid and scope. -/// MongoDB rejects the name locally and remotely alike, so this guards both local runs and deploys. -/// -public static class StorageNameValidator -{ - // Worst-case ids are snowflake longs of up to 19 digits: cid (19), pid "DE_" + 19, zid "ZONE_" + 19. - private const int WorstCaseCidLength = 19; - public const int WorstCaseRealmStorageNameLength = StorageDatabaseName.MaxLength - WorstCaseCidLength - 22 - 1; - public const int WorstCaseZoneStorageNameLength = StorageDatabaseName.MaxLength - WorstCaseCidLength - 24 - 1; - - /// - /// Validates the name of a storage about to be created. With a known cid and scope (pid, or zid for a zone - /// storage) a name that doesn't fit throws; without them, the name is only compared against the worst-case - /// budget and a warning is logged, since the target isn't known yet. - /// - public static void ValidateNewStorage(string cid, string scope, string storageName, bool isZone) - { - if (IsNumericCid(cid) && !string.IsNullOrEmpty(scope)) - { - if (StorageDatabaseName.IsTooLong(cid, scope, storageName)) - { - ThrowIfAny(new List { StorageDatabaseName.DescribeTooLong(cid, scope, storageName) }); - } - - return; - } - - var worstCase = isZone ? WorstCaseZoneStorageNameLength : WorstCaseRealmStorageNameLength; - if (storageName.Length > worstCase) - { - Log.Warning($"Storage [{storageName}] is {storageName.Length} characters long. Its MongoDB database name " + - $"is {{cid}}{{{(isZone ? "zid" : "pid")}}}_{{storageName}} (at most {StorageDatabaseName.MaxLength} " + - $"characters), and the {(isZone ? "zone" : "realm")} isn't known yet, so it can't be checked. " + - $"Names up to {worstCase} characters always fit; a longer name may fail once you run or deploy it."); - } - } - - /// - /// Returns one message per local storage whose database name would be too long. Realm storages are checked - /// against and zone storages against ; a storage whose scope id is - /// unknown (null/empty) is skipped, as is everything when isn't a numeric customer id. - /// - /// The manifest's definitions; only local storages are checked. - /// When set, only these storages are checked. - public static List FindTooLong( - IEnumerable definitions, - string cid, - string pid, - string zid, - ICollection includeOnlyBeamoIds = null) - { - var errors = new List(); - if (!IsNumericCid(cid)) - { - return errors; - } - - foreach (var definition in definitions) - { - if (!definition.IsLocal || definition.Protocol != BeamoProtocolType.EmbeddedMongoDb) - continue; - if (includeOnlyBeamoIds != null && !includeOnlyBeamoIds.Contains(definition.BeamoId)) - continue; - - var scope = definition.IsZoneScoped ? zid : pid; - if (string.IsNullOrEmpty(scope)) - { - Log.Trace($"Skipping storage name length check for storage=[{definition.BeamoId}]; no " + - $"{(definition.IsZoneScoped ? "zid" : "pid")} is known."); - continue; - } - - if (StorageDatabaseName.IsTooLong(cid, scope, definition.BeamoId)) - { - errors.Add(StorageDatabaseName.DescribeTooLong(cid, scope, definition.BeamoId)); - } - } - - return errors; - } - - /// - /// Throws a single listing every storage in , if any. - /// - public static void ThrowIfAny(List errors) - { - if (errors.Count == 0) - { - return; - } - - var lines = string.Join(Environment.NewLine, errors.Select(e => " - " + e)); - throw new CliException( - $"Some storage names are too long for MongoDB:{Environment.NewLine}{lines}{Environment.NewLine}" + - "Rename these storages to fit: change the property (or the .csproj name when isn't set) " + - "and the matching [StorageObject] attribute name."); - } - - private static bool IsNumericCid(string cid) => !string.IsNullOrEmpty(cid) && cid.All(char.IsDigit); -} From fd55878be30b537ef770d5bfb72da6c4b3f35866 Mon Sep 17 00:00:00 2001 From: Gabriel Ramos Date: Wed, 7 Oct 2026 17:27:29 -0300 Subject: [PATCH 3/3] tests --- ...atorTests.cs => StorageNameLengthTests.cs} | 58 ++++++++----------- 1 file changed, 25 insertions(+), 33 deletions(-) rename cli/tests/{StorageNameValidatorTests.cs => StorageNameLengthTests.cs} (65%) diff --git a/cli/tests/StorageNameValidatorTests.cs b/cli/tests/StorageNameLengthTests.cs similarity index 65% rename from cli/tests/StorageNameValidatorTests.cs rename to cli/tests/StorageNameLengthTests.cs index 09cb10da79..7958344eac 100644 --- a/cli/tests/StorageNameValidatorTests.cs +++ b/cli/tests/StorageNameLengthTests.cs @@ -12,7 +12,7 @@ namespace tests; /// name that doesn't fit before running or deploying it. /// [TestFixture] -public class StorageNameValidatorTests +public class StorageNameLengthTests { const string Cid = "1706624984549280"; // 16 const string Pid = "DE_1706624984549283"; // 19 -> realm budget 63 - 16 - 19 - 1 = 27 @@ -75,14 +75,14 @@ public void DescribeTooLong_NamesTheStorageDatabaseAndBudget() Assert.That(message, Does.Contain("at most 23 characters")); } - // ---- FindTooLong ---- + // ---- FindTooLongStorageNames ---- [Test] - public void FindTooLong_ChecksRealmStoragesAgainstPidAndZoneStoragesAgainstZid() + public void FindTooLongStorageNames_ChecksRealmStoragesAgainstPidAndZoneStoragesAgainstZid() { // 25 characters fits the realm (27) but not the zone (23). var name = new string('a', 25); - var errors = StorageNameValidator.FindTooLong( + var errors = ProjectService.FindTooLongStorageNames( new[] { Storage(name), Storage("Z" + name, zone: true) }, Cid, Pid, Zid); Assert.That(errors, Has.Count.EqualTo(1)); @@ -90,88 +90,80 @@ public void FindTooLong_ChecksRealmStoragesAgainstPidAndZoneStoragesAgainstZid() } [Test] - public void FindTooLong_SkipsZoneStoragesWhenNoZidIsKnown() + public void FindTooLongStorageNames_SkipsZoneStoragesWhenNoZidIsKnown() { - var errors = StorageNameValidator.FindTooLong( + var errors = ProjectService.FindTooLongStorageNames( new[] { Storage(new string('a', 40), zone: true) }, Cid, Pid, zid: null); Assert.That(errors, Is.Empty); } [Test] - public void FindTooLong_SkipsEverythingWithoutANumericCid() + public void FindTooLongStorageNames_SkipsEverythingWithoutANumericCid() { var definitions = new[] { Storage(new string('a', 40)) }; - Assert.That(StorageNameValidator.FindTooLong(definitions, null, Pid, Zid), Is.Empty); - Assert.That(StorageNameValidator.FindTooLong(definitions, "my-alias", Pid, Zid), Is.Empty); + Assert.That(ProjectService.FindTooLongStorageNames(definitions, null, Pid, Zid), Is.Empty); + Assert.That(ProjectService.FindTooLongStorageNames(definitions, "my-alias", Pid, Zid), Is.Empty); } [Test] - public void FindTooLong_IgnoresServicesAndRemoteOnlyStorages() + public void FindTooLongStorageNames_IgnoresServicesAndRemoteOnlyStorages() { var longName = new string('a', 40); - var errors = StorageNameValidator.FindTooLong( + var errors = ProjectService.FindTooLongStorageNames( new[] { Service(longName), Storage("R" + longName, local: false) }, Cid, Pid, Zid); Assert.That(errors, Is.Empty); } [Test] - public void FindTooLong_OnlyChecksIncludedStorages() + public void FindTooLongStorageNames_OnlyChecksIncludedStorages() { var tooLong = new string('a', 30); var definitions = new[] { Storage(tooLong), Storage("Other") }; - Assert.That(StorageNameValidator.FindTooLong(definitions, Cid, Pid, Zid, new HashSet { "Other" }), + Assert.That(ProjectService.FindTooLongStorageNames(definitions, Cid, Pid, Zid, new HashSet { "Other" }), Is.Empty); } [Test] - public void ThrowIfAny_ListsEveryOffenderInOneError() + public void ThrowIfStorageNamesTooLong_ListsEveryOffenderInOneError() { - var errors = StorageNameValidator.FindTooLong( + var errors = ProjectService.FindTooLongStorageNames( new[] { Storage(new string('a', 30)), Storage(new string('b', 30)) }, Cid, Pid, Zid); - var ex = Assert.Throws(() => StorageNameValidator.ThrowIfAny(errors)); + var ex = Assert.Throws(() => ProjectService.ThrowIfStorageNamesTooLong(errors)); Assert.That(ex!.Message, Does.Contain(new string('a', 30))); Assert.That(ex.Message, Does.Contain(new string('b', 30))); } [Test] - public void ThrowIfAny_WithNoErrors_DoesNothing() + public void ThrowIfStorageNamesTooLong_WithNoErrors_DoesNothing() { - Assert.DoesNotThrow(() => StorageNameValidator.ThrowIfAny(new List())); + Assert.DoesNotThrow(() => ProjectService.ThrowIfStorageNamesTooLong(new List())); } - // ---- ValidateNewStorage ---- + // ---- ValidateNewStorageName ---- [Test] - public void ValidateNewStorage_WithKnownTarget_ThrowsWhenTooLong() + public void ValidateNewStorageName_WithKnownTarget_ThrowsWhenTooLong() { Assert.Throws(() => - StorageNameValidator.ValidateNewStorage(Cid, Zid, new string('a', 24), isZone: true)); + ProjectService.ValidateNewStorageName(Cid, Zid, new string('a', 24), isZone: true)); } [Test] - public void ValidateNewStorage_WithKnownTarget_PassesAtTheLimit() + public void ValidateNewStorageName_WithKnownTarget_PassesAtTheLimit() { Assert.DoesNotThrow(() => - StorageNameValidator.ValidateNewStorage(Cid, Zid, new string('a', 23), isZone: true)); + ProjectService.ValidateNewStorageName(Cid, Zid, new string('a', 23), isZone: true)); } [Test] - public void ValidateNewStorage_WithoutTarget_OnlyWarns() + public void ValidateNewStorageName_WithoutTarget_OnlyWarns() { Assert.DoesNotThrow(() => - StorageNameValidator.ValidateNewStorage(null, null, new string('a', 40), isZone: true)); - } - - [Test] - public void WorstCaseBudgets_MatchTheLongestPossibleIds() - { - // cid 19 digits, pid "DE_" + 19, zid "ZONE_" + 19. - Assert.That(StorageNameValidator.WorstCaseRealmStorageNameLength, Is.EqualTo(21)); - Assert.That(StorageNameValidator.WorstCaseZoneStorageNameLength, Is.EqualTo(19)); + ProjectService.ValidateNewStorageName(null, null, new string('a', 40), isZone: true)); } }