2

I have the following structure:

public interface IChainByPreviousId
{
    int Id { get; set; }
    int? PreviousDeviceId { get; set; }
}

public class RegisteredDevice : IChainByPreviousId
{
    public int Id { get; set; }

    public int? PreviousDeviceId { get; set; }
    public int? Position { get; set; }

    public string DeviceName { get; set; }
    public string ModelNumber { get; set; }

    public virtual RegisteredDevice? PreviousDevice { get; set; }
}

so, as we can see, every device can have previous device or null (if it's the first device in the our ordered list)

        builder.HasOne(a => a.PreviousDevice)
            .WithOne()
            .HasForeignKey<RegisteredDevice>(p => p.PreviousDeviceId)
            .OnDelete(DeleteBehavior.NoAction);

then I want to "reorder" devices. model.DeviceReordered is collection of

public class DeviceReorderedDto
{
    public int DeviceId { get; set; }
    public int? NewPreviousDeviceId { get; set; }
}

so, this class describes reordering device (new previous deviceId)

for my case there are (moved device with Id == 2 from second position (after id=1) to devices between id == 4 and id == 5) :

[
  {
    "DeviceId": 3,
    "NewPreviousDeviceId": 1
  },
  {
    "DeviceId": 2,
    "NewPreviousDeviceId": 4
  },
  {
    "DeviceId": 5,
    "NewPreviousDeviceId": 2
  }
]

get devices:

var localDevices = _dataContext.RegisteredDevices.ToList();

init localDevices (after get data and before applying changes) is the following:

[
  {
    "Id": 1,
    "ModelNumber": "20I",
    "CompanyName": "J. J. Keller \u0026 Associates, Inc.",
    "DeviceName": "J. J. Keller ELD - iOS 2.0",
    "PreviousDeviceId": null,
    "Position": 0
  },
  {
    "Id": 2,
    "ModelNumber": "25I",
    "CompanyName": "J. J. Keller \u0026 Associates, Inc.",
    "DeviceName": "J. J. Keller ELD - iOS 2.5",
    "PreviousDeviceId": 1,
    "Position": 1
  },
  {
    "Id": 3,
    "ModelNumber": "FLT3",
    "CompanyName": "HOS247 LLC",
    "DeviceName": "#1 ELD by HOS247",
    "PreviousDeviceId": 2,
    "Position": 2
  },
  {
    "Id": 4,
    "ModelNumber": "N775G",
    "CompanyName": "XPERT-IT SOLUTIONS INC.",
    "DeviceName": "Xpert ELD",
    "PreviousDeviceId": 3,
    "Position": 3
  },
  {
    "Id": 5,
    "ModelNumber": "PMG001",
    "CompanyName": "PeopleNet",
    "DeviceName": "PeopleNet Mobile Gateway - Trimble Driver ELD",
    "PreviousDeviceId": 4,
    "Position": 4
  }
]

and applying changes:

for (int i = 0; i < model.DeviceReordered.Count; i++)
{
   var item = model.DeviceReordered[i];
   var device = localDevices.Where(a => a.Id == item.DeviceId).First();
   var previousDevice = localDevices.Where(a => a.Id == item.NewPreviousDeviceId).FirstOrDefault();
   device.PreviousDevice = previousDevice;
}
localDevices = localDevices.OrderBy(a => a.Position).ToList();

and save changes:

        await _dataContext.SaveChangesAsync();

I got error:

Cannot insert duplicate key row in object 'dbo.RegisteredDevice' with unique index 'IX_RegisteredDevice_PreviousDeviceId'. The duplicate key value is (4).

But I don't have duplicates in localDevices Ok, before calling await _dataContext.SaveChangesAsync(); I add: ValidateDuplicates(list);

    void ValidateDuplicates(IList<T> positionList)
    {
        var duplicates = positionList
            .Where(x => x.PreviousDeviceId.HasValue)
            .GroupBy(x => x.PreviousDeviceId)
                                        .Where(g => g.Count() > 1)
                                        .Select(x => x.Key).ToList();
        if (duplicates.Any())
            throw new PositionReorderDuplicateException(duplicates);
    }

none duplicate. Ok, check again:

var prev = localDevices.Where(a => a.PreviousDeviceId == 4).ToList();

prev has only one record.

My localDevices before call await _dataContext.SaveChangesAsync():

[
  {
    "Id": 1,
    "ModelNumber": "20I",
    "CompanyName": "J. J. Keller \u0026 Associates, Inc.",
    "DeviceName": "J. J. Keller ELD - iOS 2.0",
    "PreviousDeviceId": null,
    "Position": 0
  },
  {
    "Id": 3,
    "ModelNumber": "FLT3",
    "CompanyName": "HOS247 LLC",
    "DeviceName": "#1 ELD by HOS247",
    "PreviousDeviceId": 1,
    "Position": 1
  },
  {
    "Id": 4,
    "ModelNumber": "N775G",
    "CompanyName": "XPERT-IT SOLUTIONS INC.",
    "DeviceName": "Xpert ELD",
    "PreviousDeviceId": 3,
    "Position": 2
  },
  {
    "Id": 2,
    "ModelNumber": "25I",
    "CompanyName": "J. J. Keller \u0026 Associates, Inc.",
    "DeviceName": "J. J. Keller ELD - iOS 2.5",
    "PreviousDeviceId": 4,
    "Position": 3
  },
  {
    "Id": 5,
    "ModelNumber": "PMG001",
    "CompanyName": "PeopleNet",
    "DeviceName": "PeopleNet Mobile Gateway - Trimble Driver ELD",
    "PreviousDeviceId": 2,
    "Position": 4
  }
]

So, seems, anything is ok. Reordered as expected, no duplicates, only one device has PreviousDeviceId == 4

My full code is:

        var localDevices = _dataContext.RegisteredDevices.ToList();

        if (model.DeviceReordered.Any())
        {
            for (int i = 0; i < model.DeviceReordered.Count; i++)
            {
                var item = model.DeviceReordered[i];
                var device = localDevices.Where(a => a.Id == item.DeviceId).First();
                var previousDevice = localDevices.Where(a => a.Id == item.NewPreviousDeviceId).FirstOrDefault();
                device.PreviousDeviceId = previousDevice?.Id;
            }
        }

        int? previousId = null;
        var liveRecordsCount = localDevices.Where(a => a.Position.HasValue).Count();

        for (int i = 0; i < liveRecordsCount; i++)
        {
            var device = localDevices.Where(a => a.PreviousDeviceId == previousId).First();
            device.Position = i;
            previousId = device.Id;
        }

        localDevices = localDevices.OrderBy(a => a.Position).ToList();

        await _dataContext.SaveChangesAsync();

compiled project with the issue I added to https://github.com/oshastitko/reordering_problem

What is wrong in my code?

ADDED 29/07/2023

it generates the following SQL code:

exec sp_executesql N'SET NOCOUNT ON;
UPDATE [RegisteredDevice] SET [PreviousDeviceId] = @p0
OUTPUT 1
WHERE [Id] = @p1;
UPDATE [RegisteredDevice] SET [PreviousDeviceId] = @p2
OUTPUT 1
WHERE [Id] = @p3;
UPDATE [RegisteredDevice] SET [PreviousDeviceId] = @p4
OUTPUT 1
WHERE [Id] = @p5;
',N'@p1 int,@p0 int,@p3 int,@p2 int,@p5 int,@p4 int',@p1=5,@p0=2,@p3=2,@p2=4,@p5=3,@p4=1

which can't be executed too:

Msg 2601, Level 14, State 1, Line 2 Cannot insert duplicate key row in object 'dbo.RegisteredDevice' with unique index 'IX_RegisteredDevice_PreviousDeviceId'. The duplicate key value is (2). The statement has been terminated. Msg 2601, Level 14, State 1, Line 5 Cannot insert duplicate key row in object 'dbo.RegisteredDevice' with unique index 'IX_RegisteredDevice_PreviousDeviceId'. The duplicate key value is (4). The statement has been terminated. Msg 2601, Level 14, State 1, Line 8 Cannot insert duplicate key row in object 'dbo.RegisteredDevice' with unique index 'IX_RegisteredDevice_PreviousDeviceId'. The duplicate key value is (1). The statement has been terminated.

as I understand, it's not in transaction (don't know why). How to "cover" it to transaction?

ADDED 29/07/2023 #2

Also, the same code here, for copy/paste to console app:

DataContext.cs:

public class DataContext : DbContext
{
    public DataContext()
    {
    }

    public DataContext(DbContextOptions<DataContext> options) : base(options)
    {
    }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.ApplyConfigurationsFromAssembly(typeof(RegisteredDeviceMap).Assembly);
    }

    public virtual DbSet<RegisteredDevice> RegisteredDevices { get; set; }
}

RegisteredDevice.cs

public interface IChainByPreviousId
{
    int Id { get; set; }
    int? PreviousDeviceId { get; set; }
}

public class RegisteredDevice : IChainByPreviousId
{
    public int Id { get; set; }

    public int? PreviousDeviceId { get; set; }
    public int? Position { get; set; }

    public string DeviceName { get; set; }
    public string ModelNumber { get; set; }

    public virtual RegisteredDevice? PreviousDevice { get; set; }
}

RegisteredDeviceMap.cs

public class RegisteredDeviceMap : IEntityTypeConfiguration<RegisteredDevice>
{
    public void Configure(EntityTypeBuilder<RegisteredDevice> builder)
    {
        builder.ToTable(nameof(RegisteredDevice));
        builder.HasKey(p => p.Id);

        builder.HasOne(a => a.PreviousDevice)
            .WithOne()
            .HasForeignKey<RegisteredDevice>(p => p.PreviousDeviceId)
            .OnDelete(DeleteBehavior.NoAction);

        string data = "[{\"Id\":1,\"ModelNumber\":\"20I\",\"CompanyName\":\"J. J. Keller \\u0026 Associates, Inc.\",\"DeviceName\":\"J. J. Keller ELD - iOS 2.0\",\"PreviousDeviceId\":null,\"Position\":0},{\"Id\":2,\"ModelNumber\":\"25I\",\"CompanyName\":\"J. J. Keller \\u0026 Associates, Inc.\",\"DeviceName\":\"J. J. Keller ELD - iOS 2.5\",\"PreviousDeviceId\":1,\"Position\":1},{\"Id\":3,\"ModelNumber\":\"FLT3\",\"CompanyName\":\"HOS247 LLC\",\"DeviceName\":\"#1 ELD by HOS247\",\"PreviousDeviceId\":2,\"Position\":2},{\"Id\":4,\"ModelNumber\":\"N775G\",\"CompanyName\":\"XPERT-IT SOLUTIONS INC.\",\"DeviceName\":\"Xpert ELD\",\"PreviousDeviceId\":3,\"Position\":3},{\"Id\":5,\"ModelNumber\":\"PMG001\",\"CompanyName\":\"PeopleNet\",\"DeviceName\":\"PeopleNet Mobile Gateway - Trimble Driver ELD\",\"PreviousDeviceId\":4,\"Position\":4}]";

        var devices = (List<RegisteredDevice>)JsonSerializer.Deserialize(data, typeof(List<RegisteredDevice>));
        int id = 1;
        int? previousDeviceId = null;
        foreach (var device in devices)
        {
            builder.HasData(new RegisteredDevice
            {
                Id = id,
                DeviceName = device.DeviceName,
                ModelNumber = device.ModelNumber,
                Position = id - 1,
                PreviousDeviceId = previousDeviceId
            });
            id++;
            previousDeviceId = device.Id;
        }
    }
}

Init migration:

public partial class Init : Migration
{
    /// <inheritdoc />
    protected override void Up(MigrationBuilder migrationBuilder)
    {
        migrationBuilder.CreateTable(
            name: "RegisteredDevice",
            columns: table => new
            {
                Id = table.Column<int>(type: "int", nullable: false)
                    .Annotation("SqlServer:Identity", "1, 1"),
                PreviousDeviceId = table.Column<int>(type: "int", nullable: true),
                Position = table.Column<int>(type: "int", nullable: true),
                DeviceName = table.Column<string>(type: "nvarchar(max)", nullable: false),
                ModelNumber = table.Column<string>(type: "nvarchar(max)", nullable: false)
            },
            constraints: table =>
            {
                table.PrimaryKey("PK_RegisteredDevice", x => x.Id);
                table.ForeignKey(
                    name: "FK_RegisteredDevice_RegisteredDevice_PreviousDeviceId",
                    column: x => x.PreviousDeviceId,
                    principalTable: "RegisteredDevice",
                    principalColumn: "Id");
            });

        migrationBuilder.InsertData(
            table: "RegisteredDevice",
            columns: new[] { "Id", "DeviceName", "ModelNumber", "Position", "PreviousDeviceId" },
            values: new object[,]
            {
                { 1, "J. J. Keller ELD - iOS 2.0", "20I", 0, null },
                { 2, "J. J. Keller ELD - iOS 2.5", "25I", 1, 1 },
                { 3, "#1 ELD by HOS247", "FLT3", 2, 2 },
                { 4, "Xpert ELD", "N775G", 3, 3 },
                { 5, "PeopleNet Mobile Gateway - Trimble Driver ELD", "PMG001", 4, 4 }
            });

        migrationBuilder.CreateIndex(
            name: "IX_RegisteredDevice_PreviousDeviceId",
            table: "RegisteredDevice",
            column: "PreviousDeviceId",
            unique: true,
            filter: "[PreviousDeviceId] IS NOT NULL");
    }

    /// <inheritdoc />
    protected override void Down(MigrationBuilder migrationBuilder)
    {
        migrationBuilder.DropTable(
            name: "RegisteredDevice");
    }
}

Service:

internal interface ISaveDataService
{
    void ApplyChanges(List<DeviceReorderedDto> model);
}

internal class RegisteredDevicesSaveDataService : ISaveDataService
{
    protected readonly DataContext _dataContext;

    public RegisteredDevicesSaveDataService(DataContext dataContext)
    {
        _dataContext = dataContext;
    }

    public void ApplyChanges(List<DeviceReorderedDto> model)
    {
        var localDevices = _dataContext.RegisteredDevices.ToList();
        for (int i = 0; i < model.Count; i++)
        {
            var item = model[i];
            var device = localDevices.Where(a => a.Id == item.DeviceId).First();
            var previousDevice = localDevices.Where(a => a.Id == item.NewPreviousDeviceId).FirstOrDefault();
            device.PreviousDevice = previousDevice;
            //device.PreviousDeviceId = previousDevice?.Id;
        }

        int? previousId = null;
        var liveRecordsCount = localDevices.Where(a => a.Position.HasValue).Count();

        for (int i = 0; i < liveRecordsCount; i++)
        {
            var device = localDevices.Where(a => a.PreviousDeviceId == previousId).First();
            device.Position = i;
            previousId = device.Id;
        }

        localDevices = localDevices.OrderBy(a => a.Position).ToList();

        _dataContext.SaveChanges();
    }
}

Program.cs:

var configuration = new ConfigurationBuilder()
    .AddJsonFile("appsettings.test.json", false, true).Build();

string connectionString = "Server=(local)\\sqlexpress;Database=Test;Trusted_Connection=True;TrustServerCertificate=True;MultipleActiveResultSets=true;";

var services = new ServiceCollection();

services.AddDbContext<DataContext>(options => options
            .UseLazyLoadingProxies()
            .UseSqlServer(connectionString)
            , ServiceLifetime.Transient
            );

services.AddTransient<ISaveDataService, RegisteredDevicesSaveDataService>();

var sp = services.BuildServiceProvider();


var _dataContext = sp.GetService<DataContext>();
var applyService = sp.GetService<ISaveDataService>();

_dataContext.Database.Migrate();
_dataContext.SaveChanges();

List<DeviceReorderedDto> changes = new List<DeviceReorderedDto>
{
      new DeviceReorderedDto{
    DeviceId = 3,
    NewPreviousDeviceId = 1
  },
  new DeviceReorderedDto{
    DeviceId = 2,
    NewPreviousDeviceId = 4
  },
  new DeviceReorderedDto{
    DeviceId = 5,
    NewPreviousDeviceId = 2
  }
};

applyService.ApplyChanges(changes);
Oleg Sh
  • 8,496
  • 17
  • 89
  • 159
  • Can you please provide sample input in valid C# code that populates the `localDevices` and `model` variables? You also haven't defined `model` for us. I'm, in effect, asking for a [mcve]. Perhaps also read [ask]. – Enigmativity Jul 26 '23 at 01:05
  • @Enigmativity I added `model.DeviceReordered` description. About `localDevices`... there are from DB. But I even imagine, where there can be a mistake. Data from DB has foreign keys (otherwise they could not be added to DB) and all records exist (otherwise I get an exception on `device.PreviousDevice =...` – Oleg Sh Jul 26 '23 at 01:40
  • Can you please provide sample input in valid C# code that populates the `localDevices` and `model` variables? You should be able to pull these from the database and trim the results that we can see the error you get. Without it I think your question is unanswerable. Well asked questions get answers in minutes on this site. You should make sure you're asking a good question. – Enigmativity Jul 26 '23 at 07:21
  • What happens if you use the id instead of the navigation property, `device.PreviousDeviceId = previousDevice?.Id`? – nikstra Jul 26 '23 at 09:23
  • @nikstra the same – Oleg Sh Jul 26 '23 at 16:25
  • @OlegSh - A [mcve] for be helpful. Then we wouldn't need to guess. – Enigmativity Jul 26 '23 at 21:29
  • @Enigmativity added any details. I hope, anything is clear now :) Feel free to ask, if not – Oleg Sh Jul 26 '23 at 23:04
  • @OlegSh - I'm hoping that you make it super easy for me to run your code. JSON just adds a bunch of work. C# would be good. – Enigmativity Jul 27 '23 at 03:11
  • @Enigmativity added here https://github.com/oshastitko/reordering_problem – Oleg Sh Jul 27 '23 at 14:30
  • @OlegSh - You're intent on making this hard for me to answer. Please provide the full code, in C# format, in the question itself, so that I can copy and paste into a Console app and run your code and see what it is doing. – Enigmativity Jul 28 '23 at 04:20
  • @Enigmativity, thank you for trying to help me, but i can't add code to question now, i live in Ukraine and today after russian attack i don't have internet on pc :( if i will have ability, i will add it. But i can't imagine how to do it with fill init data, which is in migration.on github it's as zip file with all code inside – Oleg Sh Jul 28 '23 at 13:41
  • @OlegSh - Best wishes with the situation there. – Enigmativity Jul 28 '23 at 14:21
  • @Enigmativity thank you for the wish! Now electricity is restored, so, I added code to my main question (to the end). If you have time please, review. Thank you! – Oleg Sh Jul 28 '23 at 23:37
  • @OlegSh - Thank you for taking the time to do this, but you're killing me slowly with the work required. Please make it super easy. Can you try copying and pasting your code into a vanilla console app and see if it compiles and runs? Please give me the instructions needed to run this. Also, note, that I do not have any database environment set up. It needs to run without the database. You source data should just be plain C# code. – Enigmativity Jul 29 '23 at 05:49
  • @Enigmativity but I can't do easy things, which are not simple :) I tried as I could, but I can't do it more simple... We need to create DB table and fill data before trying to edit it ... Without DB is not guarantee of "pure of experiment".... Ok, don't worry, I will select to set `null` on first step and then set new values, as 'Guru Stron' proposed below. Thank you in any case! – Oleg Sh Jul 29 '23 at 19:03
  • @OlegSh - If you query your database for the data that is failing then you can take that source data and output it as pure C# code. Then I can run your code without a database. It's quite easy to do. It's actually more work for you to write the code to populate your database with JSON. – Enigmativity Jul 30 '23 at 04:01

1 Answers1

1

I suggest to split the modification into several steps (ideally in a transaction so no inconsistent state can happen) - 1) "clean" the previous device ids for all changed items 2) update the previous device ids 3) recalculate the positions:

// clean the previous ids
_dataContext.RegisteredDevices
    .Where(device => model.Select(dto => dto.DeviceId).Contains(device.Id))
    .ExecuteUpdate(up => up.SetProperty(device => device.PreviousDeviceId, (int?)null));

// set new ones
var changedDevices = _dataContext.RegisteredDevices
    .Where(device => model.Select(dto => dto.DeviceId).Contains(device.Id))
    .ToList();
for (int i = 0; i < model.Count; i++)
{
    var item = model[i];
    var device = changedDevices.Where(a => a.Id == item.DeviceId).First();
    device.PreviousDeviceId = item.NewPreviousDeviceId;
}
_dataContext.SaveChanges();

// recalculate and update the positions
_dataContext.ChangeTracker.Clear();
var localDevices = _dataContext.RegisteredDevices.ToList();
// ...

Enable the query logging, as far as I remember EF will send updates by one so you can end up in situation when you are trying to insert a PreviousDeviceId which is still used by someone.

Submitted PR with code that worked for me.

Guru Stron
  • 102,774
  • 10
  • 95
  • 132
  • I thought about it, but it's bad solution, if something will be wrong during second call, then i lose sensitive data. And this solution is more complex. I want to know, why my solution does not work – Oleg Sh Jul 28 '23 at 20:48
  • @OlegSh _"if something will be wrong during second call"_ - that' s why you wrap everything in transaction (as in PR I have submitted). _"I want to know, why my solution does not work "_ - as I said - enable query logging and see what commands EF will send. – Guru Stron Jul 28 '23 at 20:49