Ticket #1021 (closed defect: fixed)

Opened 6 years ago

Last modified 6 years ago

[REGRESSION-TESTCASE] osync_xmlfield_set_key_value() is missing child links

Reported by: henrik Owned by: dgollub
Priority: highest Milestone: OpenSync 0.39
Component: OpenSync: XMLFormat API Version: 0.38
Severity: blocker Keywords:
Cc:

Description

OpenSync? -r5044 creates duplicates on slowsync.

  1. Create a new Thunderbird address-book with one single contact.
  2. Create OpenSync? group and sync the address-book (mozilla-sync) with file-sync
  3. Everything is file, the file-sync dir contains one VCARD
  4. Create a new OpenSync? group with exactly the same settings as before and sync this
  5. Now OpenSync? adds the thunderbird contact to the file-sync directory again!
  6. So, now we are left with identical duplicates!!

mozilla-sync sends this xml:

<?xml version="1.0"?>
<contact><FormattedName><Content>ffffffffff llllllllll</Content></FormattedName><Name><LastName>llllllllll</LastName><FirstName>ffffffffff</FirstName></Name><Telephone Location="Home"><Content>hhhhhhhhhh</Content></Telephone><Telephone Location="Work"><Content>wwwwwwwwww</Content></Telephone></contact>

The first slowsync turns this into the file-sync VCARD

BEGIN:VCARD
VERSION:2.1
FN:ffffffffff llllllllll
N:llllllllll;ffffffffff;;;
TEL;HOME:hhhhhhhhhh
TEL;WORK:wwwwwwwwww

In the second slow-sync the trace shows that the VCARD is internally converted into

<?xml version="1.0"?>
<contact>
  <FormattedName>
    <Content>ffffffffff llllllllll</Content>
  </FormattedName>
  <Name>
    <LastName>llllllllll</LastName>
    <FirstName>ffffffffff</FirstName>
  </Name>
  <Telephone Location="Home">
    <Content>hhhhhhhhhh</Content>
  </Telephone>
  <Telephone Location="Work">
    <Content>wwwwwwwwww</Content>
  </Telephone>
</contact>

So, this should compare identical to the xml from mozilla-sync. However, the traces show

>>>>>>>  compare_contact(0xaff4a8c8, 28, 0xaff4d998, 28)
	>>>>>>>  xmlformat_compare(0xaff4a8c8, 0xaff4d998, 0xb67d7f14, 0, 100)
		result of strcmp(): 0 (FormattedName || FormattedName)
		result of strcmp(): 0 (Name || Name)
		>>>>>>>  xmlfield_compare(0xaff4f138, 0xaff4f238)
		<<<<<<<  xmlfield_compare: 0
		one field is alone: Name
		>>>>>>>  xmlfield_compare_similar(0xaff4f138, 0xaff4f238, 0xb67d7f68)
		<<<<<<<  xmlfield_compare_similar: 0
		subtracting 90 point(s) for missing field: Name
		result of strcmp(): 0 (Telephone || Telephone)
		>>>>>>>  xmlfield_compare_similar(0xaff63f08, 0xaff4fd38, 0xb67d7f74)
		<<<<<<<  xmlfield_compare_similar: 0
		>>>>>>>  xmlfield_compare_similar(0xaff63f08, 0xaff4fcb0, 0xb67d7f74)
		<<<<<<<  xmlfield_compare_similar: 0
		subtracting 10 point(s) for missing field: Telephone
		>>>>>>>  xmlfield_compare_similar(0xaff63ec8, 0xaff4fd38, 0xb67d7f74)
		<<<<<<<  xmlfield_compare_similar: 0
		>>>>>>>  xmlfield_compare_similar(0xaff63ec8, 0xaff4fcb0, 0xb67d7f74)
		<<<<<<<  xmlfield_compare_similar: 0
		subtracting 10 point(s) for missing field: Telephone
		Result is: -110, Treshold is: 100
	<<<<<<<  xmlformat_compare: MISMATCH
<<<<<<<  compare_contact: 1

Full traces attached.

Attachments

OSYNC_TRACE_1.zip (47.5 KB) - added by henrik 6 years ago.
OSYNC_TRACE_2.zip (53.8 KB) - added by henrik 6 years ago.
VCARD.zip (410 bytes) - added by henrik 6 years ago.
5106_vcard.zip (392 bytes) - added by henrik 6 years ago.
5106_trace1.zip (45.6 KB) - added by henrik 6 years ago.
5106_trace2.zip (54.6 KB) - added by henrik 6 years ago.
5119_VCARD.ZIP (410 bytes) - added by henrik 6 years ago.
5119_TRACE_1.ZIP (46.4 KB) - added by henrik 6 years ago.
5119_TRACE_2.ZIP (56.3 KB) - added by henrik 6 years ago.

Change History

Changed 6 years ago by henrik

Changed 6 years ago by henrik

Changed 6 years ago by henrik

comment:1 Changed 6 years ago by henrik

If I turn capabilities off in mozilla-sync, the result is still the same...

comment:2 Changed 6 years ago by dgollub

Hmm .. this could be related to the OSyncXMLField/OSyncXMLFormat multi-level support, which could break things. Or maybe the result is not sorted? I'll instrument the compare_xmlformat function to dump the content before comparing.

Changed 6 years ago by henrik

Changed 6 years ago by henrik

Changed 6 years ago by henrik

comment:3 Changed 6 years ago by henrik

Retrying with -r5106. Traces attached.

Note that now (-r5106) the additional VCARD has CR+LF as line separator (and missing the LF at the end of the file), whereas in the original run (-r5044) it had just LF. In both cases the original VCARD had just LF.

comment:4 Changed 6 years ago by henrik

Correction: In the original -r5044 case the original file had CR+LF.

comment:5 Changed 6 years ago by dgollub

  • Status changed from new to assigned
  • Component changed from OpenSync to OpenSync: XMLFormat API

This looks really pretty much the same:

left change (UID:1):
<?xml version="1.0"?>
<contact>
  <FormattedName>
    <Content>ffffffffff llllllllll</Content>
  </FormattedName>
  <Name>
    <LastName>llllllllll</LastName>
    <FirstName>ffffffffff</FirstName>
  </Name>
  <Telephone Location="Home">
    <Content>hhhhhhhhhh</Content>
  </Telephone>
  <Telephone Location="Work">
    <Content>wwwwwwwwww</Content>
  </Telephone>
</contact>

right change (UID:V):
<?xml version="1.0"?>
<contact>
  <FormattedName>
    <Content>ffffffffff llllllllll</Content>
  </FormattedName>
  <Name>
    <LastName>llllllllll</LastName>
    <FirstName>ffffffffff</FirstName>
  </Name>
  <Telephone Location="Home">
    <Content>hhhhhhhhhh</Content>
  </Telephone>
  <Telephone Location="Work">
    <Content>wwwwwwwwww</Content>
  </Telephone>
</contact>

comparison does this:

osync_data_compare(0xb3e020d8, 0xb3e00620)
>>>>>>  compare_contact(0xb3e00d20, 28, 0xb3e01ad0, 28)
       >>>>>>>  xmlformat_compare(0xb3e00d20, 0xb3e01ad0, 0xb672cd64, 0, 100)
        result of strcmp(): 0 (FormattedName || FormattedName)
        result of strcmp(): 0 (Name || Name)
        >>>>>>>  xmlfield_compare(0xb3e01a80, 0xb3e03200)
        <<<<<<<  xmlfield_compare: 0
        one field is alone: Name
        >>>>>>>  xmlfield_compare_similar(0xb3e01a80, 0xb3e03200, 0xb672cdb8) 
        <<<<<<<  xmlfield_compare_similar: 0
        subtracting 90 point(s) for missing field: Name 
        result of strcmp(): 0 (Telephone || Telephone)
        >>>>>>>  xmlfield_compare_similar(0xb3e072a0, 0xb3e03600, 0xb672cdc4) 
        <<<<<<<  xmlfield_compare_similar: 0
        >>>>>>>  xmlfield_compare_similar(0xb3e072a0, 0xb3e032a0, 0xb672cdc4) 
        <<<<<<<  xmlfield_compare_similar: 0
        subtracting 10 point(s) for missing field: Telephone
        >>>>>>>  xmlfield_compare_similar(0xb3e07260, 0xb3e03600, 0xb672cdc4) 
        <<<<<<<  xmlfield_compare_similar: 0
        >>>>>>>  xmlfield_compare_similar(0xb3e07260, 0xb3e032a0, 0xb672cdc4) 
        <<<<<<<  xmlfield_compare_similar: 0
        subtracting 10 point(s) for missing field: Telephone
        Result is: -110, Treshold is: 100
       <<<<<<<  xmlformat_compare: MISMATCH
<<<<<<  compare_contact: 1
osync_data_compare: 1

The xmlformat_compare() seems to be broken. Maybe related to the xmlfield-multi level thing....

comment:6 Changed 6 years ago by dgollub

  • Summary changed from Duplicates on slowsync to [NEEDINFO] Duplicates on slowsync

I tried to reproduce the compare issue, without success. I wrote a testcase using those XML dumps. And calling directly compare_contact().

Could you kill all builds of the xmlformat.so and try to build _latest_ xmlformat-plugin. Note: the plugin moved to a different repo. last week.

dgollub@bender:~/projects/OpenSync/format-plugins/xmlformat/trunk> svn info
Path: .
URL: https://svn.opensync.org/format-plugins/xmlformat/trunk
[...]
Last Changed Rev: 5085
Last Changed Date: 2009-01-11 01:09:25 +0100 (Sun, 11 Jan 2009)

comment:7 Changed 6 years ago by dgollub

The vcard V ends without END:VCARD - could you fix that? I would be surprised if this would fix the issue. But to be sure please fix the vcard and let me know if this change anything... will try to reproduce with the vcards - instead of the XML dumps.

comment:8 Changed 6 years ago by dgollub

I copied the vcards of 5106_vcard.zip into two separated directories/members. Henrik, is this issue reproducible on your platform with a file2file sync group? Or does this only appear in combination with the mozilla-sync plugin?

In meanwhile i fear there is something with the encoding...

From a file2file sync - the trace from my group - on my platform:

>>>>>>>  osync_data_compare(0x83fc790, 0x8400e60)
        >>>>>>>  compare_contact(0x84027a0, 28, 0x83fc610, 28)
                >>>>>>>  xmlformat_compare(0x84027a0, 0x83fc610, 0xb710ad04, 0, 100)
                        result of strcmp(): 0 (FormattedName || FormattedName)
                        result of strcmp(): 0 (Name || Name)
                        >>>>>>>  xmlfield_compare(0x83fce20, 0x83f6fd8)
                        <<<<<<<  xmlfield_compare: 1
                        add 90 point(s) for same fields: Name
                        result of strcmp(): 0 (Telephone || Telephone)
                        >>>>>>>  xmlfield_compare(0x8400ba0, 0x84042d0)
                        <<<<<<<  xmlfield_compare: 1
                        add 10 point(s) for same fields: Telephone
                        >>>>>>>  xmlfield_compare(0x8400c70, 0x83ffad0)
                        <<<<<<<  xmlfield_compare: 1
                        add 10 point(s) for same fields: Telephone
                        Result is: 110, Treshold is: 100
                <<<<<<<  xmlformat_compare: SAME
        <<<<<<<  compare_contact: 3
<<<<<<<  osync_data_compare: 3

comment:9 Changed 6 years ago by dgollub

Henrik, please try r5116. This includes more instrumentation to check where the xmlfield_compare detects differences. On my testcase this succeed for unknown reason. Pleasy update, and rerun with OSYNC_NOPRIVCAY=1 and provide again the snippet of xmlformat_compare().

I go in meanwhile through your log files with the hex editor...

comment:10 Changed 6 years ago by dgollub

I just saw that mozilla-sync is assembling XMLFormat-contact within the sync code :( That's really bad - please put this on your TODO and don't do any conversion inside a sync plugin....

comment:11 Changed 6 years ago by henrik

Tried again with -r5119. Still the same problem. Traces attaced. The problem does NOT exist on filesync-to-filesync.

Changed 6 years ago by henrik

Changed 6 years ago by henrik

Changed 6 years ago by henrik

comment:12 Changed 6 years ago by dgollub

  • Priority changed from high to highest
  • Severity changed from critical to blocker
  • Summary changed from [NEEDINFO] Duplicates on slowsync to osync_xmlfield_new() is missing child links

Ok - here is it:

                        result of strcmp(): 0 (Name || Name)
                        >>>>>>>  xmlfield_compare(0x82f0978, 0x82efc30)
                                At least one key is empty. key1:0x82f09b8 key2:(nil)
                        <<<<<<<  xmlfield_compare: 0

The vformat-plugin assembled vcard is broken, due to the mutli-level support. The missed osync_xmlfield_new() to set child nodes. The reason why this only happens with mozilla-sync and file-sync is that mozilla-sync entries use at some point osync_xmlformat_parse(). This interface is not affected by the child-node link bug. Only the osync_xmlfield_new() interface which gets heavily used by the vformat plugin....

WIP

comment:13 Changed 6 years ago by dgollub

  • Summary changed from osync_xmlfield_new() is missing child links to osync_xmlfield_set_key_value() is missing child links

Ooops, it's about osync_xmlfield_set_key_value() no osync_xmlfield_new()

comment:14 Changed 6 years ago by dgollub

Updated regression testcase r5123

comment:15 Changed 6 years ago by dgollub

  • Summary changed from osync_xmlfield_set_key_value() is missing child links to [REGRESSION-TESTCASE] osync_xmlfield_set_key_value() is missing child links

Should be fixed with r5132

Happy testing!

Keeping the bug open until regression-testcase for OpenSync core got committed.

comment:16 Changed 6 years ago by dgollub

  • Status changed from assigned to closed
  • Resolution set to fixed

Regression testcase added r5133

comment:17 Changed 6 years ago by henrik

I confirm that all mozilla-sync test-cases are passing with -r5132.

THANK YOU VERY MUCH FOR THE FIX !!!

Note: See TracTickets for help on using tickets.