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

added method to detect if a yaml fragment is a sequence #720

Merged
merged 2 commits into from
Sep 18, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -198,4 +198,21 @@ public void RoundTripFromYaml(string path)
}

#endregion

#region Is Sequence checks

[TestMethod]
[DataRow(@"_TestData/SchemaV3_0/Examples/Src/App.pa.yaml", false)]
[DataRow(@"_TestData/SchemaV3_0/Examples/Src/Screens/Screen1.pa.yaml", false)]
[DataRow(@"_TestData/SchemaV3_0/Examples/Src/Components/MyHeaderComponent.pa.yaml", false)]
[DataRow(@"_TestData/ValidYaml-CI/With-list-of-controls.pa.yaml", true)]
[DataRow(@"_TestData/InvalidYaml/Dupliacte-Keys.pa.yaml", false)]
public void IsSequenceCheckShouldWorkAsExpected(string path, bool expected)
Copy link
Contributor

Choose a reason for hiding this comment

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

please simplify this unit tests so it doesn't require external files for the baseline cases.
use targeted input strings for testing, don't use existing large files as these can change due to their purpose etc.

{
var yaml = File.ReadAllText(path);
var isSequence = PaYamlSerializer.CheckIsSequence(yaml);
isSequence.Should().Be(expected);
}

#endregion
}
40 changes: 40 additions & 0 deletions src/Persistence/PaYaml/Serialization/PaYamlSerializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -128,5 +128,45 @@ private static void WriteTextWriter<TValue>(TextWriter writer, in TValue? value,
// See: fluentvalidation.net
// See: https://www.youtube.com/watch?v=jblRYDMTtvg
}

/// <summary>
/// checks the input source is a sequence of YAML items.
/// </summary>
/// <param name="yaml">The YAML text to check.</param>
/// <returns></returns>
/// <exception cref="ArgumentNullException"></exception>
public static bool CheckIsSequence(string yaml)
{
_ = yaml ?? throw new ArgumentNullException(nameof(yaml));

using var reader = new StringReader(yaml);
return CheckIsSequence(reader);
}

/// <summary>
/// checks the input source is a sequence of YAML items.
/// </summary>
/// <param name="reader"></param>
/// <returns></returns>
/// <exception cref="ArgumentNullException"></exception>
public static bool CheckIsSequence(StringReader reader)
{
ArgumentNullException.ThrowIfNull(reader);

var builder = new DeserializerBuilder()
.IgnoreUnmatchedProperties()
.WithTypeConverter(new YamlSequenceTesterConverter());
var deserializer = builder.Build();
try
{
var results = deserializer.Deserialize<object[]>(reader);
return results is not null;
}
catch (YamlException)
Copy link
Contributor

Choose a reason for hiding this comment

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

we should not need to use exceptions as control-flow.

Another design option would be to deseralize into a custom type, register a converter for that type, and in the converter populate a property which has the 'result' of the detection.

e..g

private record SequenceDetectorResult
{
  public required bool IsSequence { get; init; }
}

{
return false;
}
}

#endregion
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System.Collections;
using YamlDotNet.Core;
using YamlDotNet.Core.Events;
using YamlDotNet.Serialization;

namespace Microsoft.PowerPlatform.PowerApps.Persistence.PaYaml.Serialization;

/// <summary>
/// custom converter used to test if the input is a sequence of YAML items.
/// </summary>
internal sealed class YamlSequenceTesterConverter : IYamlTypeConverter
{
public bool Accepts(Type type)
{
return type == typeof(IEnumerable) || type.IsSubclassOf(typeof(IEnumerable)) ||
type == typeof(IEnumerable<object>) || type.IsSubclassOf(typeof(IEnumerable<object>)) ||
type == typeof(object[]);
}

public object? ReadYaml(IParser parser, Type type)
{
if (parser.Current is not SequenceStart)
Copy link
Contributor

Choose a reason for hiding this comment

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

when using the SequenceDetectorResult pattern I mentioned above, this converter could just have this SequenceStart check, completely skip the contents, and then return the result value.

the benefit here is that there wouldn't be an exception thrown for code flow control.

throw new YamlException(parser.Current!.Start, parser.Current.End, $"Expected sequence start but got {parser.Current.GetType().Name}");

while (!parser.Accept<SequenceEnd>(out _))
{
parser.MoveNext();
}

parser.MoveNext();

return Array.Empty<object>();
}

public void WriteYaml(IEmitter emitter, object? value, Type type)
{
throw new NotImplementedException();
}
}
Loading