Skip to content

Commit b4e0abf

Browse files
ruiguoamzruiguoamz
authored
fix(Model): retrieve correct associated target name (#965)
* another option for the wrong columnName fix * updated Graphql level logic * addressed some comments * refactor * protocol changes * rename graphQLFilter to getGraphQLFilter * changed columnMethod a little bit * increase code coverage * addressed some of John's comments * Updated method resolveColumn * To address team discussion comments * addressed one comment about a unit test in DataStore Co-authored-by: Guo <[email protected]>
1 parent 9b9bd97 commit b4e0abf

File tree

17 files changed

+299
-53
lines changed

17 files changed

+299
-53
lines changed

‎Amplify/Categories/DataStore/Model/Collection/List+LazyLoad.swift

+3-11
Original file line numberDiff line numberDiff line change
@@ -31,21 +31,13 @@ extension List {
3131
internal func lazyLoad(_ completion: DataStoreCallback<Elements>) {
3232

3333
// if the collection has no associated field, return the current elements
34-
guard let associatedId = self.associatedId,
35-
let associatedField = self.associatedField else {
34+
guard let associatedId = associatedId,
35+
let associatedField = associatedField else {
3636
completion(.success(elements))
3737
return
3838
}
3939

40-
// TODO: this is currently done by specific plugin implementations (API or DataStore)
41-
// How to add this name resolution to Amplify?
42-
let modelName = Element.modelName
43-
var name = modelName.camelCased() + associatedField.name.pascalCased() + "Id"
44-
if case let .belongsTo(_, targetName) = associatedField.association {
45-
name = targetName ?? name
46-
}
47-
48-
let predicate: QueryPredicate = field(name) == associatedId
40+
let predicate: QueryPredicate = field(associatedField.name) == associatedId
4941
Amplify.DataStore.query(Element.self, where: predicate) {
5042
switch $0 {
5143
case .success(let elements):

‎AmplifyPlugins/API/AWSAPICategoryPluginFunctionalTests/GraphQLModelBased/GraphQLConnectionScenario1Tests.swift

-1
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,6 @@ class GraphQLConnectionScenario1Tests: XCTestCase {
171171
wait(for: [getProjectAfterDeleteCompleted], timeout: TestCommonConstants.networkTimeout)
172172
}
173173

174-
// TODO: This test will fail until https://github.com/aws-amplify/amplify-ios/pull/885 is merged in
175174
func testListProjectsByTeamID() {
176175
guard let team = createTeam(name: "name") else {
177176
XCTFail("Could not create team")

‎AmplifyPlugins/API/AWSAPICategoryPluginFunctionalTests/GraphQLModelBased/GraphQLConnectionScenario4Tests.swift

-1
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,6 @@ class GraphQLConnectionScenario4Tests: XCTestCase {
203203
wait(for: [getCommentAfterDeleteCompleted], timeout: TestCommonConstants.networkTimeout)
204204
}
205205

206-
// TODO: This test will fail until https://github.com/aws-amplify/amplify-ios/pull/885 is merged in
207206
func testListCommentsByPostID() {
208207
guard let post = createPost(title: "title") else {
209208
XCTFail("Could not create post")

‎AmplifyPlugins/API/AWSAPICategoryPluginFunctionalTests/GraphQLModelBased/GraphQLConnectionScenario5Tests.swift

-2
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ class GraphQLConnectionScenario5Tests: XCTestCase {
6565
Amplify.reset()
6666
}
6767

68-
// TODO: This test will fail until https://github.com/aws-amplify/amplify-ios/pull/885 is merged in
6968
func testListPostEditorByPost() {
7069
guard let post = createPost(title: "title") else {
7170
XCTFail("Could not create post")
@@ -98,7 +97,6 @@ class GraphQLConnectionScenario5Tests: XCTestCase {
9897
wait(for: [listPostEditorByPostIDCompleted], timeout: TestCommonConstants.networkTimeout)
9998
}
10099

101-
// TODO: This test will fail until https://github.com/aws-amplify/amplify-ios/pull/885 is merged in
102100
func testListPostEditorByUser() {
103101
guard let post = createPost(title: "title") else {
104102
XCTFail("Could not create post")

‎AmplifyPlugins/Core/AWSPluginsCore/Model/GraphQLRequest/GraphQLRequest+AnyModelWithSync.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ extension GraphQLRequest: ModelSyncGraphQLRequestFactory {
171171
operationType: .query)
172172
documentBuilder.add(decorator: DirectiveNameDecorator(type: .sync))
173173
if let predicate = predicate {
174-
documentBuilder.add(decorator: FilterDecorator(filter: predicate.graphQLFilter))
174+
documentBuilder.add(decorator: FilterDecorator(filter: predicate.graphQLFilter(for: modelSchema)))
175175
}
176176
documentBuilder.add(decorator: PaginationDecorator(limit: limit, nextToken: nextToken))
177177
documentBuilder.add(decorator: ConflictResolutionDecorator(lastSync: lastSync))

‎AmplifyPlugins/Core/AWSPluginsCore/Model/GraphQLRequest/GraphQLRequest+Model.swift

+3-3
Original file line numberDiff line numberDiff line change
@@ -169,12 +169,12 @@ extension GraphQLRequest: ModelGraphQLRequestFactory {
169169
case .delete:
170170
documentBuilder.add(decorator: ModelIdDecorator(id: model.id))
171171
if let predicate = predicate {
172-
documentBuilder.add(decorator: FilterDecorator(filter: predicate.graphQLFilter))
172+
documentBuilder.add(decorator: FilterDecorator(filter: predicate.graphQLFilter(for: modelSchema)))
173173
}
174174
case .update:
175175
documentBuilder.add(decorator: ModelDecorator(model: model))
176176
if let predicate = predicate {
177-
documentBuilder.add(decorator: FilterDecorator(filter: predicate.graphQLFilter))
177+
documentBuilder.add(decorator: FilterDecorator(filter: predicate.graphQLFilter(for: modelSchema)))
178178
}
179179
}
180180

@@ -206,7 +206,7 @@ extension GraphQLRequest: ModelGraphQLRequestFactory {
206206
documentBuilder.add(decorator: DirectiveNameDecorator(type: .list))
207207

208208
if let predicate = predicate {
209-
documentBuilder.add(decorator: FilterDecorator(filter: predicate.graphQLFilter))
209+
documentBuilder.add(decorator: FilterDecorator(filter: predicate.graphQLFilter(for: modelType.schema)))
210210
}
211211

212212
documentBuilder.add(decorator: PaginationDecorator())

‎AmplifyPlugins/Core/AWSPluginsCore/Model/Support/QueryPredicate+GraphQL.swift

+69-8
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,35 @@ import Amplify
1111
public typealias GraphQLFilter = [String: Any]
1212

1313
protocol GraphQLFilterConvertible {
14-
var graphQLFilter: GraphQLFilter { get }
14+
func graphQLFilter(for modelSchema: ModelSchema?) -> GraphQLFilter
1515
}
1616

1717
// Convert QueryPredicate to GraphQLFilter JSON, and GraphQLFilter JSON to GraphQLFilter
1818
public struct GraphQLFilterConverter {
1919

20+
/// Serialize the translated GraphQL query variable object to JSON string.
21+
/// - Warning: Although this has `public` access, it is intended for internal use and should not be used directly
22+
/// by host applications. The behavior of this may change without warning.
23+
public static func toJSON(_ queryPredicate: QueryPredicate,
24+
modelSchema: ModelSchema,
25+
options: JSONSerialization.WritingOptions = []) throws -> String {
26+
let graphQLFilterData =
27+
try JSONSerialization.data(withJSONObject: queryPredicate.graphQLFilter(for: modelSchema),
28+
options: options)
29+
30+
guard let serializedString = String(data: graphQLFilterData, encoding: .utf8) else {
31+
preconditionFailure("""
32+
Could not initialize String from the GraphQL representation of QueryPredicate:
33+
\(String(describing: graphQLFilterData))
34+
""")
35+
}
36+
37+
return serializedString
38+
}
39+
40+
@available(*, deprecated, message: """
41+
Use `toJSON(_:modelSchema:options)` instead. See https://github.com/aws-amplify/amplify-ios/pull/965 for more details.
42+
""")
2043
/// Serialize the translated GraphQL query variable object to JSON string.
2144
public static func toJSON(_ queryPredicate: QueryPredicate,
2245
options: JSONSerialization.WritingOptions = []) throws -> String {
@@ -47,11 +70,27 @@ public struct GraphQLFilterConverter {
4770
/// Extension to translate a `QueryPredicate` into a GraphQL query variables object
4871
extension QueryPredicate {
4972

73+
/// - Warning: Although this has `public` access, it is intended for internal use and should not be used directly
74+
/// by host applications. The behavior of this may change without warning.
75+
public func graphQLFilter(for modelSchema: ModelSchema?) -> GraphQLFilter {
76+
if let operation = self as? QueryPredicateOperation {
77+
return operation.graphQLFilter(for: modelSchema)
78+
} else if let group = self as? QueryPredicateGroup {
79+
return group.graphQLFilter(for: modelSchema)
80+
}
81+
82+
preconditionFailure(
83+
"Could not find QueryPredicateOperation or QueryPredicateGroup for \(String(describing: self))")
84+
}
85+
86+
@available(*, deprecated, message: """
87+
Use `graphQLFilter(for:)` instead. See https://github.com/aws-amplify/amplify-ios/pull/965 for more details.
88+
""")
5089
public var graphQLFilter: GraphQLFilter {
5190
if let operation = self as? QueryPredicateOperation {
52-
return operation.graphQLFilter
91+
return operation.graphQLFilter(for: nil)
5392
} else if let group = self as? QueryPredicateGroup {
54-
return group.graphQLFilter
93+
return group.graphQLFilter(for: nil)
5594
}
5695

5796
preconditionFailure(
@@ -60,30 +99,52 @@ extension QueryPredicate {
6099
}
61100

62101
extension QueryPredicateOperation: GraphQLFilterConvertible {
63-
var graphQLFilter: GraphQLFilter {
64-
return [field: [self.operator.graphQLOperator: self.operator.value]]
102+
103+
func graphQLFilter(for modelSchema: ModelSchema?) -> GraphQLFilter {
104+
let filterValue = [self.operator.graphQLOperator: self.operator.value]
105+
guard let modelSchema = modelSchema else {
106+
return [field: filterValue]
107+
}
108+
return [columnName(modelSchema): filterValue]
109+
}
110+
111+
func columnName(_ modelSchema: ModelSchema) -> String {
112+
guard let modelField = modelSchema.field(withName: field) else {
113+
return field
114+
}
115+
let defaultFieldName = modelSchema.name.camelCased() + field.pascalCased() + "Id"
116+
switch modelField.association {
117+
case .belongsTo(_, let targetName):
118+
return targetName ?? defaultFieldName
119+
case .hasOne(_, let targetName):
120+
return targetName ?? defaultFieldName
121+
default:
122+
return field
123+
}
65124
}
66125
}
67126

68127
extension QueryPredicateGroup: GraphQLFilterConvertible {
69-
var graphQLFilter: GraphQLFilter {
128+
129+
func graphQLFilter(for modelSchema: ModelSchema?) -> GraphQLFilter {
70130
let logicalOperator = type.rawValue
71131
switch type {
72132
case .and, .or:
73133
var graphQLPredicateOperation = [logicalOperator: [Any]()]
74134
predicates.forEach { predicate in
75-
graphQLPredicateOperation[logicalOperator]?.append(predicate.graphQLFilter)
135+
graphQLPredicateOperation[logicalOperator]?.append(predicate.graphQLFilter(for: modelSchema))
76136
}
77137
return graphQLPredicateOperation
78138
case .not:
79139
if let predicate = predicates.first {
80-
return [logicalOperator: predicate.graphQLFilter]
140+
return [logicalOperator: predicate.graphQLFilter(for: modelSchema)]
81141
} else {
82142
preconditionFailure("Missing predicate for \(String(describing: self)) with type: \(type)")
83143
}
84144
}
85145
}
86146
}
147+
87148
extension QueryOperator {
88149
var graphQLOperator: String {
89150
switch self {

‎AmplifyPlugins/Core/AWSPluginsCoreTests/Model/GraphQLDocument/GraphQLListQueryTests.swift

+63-2
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ class GraphQLListQueryTests: XCTestCase {
4343
var documentBuilder = ModelBasedGraphQLDocumentBuilder(modelSchema: Post.schema, operationType: .query)
4444
documentBuilder.add(decorator: DirectiveNameDecorator(type: .list))
4545
documentBuilder.add(decorator: PaginationDecorator())
46-
documentBuilder.add(decorator: FilterDecorator(filter: predicate.graphQLFilter))
46+
documentBuilder.add(decorator: FilterDecorator(filter: predicate.graphQLFilter(for: Post.schema)))
4747
let document = documentBuilder.build()
4848
let expectedQueryDocument = """
4949
query ListPosts($filter: ModelPostFilterInput, $limit: Int) {
@@ -115,14 +115,75 @@ class GraphQLListQueryTests: XCTestCase {
115115
XCTAssertEqual(String(data: filterJSON!, encoding: .utf8), expectedFilterJSON)
116116
}
117117

118+
func testComment4BelongsToPost4Success() {
119+
let comment4 = Comment4.keys
120+
let predicate = comment4.id == "comment4Id" && comment4.post == "post4Id"
121+
122+
var documentBuilder = ModelBasedGraphQLDocumentBuilder(modelSchema: Comment4.schema,
123+
operationType: .query)
124+
documentBuilder.add(decorator: DirectiveNameDecorator(type: .list))
125+
documentBuilder.add(decorator: PaginationDecorator())
126+
documentBuilder.add(decorator: FilterDecorator(filter: predicate.graphQLFilter(for: Comment4.schema)))
127+
let document = documentBuilder.build()
128+
let expectedQueryDocument = """
129+
query ListComment4s($filter: ModelComment4FilterInput, $limit: Int) {
130+
listComment4s(filter: $filter, limit: $limit) {
131+
items {
132+
id
133+
content
134+
postID
135+
__typename
136+
}
137+
nextToken
138+
}
139+
}
140+
"""
141+
XCTAssertEqual(document.name, "listComment4s")
142+
XCTAssertEqual(document.stringValue, expectedQueryDocument)
143+
guard let variables = document.variables else {
144+
XCTFail("The document doesn't contain variables")
145+
return
146+
}
147+
XCTAssertNotNil(variables["limit"])
148+
XCTAssertEqual(variables["limit"] as? Int, 1_000)
149+
150+
guard let filter = variables["filter"] as? GraphQLFilter else {
151+
XCTFail("variables should contain a valid filter")
152+
return
153+
}
154+
155+
// Test filter for a valid JSON format
156+
let filterJSON = try? JSONSerialization.data(withJSONObject: filter,
157+
options: .prettyPrinted)
158+
XCTAssertNotNil(filterJSON)
159+
160+
let expectedFilterJSON = """
161+
{
162+
"and" : [
163+
{
164+
"id" : {
165+
"eq" : "comment4Id"
166+
}
167+
},
168+
{
169+
"postID" : {
170+
"eq" : "post4Id"
171+
}
172+
}
173+
]
174+
}
175+
"""
176+
XCTAssertEqual(String(data: filterJSON!, encoding: .utf8), expectedFilterJSON)
177+
}
178+
118179
func testListGraphQLQueryFromSimpleModelWithSyncEnabled() {
119180
let post = Post.keys
120181
let predicate = post.id.eq("id") && (post.title.beginsWith("Title") || post.content.contains("content"))
121182

122183
var documentBuilder = ModelBasedGraphQLDocumentBuilder(modelSchema: Post.schema, operationType: .query)
123184
documentBuilder.add(decorator: DirectiveNameDecorator(type: .list))
124185
documentBuilder.add(decorator: PaginationDecorator())
125-
documentBuilder.add(decorator: FilterDecorator(filter: predicate.graphQLFilter))
186+
documentBuilder.add(decorator: FilterDecorator(filter: predicate.graphQLFilter(for: Post.schema)))
126187
documentBuilder.add(decorator: ConflictResolutionDecorator())
127188
let document = documentBuilder.build()
128189
let expectedQueryDocument = """

‎AmplifyPlugins/Core/AWSPluginsCoreTests/Model/GraphQLDocument/GraphQLSyncQueryTests.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ class GraphQLSyncQueryTests: XCTestCase {
3838

3939
var documentBuilder = ModelBasedGraphQLDocumentBuilder(modelSchema: Post.schema, operationType: .query)
4040
documentBuilder.add(decorator: DirectiveNameDecorator(type: .sync))
41-
documentBuilder.add(decorator: FilterDecorator(filter: predicate.graphQLFilter))
41+
documentBuilder.add(decorator: FilterDecorator(filter: predicate.graphQLFilter(for: Post.schema)))
4242
documentBuilder.add(decorator: PaginationDecorator(limit: 100, nextToken: "token"))
4343
documentBuilder.add(decorator: ConflictResolutionDecorator(lastSync: 123))
4444
let document = documentBuilder.build()

0 commit comments

Comments
 (0)