Skip to content

Commit

Permalink
Remove limitations on nested relations in CompositeIDs (#553)
Browse files Browse the repository at this point in the history
* Remove reliance on AnyQueryAddressableProperty for working with composite ID filtering, leverage QueryFilterInput instead.
* Remove requirement that all fields of a @CompositeID must be query-addressable.
* Remove fatal errors from Children and OptionalChild (the compiler already enforces the non-composite requirement), update fatal errors for Parent and OptionalParent to suggest the composite variants.
* Add tests for more complex relations (esp. composite) nested in composite IDs.
* SQLite CI no longer requires an external dependency
  • Loading branch information
gwynne authored Feb 26, 2023
1 parent b821ac3 commit 636a59e
Show file tree
Hide file tree
Showing 12 changed files with 163 additions and 63 deletions.
3 changes: 0 additions & 3 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,6 @@ jobs:
- { dependent: 'fluent-mysql-driver', ref: 'main' }
- { dependent: 'fluent-mongo-driver', ref: 'main' }
steps:
- name: Install SQLite dependency
run: apt-get -q update && apt-get -q install -y libsqlite3-dev
if: ${{ contains(matrix.dependent, 'sqlite') }}
- name: Check out package
uses: actions/checkout@v3
with:
Expand Down
135 changes: 127 additions & 8 deletions Sources/FluentBenchmark/Tests/CompositeRelationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ extension FluentBenchmarker {
public func testCompositeRelations() throws {
try testCompositeParent_loading()
try testCompositeChildren_loading()
try testCompositeParent_nestedInCompositeID()
}

private func testCompositeParent_loading() throws {
Expand Down Expand Up @@ -114,6 +115,35 @@ extension FluentBenchmarker {
XCTAssertNil(parent6.$additionalLinkedCompositeIdChildModel.value??.id)
}
}

private func testCompositeParent_nestedInCompositeID() throws {
try self.runTest(#function, [
GalaxyMigration(),
GalaxySeed(),
CompositeParentTheFirst.ModelMigration(),
CompositeParentTheSecond.ModelMigration(),
]) {
let anyGalaxy = try XCTUnwrap(Galaxy.query(on: self.database).first().wait())

let parentFirst = CompositeParentTheFirst(parentId: try anyGalaxy.requireID())
try parentFirst.create(on: self.database).wait()

let parentSecond = CompositeParentTheSecond(parentId: try parentFirst.requireID())
try parentSecond.create(on: self.database).wait()

XCTAssertEqual(try CompositeParentTheFirst.query(on: self.database).filter(\.$id == parentFirst.requireID()).count().wait(), 1)

let parentFirstAgain = try XCTUnwrap(CompositeParentTheFirst.query(on: self.database).filter(\.$id.$parent.$id == anyGalaxy.requireID()).with(\.$id.$parent).with(\.$children).first().wait())

XCTAssertEqual(parentFirstAgain.id?.$parent.value?.id, anyGalaxy.id)
XCTAssertEqual(parentFirstAgain.$children.value?.first?.id?.$parent.id, parentFirstAgain.id)

try Galaxy.query(on: self.database).filter(\.$id == anyGalaxy.requireID()).delete(force: true).wait()

XCTAssertEqual(try CompositeParentTheFirst.query(on: self.database).count().wait(), 0)
XCTAssertEqual(try CompositeParentTheSecond.query(on: self.database).count().wait(), 0)
}
}
}

final class CompositeIDParentModel: Model {
Expand Down Expand Up @@ -169,8 +199,6 @@ final class CompositeIDParentModel: Model {
}

struct ModelMigration: Migration {
init() {}

func prepare(on database: Database) -> EventLoopFuture<Void> {
database.schema(CompositeIDParentModel.schema)
.field("name", .string, .required)
Expand All @@ -186,8 +214,6 @@ final class CompositeIDParentModel: Model {
}

struct ModelSeed: Migration {
init() {}

func prepare(on database: Database) -> EventLoopFuture<Void> {
[
CompositeIDParentModel(name: "A", dimensions: 1),
Expand Down Expand Up @@ -237,8 +263,6 @@ final class CompositeIDChildModel: Model {
}

struct ModelMigration: Migration {
init() {}

func prepare(on database: Database) -> EventLoopFuture<Void> {
database.schema(CompositeIDChildModel.schema)
.field(.id, .int, .required, .identifier(auto: (database as? SQLDatabase)?.dialect.name != "sqlite"))
Expand Down Expand Up @@ -279,8 +303,6 @@ final class CompositeIDChildModel: Model {
}

struct ModelSeed: Migration {
init() {}

func prepare(on database: Database) -> EventLoopFuture<Void> {
[
CompositeIDChildModel(id: 1, parentId: .init(name: "A"), additionalParentId: nil, linkedId: .init(name: "A"), additionalLinkedId: nil),
Expand Down Expand Up @@ -316,3 +338,100 @@ extension DatabaseSchema.Constraint {
))))
}
}

final class CompositeParentTheFirst: Model {
static let schema = "composite_parent_the_first"

final class IDValue: Fields, Hashable {
@Parent(key: "parent_id")
var parent: Galaxy

init() {}

init(parentId: Galaxy.IDValue) {
self.$parent.id = parentId
}

static func == (lhs: IDValue, rhs: IDValue) -> Bool {
lhs.$parent.id == rhs.$parent.id
}

func hash(into hasher: inout Hasher) {
hasher.combine(self.$parent.id)
}
}

@CompositeID
var id: IDValue?

@CompositeChildren(for: \.$id.$parent)
var children: [CompositeParentTheSecond]

init() {}

init(parentId: Galaxy.IDValue) {
self.id = .init(parentId: parentId)
}

struct ModelMigration: Migration {
func prepare(on database: Database) -> EventLoopFuture<Void> {
database.schema(CompositeParentTheFirst.schema)
.field("parent_id", .uuid, .required)
.foreignKey("parent_id", references: Galaxy.schema, .id, onDelete: .cascade, onUpdate: .cascade)
.compositeIdentifier(over: "parent_id")
.create()
}

func revert(on database: Database) -> EventLoopFuture<Void> {
database.schema(CompositeParentTheFirst.schema)
.delete()
}
}
}

final class CompositeParentTheSecond: Model {
static let schema = "composite_parent_the_second"

final class IDValue: Fields, Hashable {
@CompositeParent(prefix: "ref", strategy: .snakeCase)
var parent: CompositeParentTheFirst

init() {}

init(parentId: CompositeParentTheFirst.IDValue) {
self.$parent.id.$parent.id = parentId.$parent.id
}

static func == (lhs: IDValue, rhs: IDValue) -> Bool {
lhs.$parent.id == rhs.$parent.id
}

func hash(into hasher: inout Hasher) {
hasher.combine(self.$parent.id)
}
}

@CompositeID
var id: IDValue?

init() {}

init(parentId: CompositeParentTheFirst.IDValue) {
self.id = .init(parentId: parentId)
}

struct ModelMigration: Migration {
func prepare(on database: Database) -> EventLoopFuture<Void> {
database.schema(CompositeParentTheSecond.schema)
.field("ref_parent_id", .uuid, .required)
.foreignKey("ref_parent_id", references: CompositeParentTheFirst.schema, "parent_id", onDelete: .cascade, onUpdate: .cascade)
.compositeIdentifier(over: "ref_parent_id")
.create()
}

func revert(on database: Database) -> EventLoopFuture<Void> {
database.schema(CompositeParentTheSecond.schema)
.delete()
}
}
}
15 changes: 10 additions & 5 deletions Sources/FluentKit/Database/DatabaseInput.swift
Original file line number Diff line number Diff line change
Expand Up @@ -104,34 +104,39 @@ private struct PrefixedDatabaseInput<Base: DatabaseInput>: DatabaseInput {
/// need not be the same as `BuilderModel` (the base model of the query builder). This permits filtering
/// to be applied based on a joined model, and enables support for ``ModelAlias``.
///
/// If ``QueryFilterInput/inverted`` is `true`, the added filters will use the ``DatabaseQuery/Filter/Method/notEqual``
/// method instead.
///
/// The ``DatabaseInput/wantsUnmodifiedKeys-1qajw`` flag is enabled for this input type.
///
/// The query builder is modified in-place. Callers may either retain their own reference to the builder or
/// retrieve it from this structure when ready. It is the caller's responsibility to ensure that grouping of
/// multiple filters is handled appropriately for their use case - most commonly by using the builder passed
/// to a ``QueryBuilder/group(_:_:)`` closure to create an instance of this type.
/// to a ``QueryBuilder/group(_:_:)`` closure to create an instance of this type.
///
/// > Tip: Applying a query filter via database input is especially useful as a means of providing generic
/// support for filters involving a ``CompositeIDProperty``. For example, using an instance of this type
/// as the input for a ``CompositeParentProperty`` filters the query according to the set of appropriately
/// prefixed field keys the property encapsulates.
internal struct QueryFilterInput<BuilderModel: FluentKit.Model, InputModel: Schema>: DatabaseInput {
let builder: QueryBuilder<BuilderModel>
let inverted: Bool

var wantsUnmodifiedKeys: Bool { true }

init(builder: QueryBuilder<BuilderModel>) where BuilderModel == InputModel {
self.init(BuilderModel.self, builder: builder)
init(builder: QueryBuilder<BuilderModel>, inverted: Bool = false) where BuilderModel == InputModel {
self.init(BuilderModel.self, builder: builder, inverted: inverted)
}

init(_: InputModel.Type, builder: QueryBuilder<BuilderModel>) {
init(_: InputModel.Type, builder: QueryBuilder<BuilderModel>, inverted: Bool = false) {
self.builder = builder
self.inverted = inverted
}

func set(_ value: DatabaseQuery.Value, at key: FieldKey) {
self.builder.filter(
.extendedPath([key], schema: InputModel.schemaOrAlias, space: InputModel.spaceIfNotAliased),
.equal,
self.inverted ? .notEqual : .equal,
value
)
}
Expand Down
11 changes: 4 additions & 7 deletions Sources/FluentKit/Operators/ValueOperators.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,14 @@ extension QueryBuilder {
where Joined: Schema, Joined.IDValue: Fields
{
let relation: DatabaseQuery.Filter.Relation
let inverted: Bool
switch filter.method {
case .equality(false): relation = .and
case .equality(true): relation = .or
case .equality(false): (relation, inverted) = (.and, false)
case .equality(true): (relation, inverted) = (.or, true)
default: fatalError("unreachable")
}

return self.group(relation) {
_ = filter.value.properties.map { $0 as! AnyQueryAddressableProperty }.filter { $0.anyQueryableProperty.queryableValue() != nil }.reduce($0) {
$0.filter(.extendedPath($1.queryablePath, schema: Joined.schemaOrAlias, space: Joined.spaceIfNotAliased), filter.method, $1.anyQueryableProperty.queryableValue()!)
}
}
return self.group(relation) { filter.value.input(to: QueryFilterInput(builder: $0, inverted: inverted)) }
}
}

Expand Down
4 changes: 0 additions & 4 deletions Sources/FluentKit/Properties/Children.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ public final class ChildrenProperty<From, To>
}

private init(for parentKey: Key) {
guard !(From.IDValue.self is Fields.Type) /*From().anyId is AnyQueryAddressableProperty*/ else {
fatalError("Can not use @Children with a model whose ID is not addressable (this probably means '\(From.self)' uses `@CompositeID`).")
}

self.parentKey = parentKey
}

Expand Down
7 changes: 0 additions & 7 deletions Sources/FluentKit/Properties/CompositeID.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,6 @@ public final class CompositeIDProperty<Model, Value>
}

public init() {
guard Value.init().properties.allSatisfy({ $0 is AnyQueryAddressableProperty }) else {
fatalError("""
All elements of a composite model ID must represent exactly one actual column in the database.
This error is most often caused by trying to use @Children, @Siblings, or @Group inside a @CompositeID.
""")
}
self.value = .init()
self.exists = false
self.cachedOutput = nil
Expand Down
4 changes: 0 additions & 4 deletions Sources/FluentKit/Properties/OptionalChild.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ public final class OptionalChildProperty<From, To>
}

private init(for parentKey: Key) {
guard !(From.IDValue.self is Fields.Type) /*From().anyId is AnyQueryAddressableProperty*/ else {
fatalError("Can not use @OptionalChild with a model whose ID is not addressable (this probably means '\(From.self)' uses `@CompositeID`).")
}

self.parentKey = parentKey
}

Expand Down
4 changes: 2 additions & 2 deletions Sources/FluentKit/Properties/OptionalParent.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ public final class OptionalParentProperty<From, To>
public var value: To??

public init(key: FieldKey) {
guard !(To.IDValue.self is Fields.Type) /*To().anyId is AnyQueryAddressableProperty*/ else {
fatalError("Can not use @OptionalParent to target a model whose ID is not addressable (this probably means '\(To.self)' uses `@CompositeID`).")
guard !(To.IDValue.self is Fields.Type) else {
fatalError("Can not use @OptionalParent to target a model with composite ID; use @CompositeOptionalParent instead.")
}

self._id = .init(key: key)
Expand Down
4 changes: 2 additions & 2 deletions Sources/FluentKit/Properties/Parent.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ public final class ParentProperty<From, To>
public var value: To?

public init(key: FieldKey) {
guard !(To.IDValue.self is Fields.Type) /*To().anyId is AnyQueryAddressableProperty*/ else {
fatalError("Can not use @Parent to target a model whose ID is not addressable (this probably means '\(To.self)' uses `@CompositeID`).")
guard !(To.IDValue.self is Fields.Type) else {
fatalError("Can not use @Parent to target a model with composite ID; use @CompositeParent instead.")
}

self._id = .init(key: key)
Expand Down
6 changes: 2 additions & 4 deletions Sources/FluentKit/Properties/Siblings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,8 @@ public final class SiblingsProperty<From, To, Through>
from: KeyPath<Through, Through.Parent<From>>,
to: KeyPath<Through, Through.Parent<To>>
) {
guard !(From.IDValue.self is Fields.Type) /*From().anyId is AnyQueryAddressableProperty*/,
!(To.IDValue.self is Fields.Type) /*To().anyId is AnyQueryAddressableProperty*/
else {
fatalError("Can not use @Siblings with models whose IDs are not addressable (this probably means '\(From.self)' and/or '\(To.self)' uses `@CompositeID`).")
guard !(From.IDValue.self is Fields.Type), !(To.IDValue.self is Fields.Type) else {
fatalError("Can not use @Siblings with models which have composite IDs.")
}

self.from = from
Expand Down
21 changes: 14 additions & 7 deletions Sources/FluentKit/Query/Builder/QueryBuilder+Aggregate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ extension QueryBuilder {
if Model().anyID is AnyQueryableProperty {
return self.count(\._$id)
} else if let fieldsIDType = Model.IDValue.self as? Fields.Type {
return self.aggregate(.count, (fieldsIDType.init().properties.first! as! AnyQueryAddressableProperty).anyQueryableProperty.path)
return self.aggregate(.count, fieldsIDType.keys.first!)
} else {
fatalError("Model '\(Model.self)' has neither @ID nor @CompositeID, this is not valid.")
}
Expand Down Expand Up @@ -158,6 +158,18 @@ extension QueryBuilder {
as type: Result.Type = Result.self
) -> EventLoopFuture<Result>
where Result: Codable
{
self.aggregate(.field(
.extendedPath(path, schema: Model.schemaOrAlias, space: Model.spaceIfNotAliased),
method
))
}

public func aggregate<Result>(
_ aggregate: DatabaseQuery.Aggregate,
as type: Result.Type = Result.self
) -> EventLoopFuture<Result>
where Result: Codable
{
let copy = self.copy()
// Remove all eager load requests otherwise we try to
Expand All @@ -169,12 +181,7 @@ extension QueryBuilder {
copy.query.sorts = []

// Set custom action.
copy.query.action = .aggregate(
.field(
.extendedPath(path, schema: Model.schemaOrAlias, space: Model.spaceIfNotAliased),
method
)
)
copy.query.action = .aggregate(aggregate)

let promise = self.database.eventLoop.makePromise(of: Result.self)
copy.run { output in
Expand Down
12 changes: 2 additions & 10 deletions Sources/FluentKit/Query/Builder/QueryBuilder+Filter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,7 @@ extension QueryBuilder {
@discardableResult
internal func filter(id: Model.IDValue) -> Self {
if let fields = id as? Fields {
assert(!(Model.init().anyID is AnyQueryableProperty), "Model's IDValue should not conform to Fields if it can be directly queried.")
return self.group(.and) { query in
_ = fields.properties.map { $0 as! AnyQueryAddressableProperty }.reduce(query) { query, prop in
prop.anyQueryableProperty.queryableValue().map {
query.filter(.extendedPath(prop.queryablePath, schema: Model.schemaOrAlias, space: Model.spaceIfNotAliased), .equal, $0)
} ?? query
}
}
return self.group(.and) { fields.input(to: QueryFilterInput(builder: $0)) }
} else {
return self.filter(\Model._$id == id)
}
Expand All @@ -21,8 +14,7 @@ extension QueryBuilder {
internal func filter(ids: [Model.IDValue]) -> Self {
guard let firstId = ids.first else { return self.limit(0) }
if firstId is Fields {
assert(!(Model.init().anyID is AnyQueryableProperty), "Model's IDValue should not conform to Fields if it can be directly queried.")
return self.group(.or) { _ = ids.reduce($0) { $0.filter(id: $1) } }
return self.group(.or) { q in ids.forEach { id in q.group(.and) { (id as! Fields).input(to: QueryFilterInput(builder: $0)) } } }
} else {
return self.filter(\Model._$id ~~ ids)
}
Expand Down

0 comments on commit 636a59e

Please sign in to comment.