From bb9dc994b43bee1042450a5ef731c56bdf184c48 Mon Sep 17 00:00:00 2001 From: gladysheva Date: Fri, 23 Feb 2024 06:44:28 +0500 Subject: [PATCH] CR fixes --- .../ApplicationsStorage_Tests.cs | 46 +++++++++++-------- .../EnvironmentsStorage_Tests.cs | 21 +++++---- .../EnvironmentsStorage.cs | 23 ++++++---- 3 files changed, 53 insertions(+), 37 deletions(-) diff --git a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs index 625dd2f..8dcf07f 100644 --- a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs +++ b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs @@ -226,37 +226,47 @@ public void UpdateAll_should_force_update() [Test] public void Should_delete_application_from_cache_if_app_and_env_nodes_were_deleted_when_observation_of_deleted_apps_is_disabled() { - CreateEnvironmentNode("environment1"); - CreateApplicationNode("environment1", "application1", new Dictionary {{"key", "1/1"}}); + const string environment = "environment1"; + const string app = "application1"; + + var expectedTopology = ServiceTopology.Build(new Uri[0], new Dictionary {{"key", "1/1"}}); using (var storage = GetApplicationsStorage(out var envStorage, observeNonExistentApplications: false)) { - var expectedTopology = ServiceTopology.Build(new Uri[0], new Dictionary {{"key", "1/1"}}); - ShouldReturnImmediately( - storage, - "environment1", - "application1", - expectedTopology); + for (var i = 0; i < 10; i++) + { + CreateEnvironmentNode(environment); + CreateApplicationNode(environment, app, new Dictionary {{"key", "1/1"}}); - DeleteApplicationNode("environment1", "application1"); - DeleteEnvironmentNode("environment1"); + envStorage.Get(environment).Should().Be(new EnvironmentInfo(environment, null, null)); - envStorage.UpdateAll(); - storage.UpdateAll(); + ShouldReturnImmediately( + storage, + environment, + app, + expectedTopology); - storage.Contains("environment1", "application1").Should().BeFalse(); + DeleteApplicationNode(environment, app); + DeleteEnvironmentNode(environment); + + envStorage.UpdateAll(); + storage.UpdateAll(); + + storage.Contains(environment, app).Should().BeFalse(); + } } } [Test] public void Should_not_delete_application_from_cache_when_env_exists_and_observation_of_deleted_apps_is_disabled() { - var environment = "environment1"; - var app = "application1"; + const string environment = "environment1"; + const string app = "application1"; + CreateEnvironmentNode(environment); CreateApplicationNode(environment, app, new Dictionary {{"key", "1/1"}}); - using (var storage = GetApplicationsStorage(out var envStorage, observeNonExistentApplications: true)) + using (var storage = GetApplicationsStorage(out var envStorage, observeNonExistentApplications: false)) { var expectedTopology = ServiceTopology.Build(new Uri[0], new Dictionary {{"key", "1/1"}}); ShouldReturnImmediately( @@ -280,8 +290,8 @@ public void Should_not_delete_application_from_cache_when_env_exists_and_observa [Test] public void Should_not_delete_application_from_cache_when_observation_of_deleted_apps_is_disabled_and_client_disconnected() { - var environment = "environment1"; - var app = "application1"; + const string environment = "environment1"; + const string app = "application1"; CreateEnvironmentNode(environment); CreateApplicationNode(environment, app, new Dictionary {{"key", "1/1"}}); diff --git a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs index 20fae4e..62cc48c 100644 --- a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs +++ b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs @@ -173,7 +173,7 @@ public void Should_not_delete_environment_from_cache_if_node_was_deleted_when_ob ShouldReturn(storage, "default", expectedInfo); } } - + [Test] public void Should_not_delete_environment_from_cache_when_observation_of_deleted_apps_is_disabled_and_client_disconnected() { @@ -195,19 +195,20 @@ public void Should_not_delete_environment_from_cache_when_observation_of_deleted [Test] public void Should_delete_environment_from_cache_if_node_was_deleted_when_observation_of_deleted_apps_is_disabled() { + var expectedInfo = new EnvironmentInfo("default", "parent", null); + using (var storage = GetEnvironmentsStorage(observeNonExistentEnvironment: false)) { - CreateEnvironmentNode("default", "parent"); + for (var i = 0; i < 10; i++) + { + CreateEnvironmentNode("default", "parent"); - var expectedInfo = new EnvironmentInfo("default", "parent", null); - ShouldReturnImmediately(storage, "default", expectedInfo); + ShouldReturnImmediately(storage, "default", expectedInfo); - DeleteEnvironmentNode("default"); - storage.UpdateAll(); - storage.Contains("default").Should().BeFalse(); - - CreateEnvironmentNode("default", "parent"); - ShouldReturnImmediately(storage, "default", expectedInfo); + DeleteEnvironmentNode("default"); + storage.UpdateAll(); + storage.Contains("default").Should().BeFalse(); + } } } diff --git a/Vostok.ServiceDiscovery/ServiceLocatorStorage/EnvironmentsStorage.cs b/Vostok.ServiceDiscovery/ServiceLocatorStorage/EnvironmentsStorage.cs index 97f608d..9853b45 100644 --- a/Vostok.ServiceDiscovery/ServiceLocatorStorage/EnvironmentsStorage.cs +++ b/Vostok.ServiceDiscovery/ServiceLocatorStorage/EnvironmentsStorage.cs @@ -88,7 +88,9 @@ private void Update(string name) { if (!environments.TryGetValue(name, out var container)) { - log.Warn("Failed to update '{Environment}' environment: it does not exist in local cache.", name); + if (observeNonExistentEnvironments) + log.Warn("Failed to update '{Environment}' environment: it does not exist in local cache.", name); + return; } @@ -109,11 +111,8 @@ private void Update(string name, VersionedContainer container) if (environmentExists.Stat == null) { - if (!observeNonExistentEnvironments) - { - environments.TryRemove(name, out _); + if (RemoveEnvironmentFromCacheIfNeeded(name)) return; - } container.Clear(); } @@ -125,11 +124,8 @@ private void Update(string name, VersionedContainer container) var environmentData = zooKeeperClient.GetData(new GetDataRequest(environmentPath) {Watcher = nodeWatcher}); if (environmentData.Status == ZooKeeperStatus.NodeNotFound) { - if (!observeNonExistentEnvironments) - { - environments.TryRemove(name, out _); + if (RemoveEnvironmentFromCacheIfNeeded(name)) return; - } container.Clear(); } @@ -147,6 +143,15 @@ private void Update(string name, VersionedContainer container) } } + private bool RemoveEnvironmentFromCacheIfNeeded(string name) + { + if (observeNonExistentEnvironments) + return false; + + environments.TryRemove(name, out _); + return true; + } + private void OnNodeEvent(NodeChangedEventType type, string path) { if (isDisposed)