django-nonrel/djangotoolbox — pull request #16 by wrwrwr (2012-02-07 14:28:05+00:00)
Title: Type conversions refactorization

This is something that started as an effort to understand how things work, add some comments to make the process faster for others to come, and maybe clean some things up. However, some additional goals have appeared on the way.

#### Flaws in type conversions
The first was to fill missing parts of type conversions:

* converting / deconverting embedded instance fields using the right db_subtype (rather than as RawFields);
* converting filter arguments for collection field lookups (as a collection item);
* deconverting new keys in the same way as any other values from the database.

This should fix a few issues, the ones I know about are the Mongo part of #10 and django-nonrel/mongodb-engine#47.

Change to converting / deconverting of embedded instance fields can be disabled by using LegacyEmbeddedModelField (now in toolbox) in case of incompatibility with existing code (hopefully this will be rare, see the comment on [NonrelDatabaseOperations.value_for_db_model](https://github.com/django-nonrel/djangotoolbox/blob/feature/type-conversions-refactor/djangotoolbox/db/base.py) for some insight).

#### Data types abstraction

The second goal was to build data type framework more suitable for a "general" NoSQL database rather than just the GAE datastore:

* ```db_type``` names suitable for a database driver that can process Python types directly;
* all primary keys and relations use a ```"key"``` ```db_type```, unless the back-end decides otherwise;
* introduced a ```nonrel_db_type``` method that makes it possible to override Django's ```db_type``` logic even for custom fields;
* toolbox can handle the "Django side" of type conversions -- converts Django lazy objects and wrappers to standard types before passing them to the database;
* a bit lesser reliance on ```sql.Query``` -- values passed to back-ends compilers are now always accompanied by a proper field (my feeling is that the "column" logic is not very natural for NoSQL databases);

#### Nonrel fields

The third goal was to make things easier for back-end developers, by taking the burden of handling toolbox fields away (there are many subtleties which can easily lead to mistakes). Now all the code is within the toolbox and all a back-end develop has to do is choose a suitable collection or embedded field ```db_type``` for his/her database and call a parent conversion (deconversion) method at the beginning (end) of the back-end methods. There is likely enough to choose from to satisfy most NoSQL databases (see comments in [creation.py](https://github.com/django-nonrel/djangotoolbox/blob/feature/type-conversions-refactor/djangotoolbox/db/creation.py) to learn the available options).

#### GAE: relations as db.Keys and binary serialization

The fourth was to prepare grounds for storing ```ForeignKeys``` and other relations using the special ```db.Key``` type of the GAE datastore. A database option is implemented to switch between the two storage methods, but only a "dump data; switch; reload data" procedure is possible. The default is to use the legacy storage for backwards compatibility, but it could be changed with some future major revision. See django-nonrel/djangoappengine#18 for more info.

GAE engine now also serializes ```DictFields``` and ```EmbeddedModelFields``` using the binary pickle protocol (instead of the ASCII protocol 0); this should work transparently as deconversion is the same for both protocols and no lookups / filtering is specified for these fields (as far as I can tell).

#### Additional tests and fixes

* Fix for the ```EmbeddedModelField.update``` issue (#14) is already applied;
* Added some tests for the natural usage of ```ListField``` with ```ForeignKeys``` that we've discussed on the mailing list (marked as an expected failure for now);
* Trivial fix for the test for issue #12 (failing on develop). Nevertheless, I believe there is more to collection fields serialization and as it is written now it may not work with any kind of nesting [pending verification].
* Also added some tests checking interaction between Django safe/to-be-escaped strings and lazy objects and the database layer .

#### Django changes to be considered

* We could drop all the ```related_db_type``` infrastructure -- it's not sufficient for GAE relations-as-Keys and rather hard to follow.
* Replacing various ```value_to_db_*``` and ```convert_values``` methods on DatabaseOperations with something like the new ```value_for/from_db(value, field [, lookup])``` would allow to avoid double recursion for collection fields, simplify back-end code (```get_db_prep_*``` would call these methods rather than compilers), and make it possible to avoid various hacks in toolbox code (e.g. by moving what is now in ```Field.get_db_prep_lookup``` -- and is more or less reverted in ```NonrelQuery._normalize_lookup_value``` -- to an abstract SQLDatabaseOperations). See comments on the [NonrelDatabaseOperations class](https://github.com/django-nonrel/djangotoolbox/blob/feature/type-conversions-refactor/djangotoolbox/db/base.py) if you'd like to know what I find unnecessarily hard to do with the current framework.

#### Reviewing, testing & merging

I believe the most convenient way to review these changes is by reading creation.py and base.py of toolbox and the engine that you use. If you'd prefer to go through some commits the best bet is to go through the Mongo ones -- only these have been squashed; otherwise you'll find a story of taking bad paths, learning new aspects of the system and redesigning things a couple of times. Any kind of feedback is welcome (including linguistic corrections, comments improvements, code style suggestions, pointing out errors, better design ideas, etc.).

These changes could use some testing. Particularly with preexisting data -- the intention was to keep everything compatible (possibly with the help of the "new" ```LegacyEmbeddedModelField```), and a considerable effort was put into understanding what each piece in current conversions does and not lose it, but considering the amount and nature of changes I've probably broken some things. So if you have a project with a testing environment and some existing data, trying these branches out would be really nice -- just backup, update and see if everything works. If it does and it's a GAE project, dump the data, add ```STORE_RELATIONS_AS_DB_KEYS: True``` to your database options, reload and check if it still does.

I've run toolbox / appengine / dbindexer / django.contrib (except flatpages and GeoDjango) test cases for GAE under Python 2.5 / 2.7 and the Mongo test suite under 2.6 / 2.7 some horrible amount of times. However, if you have the power to run the full Django test suite (django/tests/runtests.py) in reasonable time -- run it -- I've given up for now (or otherwise would have to postpone pushing this for another week or so ;-) Some tests are expected to fail, but no new failures should be introduced compared to develop branches.

There are branches also called ```feature/type-conversions-refactor``` in the GAE and Mongo engine repositories. Each of them depends on this one and this one depends on them -- they need to be merged together.

WARNING: Although this fixes some things it also changes how things are stored, in some cases in a subtle, dynamic way (e.g. ```DictFields``` on GAE). It really should not be used in production until going through some more extensive, real-data testing. Nevertheless, in case somebody feels that lucky (especially considering that this is not likely to be merged any time soon :-) dumping your data before and reloading it after the update gives a better chance to avoid some yet unforeseen problems and I'd also highly recommend looking into your database (e.g. at _ah/admin) and checking that things are stored as they should be shortly after that.