From 7e2e551c40e8a6c54623386c4ba7d314006e093d Mon Sep 17 00:00:00 2001 From: Kieren Eaton <499977+circulon@users.noreply.github.com> Date: Thu, 17 Oct 2024 19:16:38 +0800 Subject: [PATCH 01/11] Fixed HasOneThrough not loading distant model Fixed eager and on demand loading of distant table use vars for tablenames to increase readability. --- src/masoniteorm/query/QueryBuilder.py | 2 + .../relationships/HasOneThrough.py | 187 ++++++++++++------ 2 files changed, 127 insertions(+), 62 deletions(-) diff --git a/src/masoniteorm/query/QueryBuilder.py b/src/masoniteorm/query/QueryBuilder.py index 4e9a663b..963017a3 100644 --- a/src/masoniteorm/query/QueryBuilder.py +++ b/src/masoniteorm/query/QueryBuilder.py @@ -1965,6 +1965,8 @@ def _register_relationships_to_model( def _map_related(self, related_result, related): if related.__class__.__name__ == 'MorphTo': return related_result + elif related.__class__.__name__ == 'HasOneThrough': + return related_result.group_by(related.local_key) return related_result.group_by(related.foreign_key) diff --git a/src/masoniteorm/relationships/HasOneThrough.py b/src/masoniteorm/relationships/HasOneThrough.py index ae7c587b..f6ccf2c5 100644 --- a/src/masoniteorm/relationships/HasOneThrough.py +++ b/src/masoniteorm/relationships/HasOneThrough.py @@ -1,5 +1,4 @@ from .BaseRelationship import BaseRelationship -from ..collection import Collection class HasOneThrough(BaseRelationship): @@ -26,6 +25,10 @@ def __init__( self.local_owner_key = local_owner_key or "id" self.other_owner_key = other_owner_key or "id" + def __getattr__(self, attribute): + relationship = self.fn(self)[1]() + return getattr(relationship.builder, attribute) + def set_keys(self, distant_builder, intermediary_builder, attribute): self.local_key = self.local_key or "id" self.foreign_key = self.foreign_key or f"{attribute}_id" @@ -34,17 +37,18 @@ def set_keys(self, distant_builder, intermediary_builder, attribute): return self def __get__(self, instance, owner): - """This method is called when the decorated method is accessed. + """ + This method is called when the decorated method is accessed. - Arguments: - instance {object|None} -- The instance we called. + Arguments + instance (object|None): The instance we called. If we didn't call the attribute and only accessed it then this will be None. + owner (object): The current model that the property was accessed on. - owner {object} -- The current model that the property was accessed on. - - Returns: - object -- Either returns a builder or a hydrated model. + Returns + QueryBuilder|Model: Either returns a builder or a hydrated model. """ + attribute = self.fn.__name__ self.attribute = attribute relationship1 = self.fn(self)[0]() @@ -57,43 +61,60 @@ def __get__(self, instance, owner): if attribute in instance._relationships: return instance._relationships[attribute] - result = self.apply_query( + return self.apply_query( self.distant_builder, self.intermediary_builder, instance ) - return result else: return self def apply_query(self, distant_builder, intermediary_builder, owner): - """Apply the query and return a dictionary to be hydrated. - Used during accessing a relationship on a model + """ + Apply the query and return a dict of data for the distant model to be hydrated with. - Arguments: - query {oject} -- The relationship object - owner {object} -- The current model oject. + Method is used when accessing a relationship on a model if its not + already eager loaded - Returns: - dict -- A dictionary of data which will be hydrated. + Arguments + distant_builder (QueryBuilder): QueryVuilder attached to the distant table + intermediate_builder (QueryBuilder): QueryVuilder attached to the intermesiate (linking) table + owner (Any): the model this relationship is starting from + + Returns + dict: A dictionary of data which will be hydrated. """ - # select * from `countries` inner join `ports` on `ports`.`country_id` = `countries`.`country_id` where `ports`.`port_id` is null and `countries`.`deleted_at` is null and `ports`.`deleted_at` is null - distant_builder.join( - f"{self.intermediary_builder.get_table_name()}", - f"{self.intermediary_builder.get_table_name()}.{self.foreign_key}", - "=", - f"{distant_builder.get_table_name()}.{self.other_owner_key}", - ) - return self + dist_table = distant_builder.get_table_name() + int_table = intermediary_builder.get_table_name() + + return ( + distant_builder.select( + f"{dist_table}.*, {int_table}.{self.local_owner_key} as {self.local_key}" + ) + .join( + f"{int_table}", + f"{int_table}.{self.foreign_key}", + "=", + f"{dist_table}.{self.other_owner_key}", + ) + .where( + f"{int_table}.{self.local_owner_key}", + getattr(owner, self.local_key), + ) + .first() + ) def relate(self, related_model): + dist_table = self.distant_builder.get_table_name() + int_table = self.intermediary_builder.get_table_name() + return self.distant_builder.join( - f"{self.intermediary_builder.get_table_name()}", - f"{self.intermediary_builder.get_table_name()}.{self.foreign_key}", + f"{int_table}", + f"{int_table}.{self.foreign_key}", "=", - f"{self.distant_builder.get_table_name()}.{self.other_owner_key}", - ).where( - f"{self.intermediary_builder.get_table_name()}.{self.local_key}", - getattr(related_model, self.local_owner_key), + f"{dist_table}.{self.other_owner_key}", + ).where_column( + f"{int_table}.{self.local_owner_key}", + f"{query.get_table_name()}.{self.local_key}", ) def get_builder(self): @@ -104,42 +125,83 @@ def make_builder(self, eagers=None): return builder + def register_related(self, key, model, collection): + """ + Attach the related model to source models attribute + + Arguments + key (str): The attribute name + model (Any): The model instance + collection (Collection): The data for the related models + + Returns + None + """ + + related = collection.get(getattr(model, self.local_key), None) + model.add_relation({key: related[0] if related else None}) + def get_related(self, query, relation, eagers=None, callback=None): - builder = self.distant_builder + """ + Get the data to htdrate the model for the distant table with + Used when eager loading the model attribute + + Arguments + query (QueryBuilder): The source models QueryBuilder object + relation (HasOneThrough): this relationship object + eagers (Any): + callback (Any): + + Returns + dict: the dict to hydrate the distant model with + """ + + dist_table = self.distant_builder.get_table_name() + int_table = self.intermediary_builder.get_table_name() if callback: callback(builder) - if isinstance(relation, Collection): - return builder.where_in( - f"{builder.get_table_name()}.{self.foreign_key}", - Collection(relation._get_value(self.local_key)).unique(), - ).get() - else: - return builder.where( - f"{builder.get_table_name()}.{self.foreign_key}", - getattr(relation, self.local_owner_key), - ).first() + return ( + self.distant_builder.select( + f"{dist_table}.*, {int_table}.{self.local_owner_key} as {self.local_key}" + ) + .join( + f"{int_table}", + f"{int_table}.{self.foreign_key}", + "=", + f"{dist_table}.{self.other_owner_key}", + ) + .where( + f"{int_table}.{self.local_owner_key}", + relation._get_value(self.local_key), + ) + .get() + ) def query_where_exists( self, current_query_builder, callback, method="where_exists" ): query = self.distant_builder + dist_table = self.distant_builder.get_table_name() + int_table = self.intermediary_builder.get_table_name() getattr(current_query_builder, method)( query.join( - f"{self.intermediary_builder.get_table_name()}", - f"{self.intermediary_builder.get_table_name()}.{self.foreign_key}", + f"{int_table}", + f"{int_table}.{self.foreign_key}", "=", - f"{query.get_table_name()}.{self.other_owner_key}", + f"{dist_table}.{self.other_owner_key}", ).where_column( - f"{current_query_builder.get_table_name()}.{self.local_owner_key}", - f"{self.intermediary_builder.get_table_name()}.{self.local_key}", + f"{int_table}.{self.local_owner_key}", + f"{query.get_table_name()}.{self.local_key}", ) ).when(callback, lambda q: (callback(q))) def get_with_count_query(self, builder, callback): query = self.distant_builder + dist_table = self.distant_builder.get_table_name() + int_table = self.intermediary_builder.get_table_name() if not builder._columns: builder = builder.select("*") @@ -150,16 +212,16 @@ def get_with_count_query(self, builder, callback): ( q.count("*") .join( - f"{self.intermediary_builder.get_table_name()}", - f"{self.intermediary_builder.get_table_name()}.{self.foreign_key}", + f"{int_table}", + f"{int_table}.{self.foreign_key}", "=", - f"{query.get_table_name()}.{self.other_owner_key}", + f"{dist_table}.{self.other_owner_key}", ) .where_column( - f"{builder.get_table_name()}.{self.local_owner_key}", - f"{self.intermediary_builder.get_table_name()}.{self.local_key}", + f"{int_table}.{self.local_owner_key}", + f"{query.get_table_name()}.{self.local_key}", ) - .table(query.get_table_name()) + .table(dist_table) .when( callback, lambda q: ( @@ -186,18 +248,19 @@ def attach_related(self, current_model, related_record): ) def query_has(self, current_query_builder, method="where_exists"): - related_builder = self.get_builder() + dist_table = self.distant_builder.get_table_name() + int_table = self.intermediary_builder.get_table_name() getattr(current_query_builder, method)( - self.distant_builder.where_column( - f"{current_query_builder.get_table_name()}.{self.local_owner_key}", - f"{self.intermediary_builder.get_table_name()}.{self.local_key}", - ).join( - f"{self.intermediary_builder.get_table_name()}", - f"{self.intermediary_builder.get_table_name()}.{self.foreign_key}", + self.distant_builder.join( + f"{int_table}", + f"{int_table}.{self.foreign_key}", "=", - f"{self.distant_builder.get_table_name()}.{self.other_owner_key}", + f"{dist_table}.{self.other_owner_key}", + ).where_column( + f"{current_query_builder.get_table_name()}.{self.local_key}", + f"{int_table}.{self.local_owner_key}", ) ) - return related_builder + return self.distant_builder From 1ac4d2938f332f74c3d0aaf89d47218d03a9202e Mon Sep 17 00:00:00 2001 From: Kieren Eaton <499977+circulon@users.noreply.github.com> Date: Fri, 18 Oct 2024 13:49:25 +0800 Subject: [PATCH 02/11] Fixed HasOneThrough with additional queries --- .../relationships/HasOneThrough.py | 94 +++++++++---------- 1 file changed, 47 insertions(+), 47 deletions(-) diff --git a/src/masoniteorm/relationships/HasOneThrough.py b/src/masoniteorm/relationships/HasOneThrough.py index f6ccf2c5..1a87debb 100644 --- a/src/masoniteorm/relationships/HasOneThrough.py +++ b/src/masoniteorm/relationships/HasOneThrough.py @@ -61,13 +61,13 @@ def __get__(self, instance, owner): if attribute in instance._relationships: return instance._relationships[attribute] - return self.apply_query( + return self.apply_relation_query( self.distant_builder, self.intermediary_builder, instance ) else: return self - def apply_query(self, distant_builder, intermediary_builder, owner): + def apply_relation_query(self, distant_builder, intermediary_builder, owner): """ Apply the query and return a dict of data for the distant model to be hydrated with. @@ -114,7 +114,7 @@ def relate(self, related_model): f"{dist_table}.{self.other_owner_key}", ).where_column( f"{int_table}.{self.local_owner_key}", - f"{query.get_table_name()}.{self.local_key}", + getattr(related_model, self.local_key), ) def get_builder(self): @@ -160,7 +160,7 @@ def get_related(self, query, relation, eagers=None, callback=None): int_table = self.intermediary_builder.get_table_name() if callback: - callback(builder) + callback(query) return ( self.distant_builder.select( @@ -179,34 +179,60 @@ def get_related(self, query, relation, eagers=None, callback=None): .get() ) - def query_where_exists( - self, current_query_builder, callback, method="where_exists" - ): - query = self.distant_builder + def attach(self, current_model, related_record): + raise NotImplementedError( + "HasOneThrough relationship does not implement the attach method" + ) + + def attach_related(self, current_model, related_record): + raise NotImplementedError( + "HasOneThrough relationship does not implement the attach_related method" + ) + + def query_has(self, current_builder, method="where_exists"): dist_table = self.distant_builder.get_table_name() int_table = self.intermediary_builder.get_table_name() - getattr(current_query_builder, method)( - query.join( + getattr(current_builder, method)( + self.distant_builder.join( f"{int_table}", f"{int_table}.{self.foreign_key}", "=", f"{dist_table}.{self.other_owner_key}", ).where_column( f"{int_table}.{self.local_owner_key}", - f"{query.get_table_name()}.{self.local_key}", + f"{current_builder.get_table_name()}.{self.local_key}", ) - ).when(callback, lambda q: (callback(q))) + ) - def get_with_count_query(self, builder, callback): - query = self.distant_builder + return self.distant_builder + + def query_where_exists(self, current_builder, callback, method="where_exists"): dist_table = self.distant_builder.get_table_name() int_table = self.intermediary_builder.get_table_name() - if not builder._columns: - builder = builder.select("*") + getattr(current_builder, method)( + self.distant_builder.join( + f"{int_table}", + f"{int_table}.{self.foreign_key}", + "=", + f"{dist_table}.{self.other_owner_key}", + ) + .where_column( + f"{int_table}.{self.local_owner_key}", + f"{current_builder.get_table_name()}.{self.local_key}", + ) + .when(callback, lambda q: (callback(q))) + ) + + def get_with_count_query(self, current_builder, callback): + dist_table = self.distant_builder.get_table_name() + int_table = self.intermediary_builder.get_table_name() - return_query = builder.add_select( + if not current_builder._columns: + current_builder.select("*") + + return_query = current_builder.add_select( f"{self.attribute}_count", lambda q: ( ( @@ -219,7 +245,7 @@ def get_with_count_query(self, builder, callback): ) .where_column( f"{int_table}.{self.local_owner_key}", - f"{query.get_table_name()}.{self.local_key}", + f"{current_builder.get_table_name()}.{self.local_key}", ) .table(dist_table) .when( @@ -227,7 +253,9 @@ def get_with_count_query(self, builder, callback): lambda q: ( q.where_in( self.foreign_key, - callback(query.select(self.other_owner_key)), + callback( + self.distant_builder.select(self.other_owner_key) + ), ) ), ) @@ -236,31 +264,3 @@ def get_with_count_query(self, builder, callback): ) return return_query - - def attach(self, current_model, related_record): - raise NotImplementedError( - "HasOneThrough relationship does not implement the attach method" - ) - - def attach_related(self, current_model, related_record): - raise NotImplementedError( - "HasOneThrough relationship does not implement the attach_related method" - ) - - def query_has(self, current_query_builder, method="where_exists"): - dist_table = self.distant_builder.get_table_name() - int_table = self.intermediary_builder.get_table_name() - - getattr(current_query_builder, method)( - self.distant_builder.join( - f"{int_table}", - f"{int_table}.{self.foreign_key}", - "=", - f"{dist_table}.{self.other_owner_key}", - ).where_column( - f"{current_query_builder.get_table_name()}.{self.local_key}", - f"{int_table}.{self.local_owner_key}", - ) - ) - - return self.distant_builder From 4f41fc9ea98ed824864d1c3726c61dd0a0a6e6d9 Mon Sep 17 00:00:00 2001 From: Kieren Eaton <499977+circulon@users.noreply.github.com> Date: Fri, 18 Oct 2024 13:49:48 +0800 Subject: [PATCH 03/11] Fixed tests --- .../mysql/relationships/test_has_one_through.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/mysql/relationships/test_has_one_through.py b/tests/mysql/relationships/test_has_one_through.py index f0ae66a1..355a748c 100644 --- a/tests/mysql/relationships/test_has_one_through.py +++ b/tests/mysql/relationships/test_has_one_through.py @@ -13,7 +13,7 @@ class InboundShipment(Model): - @has_one_through("port_id", "country_id", "from_port_id", "country_id") + @has_one_through(None, "from_port_id", "country_id", "port_id", "country_id") def from_country(self): return Country, Port @@ -34,7 +34,7 @@ def test_has_query(self): self.assertEqual( sql, - """SELECT * FROM `inbound_shipments` WHERE EXISTS (SELECT * FROM `countries` INNER JOIN `ports` ON `ports`.`country_id` = `countries`.`country_id` WHERE `inbound_shipments`.`from_port_id` = `ports`.`port_id`)""", + """SELECT * FROM `inbound_shipments` WHERE EXISTS (SELECT * FROM `countries` INNER JOIN `ports` ON `ports`.`country_id` = `countries`.`country_id` WHERE `ports`.`port_id` = `inbound_shipments`.`from_port_id`)""", ) def test_or_has(self): @@ -42,7 +42,7 @@ def test_or_has(self): self.assertEqual( sql, - """SELECT * FROM `inbound_shipments` WHERE `inbound_shipments`.`name` = 'Joe' OR EXISTS (SELECT * FROM `countries` INNER JOIN `ports` ON `ports`.`country_id` = `countries`.`country_id` WHERE `inbound_shipments`.`from_port_id` = `ports`.`port_id`)""", + """SELECT * FROM `inbound_shipments` WHERE `inbound_shipments`.`name` = 'Joe' OR EXISTS (SELECT * FROM `countries` INNER JOIN `ports` ON `ports`.`country_id` = `countries`.`country_id` WHERE `ports`.`port_id` = `inbound_shipments`.`from_port_id`)""", ) def test_where_has_query(self): @@ -52,7 +52,7 @@ def test_where_has_query(self): self.assertEqual( sql, - """SELECT * FROM `inbound_shipments` WHERE EXISTS (SELECT * FROM `countries` INNER JOIN `ports` ON `ports`.`country_id` = `countries`.`country_id` WHERE `inbound_shipments`.`from_port_id` = `ports`.`port_id`) AND `inbound_shipments`.`name` = 'USA'""", + """SELECT * FROM `inbound_shipments` WHERE EXISTS (SELECT * FROM `countries` INNER JOIN `ports` ON `ports`.`country_id` = `countries`.`country_id` WHERE `ports`.`port_id` = `inbound_shipments`.`from_port_id` AND `countries`.`name` = 'USA')""", ) def test_or_where_has(self): @@ -64,7 +64,7 @@ def test_or_where_has(self): self.assertEqual( sql, - """SELECT * FROM `inbound_shipments` WHERE `inbound_shipments`.`name` = 'Joe' OR EXISTS (SELECT * FROM `countries` INNER JOIN `ports` ON `ports`.`country_id` = `countries`.`country_id` WHERE `inbound_shipments`.`from_port_id` = `ports`.`port_id`) AND `inbound_shipments`.`name` = 'USA'""", + """SELECT * FROM `inbound_shipments` WHERE `inbound_shipments`.`name` = 'Joe' OR EXISTS (SELECT * FROM `countries` INNER JOIN `ports` ON `ports`.`country_id` = `countries`.`country_id` WHERE `ports`.`port_id` = `inbound_shipments`.`from_port_id` AND `countries`.`name` = 'USA')""", ) def test_doesnt_have(self): @@ -72,7 +72,7 @@ def test_doesnt_have(self): self.assertEqual( sql, - """SELECT * FROM `inbound_shipments` WHERE NOT EXISTS (SELECT * FROM `countries` INNER JOIN `ports` ON `ports`.`country_id` = `countries`.`country_id` WHERE `inbound_shipments`.`from_port_id` = `ports`.`port_id`)""", + """SELECT * FROM `inbound_shipments` WHERE NOT EXISTS (SELECT * FROM `countries` INNER JOIN `ports` ON `ports`.`country_id` = `countries`.`country_id` WHERE `ports`.`port_id` = `inbound_shipments`.`from_port_id`)""", ) def test_or_where_doesnt_have(self): @@ -86,7 +86,7 @@ def test_or_where_doesnt_have(self): self.assertEqual( sql, - """SELECT * FROM `inbound_shipments` WHERE `inbound_shipments`.`name` = 'Joe' OR NOT EXISTS (SELECT * FROM `countries` INNER JOIN `ports` ON `ports`.`country_id` = `countries`.`country_id` WHERE `inbound_shipments`.`from_port_id` = `ports`.`port_id`) AND `inbound_shipments`.`name` = 'USA'""", + """SELECT * FROM `inbound_shipments` WHERE `inbound_shipments`.`name` = 'Joe' OR NOT EXISTS (SELECT * FROM `countries` INNER JOIN `ports` ON `ports`.`country_id` = `countries`.`country_id` WHERE `ports`.`port_id` = `inbound_shipments`.`from_port_id` AND `countries`.`name` = 'USA')""", ) def test_has_one_through_with_count(self): @@ -94,5 +94,5 @@ def test_has_one_through_with_count(self): self.assertEqual( sql, - """SELECT `inbound_shipments`.*, (SELECT COUNT(*) AS m_count_reserved FROM `countries` INNER JOIN `ports` ON `ports`.`country_id` = `countries`.`country_id` WHERE `inbound_shipments`.`from_port_id` = `ports`.`port_id`) AS from_country_count FROM `inbound_shipments`""", + """SELECT `inbound_shipments`.*, (SELECT COUNT(*) AS m_count_reserved FROM `countries` INNER JOIN `ports` ON `ports`.`country_id` = `countries`.`country_id` WHERE `ports`.`port_id` = `inbound_shipments`.`from_port_id`) AS from_country_count FROM `inbound_shipments`""", ) From 817e44eb95c983cbd371274c1f9ba54166118b7a Mon Sep 17 00:00:00 2001 From: Kieren Eaton <499977+circulon@users.noreply.github.com> Date: Sun, 20 Oct 2024 15:59:14 +0800 Subject: [PATCH 04/11] removed unused importsa and duplicate test --- tests/mysql/relationships/test_has_many_through.py | 11 ----------- tests/mysql/relationships/test_has_one_through.py | 5 +---- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/tests/mysql/relationships/test_has_many_through.py b/tests/mysql/relationships/test_has_many_through.py index 310a3b3c..61830c2b 100644 --- a/tests/mysql/relationships/test_has_many_through.py +++ b/tests/mysql/relationships/test_has_many_through.py @@ -2,10 +2,7 @@ from src.masoniteorm.models import Model from src.masoniteorm.relationships import ( - has_one, - belongs_to_many, has_many_through, - has_many, ) from dotenv import load_dotenv @@ -88,11 +85,3 @@ def test_or_where_doesnt_have(self): sql, """SELECT * FROM `inbound_shipments` WHERE `inbound_shipments`.`name` = 'Joe' OR NOT EXISTS (SELECT * FROM `countries` INNER JOIN `ports` ON `ports`.`country_id` = `countries`.`country_id` WHERE `inbound_shipments`.`from_port_id` = `ports`.`port_id`) AND `inbound_shipments`.`name` = 'USA'""", ) - - def test_has_one_through_with_count(self): - sql = InboundShipment.with_count("from_country").to_sql() - - self.assertEqual( - sql, - """SELECT `inbound_shipments`.*, (SELECT COUNT(*) AS m_count_reserved FROM `countries` INNER JOIN `ports` ON `ports`.`country_id` = `countries`.`country_id` WHERE `inbound_shipments`.`from_port_id` = `ports`.`port_id`) AS from_country_count FROM `inbound_shipments`""", - ) diff --git a/tests/mysql/relationships/test_has_one_through.py b/tests/mysql/relationships/test_has_one_through.py index 355a748c..4337dc83 100644 --- a/tests/mysql/relationships/test_has_one_through.py +++ b/tests/mysql/relationships/test_has_one_through.py @@ -2,10 +2,7 @@ from src.masoniteorm.models import Model from src.masoniteorm.relationships import ( - has_one, - belongs_to_many, has_one_through, - has_many, ) from dotenv import load_dotenv @@ -26,7 +23,7 @@ class Port(Model): pass -class MySQLRelationships(unittest.TestCase): +class MySQLHasOneThroughRelationship(unittest.TestCase): maxDiff = None def test_has_query(self): From 79fad5f46a6e380552d0df2c8e60a20ee0e9b94b Mon Sep 17 00:00:00 2001 From: Kieren Eaton <499977+circulon@users.noreply.github.com> Date: Sun, 20 Oct 2024 16:08:02 +0800 Subject: [PATCH 05/11] Fixed linting --- tests/sqlite/relationships/test_sqlite_relationships.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/sqlite/relationships/test_sqlite_relationships.py b/tests/sqlite/relationships/test_sqlite_relationships.py index d88039ff..a3be5246 100644 --- a/tests/sqlite/relationships/test_sqlite_relationships.py +++ b/tests/sqlite/relationships/test_sqlite_relationships.py @@ -3,6 +3,7 @@ from src.masoniteorm.relationships import belongs_to, has_many, has_one, belongs_to_many from tests.integrations.config.database import DB + class Profile(Model): __table__ = "profiles" __connection__ = "dev" From 59e6d1b58bd818ba33c18e873918c51f3f4524e8 Mon Sep 17 00:00:00 2001 From: Kieren Eaton <499977+circulon@users.noreply.github.com> Date: Sun, 20 Oct 2024 16:26:48 +0800 Subject: [PATCH 06/11] Fixed typos --- src/masoniteorm/relationships/HasOneThrough.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/masoniteorm/relationships/HasOneThrough.py b/src/masoniteorm/relationships/HasOneThrough.py index 1a87debb..a323f1f0 100644 --- a/src/masoniteorm/relationships/HasOneThrough.py +++ b/src/masoniteorm/relationships/HasOneThrough.py @@ -1,4 +1,6 @@ +from build.lib.masoniteorm.query import QueryBuilder from .BaseRelationship import BaseRelationship +from ..collection import Collection class HasOneThrough(BaseRelationship): @@ -75,8 +77,8 @@ def apply_relation_query(self, distant_builder, intermediary_builder, owner): already eager loaded Arguments - distant_builder (QueryBuilder): QueryVuilder attached to the distant table - intermediate_builder (QueryBuilder): QueryVuilder attached to the intermesiate (linking) table + distant_builder (QueryBuilder): QueryBuilder attached to the distant table + intermediate_builder (QueryBuilder): QueryBuilder attached to the intermediate (linking) table owner (Any): the model this relationship is starting from Returns @@ -143,7 +145,7 @@ def register_related(self, key, model, collection): def get_related(self, query, relation, eagers=None, callback=None): """ - Get the data to htdrate the model for the distant table with + Get the data to hydrate the model for the distant table with Used when eager loading the model attribute Arguments From 7d303573894d13b493694066e9101ddfcb96905c Mon Sep 17 00:00:00 2001 From: Kieren Eaton <499977+circulon@users.noreply.github.com> Date: Sun, 20 Oct 2024 16:27:38 +0800 Subject: [PATCH 07/11] Fixed eager loading --- .../relationships/HasOneThrough.py | 41 +++++++++++-------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/src/masoniteorm/relationships/HasOneThrough.py b/src/masoniteorm/relationships/HasOneThrough.py index a323f1f0..316378e7 100644 --- a/src/masoniteorm/relationships/HasOneThrough.py +++ b/src/masoniteorm/relationships/HasOneThrough.py @@ -140,10 +140,13 @@ def register_related(self, key, model, collection): None """ - related = collection.get(getattr(model, self.local_key), None) - model.add_relation({key: related[0] if related else None}) + related_id = getattr(model, self.local_key) + for id, item in collection.items(): + if id == related_id: + model.add_relation({key: item[0]}) + break - def get_related(self, query, relation, eagers=None, callback=None): + def get_related(self, current_builder, relation, eagers=None, callback=None): """ Get the data to hydrate the model for the distant table with Used when eager loading the model attribute @@ -162,24 +165,28 @@ def get_related(self, query, relation, eagers=None, callback=None): int_table = self.intermediary_builder.get_table_name() if callback: - callback(query) + callback(current_builder) - return ( - self.distant_builder.select( - f"{dist_table}.*, {int_table}.{self.local_owner_key} as {self.local_key}" - ) - .join( - f"{int_table}", - f"{int_table}.{self.foreign_key}", - "=", - f"{dist_table}.{self.other_owner_key}", + (self.distant_builder.select(f"{dist_table}.*, {int_table}.{self.local_owner_key} as {self.local_key}") + .join( + f"{int_table}", + f"{int_table}.{self.foreign_key}", + "=", + f"{dist_table}.{self.other_owner_key}", + )) + + if isinstance(relation, Collection): + self.distant_builder.where_in( + f"{int_table}.{self.local_owner_key}", + Collection(relation._get_value(self.local_key)).unique(), ) - .where( + else: + self.distant_builder.where( f"{int_table}.{self.local_owner_key}", - relation._get_value(self.local_key), + getattr(relation, self.local_key), ) - .get() - ) + + return self.distant_builder.get() def attach(self, current_model, related_record): raise NotImplementedError( From 80aeb848272697e917f0405af33f5afb27fa4a63 Mon Sep 17 00:00:00 2001 From: Kieren Eaton <499977+circulon@users.noreply.github.com> Date: Sun, 20 Oct 2024 16:29:55 +0800 Subject: [PATCH 08/11] Added tests for HasOneThrough eager loading --- .../test_sqlite_has_through_relationships.py | 119 ++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 tests/sqlite/relationships/test_sqlite_has_through_relationships.py diff --git a/tests/sqlite/relationships/test_sqlite_has_through_relationships.py b/tests/sqlite/relationships/test_sqlite_has_through_relationships.py new file mode 100644 index 00000000..e02af9bf --- /dev/null +++ b/tests/sqlite/relationships/test_sqlite_has_through_relationships.py @@ -0,0 +1,119 @@ +import unittest + +from build.lib.masoniteorm.collection import Collection +from build.lib.masoniteorm.scopes import scope +from src.masoniteorm.models import Model +from src.masoniteorm.relationships import has_one_through +from tests.integrations.config.database import DB +from tests.integrations.config.database import DATABASES +from src.masoniteorm.schema import Schema +from src.masoniteorm.schema.platforms import SQLitePlatform + + +class Port(Model): + __table__ = "ports" + __connection__ = "dev" + __fillable__ = ["port_id", "name", "port_country_id"] + + +class Country(Model): + __table__ = "countries" + __connection__ = "dev" + __fillable__ = ["country_id", "name"] + + +class IncomingShipment(Model): + __table__ = "incoming_shipments" + __connection__ = "dev" + __fillable__ = ["shipment_id", "name", "from_port_id"] + + @has_one_through(None, "from_port_id", "port_country_id", "port_id", "country_id") + def from_country(self): + return [Country, Port] + + +class TestRelationships(unittest.TestCase): + def setUp(self): + self.schema = Schema( + connection="dev", + connection_details=DATABASES, + platform=SQLitePlatform, + ).on("dev") + + with self.schema.create_table_if_not_exists("incoming_shipments") as table: + table.integer("shipment_id").primary() + table.string("name") + table.integer("from_port_id") + + with self.schema.create_table_if_not_exists("ports") as table: + table.integer("port_id").primary() + table.string("name") + table.integer("port_country_id") + + with self.schema.create_table_if_not_exists("countries") as table: + table.integer("country_id").primary() + table.string("name") + + if not Country.count(): + Country.builder.new().bulk_create( + [ + {"country_id": 10, "name": "Australia"}, + {"country_id": 20, "name": "USA"}, + {"country_id": 30, "name": "Canada"}, + {"country_id": 40, "name": "United Kingdom"}, + ] + ) + + if not Port.count(): + Port.builder.new().bulk_create( + [ + {"port_id": 100, "name": "Melbourne", "port_country_id": 10}, + {"port_id": 200, "name": "Darwin", "port_country_id": 10}, + {"port_id": 300, "name": "South Louisiana", "port_country_id": 20}, + {"port_id": 400, "name": "Houston", "port_country_id": 20}, + {"port_id": 500, "name": "Montreal", "port_country_id": 30}, + {"port_id": 600, "name": "Vancouver", "port_country_id": 30}, + {"port_id": 700, "name": "Southampton", "port_country_id": 40}, + {"port_id": 800, "name": "London Gateway", "port_country_id": 40}, + ] + ) + + if not IncomingShipment.count(): + IncomingShipment.builder.new().bulk_create( + [ + {"name": "Bread", "from_port_id": 300}, + {"name": "Milk", "from_port_id": 100}, + {"name": "Tractor Parts", "from_port_id": 100}, + {"name": "Fridges", "from_port_id": 700}, + {"name": "Wheat", "from_port_id": 600}, + {"name": "Kettles", "from_port_id": 400}, + {"name": "Bread", "from_port_id": 700}, + ] + ) + + def test_has_one_through_can_eager_load(self): + shipments = IncomingShipment.where("name", "Bread").with_("from_country").get() + self.assertEqual(shipments.count(), 2) + + shipment1 = shipments.shift() + self.assertIsInstance(shipment1.from_country, Country) + self.assertEqual(shipment1.from_country.country_id, 20) + + shipment2 = shipments.shift() + self.assertIsInstance(shipment2.from_country, Country) + self.assertEqual(shipment2.from_country.country_id, 40) + + def test_has_one_through_eager_load_can_be_empty(self): + shipments = ( + IncomingShipment.where("name", "Bread") + .with_( + "from_country", + ) + .get() + ) + self.assertEqual(shipments.count(), 2) + + def test_has_one_through_can_get_related(self): + shipment = IncomingShipment.where("name", "Milk").first() + self.assertIsInstance(shipment.from_country, Country) + self.assertEqual(shipment.from_country.country_id, 10) From cd40bf9d491d7a996b2dd514e67d7926b6e66365 Mon Sep 17 00:00:00 2001 From: Kieren Eaton <499977+circulon@users.noreply.github.com> Date: Sun, 20 Oct 2024 16:40:01 +0800 Subject: [PATCH 09/11] removed unused import --- src/masoniteorm/relationships/HasOneThrough.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/masoniteorm/relationships/HasOneThrough.py b/src/masoniteorm/relationships/HasOneThrough.py index 316378e7..910914cc 100644 --- a/src/masoniteorm/relationships/HasOneThrough.py +++ b/src/masoniteorm/relationships/HasOneThrough.py @@ -1,4 +1,3 @@ -from build.lib.masoniteorm.query import QueryBuilder from .BaseRelationship import BaseRelationship from ..collection import Collection From ad33effc96b70c12112705655421d6af2e46eea3 Mon Sep 17 00:00:00 2001 From: Kieren Eaton <499977+circulon@users.noreply.github.com> Date: Sun, 20 Oct 2024 16:46:57 +0800 Subject: [PATCH 10/11] removed unused imports --- .../relationships/test_sqlite_has_through_relationships.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/sqlite/relationships/test_sqlite_has_through_relationships.py b/tests/sqlite/relationships/test_sqlite_has_through_relationships.py index e02af9bf..46a0f913 100644 --- a/tests/sqlite/relationships/test_sqlite_has_through_relationships.py +++ b/tests/sqlite/relationships/test_sqlite_has_through_relationships.py @@ -1,10 +1,7 @@ import unittest -from build.lib.masoniteorm.collection import Collection -from build.lib.masoniteorm.scopes import scope from src.masoniteorm.models import Model from src.masoniteorm.relationships import has_one_through -from tests.integrations.config.database import DB from tests.integrations.config.database import DATABASES from src.masoniteorm.schema import Schema from src.masoniteorm.schema.platforms import SQLitePlatform From d377d3722774fc60e72699d1e2f6538430117b3d Mon Sep 17 00:00:00 2001 From: Kieren Eaton <499977+circulon@users.noreply.github.com> Date: Mon, 21 Oct 2024 09:25:28 +0800 Subject: [PATCH 11/11] Fixed using .first() fixed model attribute containing a list instead of a model when using .first() added additional tests checking .first() usage --- .../relationships/HasOneThrough.py | 17 +++++-------- .../test_sqlite_has_through_relationships.py | 24 ++++++++++++++++++- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/src/masoniteorm/relationships/HasOneThrough.py b/src/masoniteorm/relationships/HasOneThrough.py index 910914cc..0f9eb087 100644 --- a/src/masoniteorm/relationships/HasOneThrough.py +++ b/src/masoniteorm/relationships/HasOneThrough.py @@ -139,11 +139,8 @@ def register_related(self, key, model, collection): None """ - related_id = getattr(model, self.local_key) - for id, item in collection.items(): - if id == related_id: - model.add_relation({key: item[0]}) - break + related = collection.get(getattr(model, self.local_key), None) + model.add_relation({key: related[0] if related else None}) def get_related(self, current_builder, relation, eagers=None, callback=None): """ @@ -175,17 +172,15 @@ def get_related(self, current_builder, relation, eagers=None, callback=None): )) if isinstance(relation, Collection): - self.distant_builder.where_in( + return self.distant_builder.where_in( f"{int_table}.{self.local_owner_key}", Collection(relation._get_value(self.local_key)).unique(), - ) + ).get() else: - self.distant_builder.where( + return self.distant_builder.where( f"{int_table}.{self.local_owner_key}", getattr(relation, self.local_key), - ) - - return self.distant_builder.get() + ).first() def attach(self, current_model, related_record): raise NotImplementedError( diff --git a/tests/sqlite/relationships/test_sqlite_has_through_relationships.py b/tests/sqlite/relationships/test_sqlite_has_through_relationships.py index 46a0f913..d23f4847 100644 --- a/tests/sqlite/relationships/test_sqlite_has_through_relationships.py +++ b/tests/sqlite/relationships/test_sqlite_has_through_relationships.py @@ -100,17 +100,39 @@ def test_has_one_through_can_eager_load(self): self.assertIsInstance(shipment2.from_country, Country) self.assertEqual(shipment2.from_country.country_id, 40) + # check .first() and .get() produce the same result + single = ( + IncomingShipment.where("name", "Tractor Parts") + .with_("from_country") + .first() + ) + single_get = ( + IncomingShipment.where("name", "Tractor Parts").with_("from_country").get() + ) + self.assertEqual(single.from_country.country_id, 10) + self.assertEqual(single_get.count(), 1) + self.assertEqual( + single.from_country.country_id, single_get.first().from_country.country_id + ) + def test_has_one_through_eager_load_can_be_empty(self): shipments = ( IncomingShipment.where("name", "Bread") + .where_has("from_country", lambda query: query.where("name", "Ueaguay")) .with_( "from_country", ) .get() ) - self.assertEqual(shipments.count(), 2) + self.assertEqual(shipments.count(), 0) def test_has_one_through_can_get_related(self): shipment = IncomingShipment.where("name", "Milk").first() self.assertIsInstance(shipment.from_country, Country) self.assertEqual(shipment.from_country.country_id, 10) + + def test_has_one_through_has_query(self): + shipments = IncomingShipment.where_has( + "from_country", lambda query: query.where("name", "USA") + ) + self.assertEqual(shipments.count(), 2)