-
Notifications
You must be signed in to change notification settings - Fork 7
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
1409 importer handling of imvalid files #1437
Changes from 16 commits
b728d54
3f308dc
e53b503
0c922de
a219c50
50be51b
1421907
86eacc7
d76ef41
2a41306
aa1c1f8
4ea57b1
cb2c8a3
3a58ecb
066441e
bb32d68
72834f9
f6b6adc
70ba716
dd693a3
566ca79
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
using OSPSuite.Assets; | ||
|
||
namespace OSPSuite.Infrastructure.Import.Core | ||
{ | ||
public class BaseGridColumnNotFoundException : AbstractImporterException | ||
{ | ||
public BaseGridColumnNotFoundException(string columnName) : base(Error.BaseGridColumnNotFoundException(columnName)) | ||
{ | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
using OSPSuite.Assets; | ||
|
||
namespace OSPSuite.Infrastructure.Import.Core | ||
{ | ||
public class ColumnNotFoundException : AbstractImporterException | ||
{ | ||
public ColumnNotFoundException(string columnName) : base(Error.ColumnNotFound(columnName)) | ||
{ | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,28 +3,63 @@ | |
using System.Linq; | ||
using OSPSuite.Core.Domain; | ||
using OSPSuite.Core.Import; | ||
using OSPSuite.Infrastructure.Import.Core.Exceptions; | ||
using OSPSuite.Infrastructure.Import.Core.Helpers; | ||
using OSPSuite.Infrastructure.Import.Extensions; | ||
using OSPSuite.Infrastructure.Import.Services; | ||
using OSPSuite.Utility.Collections; | ||
using OSPSuite.Utility.Extensions; | ||
|
||
namespace OSPSuite.Infrastructure.Import.Core | ||
{ | ||
public class ParseErrors | ||
{ | ||
private Cache<IDataSet, List<ParseErrorDescription>> _errors = new Cache<IDataSet, List<ParseErrorDescription>>(onMissingKey: _ => new List<ParseErrorDescription>()); | ||
|
||
public bool Any() => _errors.Any(); | ||
|
||
public bool Contains(IDataSet key) => _errors.Contains(key); | ||
|
||
public IEnumerable<ParseErrorDescription> ErrorsFor(IDataSet key) => _errors[key]; | ||
|
||
public void Add(IDataSet key, ParseErrorDescription x) | ||
{ | ||
Add(key, new List<ParseErrorDescription>() { x }); | ||
} | ||
|
||
public void Add(ParseErrors other) | ||
{ | ||
foreach (var x in other._errors.KeyValues) | ||
{ | ||
Add(x.Key, x.Value); | ||
} | ||
} | ||
|
||
public void Add(IDataSet key, IEnumerable<ParseErrorDescription> list) | ||
{ | ||
if (_errors.Contains(key)) | ||
_errors[key].AddRange(list); | ||
else | ||
_errors.Add(key, new List<ParseErrorDescription>(list)); | ||
} | ||
} | ||
|
||
/// <summary> | ||
/// Collection of DataSets | ||
/// </summary> | ||
public interface IDataSource | ||
{ | ||
void SetDataFormat(IDataFormat dataFormat); | ||
void SetNamingConvention(string namingConvention); | ||
void AddSheets(Cache<string, DataSheet> dataSheets, IReadOnlyList<ColumnInfo> columnInfos, string filter); | ||
ParseErrors AddSheets(Cache<string, DataSheet> dataSheets, IReadOnlyList<ColumnInfo> columnInfos, string filter); | ||
void SetMappings(string fileName, IEnumerable<MetaDataMappingConverter> mappings); | ||
ImporterConfiguration GetImporterConfiguration(); | ||
IEnumerable<MetaDataMappingConverter> GetMappings(); | ||
Cache<string, IDataSet> DataSets { get; } | ||
IEnumerable<string> NamesFromConvention(); | ||
NanSettings NanSettings { get; set; } | ||
ImportedDataSet DataSetAt(int index); | ||
void ValidateDataSourceUnits(IReadOnlyList<ColumnInfo> columnInfos); | ||
ParseErrors ValidateDataSourceUnits(IReadOnlyList<ColumnInfo> columnInfos); | ||
} | ||
|
||
public class DataSource : IDataSource | ||
|
@@ -72,16 +107,18 @@ private Cache<string, DataSheet> filterSheets(Cache<string, DataSheet> dataSheet | |
return filteredDataSheets; | ||
} | ||
|
||
public void AddSheets(Cache<string, DataSheet> dataSheets, IReadOnlyList<ColumnInfo> columnInfos, string filter) | ||
{ | ||
public ParseErrors AddSheets(Cache<string, DataSheet> dataSheets, IReadOnlyList<ColumnInfo> columnInfos, string filter) | ||
{ | ||
_importer.AddFromFile(_configuration.Format, filterSheets(dataSheets, filter), columnInfos, this); | ||
if (NanSettings == null || !double.TryParse(NanSettings.Indicator, out var indicator)) | ||
indicator = double.NaN; | ||
var errors = new ParseErrors(); | ||
foreach (var dataSet in DataSets.KeyValues) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why are you iterating over the keyValues? You are never using the Key |
||
{ | ||
if (NanSettings != null && NanSettings.Action == NanSettings.ActionType.Throw) | ||
{ | ||
dataSet.Value.ThrowsOnNan(indicator); | ||
if (dataSet.Value.NanValuesExist(indicator)) | ||
errors.Add(dataSet.Value, new NaNParseErrorDescription()); | ||
} | ||
else | ||
{ | ||
|
@@ -91,9 +128,10 @@ public void AddSheets(Cache<string, DataSheet> dataSheets, IReadOnlyList<ColumnI | |
continue; | ||
|
||
var emptyDataSetsNames = emptyDataSets.Select(d => string.Join(".", d.Description.Where(metaData => metaData.Value != null).Select(metaData => metaData.Value))); | ||
throw new EmptyDataSetsException(emptyDataSetsNames); | ||
errors.Add(dataSet.Value, new EmptyDataSetsParseErrorDescription(emptyDataSetsNames)); | ||
} | ||
} | ||
return errors; | ||
} | ||
|
||
public void SetMappings(string fileName, IEnumerable<MetaDataMappingConverter> mappings) | ||
|
@@ -145,8 +183,9 @@ public ImportedDataSet DataSetAt(int index) | |
} | ||
|
||
//checks that the dimension of all the units coming from columns for error have the same dimension to the corresponding measurement | ||
private void validateErrorAgainstMeasurement(IReadOnlyList<ColumnInfo> columnInfos) | ||
private ParseErrors validateErrorAgainstMeasurement(IReadOnlyList<ColumnInfo> columnInfos) | ||
{ | ||
var errors = new ParseErrors(); | ||
foreach (var column in columnInfos.Where(c => !c.IsAuxiliary())) | ||
{ | ||
foreach (var relatedColumn in columnInfos.Where(c => c.IsAuxiliary() && c.RelatedColumnOf == column.Name)) | ||
|
@@ -161,7 +200,10 @@ private void validateErrorAgainstMeasurement(IReadOnlyList<ColumnInfo> columnInf | |
continue; | ||
|
||
if (errorColumn.Value != null && measurementColumn.Value.Count != errorColumn.Value.Count) | ||
throw new MismatchingArrayLengthsException(); | ||
{ | ||
errors.Add(dataSet, new MismatchingArrayLengthsParseErrorDescription()); | ||
continue; | ||
} | ||
|
||
var errorDimension = errorColumn.Key.Column.Dimension; | ||
var measurementDimension = measurementColumn.Key.Column.Dimension; | ||
|
@@ -176,7 +218,10 @@ private void validateErrorAgainstMeasurement(IReadOnlyList<ColumnInfo> columnInf | |
var measurementSupportedDimension = column.SupportedDimensions.FirstOrDefault(x => x.HasUnit(measurementColumn.Value.ElementAt(i).Unit)); | ||
var errorSupportedDimension = column.SupportedDimensions.FirstOrDefault(x => x.HasUnit(errorColumn.Value.ElementAt(i).Unit)); | ||
if (measurementSupportedDimension != errorSupportedDimension) | ||
throw new ErrorUnitException(); | ||
{ | ||
errors.Add(dataSet, new ErrorUnitParseErrorDescription()); | ||
continue; | ||
} | ||
} | ||
} | ||
else | ||
|
@@ -188,17 +233,22 @@ private void validateErrorAgainstMeasurement(IReadOnlyList<ColumnInfo> columnInf | |
continue; | ||
|
||
if (measurementDimension != errorDimension) | ||
throw new ErrorUnitException(); | ||
{ | ||
errors.Add(dataSet, new ErrorUnitParseErrorDescription()); | ||
continue; | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
return errors; | ||
} | ||
//checks that all units coming from a mapped column unit belong to a valid dimension for this mapping | ||
//and also that they are all of the same dimension within every data set. | ||
private void validateUnitsSupportedAndSameDimension(IReadOnlyList<ColumnInfo> columnInfos) | ||
private ParseErrors validateUnitsSupportedAndSameDimension(IReadOnlyList<ColumnInfo> columnInfos) | ||
{ | ||
var errors = new ParseErrors(); | ||
foreach (var columnInfo in columnInfos) | ||
{ | ||
foreach (var dataSet in DataSets) | ||
|
@@ -227,22 +277,30 @@ private void validateUnitsSupportedAndSameDimension(IReadOnlyList<ColumnInfo> co | |
|
||
//if the unit specified does not belong to one of the supported dimensions of the mapping | ||
if (dimension == null) | ||
throw new InvalidDimensionException(currentValue.Unit, columnInfo.DisplayName); | ||
{ | ||
errors.Add(dataSet, new InvalidDimensionParseErrorDescription(currentValue.Unit, columnInfo.DisplayName)); | ||
continue; | ||
} | ||
|
||
//if the unit specified is not of the same dimension as the other units of the same data set | ||
if (dimension != dimensionOfFirstUnit) | ||
throw new InconsistentDimensionBetweenUnitsException(columnInfo.DisplayName); | ||
{ | ||
errors.Add(dataSet, new InconsistentDimensionBetweenUnitsParseErrorDescription(columnInfo.DisplayName)); | ||
continue; | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
return errors; | ||
} | ||
|
||
void IDataSource.ValidateDataSourceUnits(IReadOnlyList<ColumnInfo> columnInfos) | ||
ParseErrors IDataSource.ValidateDataSourceUnits(IReadOnlyList<ColumnInfo> columnInfos) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why the IDataSource. here? |
||
{ | ||
validateUnitsSupportedAndSameDimension(columnInfos); | ||
validateErrorAgainstMeasurement(columnInfos); | ||
var errors = validateUnitsSupportedAndSameDimension(columnInfos); | ||
errors.Add(validateErrorAgainstMeasurement(columnInfos)); | ||
return errors; | ||
} | ||
} | ||
|
||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
using OSPSuite.Assets; | ||
|
||
namespace OSPSuite.Infrastructure.Import.Core | ||
{ | ||
public class EmptyNamingConventionsException : AbstractImporterException | ||
{ | ||
public EmptyNamingConventionsException() : base(Error.NamingConventionEmpty) | ||
{ | ||
} | ||
} | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
using OSPSuite.Assets; | ||
using OSPSuite.Utility.Extensions; | ||
using System.Collections.Generic; | ||
|
||
namespace OSPSuite.Infrastructure.Import.Core.Exceptions | ||
{ | ||
public abstract class ParseErrorDescription | ||
{ | ||
public string Message { get; protected set; } | ||
} | ||
|
||
public class InvalidDimensionParseErrorDescription : ParseErrorDescription | ||
{ | ||
public InvalidDimensionParseErrorDescription(string invalidUnit, string mappingName) | ||
{ | ||
Message = Error.InvalidDimensionException(invalidUnit, mappingName); | ||
} | ||
} | ||
|
||
public class NaNParseErrorDescription : ParseErrorDescription | ||
{ | ||
public NaNParseErrorDescription() | ||
{ | ||
Message = Error.NaNOnData; | ||
} | ||
} | ||
|
||
public class MismatchingArrayLengthsParseErrorDescription : ParseErrorDescription | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we have all those classes instead of using the string? associated with the message? |
||
{ | ||
public MismatchingArrayLengthsParseErrorDescription() | ||
{ | ||
Message = Error.MismatchingArrayLengths; | ||
} | ||
} | ||
|
||
public class InvalidMappingColumnParseErrorDescription : ParseErrorDescription | ||
{ | ||
public InvalidMappingColumnParseErrorDescription() | ||
{ | ||
Message = Error.InvalidMappingColumn; | ||
} | ||
} | ||
|
||
public class InconsistentDimensionBetweenUnitsParseErrorDescription : ParseErrorDescription | ||
{ | ||
public InconsistentDimensionBetweenUnitsParseErrorDescription(string mappingName) | ||
{ | ||
Message = Error.InconsistentDimensionBetweenUnitsException(mappingName); | ||
} | ||
} | ||
|
||
public class ErrorUnitParseErrorDescription : ParseErrorDescription | ||
{ | ||
public ErrorUnitParseErrorDescription() | ||
{ | ||
Message = Error.InvalidErrorDimension; | ||
} | ||
} | ||
|
||
public class EmptyDataSetsParseErrorDescription : ParseErrorDescription | ||
{ | ||
public EmptyDataSetsParseErrorDescription(IEnumerable<string> dataSetNames) | ||
{ | ||
Message = Error.EmptyDataSet($"{dataSetNames.ToString(", ")}"); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
using OSPSuite.Utility.Collections; | ||
using System.Collections.Generic; | ||
|
||
namespace OSPSuite.Infrastructure.Import.Core.Helpers | ||
{ | ||
static public class CachedListHelpers | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This needs to be removed right? |
||
{ | ||
public static void Add<T1, T2>(Cache<T1, List<T2>> cache, T1 key, T2 x) | ||
{ | ||
if (cache.Contains(key)) | ||
cache[key].Add(x); | ||
else | ||
cache.Add(key, new List<T2>() { x }); | ||
} | ||
|
||
public static void Add<T1, T2>(Cache<T1, List<T2>> cache, T1 key, IEnumerable<T2> list) | ||
{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make in an extension method (Call this CachedListExtensions) and the first one should call the other instead of duplicating. |
||
if (cache.Contains(key)) | ||
cache[key].AddRange(list); | ||
else | ||
cache.Add(key, new List<T2>(list)); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like this as a cache exposed. DataSet are added outside of the DataSource as it stands