Skip to content

Commit

Permalink
[Add] Parameter Validation
Browse files Browse the repository at this point in the history
  • Loading branch information
samatstariongroup committed Dec 8, 2024
1 parent 85845a2 commit a166898
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 40 deletions.
74 changes: 37 additions & 37 deletions DEH-CSV/CsvReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,44 @@ public CsvReader(ILogger<CsvReader> logger)
/// <returns>A <see cref="Task{T}" /> that returns a collection of mapped <see cref="Thing" />s</returns>
public async Task<IEnumerable<Thing>> ReadAsync(Stream stream, IReadOnlyCollection<TypeMap> typeMaps, ISession session)

Check warning on line 74 in DEH-CSV/CsvReader.cs

View workflow job for this annotation

GitHub Actions / Build

Split this method into two, one handling parameters check and the other handling the asynchronous code. (https://rules.sonarsource.com/csharp/RSPEC-4457)
{
ValidateReadParameters(stream, typeMaps, session);
if (stream == null)
{
throw new ArgumentNullException(nameof(stream));
}

if (typeMaps == null)
{
throw new ArgumentNullException(nameof(typeMaps));
}

if (session == null)
{
throw new ArgumentNullException(nameof(session));
}

if (typeMaps.Count == 0)
{
throw new ArgumentException("The provided collection of TypeMap is empty", nameof(typeMaps));
}

if (typeMaps.SelectMany(x => x.Properties).Any(x => x.IsIdentifierProperty && !x.ClassKind.HasValue))
{
throw new InvalidDataException("One of the IdentifierProperty do not specify the ClassKind to query");
}

return await this.ReadInternalAsync(stream, typeMaps, session);
}

/// <summary>
/// Reads the CSV content of the <see cref="Stream" /> and maps it to <see cref="Thing" />s based on the provided collection of
/// <see cref="TypeMap" />s
/// </summary>
/// <param name="stream">The <see cref="Stream" /> that contains CSV content</param>
/// <param name="typeMaps">The collection of <see cref="TypeMap" />s</param>
/// <param name="session">The <see cref="ISession" /> that helps to retrieve <see cref="Thing" /></param>
/// <returns>A <see cref="Task{T}" /> that returns a collection of mapped <see cref="Thing" />s</returns>
private async Task<IEnumerable<Thing>> ReadInternalAsync(Stream stream, IReadOnlyCollection<TypeMap> typeMaps, ISession session)
{
var things = new List<Thing>();

var accessibleThings = session.Assembler.Cache
Expand Down Expand Up @@ -455,42 +491,6 @@ private async Task ReadHeaderAsync(IReader reader, IEnumerable<TypeMap> typeMaps
}
}

/// <summary>
/// Validates all parameters provided for the <see cref="ReadAsync" /> method
/// </summary>
/// <param name="stream">The provided <see cref="Stream" /></param>
/// <param name="typeMaps">The provided collection of <see cref="TypeMap" />s</param>
/// <param name="session">The provided <see cref="ISession" /></param>
/// <exception cref="ArgumentNullException"></exception>
/// <exception cref="ArgumentException"></exception>
private static void ValidateReadParameters(Stream stream, IReadOnlyCollection<TypeMap> typeMaps, ISession session)
{
if (stream == null)
{
throw new ArgumentNullException(nameof(stream));
}

if (typeMaps == null)
{
throw new ArgumentNullException(nameof(typeMaps));
}

if (session == null)
{
throw new ArgumentNullException(nameof(session));
}

if (typeMaps.Count == 0)
{
throw new ArgumentException("The provided collection of TypeMap is empty", nameof(typeMaps));
}

if (typeMaps.SelectMany(x => x.Properties).Any(x => x.IsIdentifierProperty && !x.ClassKind.HasValue))
{
throw new InvalidDataException("One of the IdentifierProperty do not specify the ClassKind to query");
}
}

/// <summary>
/// Queries the value to use for a cell inside a CSV row based on a <see cref="PropertyMap" />
/// </summary>
Expand Down
27 changes: 27 additions & 0 deletions DEH-CSV/CsvWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ public void Write(Iteration iteration, bool includeNestedElements, IEnumerable<T
throw new ArgumentNullException(nameof(iteration));
}

if (maps == null)
{
throw new ArgumentNullException(nameof(maps));
}

var things = iteration.Cache.Select(item => item.Value.Value).ToList();

if (includeNestedElements)
Expand Down Expand Up @@ -135,6 +140,13 @@ public void Write(Iteration iteration, bool includeNestedElements, IEnumerable<T
/// <param name="target">
/// The target <see cref="DirectoryInfo"/> to which the CSV file is to be written
/// </param>
/// <param name="options">
/// an object that may contain any kind of configuration that is required
/// for the evaluation of the custom property. This may be a value property
/// such as a string or int or a complex object. It is the responsibility
/// of the interface implementation to verify that the options argument is
/// of the correct type
/// </param>
public virtual void Write(IEnumerable<Thing> things, TypeMap typeMap, DirectoryInfo target, object options)

Check warning on line 150 in DEH-CSV/CsvWriter.cs

View workflow job for this annotation

GitHub Actions / Build

Refactor this method to reduce its Cognitive Complexity from 16 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
{
if (things == null)
Expand Down Expand Up @@ -257,6 +269,16 @@ protected virtual object QueryValue(Thing thing, PropertyMap propertyMap, object
/// </param>
protected void WriteHeader(CsvHelper.CsvWriter csvWriter, TypeMap typeMap)
{
if (csvWriter == null)
{
throw new ArgumentNullException(nameof(csvWriter));
}

if (typeMap == null)
{
throw new ArgumentNullException(nameof(typeMap));
}

this.logger.LogDebug("Writing Header for {TypeMap}", typeMap.ClassKind.ToString());

foreach (var propertyMap in typeMap.Properties)
Expand All @@ -280,6 +302,11 @@ protected void WriteHeader(CsvHelper.CsvWriter csvWriter, TypeMap typeMap)
/// </returns>
protected internal string QueryFileName(TypeMap typeMap)

Check warning on line 303 in DEH-CSV/CsvWriter.cs

View workflow job for this annotation

GitHub Actions / Build

Make 'QueryFileName' a static method. (https://rules.sonarsource.com/csharp/RSPEC-2325)
{
if (typeMap == null)
{
throw new ArgumentNullException(nameof(typeMap));
}

if (!string.IsNullOrEmpty(typeMap.FileName))
{
return typeMap.FileName;
Expand Down
2 changes: 1 addition & 1 deletion DEH-CSV/Mapping/PropertyMap.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace STARIONGROUP.DEHCSV.Mapping
public class PropertyMap
{
/// <summary>
/// Gets or sets the source property name on the ECSS-E-TM-10-25 class in case of the mapping from ECSS-E-TM10-25 to CSV.
/// Gets or sets the source property name on the ECSS-E-TM-10-25 class in case of the mapping from ECSS-E-TM-10-25 to CSV.
/// Gets or sets the column name of the CSV to use to retrieve a value
/// </summary>
public string Source { get; set; }
Expand Down
2 changes: 1 addition & 1 deletion DEH-CSV/Mapping/TypeMap.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,6 @@ public class TypeMap
/// <summary>
/// Gets or sets the <see cref="PropertyMap"/>s
/// </summary>
public List<PropertyMap> Properties { get; set; } = new List<PropertyMap>();
public List<PropertyMap> Properties { get; set; } = [];

Check warning on line 45 in DEH-CSV/Mapping/TypeMap.cs

View workflow job for this annotation

GitHub Actions / Build

Make the 'Properties' property read-only by removing the property setter or making it private. (https://rules.sonarsource.com/csharp/RSPEC-4004)
}
}
26 changes: 25 additions & 1 deletion DEH-CSV/Services/IterationReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,30 @@ public async Task<Iteration> ReadAsync(ISession session, string modelShortName,
throw new ArgumentNullException(nameof(session));
}

return await ReadInternalAsync(session, modelShortName, iterationNumber, domainOfExpertiseShortName);
}

/// <summary>
/// ReadAsync the iteration from the
/// </summary>
/// <param name="session">
/// The <see cref="ISession"/> object used to read the <see cref="Iteration"/> data
/// </param>
/// <param name="modelShortName">
/// The shortName of the <see cref="EngineeringModel"/> that is to be opened
/// </param>
/// <param name="iterationNumber">
/// The number of hte <see cref="Iteration"/> that is to be read
/// </param>
/// <param name="domainOfExpertiseShortName">
/// The shortName of the <see cref="DomainOfExpertise"/> used to open the <see cref="EngineeringModel"/> with
/// </param>
/// <returns>
/// An instance of <see cref="Iteration"/>
/// </returns>
private async Task<Iteration> ReadInternalAsync(ISession session, string modelShortName, int iterationNumber,
string domainOfExpertiseShortName)
{
this.logger.LogDebug("Setting up the read request for {ModelShortName}:{IterationNumber}", modelShortName, iterationNumber);

var siteDirectory = session.RetrieveSiteDirectory();
Expand All @@ -104,7 +128,7 @@ public async Task<Iteration> ReadAsync(ISession session, string modelShortName,
{
throw new InstanceNotFoundException($"The DomainOfExpertise with shortName {domainOfExpertiseShortName} could not be found");
}

var engineeringModel = new EngineeringModel(engineeringModelSetup.EngineeringModelIid, session.Assembler.Cache, session.Credentials.Uri);
engineeringModel.EngineeringModelSetup = engineeringModelSetup;

Expand Down

0 comments on commit a166898

Please sign in to comment.