XML export producesAttributeError unicode

Bug #195761 reported by benglynn
34
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Zope 2
Fix Released
Low
Tres Seaver

Bug Description

if I try to export in XML when exporting a snapshot element, I get an error -- please see the stack trace below, thanks.
Traceback (innermost last):
  Module ZPublisher.Publish, line 119, in publish
  Module ZPublisher.mapply, line 88, in mapply
  Module ZPublisher.Publish, line 42, in call_object
  Module OFS.ObjectManager, line 579, in manage_exportObject
  Module OFS.XMLExportImport, line 56, in exportXML
  Module OFS.XMLExportImport, line 31, in XMLrecord
  Module Shared.DC.xml.ppml, line 263, in load
  Module pickle, line 872, in load
  Module Shared.DC.xml.ppml, line 418, in load_binput
AttributeError: 'unicode' object has no attribute 'id'

Revision history for this message
Andreas Jung (ajung) wrote :

Please use the standard .zexp export/import. There is usually no need for doing export/imports on the XML basis.

Changed in zope2:
importance: Undecided → Low
Revision history for this message
Simon Michael (simon) wrote :

I just got this too, with Zope 2.10.5. I'd like to use XML so that I can edit to get around import problems.

This change to ZODB/ExportImport.py omits failing objects to let the export complete. I don't know if the resulting dump is valid.

 43,147c143,153
 < pickler.dump(unpickler.load())
 < pickler.dump(unpickler.load())
 < p = newp.getvalue()
 <
 < self._storage.store(oid, None, p, version, transaction)
 ---
 > try:
 > pickler.dump(unpickler.load())
 > pickler.dump(unpickler.load())
 > p = newp.getvalue()
 >
 > self._storage.store(oid, None, p, version, transaction)
 > except:
 > import sys, traceback
 > type,val,tb = sys.exc_info()
 > try: sys.stderr.write('failed to import: %s\n' % ''.join(traceback.format_exception(type,val,tb)))
 > finally: del tb # Clean up circular reference, avoid IssueNo0536

Revision history for this message
Andreas Jung (ajung) wrote :

Omitting failing objects is definitely not the right solution :-)

Revision history for this message
auspex (auspex) wrote :

But NOT exporting XML is?

There are many reasons why you might want to export (or import) XML. I ran into this because I just wanted to see what the XML output looked like, so that I could write a generator to export data from an SQL database and import as objects to the ZODB. One might equally use XML output for _input_ to an SQL database. I shudder to think of doing that with a .zexp.

Revision history for this message
Wolfgang Scherer (wolfgang-scherer) wrote :

Hi, I had the same problem.
I was trying to export XML for analysis, which is valid reason, isn't it :))
The attached patch fixed the problem.

Revision history for this message
Andreas Jung (ajung) wrote :

>
>But NOT exporting XML is?
>There are many reasons why you might want to export (or import) XML. I ran into this because I just wanted to see what the XML output looked like, so that I could write a generator to >export data from an SQL database and import as objects to the ZODB. One might equally use XML output for _input_ to an SQL database. I shudder to think of doing that with a .zexp.

The main purpose of the export/import functionality is moving stuff between identical Zope installations. This works using .zexp fine.
XML export/import is historically broken and considered a second- or third-class cititzen of Zope.

The patch can only be applied if it has unittests.

Revision history for this message
Wolfgang Scherer (wolfgang-scherer) wrote :

I'm totally new to python, zope.
So I have no idea how to write unit tests :-(

Revision history for this message
Jim Fulton (jim-zope) wrote : Re: [Bug 195761] Re: XML export producesAttributeError unicode

On Apr 17, 2008, at 12:58 AM, Andreas Jung wrote:
>>
>> But NOT exporting XML is?
>> There are many reasons why you might want to export (or import)
>> XML. I ran into this because I just wanted to see what the XML
>> output looked like, so that I could write a generator to >export
>> data from an SQL database and import as objects to the ZODB. One
>> might equally use XML output for _input_ to an SQL database. I
>> shudder to think of doing that with a .zexp.
>
> The main purpose of the export/import functionality is moving stuff
> between identical Zope installations. This works using .zexp fine.
> XML export/import is historically broken and considered a second- or
> third-class cititzen of Zope.

Over time, I found the xml export/import to serve 2 important use cases:

1. Export to external systems

     For lots of applications, the export format is actually
(surprizingly) fairly useful.

2. Export -> change -> import

     As Simon pointed out, it's useful to be able to export, then edit
the data then reimport, usually into the *same* system.

This isn't to say that you, Andreas, or anyone else in particular,
should be on the hook to fix it. I just wanted to point out that the
XML export/import is more useful than people (including myself) have
given it credit for.

> The patch can only be applied if it has unittests.

Yup.

Jim

--
Jim Fulton
Zope Corporation

Revision history for this message
Tres Seaver (tseaver) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Andreas Jung wrote:
>> But NOT exporting XML is?
>> There are many reasons why you might want to export (or import) XML. I ran into this because I just wanted to see what the XML output looked like, so that I could write a generator to >export data from an SQL database and import as objects to the ZODB. One might equally use XML output for _input_ to an SQL database. I shudder to think of doing that with a .zexp.
>
> The main purpose of the export/import functionality is moving stuff between identical Zope installations. This works using .zexp fine.
> XML export/import is historically broken and considered a second- or third-class cititzen of Zope.

I call "bullshit." The *entire reason* for the XML export / import as
an alternative to the binary version was to allow for transforms /
fixups to be applied, which is completely infesible on the binary
version. I was *there* when top ZC folks were citing it as such to
prospective clients.

> The patch can only be applied if it has unittests.

- -1. This is not a new feature, it is a fix for a known regression.

Tres.
- --
===================================================================
Tres Seaver +1 540-429-0999 <email address hidden>
Palladion Software "Excellence by Design" http://palladion.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFIB2ah+gerLs4ltQ4RAt4JAKCJ0JwwHpg0bRGv0ruZaFz2bs4LywCeM5+M
gxAaQmwzc44IuqjyJ8DL/8I=
=jfzn
-----END PGP SIGNATURE-----

Revision history for this message
Andreas Jung (ajung) wrote :

The implementation might have worked for you years agon within the ASCII world. But it is obviously broken
for several reasons when it comes to unicode or strings with non-ascii chars. We are applying fixes since versions
to the XML export code and it breaks all over again. I would not call this "usefull". Useful for the ASCII world,
broken for the rest of the world. If you want to advertise XML export as a feature of Zope then the implementation
must work in a reliable way and and it must not break as it does right now. Right now XML export is highly fragile and can only be ssen
as a second-class functionality of Zope - same as with ZClasses. it can not be recommended in general. Those
functionalities must be marked "use with care".

Revision history for this message
Jim Fulton (jim-zope) wrote :

On Apr 17, 2008, at 11:02 AM, Tres Seaver wrote:

Overly sharp reaction snipped.

>> The patch can only be applied if it has unittests.
>
> - -1. This is not a new feature, it is a fix for a known regression.

What does that have to do with whether a test is required? Andreas is
right that a test will be required with a checkin. I don't think
Andreas should have to cook up a test. Maybe someone else will be kind
enough to create a test if the person who provided the patch can't or
won't.

Jim

--
Jim Fulton
Zope Corporation

Revision history for this message
Tres Seaver (tseaver) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Jim Fulton wrote:
> On Apr 17, 2008, at 11:02 AM, Tres Seaver wrote:
>
> Overly sharp reaction snipped.
>
>>> The patch can only be applied if it has unittests.
>> - -1. This is not a new feature, it is a fix for a known regression.
>
>
> What does that have to do with whether a test is required? Andreas is
> right that a test will be required with a checkin. I don't think
> Andreas should have to cook up a test. Maybe someone else will be kind
> enough to create a test if the person who provided the patch can't or
> won't.

There are *no* working tests at all for the module, not even the ones
intended to be run by executing the module as a script. AFAICT, this
has been so for *years* now: why would we expect somebody submitting a
"works for me" fix to provide a test.

Tres.
- --
===================================================================
Tres Seaver +1 540-429-0999 <email address hidden>
Palladion Software "Excellence by Design" http://palladion.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFIB3XD+gerLs4ltQ4RAvNDAJ9Z6cIC55wc+b79IhvcTtKPJudJWQCghc4k
/NWE81SLI8O1Y5x03ayt80M=
=3kSI
-----END PGP SIGNATURE-----

Revision history for this message
Wolfgang Scherer (wolfgang-scherer) wrote :

> The implementation might have worked for you years agon within the ASCII world. But it is obviously broken
FWIW, I moved from ASCII to latin-1 world years ago. And from there I moved to UTF-8 :))
And in UTF-8, everything is possible again, even unicode :))

Revision history for this message
Andreas Jung (ajung) wrote :

The seems to work when you export e.g. DTMLDocument with a ustring property.
If fails badly when you're trying to do the same within a unitest doing only the export:

Error in test test_OFS_ObjectManager__importObjectFromFile_xml (OFS.tests.test_XMLExportImport.XMLExportImportTests)
Traceback (most recent call last):
  File "/opt/python-2.4.4/lib/python2.4/unittest.py", line 260, in run
    testMethod()
  File "/Users/ajung/sandboxes/Zope/Zope/lib/python/OFS/tests/test_XMLExportImport.py", line 110, in test_OFS_ObjectManager__importObjectFromFile_xml
    data = exportXML(connection, oid, ostream)
  File "/Users/ajung/sandboxes/Zope/Zope/lib/python/OFS/XMLExportImport.py", line 56, in exportXML
    write(XMLrecord(oid,len(p),p))
  File "/Users/ajung/sandboxes/Zope/Zope/lib/python/OFS/XMLExportImport.py", line 31, in XMLrecord
    p=p+u.load().__str__(4)
  File "/Users/ajung/sandboxes/Zope/Zope/lib/python/Shared/DC/xml/ppml.py", line 171, in __str__
    v=v.__str__(indent+2)
  File "/Users/ajung/sandboxes/Zope/Zope/lib/python/Shared/DC/xml/ppml.py", line 184, in __str__
    return '%s<%s%s>\n%s%s</%s>\n' % (
  File "/Users/ajung/sandboxes/Zope/Zope/lib/python/Shared/DC/xml/ppml.py", line 208, in value
    self._d
  File "/Users/ajung/sandboxes/Zope/Zope/lib/python/Shared/DC/xml/ppml.py", line 198, in <lambda>
    map(lambda i, ind=' '*indent, indent=indent+4:
  File "/Users/ajung/sandboxes/Zope/Zope/lib/python/Shared/DC/xml/ppml.py", line 173, in __str__
    v=v.__str__()
UnicodeEncodeError: 'ascii' codec can't encode characters in position 0-11: ordinal not in range(128)

If you do the same within test_export_import_as_string_idempotent() it will fail in other bad way.
I still consider this functionality being totally broken for general purpose - even with this patch.

Revision history for this message
ke1g (ke1g-nh) wrote :

For what it's worth, this doesn't seem to be because of any effort to encode into XML, but rather
is a failure of the ToXMLUnpickler, which is trying to convert from a pickle (loaded from a _storage
object on a Connection object - zopedb?) to Python, so that that can later be XML encoded. The
stack has lots of instances of classes from ...xml.ppml but the last object, which happens to be the
title of the blog, was appended as a unicode string, rather than an object of some kind. The current
operation is trying to add an id to the "last" object and you can't add attributes to a Unicode object.
You can't add them to string objects either, which is why, I'd guess, that ppml has this fancy String
class.

So I'm guessing that ToXMLUnpickler just has failed to keep up with changes to the format of the
pickles coming from the zopedb. Too bad this can't be shared code with whatever unpickles an
object when it is awakened (that's been kept up to date).

I'm already way beyond my zope depth, and out of time to look at this. I'm posting in hope that
my info might jog a real zope deveolper into saying: "Oh! That's simple."

(Exporting a Quills blog: Plone 3.2.1 Zope 2.10.6-final, python 2.4.6, linux2)

Revision history for this message
mtiller (michael-tiller) wrote :

Wow, this bug has been sitting here for almost two years and the patch (which works) isn't accepted because there isn't a test for it. This is just crazy. You have a bug in the software and you won't fix the bug because there's no test for the patch. There was clearly no test in the first place (or this problem would have been caught before).

So the bottom line is you'd rather have broken software than incomplete coverage? That seems like a very strange set of priorities. Ironically, at the end of the day you've got both broken software and incomplete coverage. At least the patch would address one of those.

Revision history for this message
Federico G. Schwindt (fgsch) wrote :

The attached diff implements the missing Unicode handling and adds some tests. It'd be great if someone commits it.
Regards,

Revision history for this message
Tres Seaver (tseaver) wrote :

I plan to apply Federico's patch tomorrow.

Changed in zope2:
assignee: nobody → Tres Seaver (tseaver)
status: New → Confirmed
Revision history for this message
Federico G. Schwindt (fgsch) wrote :

That's great Tres.
Once it goes in I'd like to cleanup ppml further, but I wanted to keep the diff to the minimum, although not sure where I should send such cleanup.

Revision history for this message
Andreas Jung (ajung) wrote :

Keep in mind that the XML export functionality has been removed ZMI (I think in 2.11 or 2.12).

Revision history for this message
Federico G. Schwindt (fgsch) wrote :

Reverting 95008 is enough to get it back. Diff attached.

Revision history for this message
Federico G. Schwindt (fgsch) wrote :

And here's a diff that includes the previous 2 diffs and the cleanup.

Revision history for this message
Tres Seaver (tseaver) wrote : Re: [zope2-tracker] [Bug 195761] Re: XML export producesAttributeError unicode

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Andreas Jung wrote:
> Keep in mind that the XML export functionality has been removed ZMI (I
> think in 2.11 or 2.12).

Only because we hadn't fixed this bug.

Tres.
- --
===================================================================
Tres Seaver +1 540-429-0999 <email address hidden>
Palladion Software "Excellence by Design" http://palladion.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAktsN+QACgkQ+gerLs4ltQ5ZtwCgobQ5EIozUiRHc9feNZbAww+p
rcAAn2dFLlj2bUr7Ji4SL2SI47mxNdrr
=Fs3E
-----END PGP SIGNATURE-----

Revision history for this message
Federico G. Schwindt (fgsch) wrote :

I will hold on a bit on committing the diff. I'm trying to exercise some other code paths that are likely still broken.
Will be posting a new diff soon.

Revision history for this message
Federico G. Schwindt (fgsch) wrote :

New diff attached. It should fix the remaining issues with my previous diffs.
I've taken the liberty to remove some verbiage from the .dtml, rename a few functions and do further cleanup. More specifically, I've drop the superfluous newlines and spaces in the final XML and limited base64 encoding for the str type. The tests are also updated to cover the latest issues. At this point, I think this is good enough to go in.

Revision history for this message
Tres Seaver (tseaver) wrote :

Fix committed to the trunk and the 2.11 and 2.12 branches:

- http://svn.zope.org/Zope/trunk/?rev=109072&view=rev
- http://svn.zope.org/Zope/branches/2.12/?rev=109073&view=rev
- http://svn.zope.org/Zope/branches/2.11/?rev=109074&view=rev

The tests don't pass on 2.10: I'm leaving the patch off of that branch.

Changed in zope2:
status: Confirmed → Fix Committed
Revision history for this message
Tres Seaver (tseaver) wrote :

After a recheck, I have backported the fix to the 2.10 and 2.9 branches:

- http://svn.zope.org/Zope/branches/2.10/?rev=109075&view=rev
- http://svn.zope.org/Zope/branches/2.9/?rev=109076&view=rev

Changed in zope2:
milestone: none → 2.12.4
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.