Code

Opened 7 years ago

Closed 3 years ago

Last modified 3 years ago

#3094 closed enhancement (fixed)

XMLField not being validated.

Reported by: goliath.mailinglist@… Owned by: PaulM
Component: Database layer (models, ORM) Version: master
Severity: normal Keywords: relaxng validator
Cc: sam@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

I disliked the fact that Django calls an external program to do XML-validation. So I wrote a validation-class that uses lxml (which is a wrapper for libxml2) for validation. This class uses the error-messages found in the original class (so all messages get translated), but could support more verbose messages, as lxml tells you more details, but then the included translations would not work anymore. This class can be used as some kind of drop-in replacement. Perhaps it can be included in the official release.
Drawback: You need to write RelaxNG-files, libxml2 does not support RelaxNG-Compact (AFAIK). So perhaps it's best to let the user decide which one to use.

Attachments (6)

validators.py (3.3 KB) - added by David Danier <goliath.mailinglist@…> 7 years ago.
validator.py including RelaxNG-class
validators.2.py (3.3 KB) - added by David Danier <goliath.mailinglist@…> 7 years ago.
validators.py including RelaxNG-class
validators.3.py (5.4 KB) - added by David Danier <goliath.mailinglist@…> 6 years ago.
XMLField_deprecation.diff (5.6 KB) - added by PaulM 3 years ago.
correct_1.2.X_XMLField_docs.diff (749 bytes) - added by PaulM 3 years ago.
correct_1.1.X_XMLField_docs.diff (749 bytes) - added by PaulM 3 years ago.

Download all attachments as: .zip

Change History (33)

Changed 7 years ago by David Danier <goliath.mailinglist@…>

validator.py including RelaxNG-class

comment:1 Changed 7 years ago by David Danier <goliath.mailinglist@…>

If you want to try this, but only have RelaxNG-Compaxt files consider using http://www.gnosis.cx/download/relax/. Perhaps this could be put into the validator (it's python) to convert the file on the fly (with caching), but I don't need it myself.

Update follows:

  • RelaxNG-Errors got a different Errormessage
  • Deleted unneeded import (escape)

Changed 7 years ago by David Danier <goliath.mailinglist@…>

validators.py including RelaxNG-class

comment:2 Changed 7 years ago by Simon G. <dev@…>

  • Has patch set
  • Summary changed from RelaxNG validation using lxml to [patch] RelaxNG validation using lxml
  • Triage Stage changed from Unreviewed to Design decision needed

comment:3 Changed 7 years ago by anonymous

  • Cc sam@… added

comment:4 Changed 7 years ago by mtredinnick

  • Resolution set to wontfix
  • Status changed from new to closed

We are trying to ship a reasonably minimal set of validators in core (and to keep external dependencies down). Obviously that isn't a perfectly consistently applied policy, since some validators from the pre-history of Django's development are available. But we are trying to be more consistent these days.

However, this does look like a useful validator for some projects. I would suggest adding a new wiki page linked from CookBookValidators or a pointer to an external site on that page so that people who would like to use this can have access to it.

Thanks for writing this validator.

comment:5 Changed 6 years ago by jacob

  • Resolution wontfix deleted
  • Status changed from closed to reopened
  • Summary changed from [patch] RelaxNG validation using lxml to XML validation shouldn't require Jing; should use lxml if avaialble.

The current XML validator relies on Jing, which is even *more* of a pain to install than lxml (GJC.... ugh). I think the right approach is to refactor the XML validator to use a series of backends to try validation -- lxml if available, jing if available and if you're using RNG-Compact, and finally minidom for basic well-formedness.

I'm gonna steal this ticket for that issue since this lxml validator is a good place to start.

comment:6 Changed 6 years ago by jacob

  • Triage Stage changed from Design decision needed to Accepted

comment:7 Changed 6 years ago by jacob

Also see #5703 for a validator using RNV. I'm wondering if we might want to have some sort of "registry" for XML validation methods; there seem to be a bunch, all with their own good use cases.

comment:8 Changed 6 years ago by jacob

  • Summary changed from XML validation shouldn't require Jing; should use lxml if avaialble. to XML validation shouldn't requre Jing; should use other tools if available.

Changed 6 years ago by David Danier <goliath.mailinglist@…>

comment:9 Changed 6 years ago by David Danier <goliath.mailinglist@…>

  • Summary changed from XML validation shouldn't requre Jing; should use other tools if available. to XML validation shouldn't require Jing; should use other tools if available.

As this ticket seems to get attention again I uploaded a current version of my validator, supporting both newforms and oldforms. As sample of an XmlField for newforms is included, but commented out. Hope this helps getting things forward. ;-)

comment:10 Changed 6 years ago by jacob

  • milestone set to 1.0 maybe
  • Needs documentation set
  • Owner changed from adrian to jacob
  • Patch needs improvement set
  • Status changed from reopened to new
  • Version set to SVN

comment:11 follow-up: Changed 6 years ago by jacob

  • milestone changed from 1.0 maybe to post-1.0

comment:12 in reply to: ↑ 11 Changed 6 years ago by alerades@…

  • Summary changed from XML validation shouldn't require Jing; should use other tools if available. to XMLField not working (and it shouldn't require Jing; should use other tools if available.)

Replying to jacob:
So, as XMLField is not working at all at the moment, wouldn't be better to remove it completely?

comment:13 Changed 6 years ago by David Danier <goliath.mailinglist@…>

Perhaps the best solution would be removing XMLField and add some contrib.xml-module post-1.0. This could include even more things, for example you could think of some tools to handle XSLT of something like that. Of course such an application is not needed to be in django core, but I really would like it there.

comment:14 Changed 5 years ago by jholster

The documentation doesn't mention the current (somewhat broken) status of XMLField, which may confuse users (including me).

http://docs.djangoproject.com/en/dev/ref/models/fields/#xmlfield

comment:15 Changed 5 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

comment:16 Changed 3 years ago by masaki

I was one of the few Japanese Python programmer.
I have Django with Web development is, I'm embarrassed by the Japanese XMLField little information.
I was poor reading English, so would you please translate into Japanese?

私は数少ない日本のPythonプログラマーです。
私はDjangoによるWeb開発を進めているのですが、日本語のXMLFieldの情報が少ないので困ってます。
私は英語が下手なので、日本語に翻訳して頂けますか?

comment:17 Changed 3 years ago by PaulM

  • Has patch unset
  • milestone set to 1.3
  • Owner changed from jacob to PaulM
  • Patch needs improvement unset
  • Summary changed from XMLField not working (and it shouldn't require Jing; should use other tools if available.) to XMLField not being validated.

I'd like to clear this wart up one way or another before 1.3.

Django is unlikely to bundle an external XML validator any time in the near future.

There are a couple of options (in my order of preference):

  1. Re-position it as a lightly validated convenience field named as a reminder of the purpose. Implement the suggestion in #11474 and use xml.parsers.expat to make sure that the XML is at least reasonably well formed. Let users do any further schema validation themselves.
  2. Deprecate it. Fix the docs to say it does nothing, and remove it as expediently as possible.
  3. Treat it as a documentation problem. Fix the docs to say it does nothing special, and leave it alone.

Note: If we start validating entries to this field (which has been unvalidated for at least 2 years), we will technically be introducing a minor backwards incompatibility.

I'm assigning this ticket to myself because I am going to push to get one of these options selected in time for 1.3.

comment:18 Changed 3 years ago by gabrielhurley

  • Component changed from Validators to Database layer (models, ORM)
  • Has patch set
  • Needs documentation unset
  • Patch needs improvement set

It looks to me like the last vestiges of XMLField being anything other than a TextField that takes an extra kwarg dropped off in [8616] when oldforms was removed entirely. I don't know if it truly did any validation even then (since I've never seen the code for oldforms.XMLLargeTextField, but either way it's been over two years (and two versions) since this field did what it was advertised to do.

As such, I'd say the following:

If we can introduce validation that the file is valid XML without introducing any external dependencies, I'm in favor of that. I think the schema validation should be dropped altogether, and can have an "accelerated" deprecation since it hasn't actually worked in longer than our deprecation policy currently covers.

If validation in some form is not possible/practical then I think the field should be deprecated and removed entirely. Having a TextField by another name is rather worthless IMHO.

Either way, I think the docs should drop the bit about schema validation immediately. The reality of trying to design some system of imports and fallbacks to make schema parsing work in a way that is reliable without having a formal dependency on any other library doesn't seem at all worthwhile for a feature which apparently isn't used anyway (since people haven't been beating down the doors trying to get it fixed for the past two years).

comment:19 Changed 3 years ago by PaulM

  • Patch needs improvement unset

Following discussion on the mailing list, I've written a patch which deprecates XMLField in 1.3 for removal in 1.4. It rips the tests out completely (no sense in testing for a deprecation warning) and updates the docs. It passes the test suite on my machine.

Changed 3 years ago by PaulM

comment:20 Changed 3 years ago by PaulM

The docs description for what XMLField does should probably be backported to 1.2 and 1.1, since it doesn't do any of the claimed validation in those versions either.

comment:21 Changed 3 years ago by russellm

  • Patch needs improvement set

@PaulM: Looks good to me, but remember that we have a 2-step deprecation process. The patch should be using PendingDeprecationWarning for 1.3, DeprecationWarning for 1.4, , and will be completely removed in 1.5.

comment:22 Changed 3 years ago by gabrielhurley

Seems like given the fact that the field has been blatantly non-functional for 2+ versions already we might consider an accelerated deprecation here. I don't personally care since I'm not using the field in any of my projects, but I could see where the logic would be for axing it all the faster. ::shrug::

comment:23 Changed 3 years ago by russellm

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

In [15723]:

Fixed #3094 -- Accelerated deprecation of XMLField, since it hasn't served any useful purpose since oldforms. Thanks to PaulM for driving the issue and providing the patch.

Changed 3 years ago by PaulM

Changed 3 years ago by PaulM

comment:24 Changed 3 years ago by PaulM

  • Patch needs improvement unset
  • Resolution fixed deleted
  • Status changed from closed to reopened

I've taken the liberty of reopening this and attaching patches for the 1.2 and 1.1 docs to describe how the field actually functions in those versions.

comment:25 Changed 3 years ago by russellm

Thanks Paul -- in future, please open a new ticket rather than reopening an old one.

comment:26 Changed 3 years ago by russellm

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

In [15764]:

[1.2.X] Fixed #3094 -- Updated docs to reflect actual behavior of XMLField. Thanks to PaulM for the patch.

comment:27 Changed 3 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.