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

Sql.ExtensionAttribute chain bug #2418

Closed
FlipSs opened this issue Aug 18, 2020 · 2 comments · Fixed by #2420
Closed

Sql.ExtensionAttribute chain bug #2418

FlipSs opened this issue Aug 18, 2020 · 2 comments · Fixed by #2420
Labels
status: has-pr There is active PR for issue type: bug
Milestone

Comments

@FlipSs
Copy link

FlipSs commented Aug 18, 2020

Hello,

After upgrading version 3.0.1 from 2.9.8, some of my tests failed. I have used some extension methods (via Sql.ExtensionAttribute), that allowed me to partially update JSON column (using JSON_MODIFY).
In this extension I used my own CallBuilderType, that wraps previous calls (using ISqExtensionBuilder.GetExpression())
JSON_MODIFY({source}, {path}, {value}) - where {source} is previous call result

And it used like this:

connection.SomeTable
.Set(x => x.SomeColumn, x => x.SomeColumn
          .Set(c => x.SomeProperty, 10)
          .Set(c => x.AnotherProperty, 17)
)

It was generating an update query like this:
SomeColumn=JSON_MODIFY(JSON_MODIFY(SomeColumn, '$.SomeProperty', 10), '$.AnotherProperty', 17)

But after update, it only generates query only for the first call:
SomeColumn=JSON_MODIFY(SomeColumn, '$.SomeProperty', 10)

I think it has something to do with this commit d8e6920, especially with this change:
image

Now, the main SqlExtensionParam, taken from the end of the chain (if ChainPrecedence is not specified), whereas in previous versions it was taken from the beginning.

It should be noted that all calls appear to be correct, it is just that the main SqlExtensionParam is always the first call.

Environment details

linq2db version: 3.0.1
Database Server: Sql Server
.NET Framework: .Net Core 3.1, net472

@sdanyliv
Copy link
Member

Please share your extension and simple model

@FlipSs
Copy link
Author

FlipSs commented Aug 18, 2020

Simplified everything as I could.
Model:

public class TestConnection : DataConnection
	{
		// your connection string
		private const string DefaultConnectionString = @"";

		public TestConnection()
			: base(ProviderName.SqlServer, DefaultConnectionString)
		{
			var schema = new MappingSchema();

			schema.SetConverter<DbObject<TestJson>, string>(v => JsonConvert.SerializeObject(v.Value));
			schema.SetConverter<DbObject<TestJson>, DataParameter>(v => new DataParameter
			{
				DataType = DataType.NVarChar,
				Value = JsonConvert.SerializeObject(v.Value)
			});
			schema.SetConverter<string, DbObject<TestJson>>(json => new DbObject<TestJson>(JsonConvert.DeserializeObject<TestJson>(json)));

			AddMappingSchema(schema);
		}

		public ITable<TestTable> TestTable => GetTable<TestTable>();
	}

[Table("TestTable")]
	public class TestTable
	{
		[Column]
		public Guid Id { get; set; }

		[Column]
		public DbObject<TestJson> Json { get; set; }
	}

public class TestJson
	{
		public int Number { get; set; }

		public string String { get; set; }
	}

public class DbObject<T>
		where T : class
	{
		public DbObject(T value)
		{
			Value = value;
		}

		public T Value { get; }
	}

Extension:

internal class DbObjectSetExtensionCallBuilder : Sql.IExtensionCallBuilder
	{
		public void Build(Sql.ISqExtensionBuilder builder)
		{
			builder.Expression = "JSON_MODIFY({source}, {path}, {value})";

			builder.AddParameter("source", builder.GetExpression(0));

			var member = (MemberExpression) ((LambdaExpression) ((UnaryExpression) builder.Arguments[1]).Operand).Body;

			builder.AddParameter("path", $"$.{member.Member.Name}");

			var propertyExpression = (MemberExpression) builder.Arguments[2];
			var memberExpression = (MemberExpression) propertyExpression.Expression;
			var fieldInfo = (FieldInfo) memberExpression.Member;
			var valueExpression = (ConstantExpression) memberExpression.Expression;
			var value = ((PropertyInfo) propertyExpression.Member).GetValue(fieldInfo.GetValue(valueExpression.Value));

			builder.AddParameter("value", value.ToString());
		}
	}

public static class DbObjectExtensions
	{
		[Sql.Extension("Set", ServerSideOnly = true, BuilderType = typeof(DbObjectSetExtensionCallBuilder))]
		public static DbObject<T> Set<T, TValue>(this DbObject<T> dbObject, Expression<Func<T, TValue>> propertyExpression, [SqlQueryCacheDependedParameter] TValue propertyValue)
			where T : class
		{
			return dbObject;
		}
}

Test:

[Test]
		public async Task Test()
		{
			var newRecord = new TestTable()
			{
				Id = Guid.NewGuid(),
				Json = new DbObject<TestJson>(new TestJson
				{
					String = "Test",
					Number = 1
				})
			};

			using (var connection = new TestConnection())
			{
				connection.Insert(newRecord);

				var savedRecord = await connection.TestTable.FirstAsync(x => x.Id == newRecord.Id).ConfigureAwait(false);

				var newJson = new TestJson()
				{
					String = "Test1",
					Number = 10
				};

				await connection.TestTable
					.Where(o => o.Id == savedRecord.Id)
					.Set(o => o.Json, o => o.Json
						.Set(j => j.Number, newJson.Number)
						.Set(j => j.String, newJson.String)
					).UpdateAsync()
					 .ConfigureAwait(false);

				savedRecord = await connection.TestTable.FirstAsync(x => x.Id == newRecord.Id).ConfigureAwait(false);

				savedRecord.Json.Value.Should().BeEquivalentTo(newJson);
			}
		}

@sdanyliv sdanyliv added this to the 3.1.0 milestone Aug 18, 2020
@sdanyliv sdanyliv added status: has-pr There is active PR for issue type: bug labels Aug 18, 2020
MaceWindu added a commit that referenced this issue Aug 19, 2020
* Fix for #2418. Corrected chain sequence sorting.

* fix test

Co-authored-by: MaceWindu <MaceWindu@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: has-pr There is active PR for issue type: bug
Development

Successfully merging a pull request may close this issue.

2 participants