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

fix SqlHierarchyId serialization and deserialization #55

Merged
merged 2 commits into from
Dec 11, 2020
Merged

fix SqlHierarchyId serialization and deserialization #55

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
90313e2
fix SqlHierarchyId serialization and deserialization
olmobrutall Oct 21, 2020
87d44e4
add proper HierarchyDbTests
olmobrutall Nov 3, 2020
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
130 changes: 129 additions & 1 deletion src/Microsoft.SqlServer.Types.Tests/HierarchyId/DeserializeFromBinary.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
using Microsoft.SqlServer.Types;
using Microsoft.SqlServer.Types.SqlHierarchy;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using System;
using System.Collections;
using System.Data.SqlClient;
using System.IO;
using System.Text;

namespace Microsoft.SqlServer.Types.Tests.HierarchyId
{
Expand Down Expand Up @@ -48,5 +52,129 @@ public void TestSqlHiarchy2()
}
Assert.AreEqual("/1/-2.18/", hid.ToString());
}

static bool CompareWithSqlServer = false;

[DataTestMethod]
[DataRow("/-4294971464/")]
[DataRow("/4294972495/")]
[DataRow("/3.2725686107/")]
[DataRow("/0/")]
[DataRow("/1/")]
[DataRow("/1.0.2/")]
[DataRow("/1.1.2/")]
[DataRow("/1.2.2/")]
[DataRow("/1.3.2/")]
[DataRow("/3.0/")]
public void SerializeDeserialize(string route)
{
var parsed = SqlHierarchyId.Parse(route);
var ms = new MemoryStream();
parsed.Write(new BinaryWriter(ms));
ms.Position = 0;
var dumMem = Dump(ms);
ms.Position = 0;
var roundTrip = new Microsoft.SqlServer.Types.SqlHierarchyId();
roundTrip.Read(new BinaryReader(ms));
if (parsed != roundTrip)
Assert.AreEqual(parsed, roundTrip);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why have this if statement?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just so I could put a breakpoint in the assert when things went wrong. VS didn’t want to rewind the stacktrace after the exception inside AreEqual. But could be removed now.


if (CompareWithSqlServer)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you be able to always include these tests? In this project, you can find the DBTests.cs file where geometry tests are executed against real DB.

Something in these lines:

HierarchyScripts.cs

    internal class HierarchyScripts
    {

        public static string DropTables = @"
IF OBJECT_ID('dbo.TreeNode', 'U') IS NOT NULL DROP TABLE TreeNode;
";

        public static string CreateTables = @"
-- hierarchy_tests
CREATE TABLE TreeNode(Id IDENTITY PRIMARY KEY, Route HierarchyId NOT NULL);
";
    }

DatabaseUtil.cs (extracted from DBTests.cs)

    static class DatabaseUtil
    {
        internal static void CreateSqlDatabase(string filename)
        {
            string databaseName = System.IO.Path.GetFileNameWithoutExtension(filename);
            if (File.Exists(filename))
                File.Delete(filename);
            if (File.Exists(filename.Replace(".mdf","_log.ldf")))
                File.Delete(filename.Replace(".mdf", "_log.ldf"));
            using (var connection = new SqlConnection(
                @"Data Source=(localdb)\mssqllocaldb;Initial Catalog=master; Integrated Security=true;"))
            {
                connection.Open();
                using (var command = connection.CreateCommand())
                {
                    command.CommandText =
                        String.Format("CREATE DATABASE {0} ON PRIMARY (NAME={0}, FILENAME='{1}')", databaseName, filename);
                    command.ExecuteNonQuery();

                    command.CommandText =
                        String.Format("EXEC sp_detach_db '{0}', 'true'", databaseName);
                    command.ExecuteNonQuery();
                }
            }
        }
    }

And actual tests class

    [TestCategory("Database")]
    [TestCategory("SqlHierarchyId")]
    public class HierarchyDbTests : IDisposable
    {
        const string DataSource = @"Data Source=(localdb)\mssqllocaldb;Integrated Security=True;AttachDbFileName=";

        private readonly SqlConnection _connection;
        private static string _path;
        private static readonly object LockObj = new object();
        static HierarchyDbTests()
        {
            Init();
        }

        private static void Init()
        {
            lock(LockObj)
                if(_path == null)
                {
                    _path = Path.Combine(new FileInfo(typeof(DBTests).Assembly.Location).Directory.FullName, "HierarchyUnitTestData.mdf");
                    DatabaseUtil.CreateSqlDatabase(_path);
                    using (var conn = new SqlConnection(DataSource + _path))
                    {
                        conn.Open();
                        var cmd = conn.CreateCommand();
                        cmd.CommandText = HierarchyScripts.DropTables;
                        cmd.ExecuteNonQuery();
                        cmd.CommandText = HierarchyScripts.CreateTables;
                        cmd.ExecuteNonQuery();
                        conn.Close();
                    }
                }
        }

        private static string ConnectionString => DataSource + _path;

        public HierarchyDbTests()
        {
            Init();
            _connection = new SqlConnection(ConnectionString);
            _connection.Open();
        }
        
        public void Dispose()
        {
            _connection.Close();
            _connection.Dispose();
        }
        
/*
Your tests here
*/
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks good to me, didn’t realize other test where already speaking with the database. Didn’t want to add a dependency because potential problems with CI. But locadb is a great solution.

{
/*
CREATE TABLE [dbo].[TreeNode](
[Id] [int] IDENTITY(1,1) NOT NULL,
[Route] [hierarchyid] NOT NULL
)
*/
using (SqlConnection con = new SqlConnection(@"Data Source=.\SQLEXPRESS;Initial Catalog=HierarchyTest;Integrated Security=true"))
{
con.Open();
var id = new SqlCommand($"INSERT INTO [dbo].[TreeNode] (Route) output INSERTED.ID VALUES ('{route}') ", con).ExecuteScalar();

using (var reader = new SqlCommand($"SELECT Route FROM [dbo].[TreeNode] WHERE ID = " + id, con).ExecuteReader())
{
while (reader.Read())
{
var sqlRoundTrip = new Microsoft.SqlServer.Types.SqlHierarchyId();
var dumSql = Dump(reader.GetStream(0));
Assert.AreEqual(dumMem, dumSql);
sqlRoundTrip.Read(new BinaryReader(reader.GetStream(0)));
if (parsed != sqlRoundTrip)
Assert.AreEqual(parsed, sqlRoundTrip);
}
}
}
}
}

[TestMethod]
public void DeserializeRandom()
Copy link
Contributor

Choose a reason for hiding this comment

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

You could also replace this test method with data-driven method


        [DataTestMethod]
        [DynamicData(nameof(GetData), DynamicDataSourceType.Method)]
        public void SerializeDeserializeRandom(string route)
        {
            // TEST body
        }

        private const int CountOfGeneratedCases = 1000;
        private static HashSet<string> _usedRoutes = new HashSet<string>();
        public static IEnumerable<object[]> GetData()
        {
            Random r = new Random();
            for (var i = 0; i < CountOfGeneratedCases; i++)
            {
                var randomHierarchyId = RandomHierarchyId(r);
                while (_usedRoutes.Contains(randomHierarchyId)) 
                    randomHierarchyId = RandomHierarchyId(r);

                _usedRoutes.Add(randomHierarchyId);
                yield return new object[] {randomHierarchyId};
            }
        }

{
Random r = new Random();
for (int i = 0; i < 10000; i++)
{
SerializeDeserialize(RandomHierarhyId(r));
}
}

public static string RandomHierarhyId(Random random)
{
StringBuilder sb = new StringBuilder();
sb.Append("/");
var levels = random.Next(4);
for (int i = 0; i < levels; i++)
{
var subLevels = random.Next(1, 4);
for (int j = 0; j < subLevels; j++)
{
var pattern = KnownPatterns.RandomPattern(random);
sb.Append(random.NextLong(pattern.MinValue, pattern.MaxValue + 1).ToString());
if (j < subLevels - 1)
sb.Append(".");
}
sb.Append("/");
}

return sb.ToString();
}


static string Dump(Stream ms)
{
return new BitReader(new BinaryReader(ms)).ToString();
}
}

public static class RandomExtensionMethods
{
/// <summary>
/// Returns a random long from min (inclusive) to max (exclusive)
/// </summary>
/// <param name="random">The given random instance</param>
/// <param name="min">The inclusive minimum bound</param>
/// <param name="max">The exclusive maximum bound. Must be greater than min</param>
public static long NextLong(this Random random, long min, long max)
{
if (max <= min)
throw new ArgumentOutOfRangeException("max", "max must be > min!");

//Working with ulong so that modulo works correctly with values > long.MaxValue
ulong uRange = (ulong)(max - min);

//Prevent a modolo bias; see https://stackoverflow.com/a/10984975/238419
//for more information.
//In the worst case, the expected number of calls is 2 (though usually it's
//much closer to 1) so this loop doesn't really hurt performance at all.
ulong ulongRand;
do
{
byte[] buf = new byte[8];
random.NextBytes(buf);
ulongRand = (ulong)BitConverter.ToInt64(buf, 0);
} while (ulongRand > ulong.MaxValue - ((ulong.MaxValue % uRange) + 1) % uRange);

return (long)(ulongRand % uRange) + min;
}
}
}
}
5 changes: 4 additions & 1 deletion src/Microsoft.SqlServer.Types.Tests/Microsoft.SqlServer.Types.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
<Project Sdk="MSBuild.Sdk.Extras/2.0.54">

<PropertyGroup>
<TargetFrameworks>netcoreapp2.1;net461</TargetFrameworks>
<IsPackable>false</IsPackable>
<nullable>enable</nullable>
<LangVersion>8.0</LangVersion>
</PropertyGroup>

<ItemGroup>
<Compile Include="..\Microsoft.SqlServer.Types\SqlHierarchy\BitPattern.cs" Link="HierarchyId\BitPattern.cs" />
<Compile Include="..\Microsoft.SqlServer.Types\SqlHierarchy\BitReader.cs" Link="HierarchyId\BitReader.cs" />
<Compile Include="..\Microsoft.SqlServer.Types\SqlHierarchy\BitWriter.cs" Link="HierarchyId\BitWriter.cs" />
<Compile Include="..\Microsoft.SqlServer.Types\SqlHierarchy\KnownPatterns.cs" Link="HierarchyId\KnownPatterns.cs" />
</ItemGroup>

<ItemGroup>
Expand Down
9 changes: 3 additions & 6 deletions src/Microsoft.SqlServer.Types/SqlHierarchy/BitPattern.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,12 @@ internal bool ContainsValue(long value)

public override string ToString() => Pattern;

public ulong EncodeValue(long val, bool isLast)
public ulong EncodeValue(long val)
{
ulong expand = Expand(PatternMask, val - MinValue);

ulong value = PatternOnes | expand | 1;

if (!isLast)
value++;

return value;
}

Expand All @@ -69,12 +66,12 @@ private ulong Expand(ulong mask, long value)
return Expand(mask >> 1, value) << 1;
}

internal int Decode(ulong encodedValue, out bool isLast)
internal long Decode(ulong encodedValue, out bool isLast)
{
var decodedValue = Compress(encodedValue, PatternMask);

isLast = (encodedValue & 0x1) == 0x1;
return (int)((isLast ? decodedValue : decodedValue - 1) + MinValue);
return ((isLast ? decodedValue : decodedValue - 1) + MinValue);
}

private long Compress(ulong value, ulong mask)
Expand Down
8 changes: 8 additions & 0 deletions src/Microsoft.SqlServer.Types/SqlHierarchy/KnownPatterns.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,14 @@ internal static class KnownPatterns
new BitPattern(-4294971464, -4169, "000101xxxxxxxxxxxxxxxxxxx0xxxxxx0xxx0x1xxxT"),
};

internal static BitPattern RandomPattern(Random r)
{
var index = r.Next(PositivePatterns.Length + NegativePatterns.Length);

return index < PositivePatterns.Length ? PositivePatterns[index] :
NegativePatterns[index - PositivePatterns.Length];
}

internal static BitPattern GetPatternByValue(long value)
{
if (value >= 0)
Expand Down
22 changes: 15 additions & 7 deletions src/Microsoft.SqlServer.Types/SqlHierarchyId.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,15 +260,23 @@ public void Write(BinaryWriter w)
var subNodes = nodes[i];
for (int j = 0; j < subNodes.Length; j++)
{
long val = subNodes[j];

BitPattern p = KnownPatterns.GetPatternByValue(val);

bool isLast = j == (subNodes.Length - 1);

ulong value = p.EncodeValue(val, isLast);

bw.Write(value, p.BitLength);
long val = subNodes[j];

if (isLast)
{
BitPattern p = KnownPatterns.GetPatternByValue(val);
ulong value = p.EncodeValue(val);
bw.Write(value, p.BitLength);
}
else
{
BitPattern p = KnownPatterns.GetPatternByValue(val + 1);
ulong value = p.EncodeValue(val + 1) - 1;
bw.Write(value, p.BitLength);
}
}
}

Expand Down Expand Up @@ -305,7 +313,7 @@ public void Read(BinaryReader r)

ulong encodedValue = bitR.Read(p.BitLength);

int value = p.Decode(encodedValue, out bool isLast);
long value = p.Decode(encodedValue, out bool isLast);

step.Add(value);

Expand Down