Skip to content

Commit

Permalink
Fix compose to correctly merge any combinations that contains collect…
Browse files Browse the repository at this point in the history
…ions to collection backed queries. This code needs a refactor
  • Loading branch information
stevegeek committed Oct 4, 2024
1 parent 8fd4d31 commit 910886e
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 64 deletions.
8 changes: 0 additions & 8 deletions lib/quo/collection_backed_query.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,6 @@ module Quo
class CollectionBackedQuery < Quo.base_query_class
prop :total_count, _Nilable(Integer)

# Compose is aliased as `+`. Can optionally take `joins` parameters to add joins on merged relation.
# @rbs right: Quo::Query | ActiveRecord::Relation | Object & Enumerable[untyped]
# @rbs joins: Symbol | Hash[Symbol, untyped] | Array[Symbol | Hash[Symbol, untyped]]
def self.compose(right, joins: nil)
ComposedQuery.composer(Quo::CollectionBackedQuery, self, right, joins: joins)
end
singleton_class.alias_method :+, :compose

# Wrap an enumerable collection or a block that returns an enumerable collection
# @rbs data: untyped, props: Symbol => untyped, block: () -> untyped
# @rbs return: Quo::CollectionBackedQuery
Expand Down
58 changes: 26 additions & 32 deletions lib/quo/composed_query.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,34 +67,20 @@ def merge_instances(left_instance, right_instance, joins: nil)
if left_instance.is_a?(Quo::Query) && right_instance.is_a?(::ActiveRecord::Relation)
return composer(left_instance.is_a?(Quo::RelationBackedQuery) ? Quo::RelationBackedQuery : Quo::CollectionBackedQuery, left_instance.class, right_instance, joins: joins).new(**left_instance.to_h)
elsif right_instance.is_a?(Quo::Query) && left_instance.is_a?(::ActiveRecord::Relation)
return composer(Quo::RelationBackedQuery, left_instance, right_instance.class, joins: joins).new(**right_instance.to_h)
return composer(right_instance.is_a?(Quo::RelationBackedQuery) ? Quo::RelationBackedQuery : Quo::CollectionBackedQuery, left_instance, right_instance.class, joins: joins).new(**right_instance.to_h)
elsif left_instance.is_a?(Quo::Query) && right_instance.is_a?(Quo::Query)
props = left_instance.to_h.merge(right_instance.to_h.compact)
return composer(left_instance.is_a?(Quo::RelationBackedQuery) ? Quo::RelationBackedQuery : Quo::CollectionBackedQuery, left_instance.class, right_instance.class, joins: joins).new(**props)
return composer((left_instance.is_a?(Quo::RelationBackedQuery) && right_instance.is_a?(Quo::RelationBackedQuery)) ? Quo::RelationBackedQuery : Quo::CollectionBackedQuery, left_instance.class, right_instance.class, joins: joins).new(**props)
end
composer(Quo::RelationBackedQuery, left_instance, right_instance, joins: joins).new # Both are relations
composer(Quo::RelationBackedQuery, left_instance, right_instance, joins: joins).new # Both are AR relations
end
module_function :merge_instances

# @rbs override
def query
merge_left_and_right(left, right, _composing_joins)
merge_left_and_right
end

# @rbs return: Quo::Query | ::ActiveRecord::Relation
def left
return _left_query if is_relation?(_left_query)
_left_query.new(**child_options(_left_query))
end

# @rbs return: Quo::Query | ::ActiveRecord::Relation
def right
return _right_query if is_relation?(_right_query)
_right_query.new(**child_options(_right_query))
end

delegate :_composing_joins, :_left_query, :_right_query, to: :class

# @rbs override
def inspect
klass_name = is_a?(Quo::RelationBackedQuery) ? Quo::RelationBackedQuery.name : Quo::CollectionBackedQuery.name
Expand All @@ -114,13 +100,26 @@ def property_names(query_class)
query_class.literal_properties.properties_index.keys
end

# @rbs return: ActiveRecord::Relation | Object & Enumerable[untyped]
def merge_left_and_right(left, right, joins)
left_rel = unwrap_relation(left)
right_rel = unwrap_relation(right)
# FIXME: Skipping type checks here, as not sure how to make this type check with RBS
__skip__ = if both_relations?(left_rel, right_rel)
apply_joins(left_rel, joins).merge(right_rel)
# @rbs return: Quo::Query | ::ActiveRecord::Relation
def left
lq = self.class._left_query
return lq if is_relation?(lq)
lq.new(**child_options(lq))
end

# @rbs return: Quo::Query | ::ActiveRecord::Relation
def right
rq = self.class._right_query
return rq if is_relation?(rq)
rq.new(**child_options(rq))
end

# @rbs return: ActiveRecord::Relation | CollectionBackedQuery
def merge_left_and_right
left_rel = quo_unwrap_unpaginated_query(left)
right_rel = quo_unwrap_unpaginated_query(right)
if both_relations?(left_rel, right_rel)
apply_joins(left_rel).merge(right_rel) # ActiveRecord::Relation
elsif left_relation_right_enumerable?(left_rel, right_rel)
left_rel.to_a + right_rel
elsif left_enumerable_right_relation?(left_rel, right_rel) && left_rel.respond_to?(:+)
Expand All @@ -133,9 +132,9 @@ def merge_left_and_right(left, right, joins)
end

# @rbs left_rel: ActiveRecord::Relation
# @rbs joins: untyped
# @rbs return: ActiveRecord::Relation
def apply_joins(left_rel, joins)
def apply_joins(left_rel)
joins = self.class._composing_joins
joins.present? ? left_rel.joins(joins) : left_rel
end

Expand Down Expand Up @@ -165,10 +164,5 @@ def left_relation_right_enumerable?(left, right)
def left_enumerable_right_relation?(left, right)
!is_relation?(left) && is_relation?(right)
end

# @rbs override
def unwrap_relation(query)
query.is_a?(Quo::Query) ? query.unwrap_unpaginated : query
end
end
end
14 changes: 14 additions & 0 deletions lib/quo/query.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,20 @@ def to_s
inspect
end

# Compose is aliased as `+`. Can optionally take `joins` parameters to add joins on merged relation.
# @rbs right: Quo::Query | ActiveRecord::Relation | Object & Enumerable[untyped]
# @rbs joins: Symbol | Hash[Symbol, untyped] | Array[Symbol | Hash[Symbol, untyped]]
# @rbs return: Quo::Query & Quo::ComposedQuery
def self.compose(right, joins: nil)
super_class = if self < Quo::CollectionBackedQuery || right < Quo::CollectionBackedQuery
Quo::CollectionBackedQuery
else
Quo::RelationBackedQuery
end
ComposedQuery.composer(super_class, self, right, joins: joins)
end
singleton_class.alias_method :+, :compose

COERCE_TO_INT = ->(value) do #: (untyped value) -> Integer?
return if value == Literal::Null
value&.to_i
Expand Down
8 changes: 0 additions & 8 deletions lib/quo/relation_backed_query.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,6 @@ def self.wrap(query = nil, props: {}, &block)
klass
end

# Compose is aliased as `+`. Can optionally take `joins` parameters to add joins on merged relation.
# @rbs right: Quo::Query | ActiveRecord::Relation | Object & Enumerable[untyped]
# @rbs joins: Symbol | Hash[Symbol, untyped] | Array[Symbol | Hash[Symbol, untyped]]
def self.compose(right, joins: nil)
ComposedQuery.composer(Quo::RelationBackedQuery, self, right, joins: joins)
end
singleton_class.alias_method :+, :compose

# @rbs conditions: untyped?
# @rbs return: String
def self.sanitize_sql_for_conditions(conditions)
Expand Down
5 changes: 0 additions & 5 deletions sig/generated/quo/collection_backed_query.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,6 @@
module Quo
# @rbs inherits Quo::Query
class CollectionBackedQuery < Quo::Query
# Compose is aliased as `+`. Can optionally take `joins` parameters to add joins on merged relation.
# @rbs right: Quo::Query | ActiveRecord::Relation | Object & Enumerable[untyped]
# @rbs joins: Symbol | Hash[Symbol, untyped] | Array[Symbol | Hash[Symbol, untyped]]
def self.compose: (Quo::Query | ActiveRecord::Relation | Object & Enumerable[untyped] right, ?joins: Symbol | Hash[Symbol, untyped] | Array[Symbol | Hash[Symbol, untyped]]) -> untyped

# Wrap an enumerable collection or a block that returns an enumerable collection
# @rbs data: untyped, props: Symbol => untyped, block: () -> untyped
# @rbs return: Quo::CollectionBackedQuery
Expand Down
6 changes: 0 additions & 6 deletions sig/generated/quo/composed_query.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,6 @@ module Quo

private

# The underlying query now needs to handle the possibilty of the return from the merge being either a relation or a collection
def underlying_query: () -> ActiveRecord::Relation

# The configured query is the underlying query with paging
def configured_query: () -> ActiveRecord::Relation

# @rbs return: Hash[Symbol, untyped]
def child_options: (untyped query_class) -> Hash[Symbol, untyped]

Expand Down
10 changes: 10 additions & 0 deletions sig/generated/quo/query.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,18 @@ module Quo

def self.inspect: () -> untyped

def self.to_s: () -> untyped

def inspect: () -> untyped

def to_s: () -> untyped

# Compose is aliased as `+`. Can optionally take `joins` parameters to add joins on merged relation.
# @rbs right: Quo::Query | ActiveRecord::Relation | Object & Enumerable[untyped]
# @rbs joins: Symbol | Hash[Symbol, untyped] | Array[Symbol | Hash[Symbol, untyped]]
# @rbs return: Quo::Query & Quo::ComposedQuery
def self.compose: (Quo::Query | ActiveRecord::Relation | Object & Enumerable[untyped] right, ?joins: Symbol | Hash[Symbol, untyped] | Array[Symbol | Hash[Symbol, untyped]]) -> (Quo::Query & Quo::ComposedQuery)

COERCE_TO_INT: untyped

attr_accessor page(): Integer?
Expand Down
5 changes: 0 additions & 5 deletions sig/generated/quo/relation_backed_query.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,6 @@ module Quo
# @rbs return: Quo::RelationBackedQuery
def self.wrap: (?ActiveRecord::Relation | Quo::Query query, ?props: Hash[Symbol, untyped]) ?{ (?) -> untyped } -> Quo::RelationBackedQuery

# Compose is aliased as `+`. Can optionally take `joins` parameters to add joins on merged relation.
# @rbs right: Quo::Query | ActiveRecord::Relation | Object & Enumerable[untyped]
# @rbs joins: Symbol | Hash[Symbol, untyped] | Array[Symbol | Hash[Symbol, untyped]]
def self.compose: (Quo::Query | ActiveRecord::Relation | Object & Enumerable[untyped] right, ?joins: Symbol | Hash[Symbol, untyped] | Array[Symbol | Hash[Symbol, untyped]]) -> untyped

# @rbs conditions: untyped?
# @rbs return: String
def self.sanitize_sql_for_conditions: (untyped? conditions) -> String
Expand Down

0 comments on commit 910886e

Please sign in to comment.