From f5d24d6d977e129b7a13fd9abb83b2378630d29f Mon Sep 17 00:00:00 2001 From: JT Date: Fri, 4 May 2018 17:31:35 +0000 Subject: [PATCH 1/2] updates for sqlalchemy 1.2 --- baph/core/management/commands/killcache.py | 1 - baph/core/management/commands/loaddata.py | 5 +- baph/core/serializers/python.py | 2 +- baph/db/models/base.py | 8 +-- baph/db/models/cloning.py | 11 ++-- baph/db/models/mixins.py | 6 +- baph/db/models/utils.py | 5 ++ baph/forms/forms.py | 67 +++++++++++----------- baph/test/cloning.py | 2 +- 9 files changed, 55 insertions(+), 52 deletions(-) diff --git a/baph/core/management/commands/killcache.py b/baph/core/management/commands/killcache.py index 9d01c03..5597bad 100644 --- a/baph/core/management/commands/killcache.py +++ b/baph/core/management/commands/killcache.py @@ -10,7 +10,6 @@ from sqlalchemy import * from sqlalchemy.exc import ResourceClosedError from sqlalchemy.orm import lazyload, contains_eager, class_mapper -from sqlalchemy.orm.util import identity_key from sqlalchemy.sql import compiler from baph.core.management.base import BaseCommand #NoArgsCommand diff --git a/baph/core/management/commands/loaddata.py b/baph/core/management/commands/loaddata.py index 2ef98f8..d800832 100644 --- a/baph/core/management/commands/loaddata.py +++ b/baph/core/management/commands/loaddata.py @@ -26,11 +26,11 @@ from django.utils.functional import cached_property, memoize from sqlalchemy.orm.attributes import instance_dict from sqlalchemy.orm.session import Session -from sqlalchemy.orm.util import identity_key from baph.core.management.new_base import BaseCommand from baph.db import DEFAULT_DB_ALIAS from baph.db.models import get_app_paths +from baph.db.models.utils import identity_key from baph.db.orm import ORM from baph.utils.glob import glob_escape @@ -228,8 +228,7 @@ def load_label(self, fixture_label): if True: #router.allow_syncdb(self.using, obj.object.__class__): loaded_objects_in_fixture += 1 self.models.add(type(obj)) - ident = identity_key(instance=obj) - (cls, key) = ident[:2] + (cls, key) = identity_key(instance=obj) if any(part is None for part in key): # we can't generate an explicit key with this info session.add(obj) diff --git a/baph/core/serializers/python.py b/baph/core/serializers/python.py index f496b91..58b98aa 100644 --- a/baph/core/serializers/python.py +++ b/baph/core/serializers/python.py @@ -1,9 +1,9 @@ from django.utils.encoding import smart_text, is_protected_type -from sqlalchemy.orm.util import identity_key from baph.core.serializers import base from baph.db import DEFAULT_DB_ALIAS from baph.db.models import get_apps +from baph.db.models.utils import identity_key from baph.db.orm import Base diff --git a/baph/db/models/base.py b/baph/db/models/base.py index 6a8cd56..dd9eef4 100644 --- a/baph/db/models/base.py +++ b/baph/db/models/base.py @@ -16,7 +16,7 @@ from sqlalchemy.orm.interfaces import MANYTOONE from sqlalchemy.orm.properties import ColumnProperty, RelationshipProperty from sqlalchemy.orm.session import Session -from sqlalchemy.orm.util import has_identity, identity_key +from sqlalchemy.orm.util import has_identity from sqlalchemy.schema import ForeignKeyConstraint from sqlalchemy.util import classproperty @@ -25,7 +25,7 @@ from baph.db.models.loading import get_model, register_models from baph.db.models.mixins import CacheMixin, ModelPermissionMixin, GlobalMixin from baph.db.models.options import Options -from baph.db.models.utils import class_resolver, key_to_value +from baph.db.models.utils import class_resolver, key_to_value, identity_key from baph.utils.functional import cachedclassproperty from baph.utils.importing import safe_import, remove_class @@ -147,9 +147,7 @@ def before_flush_attrs(cls): def pk_as_query_filters(self, force=False): " returns a filter expression for the primary key of the instance " " suitable for use with Query.filter() " - ident = identity_key(instance=self) - (cls, pk_values) = ident[:2] - + (cls, pk_values) = identity_key(instance=self) if None in pk_values and not force: return None items = zip(self.pk_attrs, pk_values) diff --git a/baph/db/models/cloning.py b/baph/db/models/cloning.py index 3088fdc..04cedb2 100644 --- a/baph/db/models/cloning.py +++ b/baph/db/models/cloning.py @@ -6,8 +6,8 @@ from sqlalchemy.orm.collections import MappedCollection from sqlalchemy.orm.properties import ColumnProperty, RelationshipProperty from sqlalchemy.orm.session import object_session -from sqlalchemy.orm.util import identity_key +from baph.db.models.utils import identity_key from baph.db.orm import ORM from baph.utils.collections import duck_type_collection @@ -19,7 +19,7 @@ def reload_object(instance): """ Reloads an instance with the correct polymorphic subclass """ - cls, pk_vals = identity_key(instance=instance) + (cls, pk_vals) = identity_key(instance=instance) mapper = inspect(cls) pk_cols = [col.key for col in mapper.primary_key] pk = dict(zip(pk_cols, pk_vals)) @@ -38,7 +38,7 @@ def get_polymorphic_subclass(instance): the polymorphic map of the base class. for non-polymorphic classes, it returns the class """ - cls, pk = identity_key(instance=instance) + (cls, pk) = identity_key(instance=instance) base_mapper = inspect(cls) if base_mapper.polymorphic_on is None: # this is not a polymorphic class @@ -148,7 +148,7 @@ def get_rules(rules, rule_keys): def user_id(self): if not self.user: return None - cls, pk = identity_key(instance=self.user) + (cls, pk) = identity_key(instance=self.user) if len(pk) > 1: raise Exception('chown cannot used for multi-column user pks. To ' 'specify ownership for a user with a multi-column pk, add the ' @@ -218,8 +218,7 @@ def clone_collection(self, value, **kwargs): def clone_obj(self, instance, ruleset=None, rule_keys=None, cast_to=None): is_root = self.root is None - - base_cls, pk = identity_key(instance=instance) + (base_cls, pk) = identity_key(instance=instance) if (base_cls, pk) in self.registry: # we already cloned this return self.registry[(base_cls, pk)] diff --git a/baph/db/models/mixins.py b/baph/db/models/mixins.py index 3224d24..fb980c0 100644 --- a/baph/db/models/mixins.py +++ b/baph/db/models/mixins.py @@ -15,9 +15,10 @@ from sqlalchemy.orm import class_mapper, object_session from sqlalchemy.orm.attributes import get_history, instance_dict from sqlalchemy.orm.properties import ColumnProperty, RelationshipProperty -from sqlalchemy.orm.util import has_identity, identity_key +from sqlalchemy.orm.util import has_identity from baph.db import ORM +from baph.db.models.utils import identity_key from .utils import column_to_attr, class_resolver @@ -611,8 +612,7 @@ def get_cache_keys(self, child_updated=False, force_expire_pointers=False, # the fields which trigger this pointer were not changed continue cache_key = raw_key % data - ident_key = identity_key(instance=self) - _, ident = ident_key[:2] + (_, ident) = identity_key(instance=self) if len(ident) > 1: ident = ','.join(map(str, ident)) else: diff --git a/baph/db/models/utils.py b/baph/db/models/utils.py index 28697b4..31a8153 100644 --- a/baph/db/models/utils.py +++ b/baph/db/models/utils.py @@ -2,8 +2,13 @@ from sqlalchemy import inspect from sqlalchemy.ext.declarative.clsregistry import _class_resolver +from sqlalchemy.orm.util import identity_key as new_identity_key +def identity_key(*args, **kwargs): + """ returns a 2-tuple identity key, not the 3-tuple in newer SQLA """ + return new_identity_key(*args, **kwargs)[:2] + def has_inherited_table(cls): # TODO: a fix in sqla 0.9 should make this unnecessary, check it """ diff --git a/baph/forms/forms.py b/baph/forms/forms.py index cc9fcdd..cc7cd40 100644 --- a/baph/forms/forms.py +++ b/baph/forms/forms.py @@ -69,17 +69,17 @@ def model_to_dict(instance, fields=None, exclude=None): data = {} unloaded_attrs = inspect(instance).unloaded for f in opts.fields: - if f.name in unloaded_attrs: - if issubclass(f.data_type, orm.Base): - # skip relations that haven't been loaded yet - continue - if not getattr(f, 'editable', False): - continue - if fields and not f.name in fields: - continue - if exclude and f.name in exclude: - continue - data[f.name] = getattr(instance, f.name) + if f.name in unloaded_attrs: + if issubclass(f.data_type, orm.Base): + # skip relations that haven't been loaded yet + continue + if not getattr(f, 'editable', False): + continue + if fields and not f.name in fields: + continue + if exclude and f.name in exclude: + continue + data[f.name] = getattr(instance, f.name) return data def fields_for_model(model, fields=None, exclude=None, widgets=None, @@ -101,13 +101,13 @@ def fields_for_model(model, fields=None, exclude=None, widgets=None, continue if issubclass(f.data_type, Base): - # TODO: Auto-generate fields, control via 'fields' param - if fields is not None and f.name in fields: - # manually included field - pass - else: - # skip relations unless manually requested - continue + # TODO: Auto-generate fields, control via 'fields' param + if fields is not None and f.name in fields: + # manually included field + pass + else: + # skip relations unless manually requested + continue kwargs = {} if widgets and f.name in widgets: @@ -192,6 +192,7 @@ def __new__(cls, name, bases, attrs): except NameError: parents = None declared_fields = forms.forms.get_declared_fields(bases, attrs, False) + new_class = super(SQLAModelFormMetaclass, cls) \ .__new__(cls, name, bases, attrs) if not parents: @@ -199,6 +200,7 @@ def __new__(cls, name, bases, attrs): if 'media' not in attrs: new_class.media = forms.widgets.media_property(new_class) + opts = new_class._meta = SQLAModelFormOptions(getattr(new_class, 'Meta', None)) if opts.model: # If a model is defined, extract form fields from it. @@ -219,30 +221,31 @@ def __init__(self, data=None, files=None, auto_id='id_%s', prefix=None, opts = self._meta exclude = list(opts.exclude) if opts.model is None: - raise ValueError('ModelForm has no model class specified.') + raise ValueError('ModelForm has no model class specified.') self.nested = nested if self.nested: - exclude.extend(opts.exclude_on_nested) + exclude.extend(opts.exclude_on_nested) if instance is None: - exclude.extend(opts.exclude_on_create) - self.instance = opts.model() - object_data = {} + exclude.extend(opts.exclude_on_create) + self.instance = opts.model() + object_data = {} else: - self.instance = instance - object_data = model_to_dict(instance, opts.fields, exclude) - if has_identity(instance): - exclude.extend(opts.exclude_on_update) - else: - exclude.extend(opts.exclude_on_create) + self.instance = instance + object_data = model_to_dict(instance, opts.fields, exclude) + if has_identity(instance): + exclude.extend(opts.exclude_on_update) + else: + exclude.extend(opts.exclude_on_create) if initial is not None: - object_data.update(initial) + object_data.update(initial) object_data.update(data) + super(BaseSQLAModelForm, self).__init__(object_data, files, auto_id, prefix) for k in exclude: - if k in self.fields: - del self.fields[k] + if k in self.fields: + del self.fields[k] def save(self, commit=False): """ diff --git a/baph/test/cloning.py b/baph/test/cloning.py index 743e3f4..b9fdd63 100644 --- a/baph/test/cloning.py +++ b/baph/test/cloning.py @@ -4,9 +4,9 @@ from sqlalchemy import inspect from sqlalchemy.ext.associationproxy import ASSOCIATION_PROXY from sqlalchemy.ext.orderinglist import OrderingList -from sqlalchemy.orm.util import identity_key from baph.db.models.cloning import * +from baph.db.models.utils import identity_key from baph.utils.collections import duck_type_collection From 2cfa2bd1a9c8abdceb6e640ca255b88bb1a7279b Mon Sep 17 00:00:00 2001 From: JT Date: Mon, 7 May 2018 18:57:14 +0000 Subject: [PATCH 2/2] exclude query_expressions by default when cloning instances --- baph/db/models/cloning.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/baph/db/models/cloning.py b/baph/db/models/cloning.py index 04cedb2..e332ebf 100644 --- a/baph/db/models/cloning.py +++ b/baph/db/models/cloning.py @@ -1,6 +1,6 @@ import copy -from sqlalchemy import inspect +from sqlalchemy import inspect, Column from sqlalchemy.orm import class_mapper from sqlalchemy.orm.attributes import instance_dict from sqlalchemy.orm.collections import MappedCollection @@ -65,11 +65,18 @@ def get_cloning_rules(cls): def get_default_excludes(cls): """ - By default, exclude pks and fks + By default, exclude pks, fks, and query_expressions """ mapper = inspect(cls) + exclude_props = set() exclude_cols = set() + for attr in mapper.column_attrs: + # exclude column properties which do not refer to actual columns + # ie query_expressions + if not any(isinstance(col, Column) for col in attr.columns): + exclude_props.add(attr.key) + if mapper.polymorphic_on is not None: # do not copy the polymorphic discriminator. it will be set automatically # via the polymorphic class, and we don't want to generate flush warnings @@ -86,7 +93,7 @@ def get_default_excludes(cls): props = map(mapper.get_property_by_column, exclude_cols) keys = set(prop.key for prop in props) - return keys + return exclude_props | keys def get_default_columns(cls): """