0

I need to duplicate a Transport entity (including child entities except one of them: Documents which of type 'Mail').

Here is my model:

public class Transport
{
    [Key]
    public int Id { get; set; }
    public string TransportNumber { get; set; }
    ...
    public virtual List<Document> Documents { get; set; }
}

public class Document
{
    [Key]
    public int Id { get; set; }
    public EnumDocumentType Type { get; set; }
    public int FileId { get; set; }
    public virtual File File { get; set; }
    public virtual Transport Transport { get; set; }
}

public class File
{
    [Key]
    public int Id { get; set; }
    public string Filename { get; set; }
    public byte[] FileStream { get; set; }
}

My c# code for duplicating:

   public void DuplicateTransports(IEnumerable<int> ids) {

        var transportsToDuplicate = Context.Transports.Where(t => ids.Contains(t.Id))
                                                      .Include(c => c.Documents).ToList();

        transportsToDuplicate.ForEach(t =>
        {
            var newTransport = Context.Transports.Add(t);
            newTransport.Id = -100;
            AttributeNumber(newTransport);

            t.Documents.ForEach(doc =>
                {
                    ObjectContext.LoadProperty(doc, x => x.File);
                    Context.Documents.Add(doc);
                    Context.Files.Add(doc.File);
                });
            Context.SaveChanges();

            // (step 2) Don't duplicate child Documents of type Mail 
            var docs = Context.Documents.Where(doc => doc.TransportId == newTransport.Id && doc.Type == EnumDocumentType.Mail).ToList(); 
            for (int i=docs.Count-1; i>=0; i--) Context.Documents.Remove(docs[i]);
            Context.SaveChanges();

        });

It works as expected but as you can see in the code, I had to proceed in 2 steps:

  • step 1: duplicating element + save changes in the context
  • step 2: searching for documents of type Mail and remove them + save changes in the context

My question: is it possible to proceed only in 1 step? So avoid adding in the context documents of type Mail because in my case I need to remove it at next step which I found not optimized.

Thanks.


EDIT

As suggested by Philip Stuyckx I already try to proceed in 1 step like this:

        transportsToDuplicate.ForEach(t =>
        {
            var newTransport = Context.Transports.Add(t);
            newTransport.Id = -100;
            AttributeNumber(newTransport);

            t.Documents.ForEach(doc =>
                {
                    if (doc.Type != EnumDocumentType.Mail)
                    {
                        ObjectContext.LoadProperty(doc, x => x.File);
                        Context.Documents.Add(doc);
                        Context.Files.Add(doc.File);
                    }
                });
            Context.SaveChanges();
     });

Then I got an exception: The changes to the database were committed successfully, but an error occurred while updating the object context. The ObjectContext might be in an inconsistent state. Inner exception message: A referential integrity constraint violation occurred: The property value(s) of 'Transport.Id' on one end of a relationship do not match the property value(s) of 'Document.TransportId' on the other end.

Bronzato
  • 9,438
  • 29
  • 120
  • 212

3 Answers3

0

I think following does the trick:

t.Documents.Where(doc => doc.Type == EnumDocumentType.Mail).ForEach(doc =>
            {
                ObjectContext.LoadProperty(doc, x => x.File);
                Context.Documents.Add(doc);
                Context.Files.Add(doc.File);
            });

question is also why you keep doing saveChanges so many times, where a single one at the very end should be sufficient.

transportsToDuplicate.ForEach(t =>
   ....
}
Context.SaveChanges();

But maybe you have a reason to do it like that ?

Philip Stuyck
  • 7,344
  • 3
  • 28
  • 39
  • The code you suggested gives the following error: The changes to the database were committed successfully, but an error occurred while updating the object context. The ObjectContext might be in an inconsistent state. Inner exception message: A referential integrity constraint violation occurred: The property value(s) of 'Transport.Id' on one end of a relationship do not match the property value(s) of 'Document.TransportId' on the other end. That's why I had to code in 2 steps as showed in my question. I hope there is a better way of doing. – Bronzato Feb 15 '15 at 17:52
  • And do you really need all those calls to saveChanges ? – Philip Stuyck Feb 15 '15 at 18:00
  • Yes because I showed you here only a small portion of code and I got an exception when there are too much operations before the SaveChanges statement. – Bronzato Feb 15 '15 at 18:09
0

I think by "one step" you mean the insert step. You can if you manage to grab only the data you want to insert. But then you need more steps or more trickery in one query to get the data. I would stick to fetching data first in one simple query, then process them to filter out the data you don't want to duplicate.

This looks like your current process, but I would take a slightly different approach. (All assuming that Context is a DbContext).

To start with, you should get the transports and child data with AsNoTracking(). Then you can Add() them to the context without getting entity key errors or ambiguous associations (the context is still empty before the Add):

var transportsToDuplicate = Context.Transports
                                   .AsNoTracking()
                                   .Where(t => ids.Contains(t.Id))
                                   .Include(c => c.Documents.Select(d => d.File))
                                   .ToList();

transportsToDuplicate.ForEach(t => Context.Transports.Add(t));

Now each Transport and all child objects are in an Added state (because they were not yet tracked) and will be inserted as new objects.

If you call SaveChanges() now all data will be duplicated.

So the only thing to do now is to prevent mail documents and their files to get duplicated:

var mailDocuments = Context.Documents.Local // Local!
                           .Where(d => d.Type == EnumDocumentType.Mail)
                           .ToList();
foreach(var md in mailDocuments)
{
    Context.Entry(md).State = System.Data.Entity.EntityState.Detached;
    if (md.File != null)
    {
        Context.Entry(md.File).State = System.Data.Entity.EntityState.Detached;
    }
}

The mail documents and their files will be detached and not be inserted.

Now you can call SaveChanges.

You can read some more background here: Merge identical databases into one.

Community
  • 1
  • 1
Gert Arnold
  • 105,341
  • 31
  • 202
  • 291
  • Very instructive. Thanks for that. I noticed that it works without using `AsNoTracking` on the Transport entity but using only `Local` on the Documents entities. – Bronzato Feb 16 '15 at 08:25
0

You should avoid associating your Documents/Files with multiple Transports in the database, or you'll wind up with a cascade error. If you're not careful, you'll have a single File referred to by two separate Documents, you'll delete an older Transport - say, one with a DocumentType.Mail - its Document, and the Document's File, and then another Document belonging to another Transport will be left referring to that File, which is no longer there.

The Transport object should be treated as an aggregate root and all Documents and Files associated with it should be treated as part of the aggregate. When a Transport object is removed from the database, its associated Documents and Files should also be removed; the associated entities should be considered as belonging to the Transport object.

You should just create entirely new aggregates, rooted in a new Transport, for each Transport you wish to duplicate, to avoid multiple references to the same entity:

public void DuplicateTransports(IEnumerable<int> ids)
{
    using (var context = new MyContext())
    {
        var transportsToDuplicate = context.Transports
                                           .Include(c => c.Documents.Select(d => d.File)) // load entire aggregate record in a single query
                                           .Where(t => ids.Contains(t.Id));

        foreach (var transport in transportsToDuplicate)
        {
            var documents = transport.Documents
                                     .Where(d => d.Type != Document.DocumentType.Mail)
                                     .Select(d => new Document
                                     {
                                         Type = d.Type,
                                         File = new File
                                         {
                                             Filename = d.File.Filename,
                                             FileStream = d.File.FileStream,
                                         }
                                     });

            var newTransport = new Transport { TransportNumber = transport.TransportNumber };
            newTransport.Documents.AddRange(documents);
            context.Transports.Add(newTransport);
        }

        context.SaveChanges();
    }
}

EDIT

You might notice is all the details of Document and File spread throughout our duplicate method, which should be concentrated more on doing the duplication than what that duplication is. If we add copy constructors, we isolate the details of what goes into a Document or File into the class itself, and simplify our duplication code, which will also make it more portable to future changes in those classes.

With the following ctors:

public File() { }
public File(File file) : this()
{
    this.Filename = file.Filename;
    this.FileStream = file.FileStream;
}

public Document() { }
public Document(Document document) : this()
{
    this.Type = document.Type;
    this.File = new File(document.File);
}

Our foreach loop in DuplicateTransports simplifies:

foreach (var transport in transportsToDuplicate)
{
    var documents = transport.Documents
                             .Where(d => d.Type != Document.DocumentType.Mail)
                             .Select(d => new Document(d));
    var newTransport = new Transport { TransportNumber = transport.TransportNumber };
    newTransport.Documents.AddRange(documents);
    context.Transports.Add(newTransport);
}
John Castleman
  • 1,552
  • 11
  • 12
  • There are tricks you can play with the `EntityState` on the `DbContext` and/or accessing `DbSet.Local` (the cached collection of previously fetched entities in the `DbSet`), but these are metadata, and anytime I've considered getting cute with the metadata, I've regretted it, and replaced it with something more straightforward; (granted, many of those times it was because I was working on a team, and somebody on the team was less knowledgeable about EF metadata, but even when the only team members are me and me-a-year-from-now, simplicity is usually best). – John Castleman Feb 17 '15 at 07:00