1
Vote

Bugs found in MergeItems()

description

It looks like the actual issue is because of the null test for W as shown below. The test should check if there are no conflicts and if W is subsumed by localItem – this will handle the case when the localItem and incomingItem are the same item, such that MergeOperation.None is returned.
          private MergeOperation MergeItems(Item localItem, Item incomingItem, out Item proposedItem)
          {
                 Tracer.TraceData(this, TraceEventType.Verbose, "Applying SSE merge algorithm");
                 proposedItem = null;

                 //3.3.2
                 List<Item> L = new List<Item>();
                 L.AddRange(localItem.Sync.Conflicts);
                 localItem.Sync.Conflicts.Clear();
                 L.Add(localItem);


                 //3.3.3
                 List<Item> I = new List<Item>();
                 I.AddRange(incomingItem.Sync.Conflicts);
                 incomingItem.Sync.Conflicts.Clear();
                 I.Add(incomingItem);

                 //3.3.4
                 List<Item> M = new List<Item>();

                 //3.3.5
                 Item W = null;

                 //3.3.6 and 3.3.7
                 PerformStep7(ref L, ref I, M, ref W);
                 //3.3.8
                 PerformStep7(ref I, ref L, M, ref W);
// HERE
                 if (W == null)
                 {
                       //There is no need to update the local item
                       return MergeOperation.None;
                 }

                 proposedItem = W;

                 //3.3.10
                 if (!W.Sync.NoConflicts)
                 {
                       //3.3.11
                       foreach (Item z in M)
                       {
                              if (!z.Equals(W) && !W.Sync.Conflicts.Contains(z))
                              {
                                     W.Sync.Conflicts.Add(z);
                              }
                       }
                 }

                 if (W.Sync.Conflicts.Count > 0)
                 {
                       return MergeOperation.Conflict;
                 }
                 else
                 {
                       return MergeOperation.Updated;
                 }
          }

Proposed Solution: Change the highlighted code to ‘if ((W == null) || ((M.Count == 1) && W.IsSubsumedBy(localItem)))’

comments

dcazzulino wrote Apr 27, 2009 at 2:00 PM

Can you please provide a test that asserts what's the expected output from the public API?
Without a failed unit test that showcases how the current logic is broken, we can't really take action on it as-is.

wrote Feb 12, 2013 at 9:16 PM