From c16b226c279570a21d5bc913ddbf58db925db6da Mon Sep 17 00:00:00 2001 From: Diego Costantino Date: Thu, 10 Mar 2022 09:56:38 -0800 Subject: [PATCH 1/9] fix(datastore): fix has-one associations --- .../Schema/ModelField+Association.swift | 14 +++- .../Storage/SQLite/ModelSchema+SQLite.swift | 20 ++++- .../Storage/SQLite/SQLStatement+Select.swift | 7 +- .../DataStoreConnectionScenario1V2Tests.swift | 46 ++++------- .../Core/SQLStatementTests.swift | 79 +++++++++++++++++++ 5 files changed, 132 insertions(+), 34 deletions(-) diff --git a/Amplify/Categories/DataStore/Model/Internal/Schema/ModelField+Association.swift b/Amplify/Categories/DataStore/Model/Internal/Schema/ModelField+Association.swift index 89df8c33e1..a3e10b812a 100644 --- a/Amplify/Categories/DataStore/Model/Internal/Schema/ModelField+Association.swift +++ b/Amplify/Categories/DataStore/Model/Internal/Schema/ModelField+Association.swift @@ -108,7 +108,6 @@ public enum ModelAssociation { public static func belongsTo(associatedWith: CodingKey?, targetName: String?) -> ModelAssociation { return .belongsTo(associatedFieldName: associatedWith?.stringValue, targetName: targetName) } - } extension ModelField { @@ -207,10 +206,19 @@ extension ModelField { /// application making any change to these `public` types should be backward compatible, otherwise it will be a /// breaking change. public var isAssociationOwner: Bool { - guard case .belongsTo = association else { + switch association { + case .belongsTo: + return true + case .hasOne: + // in case of a bi-directional association + // we pick the model with a belongs-to + if case .belongsTo = associatedField?.association { + return false + } + return true + default: return false } - return true } /// - Warning: Although this has `public` access, it is intended for internal & codegen use and should not be used diff --git a/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/ModelSchema+SQLite.swift b/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/ModelSchema+SQLite.swift index 9411c75e15..c4bd8cc219 100644 --- a/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/ModelSchema+SQLite.swift +++ b/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/ModelSchema+SQLite.swift @@ -107,7 +107,13 @@ extension ModelSchema { /// the owner of a foreign key to another `Model`. Fields that reference the inverse side of /// the relationship (i.e. the "one" side of a "one-to-many" relationship) are excluded. var columns: [ModelField] { - sortedFields.filter { !$0.hasAssociation || $0.isForeignKey } + let targetFields = Set(sortedFields.compactMap { $0.association?.targetName() }) + + // exclude explicit fields that are referenced as an association target + // (i.e. team: Team => teamId: ID) + return sortedFields.filter { + (!$0.hasAssociation || $0.isForeignKey) && !targetFields.contains($0.name) + } } /// Filter the fields that represent foreign keys. @@ -126,3 +132,15 @@ extension ModelSchema { return statement } } + +extension ModelAssociation { + func targetName() -> String? { + switch self { + case .belongsTo(_, let targetName), + .hasOne(_, let targetName): + return targetName + default: + return nil + } + } +} diff --git a/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/SQLStatement+Select.swift b/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/SQLStatement+Select.swift index 8b8ad91d5f..8773ac0407 100644 --- a/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/SQLStatement+Select.swift +++ b/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/SQLStatement+Select.swift @@ -28,7 +28,12 @@ struct SelectStatementMetadata { let tableName = modelSchema.name var columnMapping: ColumnMapping = [:] var columns = fields.map { field -> String in - columnMapping.updateValue((modelSchema, field), forKey: field.name) + /// use the target name instead of the actual field name + /// when a field represent a model association (i.e. team: Team => `teamId`) + let fieldName = field.hasAssociation + ? field.association?.targetName() ?? field.name + : field.name + columnMapping.updateValue((modelSchema, field), forKey: fieldName) return field.columnName(forNamespace: rootNamespace) + " as " + field.columnAlias() } diff --git a/AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginIntegrationTests/TransformerV2/DataStoreConnectionScenario1V2Tests.swift b/AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginIntegrationTests/TransformerV2/DataStoreConnectionScenario1V2Tests.swift index 13c303ea4a..2ffcd08dad 100644 --- a/AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginIntegrationTests/TransformerV2/DataStoreConnectionScenario1V2Tests.swift +++ b/AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginIntegrationTests/TransformerV2/DataStoreConnectionScenario1V2Tests.swift @@ -35,8 +35,7 @@ class DataStoreConnectionScenario1V2Tests: SyncEngineIntegrationV2TestBase { func testSaveTeamAndProjectSyncToCloud() throws { try startAmplifyAndWaitForSync() let team = Team1V2(name: "name1") - // TODO: No need to add the `team` into the project, it is using explicit field `project1V2TeamId` - let project = Project1V2(team: team, project1V2TeamId: team.id) + let project = Project1V2(team: team) let syncedTeamReceived = expectation(description: "received team from sync path") let syncProjectReceived = expectation(description: "received project from sync path") let hubListener = Amplify.Hub.listen(to: .dataStore, @@ -47,11 +46,13 @@ class DataStoreConnectionScenario1V2Tests: SyncEngineIntegrationV2TestBase { } if let syncedTeam = try? mutationEvent.decodeModel() as? Team1V2, syncedTeam.id == team.id { - XCTAssertTrue(syncedTeam == team) + XCTAssertEqual(syncedTeam, team) syncedTeamReceived.fulfill() } else if let syncedProject = try? mutationEvent.decodeModel() as? Project1V2, syncedProject.id == project.id { - XCTAssertTrue(syncedProject == project) + // don't compare associated models as mutation events only carry + // model basic properties + XCTAssertEqual(syncedProject.name, project.name) XCTAssertEqual(mutationEvent.version, 1) syncProjectReceived.fulfill() } @@ -92,11 +93,7 @@ class DataStoreConnectionScenario1V2Tests: SyncEngineIntegrationV2TestBase { return } XCTAssertEqual(queriedProject.id, project.id) - // TODO: Should the queried project eager load the has-one team? - // XCTAssertEqual(queriedProject.team, team) - // or - // access explicit field like this? - XCTAssertEqual(queriedProject.project1V2TeamId, team.id) + XCTAssertEqual(queriedProject.team, team) queriedProjectCompleted.fulfill() case .failure(let error): XCTFail("failed \(error)") @@ -109,12 +106,10 @@ class DataStoreConnectionScenario1V2Tests: SyncEngineIntegrationV2TestBase { try startAmplifyAndWaitForSync() let team = Team1V2(name: "name1") let anotherTeam = Team1V2(name: "name1") - // TODO: No need to add the `team` into the project, it is using explicit field `project1V2TeamId` - var project = Project1V2(team: team, project1V2TeamId: team.id) + var project = Project1V2(team: team) let expectedUpdatedProject = Project1V2(id: project.id, name: project.name, - team: anotherTeam, // Not needed - project1V2TeamId: anotherTeam.id) + team: anotherTeam) let syncUpdatedProjectReceived = expectation(description: "received updated project from sync path") let hubListener = Amplify.Hub.listen(to: .dataStore, eventName: HubPayload.EventName.DataStore.syncReceived) { payload in @@ -167,7 +162,7 @@ class DataStoreConnectionScenario1V2Tests: SyncEngineIntegrationV2TestBase { wait(for: [saveProjectCompleted], timeout: networkTimeout) let updateProjectCompleted = expectation(description: "save project completed") - project.project1V2TeamId = anotherTeam.id + project.team = anotherTeam Amplify.DataStore.save(project) { result in switch result { case .success: @@ -185,11 +180,7 @@ class DataStoreConnectionScenario1V2Tests: SyncEngineIntegrationV2TestBase { XCTAssertNotNil(queriedProjectOptional) if let queriedProject = queriedProjectOptional { XCTAssertEqual(queriedProject, project) - // TODO: Should the queried project eager load the has-one team? - // XCTAssertEqual(queriedProject.team, anotherTeam) - // or - // access explicit field like this? - XCTAssertEqual(queriedProject.project1V2TeamId, anotherTeam.id) + XCTAssertEqual(queriedProject.team, anotherTeam) } queriedProjectCompleted.fulfill() @@ -203,7 +194,7 @@ class DataStoreConnectionScenario1V2Tests: SyncEngineIntegrationV2TestBase { func testCreateUpdateDeleteAndGetProjectReturnsNil() throws { try startAmplifyAndWaitForSync() guard let team = saveTeam(name: "name"), - var project = saveProject(project1V2TeamId: team.id) else { + var project = saveProject(team: team) else { XCTFail("Could not save team and project") return } @@ -275,7 +266,7 @@ class DataStoreConnectionScenario1V2Tests: SyncEngineIntegrationV2TestBase { func testDeleteAndGetProjectReturnsNilWithSync() throws { try startAmplifyAndWaitForSync() guard let team = saveTeam(name: "name"), - let project = saveProject(project1V2TeamId: team.id) else { + let project = saveProject(team: team) else { XCTFail("Could not save team and project") return } @@ -367,7 +358,7 @@ class DataStoreConnectionScenario1V2Tests: SyncEngineIntegrationV2TestBase { func testDeleteWithValidCondition() throws { try startAmplifyAndWaitForSync() guard let team = saveTeam(name: "name"), - let project = saveProject(project1V2TeamId: team.id) else { + let project = saveProject(team: team) else { XCTFail("Could not save team and project") return } @@ -398,7 +389,7 @@ class DataStoreConnectionScenario1V2Tests: SyncEngineIntegrationV2TestBase { func testDeleteWithInvalidCondition() throws { try startAmplifyAndWaitForSync() guard let team = saveTeam(name: "name"), - let project = saveProject(project1V2TeamId: team.id) else { + let project = saveProject(team: team) else { XCTFail("Could not save team and project") return } @@ -436,7 +427,7 @@ class DataStoreConnectionScenario1V2Tests: SyncEngineIntegrationV2TestBase { XCTFail("Could not save team") return } - guard let project = saveProject(project1V2TeamId: team.id) else { + guard let project = saveProject(team: team) else { XCTFail("Could not save project") return } @@ -447,7 +438,7 @@ class DataStoreConnectionScenario1V2Tests: SyncEngineIntegrationV2TestBase { case .success(let projects): XCTAssertEqual(projects.count, 1) XCTAssertEqual(projects[0].id, project.id) - XCTAssertEqual(projects[0].project1V2TeamId, team.id) + XCTAssertEqual(projects[0].team, team) listProjectByTeamIDCompleted.fulfill() case .failure(let error): XCTFail("\(error)") @@ -475,9 +466,8 @@ class DataStoreConnectionScenario1V2Tests: SyncEngineIntegrationV2TestBase { func saveProject(id: String = UUID().uuidString, name: String? = nil, - project1V2TeamId: String?, team: Team1V2? = nil) -> Project1V2? { - let project = Project1V2(id: id, name: name, team: team, project1V2TeamId: project1V2TeamId) + let project = Project1V2(id: id, name: name, team: team) var result: Project1V2? let completeInvoked = expectation(description: "request completed") Amplify.DataStore.save(project) { event in @@ -506,7 +496,5 @@ extension Project1V2: Equatable { return lhs.id == rhs.id && lhs.name == rhs.name && lhs.project1V2TeamId == rhs.project1V2TeamId - // && lhs.team == rhs.team // TODO: Should the Project have the team? - } } diff --git a/AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginTests/Core/SQLStatementTests.swift b/AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginTests/Core/SQLStatementTests.swift index 0b96bc89a5..808d655e8f 100644 --- a/AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginTests/Core/SQLStatementTests.swift +++ b/AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginTests/Core/SQLStatementTests.swift @@ -26,6 +26,14 @@ class SQLStatementTests: XCTestCase { ModelRegistry.register(modelType: UserAccount.self) ModelRegistry.register(modelType: UserProfile.self) + // one-to-one associations with custom field + ModelRegistry.register(modelType: Project2V2.self) + ModelRegistry.register(modelType: Team2V2.self) + + // one-to-one associations with default field + ModelRegistry.register(modelType: Project1V2.self) + ModelRegistry.register(modelType: Team1V2.self) + // many-to-many association ModelRegistry.register(modelType: Author.self) ModelRegistry.register(modelType: Book.self) @@ -96,6 +104,52 @@ class SQLStatementTests: XCTestCase { XCTAssertEqual(statement.stringValue, expectedStatement) } + /// - Given: a `Model` type + /// - When: + /// - the model is of type `Project2V2` + /// - the model has a foreign key + /// - Then: + /// - check if the generated SQL statement is valid: + /// - contains a `foreign key`named `teamID` referencing `Team` + func testCreateTableFromModelWithForeignKeyReferencedByField() { + let statement = CreateTableStatement(modelSchema: Project2V2.schema) + let expectedStatement = """ + create table if not exists "Project2V2" ( + "id" text primary key not null, + "createdAt" text, + "name" text, + "updatedAt" text, + "teamID" text unique, + foreign key("teamID") references "Team2V2"("id") + on delete cascade + ); + """ + XCTAssertEqual(statement.stringValue, expectedStatement) + } + + /// - Given: a `Model` type + /// - When: + /// - the model is of type `Project2V2` + /// - the model has a foreign key + /// - Then: + /// - check if the generated SQL statement is valid: + /// - contains a `foreign key`named `teamID` referencing `Team` + func testCreateTableFromModelWithForeignKeyReferencedByDefaultField() { + let statement = CreateTableStatement(modelSchema: Project1V2.schema) + let expectedStatement = """ + create table if not exists "Project1V2" ( + "id" text primary key not null, + "createdAt" text, + "name" text, + "updatedAt" text, + "project1V2TeamId" text unique, + foreign key("project1V2TeamId") references "Team1V2"("id") + on delete cascade + ); + """ + XCTAssertEqual(statement.stringValue, expectedStatement) + } + /// - Given: a `Model` type /// - When: /// - the model is of type `UserAccount` @@ -222,6 +276,31 @@ class SQLStatementTests: XCTestCase { XCTAssertEqual(variables[3] as? String, post.id) } + /// - Given: a `Model` instance + /// - When: + /// - the model is of type `Project1V2` + /// - it has a reference to a `Team1V2` + /// - Then: + /// - check if the generated SQL statement is valid + /// - check if the variables match the expected values + /// - check if the `project1V2TeamId` matches `team.id` + func testInsertStatementFromModelWithHasOneForeignKey() { + let team = Team1V2(name: "A-Team") + let projectName = "Project1" + let project = Project1V2(name: projectName, team: team) + let statement = InsertStatement(model: project, modelSchema: project.schema) + + let expectedStatement = """ + insert into "Project1V2" ("id", "createdAt", "name", "updatedAt", "project1V2TeamId") + values (?, ?, ?, ?, ?) + """ + XCTAssertEqual(statement.stringValue, expectedStatement) + + let variables = statement.variables + XCTAssertEqual(variables[2] as? String, projectName) + XCTAssertEqual(variables[4] as? String, team.id) + } + // MARK: - Update Statements /// - Given: a `Model` instance From 02804792e566b6cf807cdbb3f210955a6532f130 Mon Sep 17 00:00:00 2001 From: Diego Costantino Date: Fri, 11 Mar 2022 18:03:08 -0800 Subject: [PATCH 2/9] fix(datastore): move associatedTargets --- .../Internal/Schema/ModelField+Association.swift | 12 ++++++++++++ .../Model/Internal/Schema/ModelSchema.swift | 5 +++++ .../Model/Support/Model+GraphQL.swift | 6 ++---- .../Model/Support/ModelSchema+GraphQL.swift | 2 +- .../Auth/AWSAuthServiceBehaviorTests.swift | 2 +- .../Storage/SQLite/ModelSchema+SQLite.swift | 16 +--------------- .../Mocks/MockAuthCategoryPlugin.swift | 2 +- 7 files changed, 23 insertions(+), 22 deletions(-) diff --git a/Amplify/Categories/DataStore/Model/Internal/Schema/ModelField+Association.swift b/Amplify/Categories/DataStore/Model/Internal/Schema/ModelField+Association.swift index a3e10b812a..f2407cb3bf 100644 --- a/Amplify/Categories/DataStore/Model/Internal/Schema/ModelField+Association.swift +++ b/Amplify/Categories/DataStore/Model/Internal/Schema/ModelField+Association.swift @@ -108,6 +108,18 @@ public enum ModelAssociation { public static func belongsTo(associatedWith: CodingKey?, targetName: String?) -> ModelAssociation { return .belongsTo(associatedFieldName: associatedWith?.stringValue, targetName: targetName) } + + /// Convenience method to access the `targetName` for those associations that have one (currently `.belongsTo` and `.hasOne`). + /// Returns `nil` for associations that don't have an explicit target name. + public func targetName() -> String? { + switch self { + case .belongsTo(_, let targetName), + .hasOne(_, let targetName): + return targetName + case .hasMany: + return nil + } + } } extension ModelField { diff --git a/Amplify/Categories/DataStore/Model/Internal/Schema/ModelSchema.swift b/Amplify/Categories/DataStore/Model/Internal/Schema/ModelSchema.swift index e76472f8c4..e44bbb4227 100644 --- a/Amplify/Categories/DataStore/Model/Internal/Schema/ModelSchema.swift +++ b/Amplify/Categories/DataStore/Model/Internal/Schema/ModelSchema.swift @@ -87,6 +87,9 @@ public struct ModelSchema { public let sortedFields: [ModelField] + /// Lazy view of associations target names + public var associationsTargets: Set + public var primaryKey: ModelField { guard let primaryKey = fields.first(where: { $1.isPrimaryKey }) else { preconditionFailure("Primary Key not defined for `\(name)`") @@ -110,6 +113,8 @@ public struct ModelSchema { self.fields = fields self.sortedFields = fields.sortedFields() + + self.associationsTargets = Set(sortedFields.compactMap { $0.association?.targetName() }) } public func field(withName name: String) -> ModelField? { diff --git a/AmplifyPlugins/Core/AWSPluginsCore/Model/Support/Model+GraphQL.swift b/AmplifyPlugins/Core/AWSPluginsCore/Model/Support/Model+GraphQL.swift index d01147dc1c..de0ef6d302 100644 --- a/AmplifyPlugins/Core/AWSPluginsCore/Model/Support/Model+GraphQL.swift +++ b/AmplifyPlugins/Core/AWSPluginsCore/Model/Support/Model+GraphQL.swift @@ -17,8 +17,7 @@ extension Model { /// used as the `input` of GraphQL related operations. func graphQLInputForMutation(_ modelSchema: ModelSchema) -> GraphQLInput { var input: GraphQLInput = [:] - modelSchema.fields.forEach { - let modelField = $0.value + modelSchema.sortedFields.forEach { modelField in // When the field is read-only don't add it to the GraphQL input object if modelField.isReadOnly { @@ -93,8 +92,7 @@ extension Model { private func fixHasOneAssociationsWithExplicitFieldOnModel(_ input: GraphQLInput, modelSchema: ModelSchema) -> GraphQLInput { var input = input - modelSchema.fields.forEach { - let modelField = $0.value + modelSchema.sortedFields.forEach { modelField in if case .model = modelField.type, case .hasOne = modelField.association, input.keys.contains(modelField.name) { diff --git a/AmplifyPlugins/Core/AWSPluginsCore/Model/Support/ModelSchema+GraphQL.swift b/AmplifyPlugins/Core/AWSPluginsCore/Model/Support/ModelSchema+GraphQL.swift index 38cf7f44a2..e9dcfc01d5 100644 --- a/AmplifyPlugins/Core/AWSPluginsCore/Model/Support/ModelSchema+GraphQL.swift +++ b/AmplifyPlugins/Core/AWSPluginsCore/Model/Support/ModelSchema+GraphQL.swift @@ -49,7 +49,7 @@ extension ModelSchema { /// The list of fields formatted for GraphQL usage. var graphQLFields: [ModelField] { sortedFields.filter { field in - !field.hasAssociation || field.isAssociationOwner + (!field.hasAssociation || field.isAssociationOwner) && !associationsTargets.contains(field.name) } } } diff --git a/AmplifyPlugins/Core/AWSPluginsCoreTests/Auth/AWSAuthServiceBehaviorTests.swift b/AmplifyPlugins/Core/AWSPluginsCoreTests/Auth/AWSAuthServiceBehaviorTests.swift index 8b336685ed..0790413d6a 100644 --- a/AmplifyPlugins/Core/AWSPluginsCoreTests/Auth/AWSAuthServiceBehaviorTests.swift +++ b/AmplifyPlugins/Core/AWSPluginsCoreTests/Auth/AWSAuthServiceBehaviorTests.swift @@ -98,7 +98,7 @@ class AWSAuthServiceBehaviorTests: XCTestCase { } } -fileprivate class _MockAWSAuthService: AWSAuthServiceBehavior { +private class _MockAWSAuthService: AWSAuthServiceBehavior { let identityID: () -> Result let userPoolAccessToken: () -> Result diff --git a/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/ModelSchema+SQLite.swift b/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/ModelSchema+SQLite.swift index c4bd8cc219..b6bba68763 100644 --- a/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/ModelSchema+SQLite.swift +++ b/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/ModelSchema+SQLite.swift @@ -107,12 +107,10 @@ extension ModelSchema { /// the owner of a foreign key to another `Model`. Fields that reference the inverse side of /// the relationship (i.e. the "one" side of a "one-to-many" relationship) are excluded. var columns: [ModelField] { - let targetFields = Set(sortedFields.compactMap { $0.association?.targetName() }) - // exclude explicit fields that are referenced as an association target // (i.e. team: Team => teamId: ID) return sortedFields.filter { - (!$0.hasAssociation || $0.isForeignKey) && !targetFields.contains($0.name) + (!$0.hasAssociation || $0.isForeignKey) && !associationsTargets.contains($0.name) } } @@ -132,15 +130,3 @@ extension ModelSchema { return statement } } - -extension ModelAssociation { - func targetName() -> String? { - switch self { - case .belongsTo(_, let targetName), - .hasOne(_, let targetName): - return targetName - default: - return nil - } - } -} diff --git a/AmplifyTestCommon/Mocks/MockAuthCategoryPlugin.swift b/AmplifyTestCommon/Mocks/MockAuthCategoryPlugin.swift index 8a2bbe0709..42e884db6e 100644 --- a/AmplifyTestCommon/Mocks/MockAuthCategoryPlugin.swift +++ b/AmplifyTestCommon/Mocks/MockAuthCategoryPlugin.swift @@ -69,7 +69,7 @@ class MockAuthCategoryPlugin: MessageReporter, AuthCategoryPlugin { listener: AuthSignOutOperation.ResultListener?) -> AuthSignOutOperation { fatalError() } - + public func deleteUser(listener: AuthDeleteUserOperation.ResultListener?) -> AuthDeleteUserOperation { fatalError() } From c7c1681c3897548bb86db6bf6b55baa95e189f21 Mon Sep 17 00:00:00 2001 From: Diego Costantino Date: Mon, 14 Mar 2022 10:59:43 -0700 Subject: [PATCH 3/9] fix(datastore): fix tests --- .../Model/Support/ModelSchema+GraphQL.swift | 2 +- .../GraphQLCreateMutationTests.swift | 74 +++++++++++++++++++ .../GraphQLDeleteMutationTests.swift | 12 +++ .../GraphQLGetQueryTests.swift | 9 +++ .../GraphQLUpdateMutationTests.swift | 12 +++ 5 files changed, 108 insertions(+), 1 deletion(-) diff --git a/AmplifyPlugins/Core/AWSPluginsCore/Model/Support/ModelSchema+GraphQL.swift b/AmplifyPlugins/Core/AWSPluginsCore/Model/Support/ModelSchema+GraphQL.swift index e9dcfc01d5..ea798e1f27 100644 --- a/AmplifyPlugins/Core/AWSPluginsCore/Model/Support/ModelSchema+GraphQL.swift +++ b/AmplifyPlugins/Core/AWSPluginsCore/Model/Support/ModelSchema+GraphQL.swift @@ -49,7 +49,7 @@ extension ModelSchema { /// The list of fields formatted for GraphQL usage. var graphQLFields: [ModelField] { sortedFields.filter { field in - (!field.hasAssociation || field.isAssociationOwner) && !associationsTargets.contains(field.name) + !field.hasAssociation || field.isAssociationOwner /*&& !associationsTargets.contains(field.name)*/ } } } diff --git a/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/GraphQLDocument/GraphQLCreateMutationTests.swift b/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/GraphQLDocument/GraphQLCreateMutationTests.swift index de2ab68bfe..de248e724c 100644 --- a/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/GraphQLDocument/GraphQLCreateMutationTests.swift +++ b/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/GraphQLDocument/GraphQLCreateMutationTests.swift @@ -16,6 +16,10 @@ class GraphQLCreateMutationTests: XCTestCase { override func setUp() { ModelRegistry.register(modelType: Comment.self) ModelRegistry.register(modelType: Post.self) + ModelRegistry.register(modelType: Project2V2.self) + ModelRegistry.register(modelType: Team2V2.self) + ModelRegistry.register(modelType: Record.self) + ModelRegistry.register(modelType: RecordCover.self) } override func tearDown() { @@ -236,6 +240,66 @@ class GraphQLCreateMutationTests: XCTestCase { XCTAssertEqual(input["commentPostId"] as? String, post.id) } + /// - Given: a `Model` instance + /// - When: + /// - the model is of type `Project2V2` + /// - the model has an `hasOne` associations + /// - the mutation is of type `.create` + /// - Then: + /// - check if the generated GraphQL document is a valid mutation: + /// - it is named `CreateProject2` + /// - it contains an `input` of type `CreateProject2V2Input` + /// - it has a list of fields with a `teamId` + func testCreateGraphQLMutationFromModelWithHasOneAssociationWithSyncEnabled() { + let team = Team1V2(name: "team1v2") + let project = Project1V2(name: "project1v2", team: team) + + var documentBuilder = ModelBasedGraphQLDocumentBuilder(modelSchema: Project2V2.schema, + operationType: .mutation) + documentBuilder.add(decorator: DirectiveNameDecorator(type: .create)) + documentBuilder.add(decorator: ModelDecorator(model: project)) + documentBuilder.add(decorator: ConflictResolutionDecorator()) + let document = documentBuilder.build() + let expectedQueryDocument = """ + mutation CreateProject2V2($input: CreateProject2V2Input!) { + createProject2V2(input: $input) { + id + createdAt + name + teamID + updatedAt + team { + id + createdAt + name + updatedAt + __typename + _version + _deleted + _lastChangedAt + } + __typename + _version + _deleted + _lastChangedAt + } + } + """ + XCTAssertEqual(document.name, "createProject2V2") + XCTAssertEqual(document.stringValue, expectedQueryDocument) + guard let variables = document.variables else { + XCTFail("The document doesn't contain variables") + return + } + guard let input = variables["input"] as? GraphQLInput else { + XCTFail("Variables should contain a valid input") + return + } + XCTAssertEqual(input["id"] as? String, project.id) + XCTAssertEqual(input["name"] as? String, project.name) + XCTAssertEqual(input["teamID"] as? String, team.id) + } + func testCreateGraphQLMutationFromModelWithReadonlyFields() { let recordCover = RecordCover(artist: "artist") let record = Record(name: "name", description: "description", cover: recordCover) @@ -254,6 +318,16 @@ class GraphQLCreateMutationTests: XCTestCase { description name updatedAt + cover { + id + artist + createdAt + updatedAt + __typename + _version + _deleted + _lastChangedAt + } __typename _version _deleted diff --git a/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/GraphQLDocument/GraphQLDeleteMutationTests.swift b/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/GraphQLDocument/GraphQLDeleteMutationTests.swift index 951406fc95..7eca8f36f7 100644 --- a/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/GraphQLDocument/GraphQLDeleteMutationTests.swift +++ b/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/GraphQLDocument/GraphQLDeleteMutationTests.swift @@ -16,6 +16,8 @@ class GraphQLDeleteMutationTests: XCTestCase { override func setUp() { ModelRegistry.register(modelType: Comment.self) ModelRegistry.register(modelType: Post.self) + ModelRegistry.register(modelType: Record.self) + ModelRegistry.register(modelType: RecordCover.self) } override func tearDown() { @@ -137,6 +139,16 @@ class GraphQLDeleteMutationTests: XCTestCase { description name updatedAt + cover { + id + artist + createdAt + updatedAt + __typename + _version + _deleted + _lastChangedAt + } __typename _version _deleted diff --git a/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/GraphQLDocument/GraphQLGetQueryTests.swift b/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/GraphQLDocument/GraphQLGetQueryTests.swift index 5fc104dbcf..04a9b3836a 100644 --- a/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/GraphQLDocument/GraphQLGetQueryTests.swift +++ b/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/GraphQLDocument/GraphQLGetQueryTests.swift @@ -16,6 +16,8 @@ class GraphQLGetQueryTests: XCTestCase { override func setUp() { ModelRegistry.register(modelType: Comment.self) ModelRegistry.register(modelType: Post.self) + ModelRegistry.register(modelType: Record.self) + ModelRegistry.register(modelType: RecordCover.self) } /// - Given: a `Model` type @@ -198,6 +200,13 @@ class GraphQLGetQueryTests: XCTestCase { description name updatedAt + cover { + id + artist + createdAt + updatedAt + __typename + } __typename } } diff --git a/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/GraphQLDocument/GraphQLUpdateMutationTests.swift b/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/GraphQLDocument/GraphQLUpdateMutationTests.swift index 075c289683..04fc445c68 100644 --- a/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/GraphQLDocument/GraphQLUpdateMutationTests.swift +++ b/AmplifyPlugins/Core/AWSPluginsCoreTests/Model/GraphQLDocument/GraphQLUpdateMutationTests.swift @@ -16,6 +16,8 @@ class GraphQLUpdateMutationTests: XCTestCase { override func setUp() { ModelRegistry.register(modelType: Comment.self) ModelRegistry.register(modelType: Post.self) + ModelRegistry.register(modelType: Record.self) + ModelRegistry.register(modelType: RecordCover.self) } override func tearDown() { @@ -141,6 +143,16 @@ class GraphQLUpdateMutationTests: XCTestCase { description name updatedAt + cover { + id + artist + createdAt + updatedAt + __typename + _version + _deleted + _lastChangedAt + } __typename _version _deleted From c65ae8fe2b815d0f864b29823b01473b9afe8a61 Mon Sep 17 00:00:00 2001 From: Diego Costantino Date: Mon, 14 Mar 2022 11:33:54 -0700 Subject: [PATCH 4/9] fix(datastore): cleanup --- .../Core/AWSPluginsCore/Model/Support/ModelSchema+GraphQL.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AmplifyPlugins/Core/AWSPluginsCore/Model/Support/ModelSchema+GraphQL.swift b/AmplifyPlugins/Core/AWSPluginsCore/Model/Support/ModelSchema+GraphQL.swift index ea798e1f27..38cf7f44a2 100644 --- a/AmplifyPlugins/Core/AWSPluginsCore/Model/Support/ModelSchema+GraphQL.swift +++ b/AmplifyPlugins/Core/AWSPluginsCore/Model/Support/ModelSchema+GraphQL.swift @@ -49,7 +49,7 @@ extension ModelSchema { /// The list of fields formatted for GraphQL usage. var graphQLFields: [ModelField] { sortedFields.filter { field in - !field.hasAssociation || field.isAssociationOwner /*&& !associationsTargets.contains(field.name)*/ + !field.hasAssociation || field.isAssociationOwner } } } From 08293994375ebbc093653e37b0f66c28ef5d0b92 Mon Sep 17 00:00:00 2001 From: Diego Costantino Date: Tue, 15 Mar 2022 18:12:00 -0700 Subject: [PATCH 5/9] fix(datastore): uniqueColumns attribute for backward compatibility --- .../Storage/SQLite/ModelSchema+SQLite.swift | 15 ++++++++++++--- .../Storage/SQLite/SQLStatement+CreateTable.swift | 3 +-- .../Storage/SQLite/SQLStatement+Insert.swift | 5 +++-- .../Storage/SQLite/SQLStatement+Select.swift | 7 +------ 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/ModelSchema+SQLite.swift b/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/ModelSchema+SQLite.swift index b6bba68763..1de42b0542 100644 --- a/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/ModelSchema+SQLite.swift +++ b/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/ModelSchema+SQLite.swift @@ -107,13 +107,22 @@ extension ModelSchema { /// the owner of a foreign key to another `Model`. Fields that reference the inverse side of /// the relationship (i.e. the "one" side of a "one-to-many" relationship) are excluded. var columns: [ModelField] { - // exclude explicit fields that are referenced as an association target - // (i.e. team: Team => teamId: ID) return sortedFields.filter { - (!$0.hasAssociation || $0.isForeignKey) && !associationsTargets.contains($0.name) + !$0.hasAssociation || $0.isForeignKey } } + /// This is a temporary workaround to circumvent a Codegen issue where + /// both the field representing an association and its target are explicitly emitted. + /// Warning: don't use it unless necessary, it will be removed in future releases. + /// Returns fields that represent actual columns on the SQL table. + /// It also exclude explicit fields that are referenced as an association target + /// (i.e. team: Team => teamId: ID). + var columnsUnique: [ModelField] { + return columns + .filter { !associationsTargets.contains($0.name) } + } + /// Filter the fields that represent foreign keys. var foreignKeys: [ModelField] { sortedFields.filter { $0.isForeignKey } diff --git a/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/SQLStatement+CreateTable.swift b/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/SQLStatement+CreateTable.swift index 1b8b9cd6c5..fcee2697a6 100644 --- a/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/SQLStatement+CreateTable.swift +++ b/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/SQLStatement+CreateTable.swift @@ -20,8 +20,7 @@ struct CreateTableStatement: SQLStatement { var stringValue: String { let name = modelSchema.name var statement = #"create table if not exists "\#(name)" (\#n"# - - let columns = modelSchema.columns + let columns = modelSchema.columnsUnique let foreignKeys = modelSchema.foreignKeys for (index, column) in columns.enumerated() { diff --git a/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/SQLStatement+Insert.swift b/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/SQLStatement+Insert.swift index 210b886ea5..60c9de7a4d 100644 --- a/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/SQLStatement+Insert.swift +++ b/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/SQLStatement+Insert.swift @@ -16,11 +16,12 @@ struct InsertStatement: SQLStatement { init(model: Model, modelSchema: ModelSchema) { self.modelSchema = modelSchema - self.variables = model.sqlValues(for: modelSchema.columns, modelSchema: modelSchema) + self.variables = model.sqlValues(for: modelSchema.columnsUnique, + modelSchema: modelSchema) } var stringValue: String { - let fields = modelSchema.columns + let fields = modelSchema.columnsUnique let columns = fields.map { $0.columnName() } var statement = "insert into \"\(modelSchema.name)\" " statement += "(\(columns.joined(separator: ", ")))\n" diff --git a/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/SQLStatement+Select.swift b/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/SQLStatement+Select.swift index 8773ac0407..8b8ad91d5f 100644 --- a/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/SQLStatement+Select.swift +++ b/AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/SQLStatement+Select.swift @@ -28,12 +28,7 @@ struct SelectStatementMetadata { let tableName = modelSchema.name var columnMapping: ColumnMapping = [:] var columns = fields.map { field -> String in - /// use the target name instead of the actual field name - /// when a field represent a model association (i.e. team: Team => `teamId`) - let fieldName = field.hasAssociation - ? field.association?.targetName() ?? field.name - : field.name - columnMapping.updateValue((modelSchema, field), forKey: fieldName) + columnMapping.updateValue((modelSchema, field), forKey: field.name) return field.columnName(forNamespace: rootNamespace) + " as " + field.columnAlias() } From d5c647af525aa713893d8caf7db9addca7a85ddd Mon Sep 17 00:00:00 2001 From: Diego Costantino Date: Tue, 15 Mar 2022 18:12:23 -0700 Subject: [PATCH 6/9] fix Flutter tests to use the proper model --- .../Connection/DataStoreConnectionScenario2FlutterTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginFlutterIntegrationTests/Connection/DataStoreConnectionScenario2FlutterTests.swift b/AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginFlutterIntegrationTests/Connection/DataStoreConnectionScenario2FlutterTests.swift index 44866d3fff..79acd8ae5b 100644 --- a/AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginFlutterIntegrationTests/Connection/DataStoreConnectionScenario2FlutterTests.swift +++ b/AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginFlutterIntegrationTests/Connection/DataStoreConnectionScenario2FlutterTests.swift @@ -58,7 +58,7 @@ class DataStoreConnectionScenario2FlutterTests: SyncEngineFlutterIntegrationTest return } let saveTeamCompleted = expectation(description: "save team completed") - plugin.save(team.model, modelSchema: Team1.schema) { result in + plugin.save(team.model, modelSchema: Team2.schema) { result in switch result { case .success: saveTeamCompleted.fulfill() From 152c04db82587c7967efe35d407ca1dfb3880ec5 Mon Sep 17 00:00:00 2001 From: Diego Costantino Date: Tue, 15 Mar 2022 18:12:50 -0700 Subject: [PATCH 7/9] fix tests --- .../DataStoreConnectionScenario1V2Tests.swift | 6 +++--- .../DataStoreConnectionScenario2V2Tests.swift | 8 ++++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginIntegrationTests/TransformerV2/DataStoreConnectionScenario1V2Tests.swift b/AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginIntegrationTests/TransformerV2/DataStoreConnectionScenario1V2Tests.swift index 2ffcd08dad..2f4e19f6b4 100644 --- a/AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginIntegrationTests/TransformerV2/DataStoreConnectionScenario1V2Tests.swift +++ b/AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginIntegrationTests/TransformerV2/DataStoreConnectionScenario1V2Tests.swift @@ -120,7 +120,7 @@ class DataStoreConnectionScenario1V2Tests: SyncEngineIntegrationV2TestBase { if let syncedUpdatedProject = try? mutationEvent.decodeModel() as? Project1V2, syncedUpdatedProject.id == expectedUpdatedProject.id, - syncedUpdatedProject.project1V2TeamId == expectedUpdatedProject.project1V2TeamId { + syncedUpdatedProject.team?.id == expectedUpdatedProject.team?.id { syncUpdatedProjectReceived.fulfill() } } @@ -270,7 +270,7 @@ class DataStoreConnectionScenario1V2Tests: SyncEngineIntegrationV2TestBase { XCTFail("Could not save team and project") return } - let createReceived = expectation(description: "received created items from cloud") + let createReceived = expectation(description: "Received created items from cloud") createReceived.expectedFulfillmentCount = 2 // 1 project and 1 team let deleteReceived = expectation(description: "Delete notification received") deleteReceived.expectedFulfillmentCount = 2 // 1 project and 1 team @@ -495,6 +495,6 @@ extension Project1V2: Equatable { public static func == (lhs: Project1V2, rhs: Project1V2) -> Bool { return lhs.id == rhs.id && lhs.name == rhs.name - && lhs.project1V2TeamId == rhs.project1V2TeamId + // && lhs.project1V2TeamId == rhs.project1V2TeamId } } diff --git a/AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginIntegrationTests/TransformerV2/DataStoreConnectionScenario2V2Tests.swift b/AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginIntegrationTests/TransformerV2/DataStoreConnectionScenario2V2Tests.swift index 6550b08ebb..b9b5ddf6f5 100644 --- a/AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginIntegrationTests/TransformerV2/DataStoreConnectionScenario2V2Tests.swift +++ b/AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginIntegrationTests/TransformerV2/DataStoreConnectionScenario2V2Tests.swift @@ -101,8 +101,12 @@ class DataStoreConnectionScenario2V2Tests: SyncEngineIntegrationV2TestBase { try startAmplifyAndWaitForSync() let team = Team2V2(name: "name1") let anotherTeam = Team2V2(name: "name1") - var project = Project2V2(teamID: team.id, team: team) - let expectedUpdatedProject = Project2V2(id: project.id, name: project.name, teamID: anotherTeam.id) + var project = Project2V2(teamID: team.id, + team: team) + let expectedUpdatedProject = Project2V2(id: project.id, + name: project.name, + teamID: anotherTeam.id, + team: anotherTeam) let syncUpdatedProjectReceived = expectation(description: "received updated project from sync path") let hubListener = Amplify.Hub.listen(to: .dataStore, eventName: HubPayload.EventName.DataStore.syncReceived) { payload in From b81b8be931e685690b9155c316ef091843bcdad7 Mon Sep 17 00:00:00 2001 From: Diego Costantino Date: Wed, 16 Mar 2022 12:26:38 -0700 Subject: [PATCH 8/9] add test1 --- AmplifyPlugins/API/Podfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AmplifyPlugins/API/Podfile.lock b/AmplifyPlugins/API/Podfile.lock index 63746a349b..11e342ff88 100644 --- a/AmplifyPlugins/API/Podfile.lock +++ b/AmplifyPlugins/API/Podfile.lock @@ -116,4 +116,4 @@ SPEC CHECKSUMS: PODFILE CHECKSUM: 54e87158e45936fe60756d4a417b89eb64ea1c51 -COCOAPODS: 1.10.1 +COCOAPODS: 1.11.2 From 6f2f1cb3cb809ecd36f84fd0833eaafd677ab783 Mon Sep 17 00:00:00 2001 From: Diego Costantino Date: Wed, 16 Mar 2022 14:47:45 -0700 Subject: [PATCH 9/9] update tests --- .../TransformerV2/DataStoreConnectionScenario1V2Tests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginIntegrationTests/TransformerV2/DataStoreConnectionScenario1V2Tests.swift b/AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginIntegrationTests/TransformerV2/DataStoreConnectionScenario1V2Tests.swift index 2f4e19f6b4..73a8985122 100644 --- a/AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginIntegrationTests/TransformerV2/DataStoreConnectionScenario1V2Tests.swift +++ b/AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginIntegrationTests/TransformerV2/DataStoreConnectionScenario1V2Tests.swift @@ -495,6 +495,6 @@ extension Project1V2: Equatable { public static func == (lhs: Project1V2, rhs: Project1V2) -> Bool { return lhs.id == rhs.id && lhs.name == rhs.name - // && lhs.project1V2TeamId == rhs.project1V2TeamId + && lhs.team == rhs.team } }