From: Mike Bayer Date: Sun, 10 Jan 2016 22:47:38 +0000 (-0500) Subject: - refactor a bit the loader options system to make it a bit more X-Git-Tag: rel_1_1_0b1~84^2~47 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b301f009e18246db9277a4b9d7e3a1bf01a92ae9;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - refactor a bit the loader options system to make it a bit more intelligible, given the fixes for ref #3623. unfortunately the system is still quite weird even though it was rewritten to be... less weird --- diff --git a/lib/sqlalchemy/orm/strategy_options.py b/lib/sqlalchemy/orm/strategy_options.py index f083679416..aa818258a5 100644 --- a/lib/sqlalchemy/orm/strategy_options.py +++ b/lib/sqlalchemy/orm/strategy_options.py @@ -80,6 +80,8 @@ class Load(Generative, MapperOption): def __init__(self, entity): insp = inspect(entity) self.path = insp._path_registry + # note that this .context is shared among all descendant + # Load objects self.context = {} self.local_opts = {} @@ -88,7 +90,7 @@ class Load(Generative, MapperOption): cloned.local_opts = {} return cloned - _merge_into_path = False + is_opts_only = False strategy = None propagate_to_loaders = False @@ -201,7 +203,7 @@ class Load(Generative, MapperOption): self._set_path_strategy() @_generative - def set_column_strategy(self, attrs, strategy, opts=None): + def set_column_strategy(self, attrs, strategy, opts=None, opts_only=False): strategy = self._coerce_strat(strategy) for attr in attrs: @@ -212,21 +214,34 @@ class Load(Generative, MapperOption): cloned.propagate_to_loaders = True if opts: cloned.local_opts.update(opts) + if opts_only: + cloned.is_opts_only = True cloned._set_path_strategy() - def _set_path_strategy(self): - if self._merge_into_path: - # special helper for undefer_group - existing = self.path.get(self.context, "loader") + def _set_for_path(self, context, path, replace=True, merge_opts=False): + if merge_opts or not replace: + existing = path.get(self.context, "loader") + if existing: - existing.local_opts.update(self.local_opts) + if merge_opts: + existing.local_opts.update(self.local_opts) else: - self.path.set(self.context, "loader", self) + path.set(context, "loader", self) + else: + existing = path.get(self.context, "loader") + path.set(context, "loader", self) + if existing and existing.is_opts_only: + self.local_opts.update(existing.local_opts) - elif self.path.has_entity: - self.path.parent.set(self.context, "loader", self) + def _set_path_strategy(self): + if self.path.has_entity: + effective_path = self.path.parent else: - self.path.set(self.context, "loader", self) + effective_path = self.path + + self._set_for_path( + self.context, effective_path, replace=True, + merge_opts=self.is_opts_only) def __getstate__(self): d = self.__dict__.copy() @@ -314,7 +329,7 @@ class _UnboundLoad(Load): val._bind_loader(query, query._attributes, raiseerr) @classmethod - def _from_keys(self, meth, keys, chained, kw): + def _from_keys(cls, meth, keys, chained, kw): opt = _UnboundLoad() def _split_key(key): @@ -399,6 +414,7 @@ class _UnboundLoad(Load): loader = Load(path_element) loader.context = context loader.strategy = self.strategy + loader.is_opts_only = self.is_opts_only path = loader.path for token in start_path: @@ -420,24 +436,15 @@ class _UnboundLoad(Load): if effective_path.is_token: for path in effective_path.generate_for_superclasses(): - if self._merge_into_path: - # special helper for undefer_group - existing = path.get(context, "loader") - if existing: - existing.local_opts.update(self.local_opts) - else: - path.set(context, "loader", loader) - elif self._is_chain_link: - path.setdefault(context, "loader", loader) - else: - path.set(context, "loader", loader) + loader._set_for_path( + context, path, + replace=not self._is_chain_link, + merge_opts=self.is_opts_only) else: - # only supported for the undefer_group() wildcard opt - assert not self._merge_into_path - if self._is_chain_link: - effective_path.setdefault(context, "loader", loader) - else: - effective_path.set(context, "loader", loader) + loader._set_for_path( + context, effective_path, + replace=not self._is_chain_link, + merge_opts=self.is_opts_only) def _find_entity_prop_comparator(self, query, token, mapper, raiseerr): if _is_aliased_class(mapper): @@ -1043,11 +1050,11 @@ def undefer_group(loadopt, name): :func:`.orm.undefer` """ - loadopt._merge_into_path = True return loadopt.set_column_strategy( "*", None, - {"undefer_group_%s" % name: True} + {"undefer_group_%s" % name: True}, + opts_only=True ) diff --git a/test/orm/test_options.py b/test/orm/test_options.py index e1e26c62ce..e7b750cf4b 100644 --- a/test/orm/test_options.py +++ b/test/orm/test_options.py @@ -3,11 +3,14 @@ from sqlalchemy.orm import attributes, mapper, relationship, backref, \ configure_mappers, create_session, synonym, Session, class_mapper, \ aliased, column_property, joinedload_all, joinedload, Query,\ util as orm_util, Load, defer +from sqlalchemy.orm.query import QueryContext +from sqlalchemy.orm import strategy_options import sqlalchemy as sa from sqlalchemy import testing -from sqlalchemy.testing.assertions import eq_, assert_raises, assert_raises_message +from sqlalchemy.testing.assertions import eq_, assert_raises_message from test.orm import _fixtures + class QueryTest(_fixtures.FixtureTest): run_setup_mappers = 'once' run_inserts = 'once' @@ -17,6 +20,7 @@ class QueryTest(_fixtures.FixtureTest): def setup_mappers(cls): cls._setup_stock_mapping() + class PathTest(object): def _make_path(self, path): r = [] @@ -160,11 +164,11 @@ class LoadTest(PathTest, QueryTest): ) + + class OptionsTest(PathTest, QueryTest): def _option_fixture(self, *arg): - from sqlalchemy.orm import strategy_options - return strategy_options._UnboundLoad._from_keys( strategy_options._UnboundLoad.joinedload, arg, True, {}) @@ -768,3 +772,121 @@ class OptionsNoPropTest(_fixtures.FixtureTest): create_session().query(column).options, joinedload(eager_option)) + +class LocalOptsTest(PathTest, QueryTest): + @classmethod + def setup_class(cls): + super(LocalOptsTest, cls).setup_class() + + @strategy_options.loader_option() + def some_col_opt_only(loadopt, key, opts): + return loadopt.set_column_strategy( + (key, ), + None, + opts, + opts_only=True + ) + + @strategy_options.loader_option() + def some_col_opt_strategy(loadopt, key, opts): + return loadopt.set_column_strategy( + (key, ), + {"deferred": True, "instrument": True}, + opts + ) + + cls.some_col_opt_only = some_col_opt_only + cls.some_col_opt_strategy = some_col_opt_strategy + + def _assert_attrs(self, opts, expected): + User = self.classes.User + + query = create_session().query(User) + attr = {} + + for opt in opts: + if isinstance(opt, strategy_options._UnboundLoad): + for tb in opt._to_bind: + tb._bind_loader(query, attr, False) + else: + attr.update(opt.context) + + key = ( + 'loader', + tuple(inspect(User)._path_registry[User.name.property])) + eq_( + attr[key].local_opts, + expected + ) + + def test_single_opt_only(self): + opt = strategy_options._UnboundLoad().some_col_opt_only( + "name", {"foo": "bar"} + ) + self._assert_attrs([opt], {"foo": "bar"}) + + def test_unbound_multiple_opt_only(self): + opts = [ + strategy_options._UnboundLoad().some_col_opt_only( + "name", {"foo": "bar"} + ), + strategy_options._UnboundLoad().some_col_opt_only( + "name", {"bat": "hoho"} + ) + ] + self._assert_attrs(opts, {"foo": "bar", "bat": "hoho"}) + + def test_bound_multiple_opt_only(self): + User = self.classes.User + opts = [ + Load(User).some_col_opt_only( + "name", {"foo": "bar"} + ).some_col_opt_only( + "name", {"bat": "hoho"} + ) + ] + self._assert_attrs(opts, {"foo": "bar", "bat": "hoho"}) + + def test_bound_strat_opt_recvs_from_optonly(self): + User = self.classes.User + opts = [ + Load(User).some_col_opt_only( + "name", {"foo": "bar"} + ).some_col_opt_strategy( + "name", {"bat": "hoho"} + ) + ] + self._assert_attrs(opts, {"foo": "bar", "bat": "hoho"}) + + def test_unbound_strat_opt_recvs_from_optonly(self): + opts = [ + strategy_options._UnboundLoad().some_col_opt_only( + "name", {"foo": "bar"} + ), + strategy_options._UnboundLoad().some_col_opt_strategy( + "name", {"bat": "hoho"} + ) + ] + self._assert_attrs(opts, {"foo": "bar", "bat": "hoho"}) + + def test_unbound_opt_only_adds_to_strat(self): + opts = [ + strategy_options._UnboundLoad().some_col_opt_strategy( + "name", {"bat": "hoho"} + ), + strategy_options._UnboundLoad().some_col_opt_only( + "name", {"foo": "bar"} + ), + ] + self._assert_attrs(opts, {"foo": "bar", "bat": "hoho"}) + + def test_bound_opt_only_adds_to_strat(self): + User = self.classes.User + opts = [ + Load(User).some_col_opt_strategy( + "name", {"bat": "hoho"} + ).some_col_opt_only( + "name", {"foo": "bar"} + ), + ] + self._assert_attrs(opts, {"foo": "bar", "bat": "hoho"})