Skip to content

Commit

Permalink
Fix ValueSet type to revert a breaking API change (#829)
Browse files Browse the repository at this point in the history 
* Fix ValueSet type to revert a breaking API change
  • Loading branch information
@sanxing-chen @tellarin
sanxing-chen authored and tellarin committed Sep 3, 2018
1 parent 79f941d commit f06c68e
Show file tree
Hide file tree
Showing 8 changed files with 65 additions and 119 deletions.
18 changes: 13 additions & 5 deletions ...crosoft.Recognizers.Text.DataDrivenTests/DateTime/TestDateTimeRecognizerInitialization.cs
View file
Expand Up @@ -129,13 +129,21 @@ private void TestDateTime(IModel testedModel, IModel controlModel, string source

Assert.AreEqual(expected.TypeName, actual.TypeName, source);
Assert.AreEqual(expected.Text, actual.Text, source);
if (expected.Start != 0) Assert.AreEqual(expected.Start, actual.Start, source);
if (expected.End != 0) Assert.AreEqual(expected.End, actual.End, source);

var listValues = actual.Resolution[ResolutionKey.ValueSet] as IList<Dictionary<string, object>>;
if (expected.Start != 0)
{
Assert.AreEqual(expected.Start, actual.Start, source);
}

if (expected.End != 0)
{
Assert.AreEqual(expected.End, actual.End, source);
}

// Actual ValueSet types should not be modified as that's considered a breaking API change
var listValues = actual.Resolution[ResolutionKey.ValueSet] as IList<Dictionary<string, string>>;
var actualValues = listValues.FirstOrDefault();

var expectedObj = expected.Resolution[ResolutionKey.ValueSet] as IList<Dictionary<string, object>>;
var expectedObj = expected.Resolution[ResolutionKey.ValueSet] as IList<Dictionary<string, string>>;
var expectedValues = expectedObj.FirstOrDefault();

CollectionAssert.AreEqual(expectedValues, actualValues, source);
Expand Down
75 changes: 20 additions & 55 deletions .NET/Microsoft.Recognizers.Text.DataDrivenTests/TestBase.cs
View file
Expand Up @@ -7,7 +7,6 @@
using Microsoft.VisualStudio.TestTools.UnitTesting;

using Newtonsoft.Json;
using Newtonsoft.Json.Linq;

namespace Microsoft.Recognizers.Text.DataDrivenTests
{
Expand Down Expand Up @@ -93,29 +92,19 @@ public void TestDateTime()
Assert.AreEqual(expected.End, actual.End, GetMessage(TestSpec));

var values = actual.Resolution as IDictionary<string, object>;
var actualValues = ((List<Dictionary<string, object>>)values[ResolutionKey.ValueSet]).ToList();
var expectedValues = JsonConvert.DeserializeObject<List<Dictionary<string, object>>>(
expected.Resolution[ResolutionKey.ValueSet].ToString(),
new JsonSerializerSettings {DateParseHandling = DateParseHandling.None});

// Actual ValueSet types should not be modified as that's considered a breaking API change
var actualValues = ((List<Dictionary<string, string>>)values[ResolutionKey.ValueSet]).ToList();
var expectedValues =
JsonConvert.DeserializeObject<IList<Dictionary<string, string>>>(expected
.Resolution[ResolutionKey.ValueSet].ToString());

Assert.AreEqual(expectedValues.Count, actualValues.Count, GetMessage(TestSpec));

foreach (var value in expectedValues.Zip(actualValues, Tuple.Create))
{
Assert.AreEqual(value.Item1.Count, value.Item2.Count, GetMessage(TestSpec));

foreach (var o in value.Item1)
{
if (o.Value is string)
{
Assert.AreEqual(o.Value, value.Item2[o.Key], GetMessage(TestSpec));
}
else
{
CollectionAssert.AreEqual(((JArray)o.Value).ToObject<List<string>>(),
(List<string>)value.Item2[o.Key]);
}
}
CollectionAssert.AreEqual(value.Item1, value.Item2, GetMessage(TestSpec));
}
}
}
Expand Down Expand Up @@ -145,33 +134,21 @@ public void TestDateTimeAlt()
GetMessage(TestSpec));
}

var values = actual.Resolution as IDictionary<string, object>;
var listValues = values[ResolutionKey.ValueSet] as IList<Dictionary<string, object>>;
var actualValues = listValues;
// Actual ValueSet types should not be modified as that's considered a breaking API change
var actualValues =
((IDictionary<string, object>)actual.Resolution)[ResolutionKey.ValueSet] as
IList<Dictionary<string, string>>;

var expectedObj = JsonConvert.DeserializeObject<IList<Dictionary<string, object>>>(
expected.Resolution[ResolutionKey.ValueSet].ToString(),
new JsonSerializerSettings {DateParseHandling = DateParseHandling.None});
var expectedValues = expectedObj;
var expectedValues =
JsonConvert.DeserializeObject<IList<Dictionary<string, string>>>(expected
.Resolution[ResolutionKey.ValueSet].ToString());

Assert.AreEqual(expectedValues.Count, actualValues.Count, GetMessage(TestSpec));

foreach (var value in expectedValues.Zip(actualValues, Tuple.Create))
{
Assert.AreEqual(value.Item1.Count, value.Item2.Count, GetMessage(TestSpec));

foreach (var o in value.Item1)
{
if (o.Value is string)
{
Assert.AreEqual(o.Value, value.Item2[o.Key], GetMessage(TestSpec));
}
else
{
CollectionAssert.AreEqual(((JArray)o.Value).ToObject<List<string>>(),
(List<string>)value.Item2[o.Key]);
}
}
CollectionAssert.AreEqual(value.Item1, value.Item2, GetMessage(TestSpec));
}
}
}
Expand Down Expand Up @@ -273,30 +250,18 @@ public void TestDateTimeMergedParser()

if (actual.Value is IDictionary<string, object> values)
{
var actualValues = values[ResolutionKey.ValueSet] as IList<Dictionary<string, object>>;
// Actual ValueSet types should not be modified as that's considered a breaking API change
var actualValues = values[ResolutionKey.ValueSet] as IList<Dictionary<string, string>>;

var expectedObj =
JsonConvert.DeserializeObject<IDictionary<string, IList<Dictionary<string, object>>>>(
expected.Value.ToString(),
new JsonSerializerSettings {DateParseHandling = DateParseHandling.None});
JsonConvert.DeserializeObject<IDictionary<string, IList<Dictionary<string, string>>>>(
expected.Value.ToString());
var expectedValues = expectedObj[ResolutionKey.ValueSet];

foreach (var value in expectedValues.Zip(actualValues, Tuple.Create))
{
Assert.AreEqual(value.Item1.Count, value.Item2.Count, GetMessage(TestSpec));

foreach (var o in value.Item1)
{
if (o.Value is string)
{
Assert.AreEqual(o.Value, value.Item2[o.Key], GetMessage(TestSpec));
}
else
{
CollectionAssert.AreEqual(((JArray)o.Value).ToObject<List<string>>(),
(List<string>)value.Item2[o.Key]);
}
}
CollectionAssert.AreEqual(value.Item1, value.Item2, GetMessage(TestSpec));
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion .NET/Microsoft.Recognizers.Text.DateTime/Parsers/BaseDatePeriodParser.cs
View file
Expand Up @@ -1268,6 +1268,7 @@ private DateTimeResolutionResult ParseDuration(string text, DateObject reference
return ret;
}

// TODO: This method should be refactored as it takes too many parameters and has too many side-effects
private static void GetModAndDate(out DateObject beginDate, ref DateObject endDate, DateObject referenceDate,
string timex, bool future, out string mod, out List<DateObject> dateList)
{
Expand Down Expand Up @@ -1919,4 +1920,4 @@ public List<DateTimeParseResult> FilterResults(string query, List<DateTimeParseR
return candidateResults;
}
}
}
}
39 changes: 20 additions & 19 deletions .NET/Microsoft.Recognizers.Text.DateTime/Parsers/BaseMergedDateTimeParser.cs
View file
Expand Up @@ -284,15 +284,15 @@ public DateTimeParseResult SetInclusivePeriodEnd(DateTimeParseResult slot)

if (value != null && value.ContainsKey(ResolutionKey.ValueSet))
{
if (value[ResolutionKey.ValueSet] is IList<Dictionary<string, object>> valueSet && valueSet.Any())
if (value[ResolutionKey.ValueSet] is IList<Dictionary<string, string>> valueSet && valueSet.Any())
{
foreach (var values in valueSet)
{
// This is only a sanity check, as here we only handle DatePeriod like "(StartDate,EndDate,Duration)"
if (values.ContainsKey(DateTimeResolutionKey.START) && values.ContainsKey(DateTimeResolutionKey.END) && values.ContainsKey(DateTimeResolutionKey.Timex))
{
var startDate = DateObject.Parse(values[DateTimeResolutionKey.START].ToString());
var endDate = DateObject.Parse(values[DateTimeResolutionKey.END].ToString());
var startDate = DateObject.Parse(values[DateTimeResolutionKey.START]);
var endDate = DateObject.Parse(values[DateTimeResolutionKey.END]);
var durationStr = timexComponents[2];
var datePeriodTimexType = TimexUtility.GetDatePeriodTimexType(durationStr);
endDate = TimexUtility.OffsetDateObject(endDate, offset: 1, timexType: datePeriodTimexType);
Expand All @@ -301,7 +301,7 @@ public DateTimeParseResult SetInclusivePeriodEnd(DateTimeParseResult slot)

if (string.IsNullOrEmpty(altTimex))
{
altTimex = values[DateTimeResolutionKey.Timex].ToString();
altTimex = values[DateTimeResolutionKey.Timex];
}
}
}
Expand Down Expand Up @@ -406,7 +406,7 @@ public List<DateTimeParseResult> DateTimeResolutionForSplit(DateTimeParseResult
return null;
}

var resolutions = new List<Dictionary<string, object>>();
var resolutions = new List<Dictionary<string, string>>();
var res = new Dictionary<string, object>();

var type = slot.Type;
Expand All @@ -418,14 +418,14 @@ public List<DateTimeParseResult> DateTimeResolutionForSplit(DateTimeParseResult
return null;
}

var islunar = val.IsLunar;
var isLunar = val.IsLunar;
var mod = val.Mod;
List<string> list = null;
string list = null;

// Resolve dates list for date periods
if (slot.Type.Equals(Constants.SYS_DATETIME_DATEPERIOD) && val.List != null)
{
list = val.List.Select(o => FormatUtil.LuisDate((DateObject)o)).ToList();
list = string.Join(",", val.List.Select(o => FormatUtil.LuisDate((DateObject)o)).ToArray());
}

// With modifier, output Type might not be the same with type in resolution result
Expand All @@ -438,7 +438,7 @@ public List<DateTimeParseResult> DateTimeResolutionForSplit(DateTimeParseResult
AddResolutionFields(res, Constants.Comment, comment);
AddResolutionFields(res, DateTimeResolutionKey.Mod, mod);
AddResolutionFields(res, ResolutionKey.Type, typeOutput);
AddResolutionFields(res, DateTimeResolutionKey.IsLunar, islunar ? islunar.ToString() : string.Empty);
AddResolutionFields(res, DateTimeResolutionKey.IsLunar, isLunar ? isLunar.ToString() : string.Empty);

var hasTimeZone = false;

Expand Down Expand Up @@ -523,12 +523,12 @@ public List<DateTimeParseResult> DateTimeResolutionForSplit(DateTimeParseResult
{
if (p.Value is Dictionary<string, string> dictionary)
{
var value = new Dictionary<string, object>();
var value = new Dictionary<string, string>();

AddResolutionFields(value, DateTimeResolutionKey.Timex, timex);
AddResolutionFields(value, DateTimeResolutionKey.Mod, mod);
AddResolutionFields(value, ResolutionKey.Type, typeOutput);
AddResolutionFields(value, DateTimeResolutionKey.IsLunar, islunar ? islunar.ToString() : string.Empty);
AddResolutionFields(value, DateTimeResolutionKey.IsLunar, isLunar ? isLunar.ToString() : string.Empty);
AddResolutionFields(value, DateTimeResolutionKey.List, list);

if (hasTimeZone)
Expand Down Expand Up @@ -556,7 +556,7 @@ public List<DateTimeParseResult> DateTimeResolutionForSplit(DateTimeParseResult

if (resolutionPast.Count == 0 && resolutionFuture.Count == 0 && val.TimeZoneResolution == null)
{
var notResolved = new Dictionary<string, object> {
var notResolved = new Dictionary<string, string> {
{
DateTimeResolutionKey.Timex, timex
},
Expand Down Expand Up @@ -592,16 +592,17 @@ private string DetermineResolutionDateTimeType(Dictionary<string, string> pastRe
}
}

internal void AddResolutionFields(Dictionary<string, object> dic, string key, object value)
internal void AddResolutionFields(Dictionary<string, string> dic, string key, string value)
{
if (value is string v)
if (!string.IsNullOrEmpty(value))
{
if (!string.IsNullOrEmpty(v))
{
dic.Add(key, v);
}
dic.Add(key, value);
}
else if (value != null)
}

internal void AddResolutionFields(Dictionary<string, object> dic, string key, object value)
{
if (value != null)
{
dic.Add(key, value);
}
Expand Down
6 changes: 3 additions & 3 deletions .NET/Microsoft.Recognizers.Text.DateTime/Parsers/FullDateTimeParser.cs
View file
Expand Up @@ -191,7 +191,7 @@ public string DetermineDateTimeType(string type, bool hasBefore, bool hasAfter,

public SortedDictionary<string, object> DateTimeResolution(DateTimeParseResult slot, bool hasBefore, bool hasAfter, bool hasSince)
{
var resolutions = new List<Dictionary<string, object>>();
var resolutions = new List<Dictionary<string, string>>();
var res = new Dictionary<string, object>();

var type = slot.Type;
Expand Down Expand Up @@ -278,7 +278,7 @@ public string DetermineDateTimeType(string type, bool hasBefore, bool hasAfter,
{
if (p.Value is Dictionary<string, string> dictionary)
{
var value = new Dictionary<string, object>();
var value = new Dictionary<string, string>();

if (!string.IsNullOrEmpty(timex))
{
Expand Down Expand Up @@ -313,7 +313,7 @@ public string DetermineDateTimeType(string type, bool hasBefore, bool hasAfter,

if (resolutionPast.Count == 0 && resolutionFuture.Count == 0)
{
var notResolved = new Dictionary<string, object> {
var notResolved = new Dictionary<string, string> {
{
DateTimeResolutionKey.Timex, timex
}, {
Expand Down
1 change: 1 addition & 0 deletions .gitignore
View file
@@ -1,5 +1,6 @@
/.vs/Microsoft.Recognizers.Text/v15
/.vs/Microsoft.Recognizers.Text/v14
/.vs/slnx.sqlite
/TestResults
/packages
/.idea
21 changes: 3 additions & 18 deletions Specs/DateTime/English/DateTimeModel.json
View file
Expand Up @@ -6186,12 +6186,7 @@
{
"timex": "(2018-08-21,2018-08-25,P4BD)",
"type": "daterange",
"list": [
"2018-08-21",
"2018-08-22",
"2018-08-23",
"2018-08-24"
],
"list": "2018-08-21,2018-08-22,2018-08-23,2018-08-24",
"start": "2018-08-21",
"end": "2018-08-25"
}
Expand All @@ -6217,12 +6212,7 @@
{
"timex": "(2018-08-22,2018-08-28,P4BD)",
"type": "daterange",
"list": [
"2018-08-22",
"2018-08-23",
"2018-08-24",
"2018-08-27"
],
"list": "2018-08-22,2018-08-23,2018-08-24,2018-08-27",
"start": "2018-08-22",
"end": "2018-08-28"
}
Expand All @@ -6248,12 +6238,7 @@
{
"timex": "(2018-08-15,2018-08-21,P4BD)",
"type": "daterange",
"list": [
"2018-08-15",
"2018-08-16",
"2018-08-17",
"2018-08-20"
],
"list": "2018-08-15,2018-08-16,2018-08-17,2018-08-20",
"start": "2018-08-15",
"end": "2018-08-21"
}
Expand Down
21 changes: 3 additions & 18 deletions Specs/DateTime/English/DateTimeModelComplexCalendar.json
View file
Expand Up @@ -5200,12 +5200,7 @@
{
"timex": "(2018-08-21,2018-08-25,P4BD)",
"type": "daterange",
"list": [
"2018-08-21",
"2018-08-22",
"2018-08-23",
"2018-08-24"
],
"list": "2018-08-21,2018-08-22,2018-08-23,2018-08-24",
"start": "2018-08-21",
"end": "2018-08-25"
}
Expand All @@ -5231,12 +5226,7 @@
{
"timex": "(2018-08-22,2018-08-28,P4BD)",
"type": "daterange",
"list": [
"2018-08-22",
"2018-08-23",
"2018-08-24",
"2018-08-27"
],
"list": "2018-08-22,2018-08-23,2018-08-24,2018-08-27",
"start": "2018-08-22",
"end": "2018-08-28"
}
Expand All @@ -5262,12 +5252,7 @@
{
"timex": "(2018-08-15,2018-08-21,P4BD)",
"type": "daterange",
"list": [
"2018-08-15",
"2018-08-16",
"2018-08-17",
"2018-08-20"
],
"list": "2018-08-15,2018-08-16,2018-08-17,2018-08-20",
"start": "2018-08-15",
"end": "2018-08-21"
}
Expand Down

0 comments on commit f06c68e

Please sign in to comment.