#54 - Better error handling for malformed JSON (#56)

Kieranties · Feb 19, 2019 · f68e161 · f68e161
1 parent ea77fa3
commit f68e161
Show file tree

Hide file tree

Showing 2 changed files with 103 additions and 7 deletions.
diff --git a/src/SimpleVersion.Core/Pipeline/ResolveConfigurationProcess.cs b/src/SimpleVersion.Core/Pipeline/ResolveConfigurationProcess.cs
@@ -22,7 +22,7 @@ public void Apply(VersionContext context)
 
             using(var repo = new Repository(context.RepositoryPath)){
                 var config = GetConfiguration(repo.Head?.Tip)
-                    ?? throw new InvalidOperationException($"No commits found for '{Constants.VersionFileName}'");
+                    ?? throw new InvalidOperationException($"Could not read '{Constants.VersionFileName}', has it been committed?");
 
                 context.Configuration = config;
 
@@ -66,7 +66,7 @@ private bool HasVersionChange(TreeChanges diff, Commit commit, SVM.Configuration
             if (diff.Any(d => d.Path == Constants.VersionFileName))
             {
                 var commitConfig = GetConfiguration(commit);
-                return !_comparer.Equals(config, commitConfig);
+                return commitConfig != null && !_comparer.Equals(config, commitConfig);
             }
 
             return false;
@@ -93,6 +93,17 @@ private SVM.Configuration GetConfiguration(Commit commit)
             return Read((gitObj as Blob).GetContentText());
         }
 
-        private SVM.Configuration Read(string rawConfiguration) => JsonConvert.DeserializeObject<SVM.Configuration>(rawConfiguration);
+        private SVM.Configuration Read(string rawConfiguration)
+        {
+            try
+            {
+                return JsonConvert.DeserializeObject<SVM.Configuration>(rawConfiguration);
+            }
+            catch
+            {
+                //TODO handle logger of invalid parsing
+                return null;
+            }
+        }
     }
 }
diff --git a/test/SimpleVersion.Core.Tests/Pipeline/ResolveConfigurationProcessFixture.cs b/test/SimpleVersion.Core.Tests/Pipeline/ResolveConfigurationProcessFixture.cs
@@ -3,6 +3,7 @@
 using SimpleVersion.Model;
 using SimpleVersion.Pipeline;
 using System;
+using System.IO;
 using Xunit;
 
 namespace SimpleVersion.Core.Tests.Pipeline
@@ -56,7 +57,7 @@ public void Apply_NoCommits_ShouldThrow()
 
                 // Assert
                 action.Should().Throw<InvalidOperationException>()
-                    .WithMessage($"No commits found for '{Constants.VersionFileName}'");
+                    .WithMessage($"Could not read '{Constants.VersionFileName}', has it been committed?");
             }
         }
 
@@ -77,7 +78,7 @@ public void Apply_CommitsForFile_ShouldThrow()
 
                 // Assert
                 action.Should().Throw<InvalidOperationException>()
-                    .WithMessage($"No commits found for '{Constants.VersionFileName}'");
+                    .WithMessage($"Could not read '{Constants.VersionFileName}', has it been committed?");
             }
         }
 
@@ -181,7 +182,7 @@ public void Apply_Feature_Branch_No_Change_Increments_Merge_Once()
 
                 // Act
                 _sut.Apply(context);
- 
+
                 context.Result.Height.Should().Be(6);
             }
         }
@@ -287,13 +288,97 @@ public void Apply_Feature_Branch_Sets_BranchName()
                 fixture.MakeACommit(); // feature 1
                 fixture.MakeACommit(); // feature 2
                 fixture.MakeACommit(); // feature 3
-                
+
                 // Act
                 _sut.Apply(context);
 
                 context.Result.BranchName.Should().Be("feature/other");
                 context.Result.CanonicalBranchName.Should().Be("refs/heads/feature/other");
             }
         }
+
+        [Fact]
+        public void Apply_Malformed_Json_At_Commit_Throws()
+        {
+            using (var fixture = new EmptyRepositoryFixture())
+            {
+                // Arrange
+                var context = new VersionContext { RepositoryPath = fixture.RepositoryPath };
+
+                // write the version file (Well formaed)
+                var config = new Configuration { Version = "0.1.0" };
+                Utils.WriteConfiguration(config, fixture); // 1
+
+                fixture.MakeACommit(); // 2
+                fixture.MakeACommit(); // 3
+                fixture.MakeACommit(); // 4
+
+
+                // Write the version file (with parsing errors)
+                var file = Path.Combine(fixture.RepositoryPath, Constants.VersionFileName);
+                using (var writer = File.AppendText(file))
+                {
+                    writer.WriteLine("This will not parse");
+                    writer.Flush();
+                }
+
+                fixture.Repository.Index.Add(Constants.VersionFileName);
+                fixture.Repository.Index.Write();
+                fixture.MakeACommit(); // 5
+                fixture.MakeACommit(); // 6
+                fixture.MakeACommit(); // 7
+                fixture.MakeACommit(); // 8
+
+                // Act
+                Action action = () => _sut.Apply(context);
+
+                // Assert
+                action.Should().Throw<InvalidOperationException>()
+                    .WithMessage($"Could not read '{Constants.VersionFileName}', has it been committed?");
+            }
+        }
+
+        [Fact]
+        public void Apply_Malformed_Json_Committed_Counts_As_No_Change()
+        {
+            using (var fixture = new EmptyRepositoryFixture())
+            {
+                // Arrange
+                var context = new VersionContext { RepositoryPath = fixture.RepositoryPath };
+
+                // write the version file (Well formaed)
+                var config = new Configuration { Version = "0.1.0" };
+                Utils.WriteConfiguration(config, fixture); // 1
+
+                fixture.MakeACommit(); // 2
+                fixture.MakeACommit(); // 3
+                fixture.MakeACommit(); // 4
+
+
+                // Write the version file (with parsing errors)
+                var file = Path.Combine(fixture.RepositoryPath, Constants.VersionFileName);
+
+                using (var writer = File.AppendText(file))
+                {
+                    writer.WriteLine("This will not parse");
+                    writer.Flush();
+                }
+
+                fixture.Repository.Index.Add(Constants.VersionFileName);
+                fixture.Repository.Index.Write();
+                fixture.MakeACommit(); // 5
+                fixture.MakeACommit(); // 6
+                fixture.MakeACommit(); // 7
+                fixture.MakeACommit(); // 8
+
+                config = new Configuration { Version = "0.1.0" };
+                Utils.WriteConfiguration(config, fixture); // 9
+
+                // Act
+                _sut.Apply(context);
+
+                context.Result.Height.Should().Be(9);
+            }
+        }
     }
 }