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.

Sign up for GitHub

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

Do not accept invalid units #982 #986

Merged
merged 2 commits into from
Apr 29, 2021
Merged

Do not accept invalid units #982 #986

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
773b06f
Do not accept invalid units #982
abdelr Apr 29, 2021
d9d5d95
better using dimension factory to check if the unit exists
abdelr Apr 29, 2021
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
10 changes: 8 additions & 2 deletions src/OSPSuite.Infrastructure.Import/Core/DataFormat/MixColumnsDataFormat.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
using System;
using System.Collections.Generic;
using System.Collections.Generic;
using System.Linq;
using OSPSuite.Core.Domain.UnitSystem;
using OSPSuite.Infrastructure.Import.Core.Extensions;
using System.Text.RegularExpressions;
using OSPSuite.Core.Import;
Expand All @@ -12,7 +12,12 @@ public class MixColumnsDataFormat : AbstractColumnsDataFormat
private const string _name = "Mixin";
private const string _description = "https://github.com/Open-Systems-Pharmacology/OSPSuite.Core/issues/639\rhttps://github.com/Open-Systems-Pharmacology/OSPSuite.Core/issues/797";
public override string Name => _name;
private IDimensionFactory _dimensionFactory;
public override string Description => _description;
public MixColumnsDataFormat(IDimensionFactory dimensionFactory)
{
_dimensionFactory = dimensionFactory;
}
protected override string ExtractLloq(string description, IUnformattedData data, List<string> keys, ref double rank)
{
if (data.GetColumn(description).Any(element => element.Trim().StartsWith("<")))
Expand All @@ -39,6 +44,7 @@ protected override UnitDescription ExtractUnits(string description, IUnformatted
.Substring(1, units.Length - 2) //remove the brackets
.Trim() //remove whitespace
.Split(',') //split comma separated list
.Where(unitName => _dimensionFactory.Dimensions.SelectMany(dimension => dimension.Units).Any(unitFromDimension => unitFromDimension.Name == unitName)) //only accepts valid units
Copy link
Member

Choose a reason for hiding this comment

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

I do not think this is correct here. We have a lot of methods checking whether a unit is valid or not (taking care of case, synonyms etc...)
This needs to change

.FirstOrDefault() ?? UnitDescription.InvalidUnit; //default = ?
rank++;
return new UnitDescription(unit);
Expand Down
45 changes: 44 additions & 1 deletion tests/OSPSuite.Presentation.Tests/Importer/Services/ImporterSpecs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
using OSPSuite.Infrastructure.Import.Core.Mappers;
using OSPSuite.Presentation.Importer.Core.DataFormat;
using OSPSuite.Utility.Collections;
using OSPSuite.Core.Domain.UnitSystem;
using OSPSuite.Core.Import;

namespace OSPSuite.Presentation.Importer.Services
{
Expand Down Expand Up @@ -229,7 +231,7 @@ public override void GlobalContext()
new MetaDataCategory() {Name = "Route"}
};

A.CallTo(() => _container.ResolveAll<IDataFormat>()).Returns(new List<IDataFormat>() { new DataFormatHeadersWithUnits(), new DataFormatNonmem(), new MixColumnsDataFormat() });
A.CallTo(() => _container.ResolveAll<IDataFormat>()).Returns(new List<IDataFormat>() { new DataFormatHeadersWithUnits(), new DataFormatNonmem(), new MixColumnsDataFormat(new DimensionFactory()) });
_parser = A.Fake<IDataSourceFileParser>();
_dataRepositoryMapper = A.Fake<IDataSetToDataRepositoryMapper>();
A.CallTo(() => _container.Resolve<IDataSourceFileParser>()).Returns(_parser);
Expand Down Expand Up @@ -374,4 +376,45 @@ public void rank_mixin_highest_for_its_format()
formats.First().ShouldBeAnInstanceOf(typeof(MixColumnsDataFormat));
}
}

public class When_listing_available_formats_on_mixin_format_with_invalid_unit : ConcernForImporter2
{
protected override void Because()
{
_basicFormat = new TestUnformattedData
(
new Cache<string, ColumnDescription>()
{
{
"Time [invalidUnit]",
new ColumnDescription(5, null,ColumnDescription.MeasurementLevel.Numeric )
},
{
"Concentration",
new ColumnDescription(6, null, ColumnDescription.MeasurementLevel.Numeric)
},
{
"Concentration_unit",
new ColumnDescription(5, new List<string>() { "pmol/l" }, ColumnDescription.MeasurementLevel.Discrete)
},
{
"Error",
new ColumnDescription(7, null, ColumnDescription.MeasurementLevel.Numeric)
},
{
"Error_unit",
new ColumnDescription(5, new List<string>() { "pmol/l" }, ColumnDescription.MeasurementLevel.Discrete)

}
}
);
}

[TestCase]
public void the_invalid_unit_is_detected()
{
var formats = sut.AvailableFormats(_basicFormat, _columnInfos, _metaDataCategories);
(formats.First().Parameters.First(parameter => parameter.ColumnName == "Time [invalidUnit]") as MappingDataFormatParameter).MappedColumn.Unit.SelectedUnit.ShouldBeEqualTo(UnitDescription.InvalidUnit);
}
}
}