Resolves #1956 - Trim blanks when updating OSM

IsraelHikingMap · Nov 30, 2023 · 6d2b74e · 6d2b74e
1 parent ed8ed0f
commit 6d2b74e
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 4 deletions.
diff --git a/IsraelHiking.API/Services/Poi/PointsOfInterestProvider.cs b/IsraelHiking.API/Services/Poi/PointsOfInterestProvider.cs
@@ -204,15 +204,21 @@ private void RemoveTagsByIcon(TagsCollectionBase tags, string icon)
             }
         }
 
-        private void RemoveEmptyTags(TagsCollectionBase tags)
+        private void RemoveEmptyTagsAndWhiteSpaces(TagsCollectionBase tags)
         {
             for (int i = tags.Count - 1; i >= 0; i--)
             {
                 var currentTag = tags.ElementAt(i);
-                if (string.IsNullOrWhiteSpace(currentTag.Value))
+                var valueWithOutExtraSpaces = Regex.Replace(currentTag.Value, @"\s+", " ", RegexOptions.Multiline).Trim();
+                if (string.IsNullOrWhiteSpace(valueWithOutExtraSpaces))
                 {
                     tags.RemoveKeyValue(currentTag);
                 }
+                else
+                {
+                    currentTag.Value = valueWithOutExtraSpaces;
+                    tags.AddOrReplace(currentTag);
+                }
             }
         }
 
@@ -339,7 +345,7 @@ public async Task<IFeature> AddFeature(IFeature feature, IAuthClient osmGateway,
             SetTagByLanguage(node.Tags, FeatureAttributes.NAME, feature.GetTitle(language), language);
             SetTagByLanguage(node.Tags, FeatureAttributes.DESCRIPTION, feature.GetDescription(language), language);
             AddTagsByIcon(node.Tags, feature.Attributes[FeatureAttributes.POI_ICON].ToString());
-            RemoveEmptyTags(node.Tags);
+            RemoveEmptyTagsAndWhiteSpaces(node.Tags);
             await osmGateway.UploadToOsmWithRetries(
                 $"Added {feature.GetTitle(language)} using IsraelHiking.osm.org.il",
                 async changeSetId =>
@@ -386,7 +392,7 @@ public async Task<IFeature> UpdateFeature(IFeature partialFeature, IAuthClient o
 
             await UpdateLists(partialFeature, completeOsmGeo, osmGateway, language);
 
-            RemoveEmptyTags(completeOsmGeo.Tags);
+            RemoveEmptyTagsAndWhiteSpaces(completeOsmGeo.Tags);
             if (oldTags.SequenceEqual(completeOsmGeo.Tags.ToArray()) &&
                 !locationWasUpdated)
             {

diff --git a/Tests/IsraelHiking.API.Tests/Services/Poi/PointsOfInterestProviderTests.cs b/Tests/IsraelHiking.API.Tests/Services/Poi/PointsOfInterestProviderTests.cs
@@ -268,6 +268,32 @@ public void AddFeature_ShouldUpdateOsmAndElasticSearch()
             gateway.Received().CreateChangeset(Arg.Any<TagsCollectionBase>());
             gateway.Received().CloseChangeset(Arg.Any<long>());
         }
+
+        [TestMethod]
+        public void AddFeature_WithExtraSpaces_ShouldRemoveExtraSpaces()
+        {
+            var user = new User { DisplayName = "DisplayName" };
+            var gateway = SetupHttpFactory();
+            gateway.GetUserDetails().Returns(user);
+            var language = "he";
+            gateway.CreateElement(Arg.Any<long>(), Arg.Any<Node>()).Returns(42);
+            var feature = GetValidFeature("42", Sources.OSM);
+            feature.Attributes.AddOrUpdate(FeatureAttributes.POI_ICON, _tagsHelper.GetCategoriesByGroup(Categories.POINTS_OF_INTEREST).First().Icon);
+            feature.Attributes.AddOrUpdate(FeatureAttributes.NAME, " a   b  c ");
+            feature.Attributes.AddOrUpdate(FeatureAttributes.DESCRIPTION, "  ");
+            _imagesUrlsStorageExecutor.GetImageUrlIfExists(Arg.Any<MD5>(), Arg.Any<byte[]>()).Returns((string)null);
+            _pointsOfInterestRepository.GetPointOfInterestById(Arg.Any<string>(), Arg.Any<string>()).Returns(null as IFeature);
+
+            var results = _adapter.AddFeature(feature, gateway, language).Result;
+
+            Assert.IsNotNull(results);
+            _pointsOfInterestRepository.Received(1).UpdatePointsOfInterestData(Arg.Any<List<IFeature>>());
+            gateway.Received().CreateElement(Arg.Any<long>(), Arg.Is<OsmGeo>(x => 
+                x.Tags[FeatureAttributes.NAME + ":" + language].Equals("a b c") && 
+                x.Tags.All(t => t.Key != FeatureAttributes.DESCRIPTION)));
+            gateway.Received().CreateChangeset(Arg.Any<TagsCollectionBase>());
+            gateway.Received().CloseChangeset(Arg.Any<long>());
+        }
 
         [TestMethod]
         public void AddFeature_WikipediaMobileLink_ShouldUpdateOsmAndElasticSearch()
@@ -366,6 +392,35 @@ public void UpdateFeature_UpdateLocationToALocationTooClose_ShouldNotUpdate()
             gateway.DidNotReceive().UpdateElement(Arg.Any<long>(), Arg.Any<ICompleteOsmGeo>());
         }
 
+        [TestMethod]
+        public void UpdateFeature_OnlyChangeExtraSpaces_ShouldNotUpdate()
+        {
+            var gateway = SetupHttpFactory();
+            var featureBeforeUpdate = GetValidFeature("Node_1", Sources.OSM);
+            var featureUpdate = new Feature(featureBeforeUpdate.Geometry, new AttributesTable
+            {
+                { FeatureAttributes.POI_ID, featureBeforeUpdate.GetId()},
+                { FeatureAttributes.ID, featureBeforeUpdate.Attributes[FeatureAttributes.ID]},
+                { FeatureAttributes.NAME, "name " }
+            });
+            _pointsOfInterestRepository.GetPointOfInterestById(Arg.Any<string>(), Arg.Any<string>()).Returns(GetValidFeature("Node_1", Sources.OSM));
+            gateway.GetNode(1).Returns(new Node
+            {
+                Id = 1,
+                Tags = new TagsCollection
+                {
+                    {FeatureAttributes.NAME, "name"},
+                    {FeatureAttributes.NAME + ":en", "name"}
+                },
+                Latitude = 1,
+                Longitude = 1
+            });
+
+            _adapter.UpdateFeature(featureUpdate, gateway, "en").Wait();
+
+            gateway.DidNotReceive().UpdateElement(Arg.Any<long>(), Arg.Any<ICompleteOsmGeo>());
+        }
+
         [TestMethod]
         public void UpdateFeature_UpdateLocationOfWay_ShouldNotUpdate()
         {