Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
@@ -1,5 +1,6 @@
using System.Data;

Check notice on line 1 in src/Umbraco.Cms.Persistence.Sqlite/Services/SqliteSyntaxProvider.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (v17/dev)

✅ Getting better: Primitive Obsession

The ratio of primitive types in function arguments decreases from 45.83% to 43.14%, threshold = 30.0% The functions in this file have too many primitive types (e.g. int, double, float) in their function argument lists. Using many primitive types lead to the code smell Primitive Obsession. Avoid adding more primitive arguments.
using System.Data.Common;
using System.Globalization;
using System.Linq.Expressions;
using System.Reflection;
using System.Runtime.InteropServices;
Expand Down Expand Up @@ -104,6 +105,22 @@
/// </remarks>
public override string FormatGuid(Guid guid) => guid.ToString().ToUpperInvariant();

/// <inheritdoc />
/// <remarks>
/// SQLite stores date and time values as text, so this and the other date and time formats match those the ADO.NET provider writes for
Comment thread
AndyButland marked this conversation as resolved.
Resolved
/// parameters, keeping values comparable and sortable whichever way they were written.
/// </remarks>
public override string FormatDateTimeOffset(DateTimeOffset dateTimeOffset)
=> dateTimeOffset.ToString(@"yyyy\-MM\-dd HH\:mm\:ss.FFFFFFFzzz", CultureInfo.InvariantCulture);

/// <inheritdoc />
public override string FormatDateOnly(DateOnly date)
=> date.ToString(@"yyyy\-MM\-dd", CultureInfo.InvariantCulture);

/// <inheritdoc />
public override string FormatTimeOnly(TimeOnly time)
=> time.ToString(time.Ticks % TimeSpan.TicksPerSecond == 0 ? "HH:mm:ss" : "HH:mm:ss.fffffff", CultureInfo.InvariantCulture);

/// <inheritdoc />
public override string Format(TableDefinition table)
{
Expand Down
20 changes: 14 additions & 6 deletions src/Umbraco.Infrastructure/Migrations/MigrationExpressionBase.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Runtime.ConstrainedExecution;
using System.Globalization;
using System.Text;
using Microsoft.Extensions.Logging;
using NPoco;
Expand Down Expand Up @@ -181,34 +181,42 @@
return "NULL";
}

// Format Guids via the syntax provider to ensure consistent storage (e.g. Guids need to be uppercase for SQLite).
if (val is Guid guid)
// Format Guids and date/time types via the syntax provider to ensure consistent storage
// (e.g. Guids need to be uppercase for SQLite) and literals the database reads unambiguously.
switch (val)
{
return SqlSyntax.GetQuotedValue(SqlSyntax.FormatGuid(guid));
case Guid guid:
return SqlSyntax.GetQuotedValue(SqlSyntax.FormatGuid(guid));
case DateTimeOffset dateTimeOffset:
return SqlSyntax.GetQuotedValue(SqlSyntax.FormatDateTimeOffset(dateTimeOffset));
case DateOnly date:
return SqlSyntax.GetQuotedValue(SqlSyntax.FormatDateOnly(date));
case TimeOnly time:
return SqlSyntax.GetQuotedValue(SqlSyntax.FormatTimeOnly(time));
}

Type type = val.GetType();

switch (Type.GetTypeCode(type))
{
case TypeCode.Boolean:
return (bool)val ? "1" : "0";
case TypeCode.Single:
case TypeCode.Double:
case TypeCode.Decimal:
case TypeCode.SByte:
case TypeCode.Int16:
case TypeCode.Int32:
case TypeCode.Int64:
case TypeCode.Byte:
case TypeCode.UInt16:
case TypeCode.UInt32:
case TypeCode.UInt64:
return val.ToString()!;
return Convert.ToString(val, CultureInfo.InvariantCulture)!;
case TypeCode.DateTime:
return SqlSyntax.GetQuotedValue(SqlSyntax.FormatDateTime((DateTime)val));
default:
return SqlSyntax.GetQuotedValue(val.ToString()!);
return SqlSyntax.GetQuotedValue(Convert.ToString(val, CultureInfo.InvariantCulture)!);

Check warning on line 219 in src/Umbraco.Infrastructure/Migrations/MigrationExpressionBase.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (v17/dev)

❌ Getting worse: Complex Method

GetQuotedValue increases in cyclomatic complexity from 18 to 21, threshold = 9 This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Data;

Check notice on line 1 in src/Umbraco.Infrastructure/Persistence/SqlSyntax/ISqlSyntaxProvider.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (v17/dev)

✅ Getting better: Primitive Obsession

The ratio of primitive types in function arguments decreases from 70.59% to 60.00%, threshold = 30.0% The functions in this file have too many primitive types (e.g. int, double, float) in their function argument lists. Using many primitive types lead to the code smell Primitive Obsession. Avoid adding more primitive arguments.

Check notice on line 1 in src/Umbraco.Infrastructure/Persistence/SqlSyntax/ISqlSyntaxProvider.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (v17/dev)

✅ Getting better: String Heavy Function Arguments

The ratio of strings in function arguments decreases from 64.71% to 55.00%, threshold = 39.0% The functions in this file have a high ratio of strings as arguments. Avoid adding more.
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Linq.Expressions;
using System.Text.RegularExpressions;
using NPoco;
Expand All @@ -9,8 +10,6 @@

namespace Umbraco.Cms.Infrastructure.Persistence.SqlSyntax;

// TODO (V18): Remove the default implementations in this interface.

/// <summary>
/// Defines an SqlSyntaxProvider.
/// </summary>
Expand Down Expand Up @@ -347,6 +346,31 @@
/// <returns>A string representation of the guid formatted for SQL.</returns>
string FormatGuid(Guid guid) => guid.ToString();

/// <summary>
/// Formats a <see cref="DateTimeOffset"/> value as a string suitable for use in SQL queries.
/// </summary>
/// <param name="dateTimeOffset">The date, time and offset value to format.</param>
/// <returns>A string representation of the value formatted for SQL, independent of the current culture.</returns>
// TODO (V20): Remove the default implementation.
string FormatDateTimeOffset(DateTimeOffset dateTimeOffset)
=> dateTimeOffset.ToString("yyyy-MM-ddTHH:mm:ss.fffffffzzz", CultureInfo.InvariantCulture);

/// <summary>
/// Formats a <see cref="DateOnly"/> value as a string suitable for use in SQL queries.
/// </summary>
/// <param name="date">The date value to format.</param>
/// <returns>A string representation of the date formatted for SQL, independent of the current culture.</returns>
// TODO (V20): Remove the default implementation.
string FormatDateOnly(DateOnly date) => date.ToString("yyyyMMdd", CultureInfo.InvariantCulture);

/// <summary>
/// Formats a <see cref="TimeOnly"/> value as a string suitable for use in SQL queries.
/// </summary>
/// <param name="time">The time value to format.</param>
/// <returns>A string representation of the time formatted for SQL, independent of the current culture.</returns>
// TODO (V20): Remove the default implementation.
string FormatTimeOnly(TimeOnly time) => time.ToString("HH:mm:ss.fffffff", CultureInfo.InvariantCulture);

/// <summary>
/// Formats a <see cref="TableDefinition"/> into its corresponding SQL statement.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Don't remove the unused System using, for some reason this breaks docfx, and I have no clue why.

Check notice on line 1 in src/Umbraco.Infrastructure/Persistence/SqlSyntax/SqlSyntaxProviderBase.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (v17/dev)

✅ Getting better: Primitive Obsession

The ratio of primitive types in function arguments decreases from 55.42% to 53.49%, threshold = 30.0% The functions in this file have too many primitive types (e.g. int, double, float) in their function argument lists. Using many primitive types lead to the code smell Primitive Obsession. Avoid adding more primitive arguments.

Check notice on line 1 in src/Umbraco.Infrastructure/Persistence/SqlSyntax/SqlSyntaxProviderBase.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (v17/dev)

✅ Getting better: String Heavy Function Arguments

The ratio of strings in function arguments decreases from 46.99% to 45.35%, threshold = 39.0% The functions in this file have a high ratio of strings as arguments. Avoid adding more.
using System;
using System.Data;
using System.Diagnostics.CodeAnalysis;
Expand All @@ -11,7 +11,6 @@
using Umbraco.Cms.Core.Persistence;
using Umbraco.Cms.Infrastructure.Persistence.DatabaseAnnotations;
using Umbraco.Cms.Infrastructure.Persistence.DatabaseModelDefinitions;
using Umbraco.Cms.Infrastructure.Persistence.Dtos;
using Umbraco.Extensions;

namespace Umbraco.Cms.Infrastructure.Persistence.SqlSyntax;
Expand Down Expand Up @@ -311,11 +310,11 @@
public virtual string GetQuotedName(string? name) => $"\"{name}\"";

/// <summary>
/// Returns the specified string value enclosed in single quotes.
/// Returns the specified string value enclosed in single quotes, with any single quotes it contains escaped.
/// </summary>
/// <param name="value">The string value to quote.</param>
/// <returns>The quoted string value.</returns>
public virtual string GetQuotedValue(string value) => $"'{value}'";
public virtual string GetQuotedValue(string value) => $"'{value.Replace("'", "''")}'";
Comment thread
AndyButland marked this conversation as resolved.
Resolved

/// <inheritdoc />
public virtual string GetNullCastSuffix<T>() => string.Empty;
Expand Down Expand Up @@ -569,6 +568,24 @@
/// <inheritdoc />
public virtual string FormatGuid(Guid guid) => guid.ToString();

/// <inheritdoc />
/// <remarks>
/// ISO 8601 with a "T" separator is unambiguous for SQL Server date types, whatever the language
/// or DATEFORMAT setting of the connection.
/// </remarks>
public virtual string FormatDateTimeOffset(DateTimeOffset dateTimeOffset)
=> dateTimeOffset.ToString("yyyy-MM-ddTHH:mm:ss.fffffffzzz", CultureInfo.InvariantCulture);

/// <inheritdoc />
/// <remarks>
/// YYYYMMDD is unambiguous for all SQL Server date types, whatever the language or DATEFORMAT
/// setting of the connection.
/// </remarks>
public virtual string FormatDateOnly(DateOnly date) => date.ToString("yyyyMMdd", CultureInfo.InvariantCulture);

/// <inheritdoc />
public virtual string FormatTimeOnly(TimeOnly time) => time.ToString("HH:mm:ss.fffffff", CultureInfo.InvariantCulture);

/// <summary>
/// Formats a SQL create table statement for the specified table definition.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Umbraco.
// See LICENSE for more details.

using System.Globalization;
using Microsoft.Extensions.Options;
using NUnit.Framework;
using Umbraco.Cms.Core.Configuration.Models;
Expand All @@ -11,16 +12,53 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Cms.Persistence.SqlServer;
[TestFixture]
public class SqlServerSyntaxProviderTests
{
private static readonly DateTime _dateTime = new(2026, 11, 3, 10, 30, 15, 250);
private static readonly DateTimeOffset _dateTimeOffset = new(2026, 11, 3, 10, 30, 15, 250, TimeSpan.FromHours(-1));
private static readonly DateOnly _date = new(2026, 11, 3);
private static readonly TimeOnly _time = new(10, 30, 15, 250);

private CultureInfo _originalCulture = null!;

// fi-FI formats dates, times, decimals and negative signs differently from the invariant culture,
// so any formatting that depends on the current culture shows up in the results.
[SetUp]
public void SetUp()
{
_originalCulture = CultureInfo.CurrentCulture;
CultureInfo.CurrentCulture = CultureInfo.GetCultureInfo("fi-FI");
}

[TearDown]
public void TearDown() => CultureInfo.CurrentCulture = _originalCulture;

[Test]
public void Can_Format_Guid_Unchanged()
{
var sut = new SqlServerSyntaxProvider(Options.Create(new GlobalSettings()));

var result = sut.FormatGuid(new Guid("a1b2c3d4-e5f6-7890-abcd-ef1234567890"));
var result = CreateSut().FormatGuid(new Guid("a1b2c3d4-e5f6-7890-abcd-ef1234567890"));

Assert.That(result, Is.EqualTo("a1b2c3d4-e5f6-7890-abcd-ef1234567890"));
}

[TestCase(true, ExpectedResult = "20261103 10:30:15")]
[TestCase(false, ExpectedResult = "20261103")]
public string Can_Format_DateTime(bool includeTime) => CreateSut().FormatDateTime(_dateTime, includeTime);

[Test]
public void Can_Format_DateTimeOffset()
=> Assert.That(CreateSut().FormatDateTimeOffset(_dateTimeOffset), Is.EqualTo("2026-11-03T10:30:15.2500000-01:00"));

[Test]
public void Can_Format_DateOnly()
=> Assert.That(CreateSut().FormatDateOnly(_date), Is.EqualTo("20261103"));

[Test]
public void Can_Format_TimeOnly()
=> Assert.That(CreateSut().FormatTimeOnly(_time), Is.EqualTo("10:30:15.2500000"));

[Test]
public void Can_Quote_And_Escape_A_String_Value()
=> Assert.That(CreateSut().GetQuotedValue("O'Brien's"), Is.EqualTo("'O''Brien''s'"));

[TestCase("", SqlServerSyntaxProvider.VersionName.Invalid)]
[TestCase("13.0.7050.2", SqlServerSyntaxProvider.VersionName.V2016)]
[TestCase("14.0.3485.1", SqlServerSyntaxProvider.VersionName.V2017)]
Expand All @@ -40,4 +78,6 @@ public void Can_Map_Product_Version_To_Version_Name(string productVersion, SqlSe

Assert.That(result.ProductVersionName, Is.EqualTo(expected));
}

private static SqlServerSyntaxProvider CreateSut() => new(Options.Create(new GlobalSettings()));
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,74 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Cms.Persistence.Sqlite;
[TestFixture]
public class SqliteSyntaxProviderTests
{
private static readonly DateTime _dateTime = new(2026, 11, 3, 10, 30, 15, 250);
private static readonly DateTimeOffset _dateTimeOffset = new(2026, 11, 3, 10, 30, 15, 250, TimeSpan.FromHours(-1));
private static readonly DateOnly _date = new(2026, 11, 3);
private static readonly TimeOnly _time = new(10, 30, 15, 250);
private static readonly TimeOnly _wholeSecondTime = new(10, 30, 15);

private CultureInfo _originalCulture = null!;

// fi-FI formats dates, times, decimals and negative signs differently from the invariant culture,
// so any formatting that depends on the current culture shows up in the results.
[SetUp]
public void SetUp()
{
_originalCulture = CultureInfo.CurrentCulture;
CultureInfo.CurrentCulture = CultureInfo.GetCultureInfo("fi-FI");
}

[TearDown]
public void TearDown() => CultureInfo.CurrentCulture = _originalCulture;

[Test]
public void Can_Format_Guid_Uppercase()
{
var sut = new SqliteSyntaxProvider(
Options.Create(new GlobalSettings()),
Mock.Of<ILogger<SqliteSyntaxProvider>>());

var result = sut.FormatGuid(new Guid("a1b2c3d4-e5f6-7890-abcd-ef1234567890"));
var result = CreateSut().FormatGuid(new Guid("a1b2c3d4-e5f6-7890-abcd-ef1234567890"));

Assert.That(result, Is.EqualTo("A1B2C3D4-E5F6-7890-ABCD-EF1234567890"));
}

[TestCase(true, ExpectedResult = "20261103 10:30:15")]
[TestCase(false, ExpectedResult = "20261103")]
public string Can_Format_DateTime(bool includeTime) => CreateSut().FormatDateTime(_dateTime, includeTime);

[Test]
public void Can_Format_DateTimeOffset()
=> Assert.That(CreateSut().FormatDateTimeOffset(_dateTimeOffset), Is.EqualTo("2026-11-03 10:30:15.25-01:00"));

[Test]
public void Can_Format_DateOnly()
=> Assert.That(CreateSut().FormatDateOnly(_date), Is.EqualTo("2026-11-03"));

[Test]
public void Can_Format_TimeOnly()
=> Assert.That(CreateSut().FormatTimeOnly(_time), Is.EqualTo("10:30:15.2500000"));

[Test]
public void Can_Format_TimeOnly_Without_Fractional_Seconds()
=> Assert.That(CreateSut().FormatTimeOnly(_wholeSecondTime), Is.EqualTo("10:30:15"));

[Test]
public void Can_Quote_And_Escape_A_String_Value()
=> Assert.That(CreateSut().GetQuotedValue("O'Brien's"), Is.EqualTo("'O''Brien''s'"));

[Test]
public void Date_And_Time_Values_Are_Formatted_As_Sqlite_Stores_Parameter_Values()
{
SqliteSyntaxProvider sut = CreateSut();
using var connection = new SqliteConnection("Data Source=:memory:");
connection.Open();

Assert.Multiple(() =>
{
Assert.That(sut.FormatDateTimeOffset(_dateTimeOffset), Is.EqualTo(GetStoredText(connection, _dateTimeOffset)));
Assert.That(sut.FormatDateOnly(_date), Is.EqualTo(GetStoredText(connection, _date)));
Assert.That(sut.FormatTimeOnly(_time), Is.EqualTo(GetStoredText(connection, _time)));
Assert.That(sut.FormatTimeOnly(_wholeSecondTime), Is.EqualTo(GetStoredText(connection, _wholeSecondTime)));
});
}

[Test]
public void Can_Order_Integers_Numerically_By_Orderable_String()
{
Expand Down Expand Up @@ -62,6 +118,14 @@ public void Can_Order_Decimals_Numerically_By_Orderable_String()
private static SqliteSyntaxProvider CreateSut()
=> new(Options.Create(new GlobalSettings()), Mock.Of<ILogger<SqliteSyntaxProvider>>());

private static string GetStoredText(SqliteConnection connection, object value)
{
using SqliteCommand command = connection.CreateCommand();
command.CommandText = "SELECT @value";
command.Parameters.AddWithValue("@value", value);
return (string)command.ExecuteScalar()!;
}

private static IEnumerable<T> OrderByExpression<T>(string expression, string columnType, IEnumerable<object> values, Func<object, T> read)
{
using var connection = new SqliteConnection("Data Source=:memory:");
Expand Down
Loading
Loading