Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SLVS-1434 Extend JsonFileHandler #5661

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Core.UnitTests/Binding/ServerConnectionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ public void Ctor_SonarCloud_NullCredentials_SetsNull()
[TestMethod]
public void Ctor_SonarCloud_SetsProperties()
{
var expectedServerUri = new Uri("https://sonarcloud.io");
var serverConnectionSettings = new ServerConnectionSettings(false);
var credentials = Substitute.For<ICredentials>();
var sonarCloud = new ServerConnection.SonarCloud(Org, serverConnectionSettings, credentials);
Expand Down
65 changes: 9 additions & 56 deletions src/Core.UnitTests/Persistence/JsonFileHandlerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,76 +55,29 @@ public void MefCtor_CheckExports()
[TestMethod]
public void Mef_CheckIsSingleton()
{
MefTestHelpers.CheckIsSingletonMefComponent<JsonFileHandler>();
MefTestHelpers.CheckIsNonSharedMefComponent<JsonFileHandler>();
}

[TestMethod]
public void TryReadFile_FileDoesNotExist_ReturnsFalse()
{
fileSystem.File.Exists(FilePath).Returns(false);

var succeeded = testSubject.TryReadFile(FilePath, out TestType deserializedContent);

succeeded.Should().BeFalse();
deserializedContent.Should().BeNull();
fileSystem.File.Received(1).Exists(FilePath);
}

[TestMethod]
public void TryReadFile_FileExists_ReturnsTrueAndDeserializeContent()
{
var expectedContent = new TestType("test");
var serializedContent = JsonConvert.SerializeObject(expectedContent);
fileSystem.File.Exists(FilePath).Returns(true);
fileSystem.File.ReadAllText(FilePath).Returns(serializedContent);
serializer.TryDeserialize(Arg.Any<string>(), out Arg.Any<TestType>()).Returns(true);

var succeeded = testSubject.TryReadFile(FilePath, out TestType _);

succeeded.Should().BeTrue();
Received.InOrder(() =>
{
fileSystem.File.Exists(FilePath);
fileSystem.File.ReadAllText(FilePath);
serializer.TryDeserialize(Arg.Any<string>(), out Arg.Any<TestType>());
});
}

[TestMethod]
public void TryReadFile_ReadingFileThrowsException_WritesLogAndReturnsFalse()
public void ReadFile_ReadingFileThrowsException_TrowsException()
{
var exceptionMsg = "IO failed";
fileSystem.File.Exists(FilePath).Returns(true);
fileSystem.File.When(x => x.ReadAllText(FilePath)).Do(x => throw new Exception(exceptionMsg));

var succeeded = testSubject.TryReadFile(FilePath, out TestType _);

succeeded.Should().BeFalse();
logger.Received(1).WriteLine(exceptionMsg);
}

[TestMethod]
public void TryReadFile_DeserializationThrowsException_WritesLogAndReturnsFalse()
{
var exceptionMsg = "deserialization failed";
fileSystem.File.Exists(FilePath).Returns(true);
serializer.When(x => x.TryDeserialize(Arg.Any<string>(), out Arg.Any<TestType>())).Do(x => throw new Exception(exceptionMsg));
Action act = () => testSubject.ReadFile<TestType>(FilePath);

var succeeded = testSubject.TryReadFile(FilePath, out TestType _);

succeeded.Should().BeFalse();
logger.Received(1).WriteLine(exceptionMsg);
act.Should().Throw<Exception>().WithMessage(exceptionMsg);
}

[TestMethod]
public void TryReadFile_DeserializationFails_WritesLogAndReturnsFalse()
public void ReadFile_DeserializationThrowsException_TrowsException()
{
fileSystem.File.Exists(FilePath).Returns(true);
serializer.TryDeserialize(Arg.Any<string>(), out Arg.Any<TestType>()).Returns(false);
var exceptionMsg = "IO failed";
serializer.When(x => x.Deserialize<TestType>(Arg.Any<string>())).Do(x => throw new Exception(exceptionMsg));

var succeeded = testSubject.TryReadFile(FilePath, out TestType _);
Action act = () => testSubject.ReadFile<TestType>(FilePath);

succeeded.Should().BeFalse();
act.Should().Throw<Exception>().WithMessage(exceptionMsg);
}

[TestMethod]
Expand Down
20 changes: 20 additions & 0 deletions src/Core.UnitTests/Persistence/JsonSerializerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -96,4 +96,24 @@ public void TryDeserialize_Fails_LogsAndReturnsFalse()
deserializedObj.Should().BeNull();
logger.Received(1).WriteLine(string.Format(PersistenceStrings.FailedToDeserializeObject, nameof(TestType)));
}

[TestMethod]
public void Deserialize_Succeeds_ReturnsString()
{
var serializedString = "{\"PropName\":\"abc\"}";

var deserializedObj = testSubject.Deserialize<TestType>(serializedString);

deserializedObj.Should().BeEquivalentTo(new TestType("abc"));
}

[TestMethod]
public void Deserialize_Fails_ThrowsException()
{
var serializedString = "invalid";

Action act = () => testSubject.Deserialize<TestType>(serializedString);

act.Should().Throw<Exception>();
}
}
74 changes: 35 additions & 39 deletions src/Core/Persistence/JsonFileHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,32 @@ namespace SonarLint.VisualStudio.Core.Persistence;

public interface IJsonFileHandler
{
bool TryReadFile<T>(string filePath, out T content) where T : class;
/// <summary>
/// Reads the json file and deserializes its content to the provided type.
/// </summary>
/// <typeparam name="T">The type of the model that will be serialized</typeparam>
/// <param name="filePath">The path to the file</param>
/// <returns>Returns the content of the json file deserialized to the provided type.</returns>
T ReadFile<T>(string filePath) where T : class;

/// <summary>
/// Tries to deserialize the model and write it to the json file.
/// If the file does not exist, it will be created.
/// </summary>
/// <typeparam name="T">The type of the model that will be deserialized</typeparam>
/// <param name="filePath">The path to the file</param>
/// <param name="model">The model that will be deserialized</param>
/// <returns>True if the model was deserialized successfully and written to the file. False otherwise</returns>
bool TryWriteToFile<T>(string filePath, T model) where T : class;
}

[Export(typeof(IJsonFileHandler))]
[PartCreationPolicy(CreationPolicy.Shared)]
[PartCreationPolicy(CreationPolicy.NonShared)]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that you removed the lock it doesn't actually matter if it's shared or not 😆

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but I don't think there is a good reason for it to be a singleton. In fact, it might be dangerous if in some point any state is being added to the class.

public class JsonFileHandler : IJsonFileHandler
{
private readonly ILogger logger;
private readonly IFileSystem fileSystem;
private readonly IJsonSerializer jsonSerializer;
private static readonly object Locker = new();

[ImportingConstructor]
public JsonFileHandler(IJsonSerializer jsonSerializer, ILogger logger) : this(new FileSystem(), jsonSerializer, logger) { }
Expand All @@ -50,52 +64,34 @@ public JsonFileHandler(IJsonSerializer jsonSerializer, ILogger logger) : this(ne
this.logger = logger;
}

public bool TryReadFile<T>(string filePath, out T content) where T: class
public T ReadFile<T>(string filePath) where T : class
{
content = null;
if (!fileSystem.File.Exists(filePath))
{
return false;
}

try
{
var jsonContent = fileSystem.File.ReadAllText(filePath);
return jsonSerializer.TryDeserialize(jsonContent, out content);

}
catch (Exception ex) when (!ErrorHandler.IsCriticalException(ex))
{
logger.WriteLine(ex.Message);
return false;
}
var jsonContent = fileSystem.File.ReadAllText(filePath);
return jsonSerializer.Deserialize<T>(jsonContent);
}

public bool TryWriteToFile<T>(string filePath, T model) where T : class
{
lock (Locker)
try
{
try
var directoryName = Path.GetDirectoryName(filePath);
if (!fileSystem.Directory.Exists(directoryName))
{
var directoryName = Path.GetDirectoryName(filePath);
if (!fileSystem.Directory.Exists(directoryName))
{
fileSystem.Directory.CreateDirectory(directoryName);
}

var wasContentDeserialized = jsonSerializer.TrySerialize(model, out string serializedObj, Formatting.Indented);
if (wasContentDeserialized)
{
fileSystem.File.WriteAllText(filePath, serializedObj);
return true;
}
fileSystem.Directory.CreateDirectory(directoryName);
}
catch (Exception ex) when (!ErrorHandler.IsCriticalException(ex))

var wasContentDeserialized = jsonSerializer.TrySerialize(model, out string serializedObj, Formatting.Indented);
if (wasContentDeserialized)
{
logger.WriteLine(ex.Message);
fileSystem.File.WriteAllText(filePath, serializedObj);
return true;
}

return false;
}
catch (Exception ex) when (!ErrorHandler.IsCriticalException(ex))
{
logger.WriteLine(ex.Message);
}

return false;
}
}
8 changes: 7 additions & 1 deletion src/Core/Persistence/JsonSerializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ namespace SonarLint.VisualStudio.Core.Persistence;
public interface IJsonSerializer
{
bool TryDeserialize<T>(string json, out T deserializedObj, JsonSerializerSettings serializerSettings = null) where T : class;
T Deserialize<T>(string json, JsonSerializerSettings serializerSettings = null) where T : class;
bool TrySerialize<T>(T objectToSerialize, out string serializedObj, Formatting formatting = Formatting.None,
JsonSerializerSettings serializerSettings = null) where T : class;
}
Expand Down Expand Up @@ -55,7 +56,7 @@ public bool TryDeserialize<T>(string json, out T deserializedObj, JsonSerializer
deserializedObj = null;
try
{
deserializedObj = JsonConvert.DeserializeObject<T>(json, serializerSettings);
deserializedObj = Deserialize<T>(json, serializerSettings);
return true;
}
catch (Exception)
Expand All @@ -65,6 +66,11 @@ public bool TryDeserialize<T>(string json, out T deserializedObj, JsonSerializer
}
}

public T Deserialize<T>(string json, JsonSerializerSettings serializerSettings = null) where T : class
{
return JsonConvert.DeserializeObject<T>(json, serializerSettings);
}

public bool TrySerialize<T>(T objectToSerialize, out string serializedObj, Formatting formatting = Formatting.None, JsonSerializerSettings serializerSettings = null) where T: class
{
serializedObj = null;
Expand Down