Strange behaviour on RemovedEntitiesTracker

Posts   
 
    
saggett
User
Posts: 50
Joined: 12-Nov-2007
# Posted on: 21-Nov-2007 13:08:49   

Running LLBLGen v2.5, Build 2.5.7.1119, Adapter Pattern.

Compare this:

            PriceParentAddOnCostEntity ppAddOnCost = new PriceParentAddOnCostEntity();
            ppAddOnCost.AddOnCost = addOnCost;
//PriceParentAddOnCosts.RemovedEntitiesTracker.Count = 0, PriceParentAddOnCosts.Count = 0
            ppAddOnCost.PriceParent = this;
//PriceParentAddOnCosts.RemovedEntitiesTracker.Count = 0, PriceParentAddOnCosts.Count = 1
           PriceParentAddOnCosts.Add(ppAddOnCost);
//PriceParentAddOnCosts.RemovedEntitiesTracker.Count = 1, PriceParentAddOnCosts.Count =  1

with this:

            PriceParentAddOnCostEntity ppAddOnCost = new PriceParentAddOnCostEntity();
            ppAddOnCost.AddOnCost = addOnCost;
//PriceParentAddOnCosts.RemovedEntitiesTracker.Count = 0, PriceParentAddOnCosts.Count = 0
            PriceParentAddOnCosts.Add(ppAddOnCost);
//PriceParentAddOnCosts.RemovedEntitiesTracker.Count = 0, PriceParentAddOnCosts.Count = 1

I fixed the first code sample by removing the line ppAddOnCost.PriceParent = this;, but I thought I should bring it to your attention because it doesn't look like the RemovedEntitiesTracker is behaving correctly in this scenario.

Walaa avatar
Walaa
Support Team
Posts: 14950
Joined: 21-Aug-2005
# Posted on: 21-Nov-2007 15:25:37   

Would you please elaborate a little bit? What are these commented lines for? And what does this stands for? (Where is the code being executed?)

saggett
User
Posts: 50
Joined: 12-Nov-2007
# Posted on: 21-Nov-2007 15:29:48   

This code exists in a PriceParent entity partial class. In each of the code samples, a linking entity known as PriceParentAddOnCost (linking PriceParent and AddOnCost) is being added to the PriceParentAddOnCosts collection. My point is that when the former code is used to affect the addition of this new entity, RemovedEntitiesTracker behaves incorrectly - a item is added to it that has never been removed from the collection being tracked (PriceParentAddOnCosts).

Walaa avatar
Walaa
Support Team
Posts: 14950
Joined: 21-Aug-2005
# Posted on: 21-Nov-2007 16:28:40   

ppAddOnCost.PriceParent = this; //PriceParentAddOnCosts.RemovedEntitiesTracker.Count = 0, PriceParentAddOnCosts.Count = 1 PriceParentAddOnCosts.Add(ppAddOnCost); //PriceParentAddOnCosts.RemovedEntitiesTracker.Count = 1, PriceParentAddOnCosts.Count = 1

I think that's because the above code attempts to associate or attach the ppAddOnCost to this PriceParenttwice. So I guess when this association is done for the second time, one of those associations were removed from the collection of PriceParentAddOnCosts.

We'll check to see if this is a necessary behaviour or not.

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39614
Joined: 17-Aug-2003
# Posted on: 22-Nov-2007 11:05:28   

saggett wrote:

Running LLBLGen v2.5, Build 2.5.7.1119, Adapter Pattern.

Compare this:

            PriceParentAddOnCostEntity ppAddOnCost = new PriceParentAddOnCostEntity();
            ppAddOnCost.AddOnCost = addOnCost;
//PriceParentAddOnCosts.RemovedEntitiesTracker.Count = 0, PriceParentAddOnCosts.Count = 0
            ppAddOnCost.PriceParent = this;
//PriceParentAddOnCosts.RemovedEntitiesTracker.Count = 0, PriceParentAddOnCosts.Count = 1
           PriceParentAddOnCosts.Add(ppAddOnCost);
//PriceParentAddOnCosts.RemovedEntitiesTracker.Count = 1, PriceParentAddOnCosts.Count =  1

with this:

            PriceParentAddOnCostEntity ppAddOnCost = new PriceParentAddOnCostEntity();
            ppAddOnCost.AddOnCost = addOnCost;
//PriceParentAddOnCosts.RemovedEntitiesTracker.Count = 0, PriceParentAddOnCosts.Count = 0
            PriceParentAddOnCosts.Add(ppAddOnCost);
//PriceParentAddOnCosts.RemovedEntitiesTracker.Count = 0, PriceParentAddOnCosts.Count = 1

I fixed the first code sample by removing the line ppAddOnCost.PriceParent = this;, but I thought I should bring it to your attention because it doesn't look like the RemovedEntitiesTracker is behaving correctly in this scenario.

The reason for this is that it gets dereferenced first as the entity is already added to PriceParentAdditionalCosts due to: ppAddOnCost.PriceParent = this;

(myOrder.Customer = myCustomer; also does: myCustomer.Orders.Add(myOrder); )

I see this can lead to phantom deletes and this is a troubling issue. I'll try to fix this right away.

Frans Bouma | Lead developer LLBLGen Pro
Otis avatar
Otis
LLBLGen Pro Team
Posts: 39614
Joined: 17-Aug-2003
# Posted on: 22-Nov-2007 12:07:39   

Reproduced: disappointed


[Test]
public void AddingEntityTwiceShouldNotEndUpInRemovedEntitiesTracker()
{
    OrderEntity order = new OrderEntity(10254);
    ProductEntity product = new ProductEntity(3);

    using(DataAccessAdapter adapter = new DataAccessAdapter())
    {
        Assert.IsTrue(adapter.FetchEntity(order));
        Assert.IsTrue(adapter.FetchEntity(product));
    }

    EntityCollection<OrderDetailsEntity> tracker = new EntityCollection<OrderDetailsEntity>();
    order.OrderDetails.RemovedEntitiesTracker = tracker;
    OrderDetailsEntity newOD = new OrderDetailsEntity();
    newOD.Orders = order;
    newOD.Products = product;

    Assert.AreEqual(0, tracker.Count);
    Assert.AreEqual(1, order.OrderDetails.Count);
    Assert.IsTrue(order.OrderDetails.Contains(newOD));

    order.OrderDetails.Add(newOD);
    Assert.AreEqual(0, tracker.Count);  // fails.
    Assert.AreEqual(1, order.OrderDetails.Count);
}

Looking into it.

Frans Bouma | Lead developer LLBLGen Pro
Otis avatar
Otis
LLBLGen Pro Team
Posts: 39614
Joined: 17-Aug-2003
# Posted on: 22-Nov-2007 12:23:44   

The thing is this: this call: PriceParentAddOnCosts.Add(ppAddOnCost); adds the entity AGAIN. This is because the flag PriceParentAddOnCosts.DoNotPerformAddIfPresent is set to false by default to allow faster additions to collections (otherwise it will perform a linear search before adding the new entity). As the flag is false, the addition isn't stopped with the Contains check, as that's not performed, the entity is added as normal. As the collection is contained inside an entity (PriceParent), it will start a sync setup cycle. This means that ppAddOnCost is notified it is been assigned to that PriceParent instance, which will first dereference an existing reference (as it is a m:1 relation) and then assign the new reference. The dereferencing will remove the FIRST added entity (which was added for you by LLBLGen Pro when PriceParentAddOnCosts.Add(ppAddOnCost); was executed) from the collection.

The end result is what you had in mind, but the tracker collection got a reference to the removed entity which wasn't intended. The flaw is that the entity is added to the tracker despite the fact that it's still in the collection. I'll add that check which should prevent entities which are still in the collection because they're added twice, are ending up in the tracker, simply because they're not removed completely, which is purpose of the tracker: track removals so the entity can be removed from the db.

(edit) Fixed in next build. (11222007)

Frans Bouma | Lead developer LLBLGen Pro
Posts: 17
Joined: 12-Mar-2007
# Posted on: 14-Jul-2008 20:31:56   

We're seeing some odd behavior ourselves. The following is from a method in a entity partial:


Debug.Assert(specialty != null);
var opportunitySpecialty = GetOpportunitySpecialtyBySortOrder(sortOrder);
if (opportunitySpecialty == null || !opportunitySpecialty.Specialty.Equals(specialty))
{
    if(opportunitySpecialty != null)
    {
        if (OpportunitySpecialties.RemovedEntitiesTracker == null)
        {
            OpportunitySpecialties.RemovedEntitiesTracker = new EntityCollection<OpportunitySpecialtyEntity>();
        }
        OpportunitySpecialties.Remove(opportunitySpecialty);
        Debug.Assert(OpportunitySpecialties.RemovedEntitiesTracker != null);
        Debug.Assert(OpportunitySpecialties.RemovedEntitiesTracker.Contains(opportunitySpecialty));
    }
    var newOpportunitySpecialty = new OpportunitySpecialtyEntity {
                                                                    SortOrder = sortOrder,
                                                                    Created = DateTime.UtcNow,
                                                                    Opportunity = this,
                                                                    Specialty = specialty,
                                                                 };
    OpportunitySpecialties.Add(newOpportunitySpecialty);
}

When we persist the entity, we pull the items from the RemovedEntitiesTracker and add them to the UnitOfWork2 for delete. After persisting, the RemovedEntitiesTracker quits picking up on the removed items, whether or not we set it to a new collection. The following assertion fails:

Debug.Assert(OpportunitySpecialties.RemovedEntitiesTracker.Contains(opportunitySpecialty));

Framework 3.5 LLBLGen Pro 2.6

daelmo avatar
daelmo
Support Team
Posts: 8245
Joined: 28-Nov-2005
# Posted on: 15-Jul-2008 06:39:33   

Hi David,

Could you please check this:


// this should pass ok
Debug.Assert(OpportunitySpecialties.Remove(opportunitySpecialty));

Are you sure the involved entity to be removed is actually part of the OpportunitySpecialties collection? Otherwise it will fail, returns false and the entity wouldn't be added to the tracker.

David Elizondo | LLBLGen Support Team
Posts: 17
Joined: 12-Mar-2007
# Posted on: 15-Jul-2008 15:34:37   

David,

The call to Remove returns true, and the OpportunitySpecialtyEntity is still not showing up in the RemovedEntityTracker.

The only thing that may be a little weird about this case is that the entity (OpportunitySpecialty) has a composite primary key.

Thanks, David

Walaa avatar
Walaa
Support Team
Posts: 14950
Joined: 21-Aug-2005
# Posted on: 15-Jul-2008 16:18:18   

Which LLBLGen Pro runtime library version are you using?

Posts: 17
Joined: 12-Mar-2007
# Posted on: 15-Jul-2008 16:24:40   

SD.LLBLGen.Pro.ORMSupportClasses.NET20 2.6.8.613

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39614
Joined: 17-Aug-2003
# Posted on: 15-Jul-2008 17:21:44   

Will check it out.

Frans Bouma | Lead developer LLBLGen Pro
Otis avatar
Otis
LLBLGen Pro Team
Posts: 39614
Joined: 17-Aug-2003
# Posted on: 16-Jul-2008 10:09:08   

davidsidlinger wrote:

We're seeing some odd behavior ourselves. The following is from a method in a entity partial:


Debug.Assert(specialty != null);
var opportunitySpecialty = GetOpportunitySpecialtyBySortOrder(sortOrder);
if (opportunitySpecialty == null || !opportunitySpecialty.Specialty.Equals(specialty))
{
    if(opportunitySpecialty != null)
    {
        if (OpportunitySpecialties.RemovedEntitiesTracker == null)
        {
            OpportunitySpecialties.RemovedEntitiesTracker = new EntityCollection<OpportunitySpecialtyEntity>();
        }
        OpportunitySpecialties.Remove(opportunitySpecialty);
        Debug.Assert(OpportunitySpecialties.RemovedEntitiesTracker != null);
        Debug.Assert(OpportunitySpecialties.RemovedEntitiesTracker.Contains(opportunitySpecialty));
    }
    var newOpportunitySpecialty = new OpportunitySpecialtyEntity {
                                                                    SortOrder = sortOrder,
                                                                    Created = DateTime.UtcNow,
                                                                    Opportunity = this,
                                                                    Specialty = specialty,
                                                                 };
    OpportunitySpecialties.Add(newOpportunitySpecialty);
}

When we persist the entity, we pull the items from the RemovedEntitiesTracker and add them to the UnitOfWork2 for delete. After persisting, the RemovedEntitiesTracker quits picking up on the removed items, whether or not we set it to a new collection. The following assertion fails:

Debug.Assert(OpportunitySpecialties.RemovedEntitiesTracker.Contains(opportunitySpecialty));

Framework 3.5 LLBLGen Pro 2.6

I can't work with this information, so please provide more code, i.e. the code you described in text. Also, you posted a snippet of code, but is that inside a method?

The thing is: I don't really understand what you're doing, i.e. what the order of actions is that are taking place: do you use the Unit of work for persistence as well, etc.

Using a unitofwork means: add all actions to the unitofwork object, then call commit. So if you're adding entities to the unitofwork during commit, that's not what should be done: add the actions to perform before commit. But again, I don't know if that's what you're doing, so please provide more information so I can see what you're doing in code and which I can use to reproduce the problem.

Also, please next time start a new thread. Thanks. simple_smile

Frans Bouma | Lead developer LLBLGen Pro
Posts: 17
Joined: 12-Mar-2007
# Posted on: 16-Jul-2008 17:33:50   

The code I initially posted is in a partial for the OpportunityEntity:


public SpecialtyEntity PreferredSpecialty
{
    get { return GetSpecialtyEntityBySortOrder(1); }
    set
    {
        if(value == null)
        {
            throw new ArgumentNullException("value");
        }
        SetSpecialtyBySortOrder(value, 1);
    }
}

…

private void SetSpecialtyBySortOrder(SpecialtyEntity specialty, short sortOrder)
{
    Debug.Assert(specialty != null);
    var opportunitySpecialty = GetOpportunitySpecialtyBySortOrder(sortOrder);
    if (opportunitySpecialty == null || !opportunitySpecialty.Specialty.Equals(specialty))
    {
        if(opportunitySpecialty != null)
        {
            if (OpportunitySpecialties.RemovedEntitiesTracker == null)
            {
                OpportunitySpecialties.RemovedEntitiesTracker = new EntityCollection<OpportunitySpecialtyEntity>();
            }
            Debug.Assert(OpportunitySpecialties.Contains(opportunitySpecialty));
                    OpportunitySpecialties.Remove(opportunitySpecialty);
                    Debug.Assert(OpportunitySpecialties.RemovedEntitiesTracker != null);
            Debug.Assert(OpportunitySpecialties.RemovedEntitiesTracker.Contains(opportunitySpecialty));
                }
        var newOpportunitySpecialty = new OpportunitySpecialtyEntity {
                                                                        SortOrder = sortOrder,
                                                                        Created = DateTime.UtcNow,
                                                                        Opportunity = this,
                                                                        Specialty = specialty,
                                                                     };
        OpportunitySpecialties.Add(newOpportunitySpecialty);
    }
}

…

private OpportunitySpecialtyEntity GetOpportunitySpecialtyBySortOrder(short sortOrder)
{
    return (from opportunitySpecialty in OpportunitySpecialties
            where opportunitySpecialty.SortOrder == sortOrder
            select opportunitySpecialty).FirstOrDefault();
}

Here is the persistence code, where we use the UnitOfWork:


public void Save(OpportunityEntity opportunity)
{
    if (opportunity == null)
    {
        throw new ArgumentNullException("opportunity");
    }
    var unitOfWork = new UnitOfWork2(new List<UnitOfWorkBlockType> {
                                                                    UnitOfWorkBlockType.Deletes,
UnitOfWorkBlockType.Updates,                                                                                                                                    UnitOfWorkBlockType.Inserts
                                                                   });

    Debug.Assert(opportunity.OpportunitySpecialties != null);
    if(opportunity.OpportunitySpecialties.RemovedEntitiesTracker != null)
    {
        foreach(var opportunitySpecialty in opportunity.OpportunitySpecialties.RemovedEntitiesTracker.Cast<OpportunitySpecialtyEntity>())
        {
            if(!opportunitySpecialty.IsNew)
            {
                unitOfWork.AddForDelete(opportunitySpecialty);
            }
        }

        opportunity.OpportunitySpecialties.RemovedEntitiesTracker.Clear();
    }

    unitOfWork.AddForSave(opportunity, new PredicateExpression(), true, true);
    WithAdapter(adapter => unitOfWork.Commit(adapter));
}

Here's an automated integration test, which passes. After the first save, the last Debug.Assert in SetSpecialtyBySortOrder is tripped.


[TestMethod]
[ExpectedException(typeof(ORMQueryExecutionException))]
public void SetSpecialtyMultipleTimes()
{
    var target = new SqlData(_factory);

    var opportunity = target.FetchOpportunity(target.FetchOpportunityBrowseList().First().OpportunityId);

    var oldSpecialty = opportunity.PreferredSpecialty;
    var newSpecialty = 
        target.FetchSpecialty(
        target.FetchSpecialties().First(row => row.Id != oldSpecialty.Id).Id);

    opportunity.PreferredSpecialty = newSpecialty;
    target.Save(opportunity);

    opportunity.PreferredSpecialty = oldSpecialty;
    target.Save(opportunity);
}

Sorry for hijacking the thread. Let me know, if there is anything else that would help.

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39614
Joined: 17-Aug-2003
# Posted on: 17-Jul-2008 11:26:59   

Well, it's hard to follow what you're doing. After a while it occured to me all this code is to manage a m:n intermediate entity, for a m:n relation between Opportunity and Specialty, correct? In general, these kind of issues are almost impossible to track down without information which allows us to reproduce the issue, either in a repro case or with explanation what's going on.

Remove() calls a routine which places the entity in the tracker collection if: 1) there's a tracker collection and 2) Contains() on that tracker collection returns false.

Contains() does an instance compare if one of the entities in the comparison is new, and does a PK-PK compare if both aren't new.

What I wonder is: in the debugger, when you place a breakpoint right after Remove() and check the RemovedEntitiesTracker collection, does it contain an entity, and if so, which one? I guess it is empty? Also, is the oppertunitySpecialty entity really removed from the collection?

Is the first Save() indeed deleting anything? Unclear.

I'll try to create a repro case, however keep in mind that we will likely ask you for a reprocase.

Frans Bouma | Lead developer LLBLGen Pro
Otis avatar
Otis
LLBLGen Pro Team
Posts: 39614
Joined: 17-Aug-2003
# Posted on: 17-Jul-2008 12:01:05   

You also forgot to paste a couple of routines: - GetSpecialtyEntityBySortOrder(int); - FetchOpportunity(int) - FetchSpecialty

These would have given us insight in where the entity comes from you're comparing with (is it fetched every time?) and how you're fetching what initially. That's unknown at this point. I assume a prefetch path, but it also could be a webservice you're communicating with every time, which would create copies...

I'll write my own to see if they can get me the result you're seeing. If not, I'll give up and you've to create a repro case which shows the behavior.

You can also try to debug some more, i.e. with a debug build of the ormsupport classes. The routine of interest is in EntityCollectionBase2.cs, line 1446, PlaceInRemovedEntitiesTracker

I also run into a big problem with the OppertunitySpecialty entity: what's the PK? because your code seems to be buggy if the PK is simply the two Ids form Oppertunity and Specialty: I have OppertunitySpecialty setup as:


CREATE TABLE [OppertunitySpecialty] (
    [OppertunityId] [int] NOT NULL ,
    [SpecialtyId] [int] NOT NULL ,
    [SortOrder] [int] NOT NULL ,
    [CreatedOn] [datetime] NULL ,
    CONSTRAINT [PK_OppertunitySpecialty] PRIMARY KEY  CLUSTERED 
    (
        [OppertunityId],
        [SpecialtyId]
    )  ON [PRIMARY] ,
    CONSTRAINT [FK_OppertunitySpecialty_Oppertunity] FOREIGN KEY 
    (
        [OppertunityId]
    ) REFERENCES [Oppertunity] (
        [Id]
    ),
    CONSTRAINT [FK_OppertunitySpecialty_Specialty] FOREIGN KEY 
    (
        [SpecialtyId]
    ) REFERENCES [Specialty] (
        [Id]
    )
) ON [PRIMARY]
GO

And I have in this table 1 | 1 | 1 | NULL 1 | 2 | 2 | NULL 2 | 1 | 2 | NULL

I fetch the first oppetunity, 1. I change the specialty from 1 to 2 for sortorder 1 -> insert crash because it removes 1 | 1 | 1 and inserts 1 | 2 | 1, however the PK is just the first two fields (IMHO), but I don't know that. I'll work around this for now, but as you can see, providing as much info as possible is essential for solving these kind of cases.

Frans Bouma | Lead developer LLBLGen Pro
Otis avatar
Otis
LLBLGen Pro Team
Posts: 39614
Joined: 17-Aug-2003
# Posted on: 17-Jul-2008 12:22:34   

The entity is in the collection twice when the Remove occurs. So it removes 1 version, then the removal tracker add routine simply does a Contains() check if it's still in the entity, yes it is, so it does nothing. Checking now why it's in the collection twice. Likely due to the removal on one side but not on both sides: dereferencing an entity is better.

the removal tracker isn't adding it if the entity is still in the collection because:

        // if there's no tracker, return. Also if the entity is still in the collection, don't add it, because the tracker is used for
        // tracking entities to delete. As the entity is still in the collection, it can't be deleted from the db, as it's still present in the
        // collection. This is a safety check, to make sure that when a user does this:
        // myOrder.Customer = myCustomer;
        // myCustomer.Orders.Add(myOrder);// A
        // at line A, myOrder is already in myCustomer.Orders, because LLBLGen Pro syncs relations. Re-adding it will dereference myOrder first, making it
        // getting removed from myCustomer.Orders, then it gets re-added, and resynced. 

(edit) I found the bug in your code. It's due to a change in v2.6. This line: newOppertunitySpecialty.Oppertunity = this; already makes sure newOppertunitySpecialty is in this.OppertunitySpecialties. So adding it AGAIN adds it again.

In previous versions, the addition: this.OppertunitySpecialties.Add(newOppertunitySpecialty); would simply desync newOppertunitySpecialty first from the Oppertunity entity, and then re-sync it, causing unnecessary events to take place.

The second addition isn't trapped because DoNotperformAddIfPresent is set to false by default (otherwise it will perform a linear search for every addition.)

You also could have seen this within a debugger: before the remove, the OppertunitySpecialties collection would have contained the entity to remove twice. simple_smile

Frans Bouma | Lead developer LLBLGen Pro
Posts: 17
Joined: 12-Mar-2007
# Posted on: 17-Jul-2008 15:34:17   

Frans,

You're correct that OpportunitySpecialty is a join table between Opportunity and Specialty and that the PK is the first two columns, Opportunity_Id and Specialty_Id. There is a unique constraint on the Opportunity_Id and Sort_Order columns as well.

We are using prefetch paths to fetch the child collection. Nothing squirrley there.

I will get the source for the methods that you noted and post an update.

Thanks, David

Posts: 17
Joined: 12-Mar-2007
# Posted on: 17-Jul-2008 15:53:39   

Frans,

We removed the redundant addition, and it worked.

Thanks, David