Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Foundational Work For Codegen Rewrite #1817

Merged
merged 13 commits into from
Jun 10, 2021
Merged

Foundational Work For Codegen Rewrite #1817

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
7a1170b
WIP
AnthonyMDev Jun 3, 2021
d968a46
WIP: ApolloCore - ApolloUtils + ApolloModels
AnthonyMDev Jun 3, 2021
5755e2d
Create ApolloModels
AnthonyMDev Jun 3, 2021
e3e6a38
Fix callsites in ApolloWebSocket and ApolloSQL
AnthonyMDev Jun 3, 2021
562951f
Fix callsites ApolloTestSupport
AnthonyMDev Jun 3, 2021
86ffccb
Move InputValue, GraphQLError, and CacheReference back to Apollo
AnthonyMDev Jun 3, 2021
3494a9d
Fixed callsites after moving JSON back to Apollo
AnthonyMDev Jun 3, 2021
90f1f73
Fix all callsites in tests
AnthonyMDev Jun 3, 2021
013a822
Setup Podspec
AnthonyMDev Jun 3, 2021
bb83598
Fix minor error
AnthonyMDev Jun 3, 2021
625886c
Minor cleanup
AnthonyMDev Jun 4, 2021
eb22a94
Fix codegen tests
AnthonyMDev Jun 4, 2021
001fd7e
Rename ApolloModels -> ApolloAPI
AnthonyMDev Jun 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Apollo.podspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ Pod::Spec.new do |s|
s.watchos.deployment_target = '5.0'

s.subspec 'Core' do |ss|
ss.source_files = 'Sources/Apollo/*.swift','Sources/ApolloCore/*.swift'
ss.source_files = 'Sources/Apollo/*.swift','Sources/ApolloUtils/*.swift','Sources/ApolloModels/*.swift'
ss.exclude_files = 'Sources/ApolloModels/CodegenV1/*.swift'
ss.preserve_paths = [
'scripts/run-bundled-codegen.sh',
]
Expand Down
248 changes: 183 additions & 65 deletions Apollo.xcodeproj/project.pbxproj
View file
Open in desktop

Large diffs are not rendered by default.

67 changes: 67 additions & 0 deletions Apollo.xcodeproj/xcshareddata/xcschemes/ApolloModels.xcscheme
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
<?xml version="1.0" encoding="UTF-8"?>
<Scheme
LastUpgradeVersion = "1250"
version = "1.3">
<BuildAction
parallelizeBuildables = "YES"
buildImplicitDependencies = "YES">
<BuildActionEntries>
<BuildActionEntry
buildForTesting = "YES"
buildForRunning = "YES"
buildForProfiling = "YES"
buildForArchiving = "YES"
buildForAnalyzing = "YES">
<BuildableReference
BuildableIdentifier = "primary"
BlueprintIdentifier = "DE058606266978A100265760"
BuildableName = "ApolloModels.framework"
BlueprintName = "ApolloModels"
ReferencedContainer = "container:Apollo.xcodeproj">
</BuildableReference>
</BuildActionEntry>
</BuildActionEntries>
</BuildAction>
<TestAction
buildConfiguration = "Debug"
selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
shouldUseLaunchSchemeArgsEnv = "YES">
<Testables>
</Testables>
</TestAction>
<LaunchAction
buildConfiguration = "Debug"
selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
launchStyle = "0"
useCustomWorkingDirectory = "NO"
ignoresPersistentStateOnLaunch = "NO"
debugDocumentVersioning = "YES"
debugServiceExtension = "internal"
allowLocationSimulation = "YES">
</LaunchAction>
<ProfileAction
buildConfiguration = "Release"
shouldUseLaunchSchemeArgsEnv = "YES"
savedToolIdentifier = ""
useCustomWorkingDirectory = "NO"
debugDocumentVersioning = "YES">
<MacroExpansion>
<BuildableReference
BuildableIdentifier = "primary"
BlueprintIdentifier = "DE058606266978A100265760"
BuildableName = "ApolloModels.framework"
BlueprintName = "ApolloModels"
ReferencedContainer = "container:Apollo.xcodeproj">
</BuildableReference>
</MacroExpansion>
</ProfileAction>
<AnalyzeAction
buildConfiguration = "Debug">
</AnalyzeAction>
<ArchiveAction
buildConfiguration = "Release"
revealArchiveInOrganizer = "YES">
</ArchiveAction>
</Scheme>
8 changes: 4 additions & 4 deletions ...cshareddata/xcschemes/ApolloCore.xcscheme → ...shareddata/xcschemes/ApolloUtils.xcscheme
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
<BuildableReference
BuildableIdentifier = "primary"
BlueprintIdentifier = "9B68353D2463481A00337AE6"
BuildableName = "ApolloCore.framework"
BlueprintName = "ApolloCore"
BuildableName = "ApolloUtils.framework"
BlueprintName = "ApolloUtils"
ReferencedContainer = "container:Apollo.xcodeproj">
</BuildableReference>
</BuildActionEntry>
Expand Down Expand Up @@ -61,8 +61,8 @@
<BuildableReference
BuildableIdentifier = "primary"
BlueprintIdentifier = "9B68353D2463481A00337AE6"
BuildableName = "ApolloCore.framework"
BlueprintName = "ApolloCore"
BuildableName = "ApolloUtils.framework"
BlueprintName = "ApolloUtils"
ReferencedContainer = "container:Apollo.xcodeproj">
</BuildableReference>
</MacroExpansion>
Expand Down
3 changes: 3 additions & 0 deletions Configuration/Apollo/Apollo-Target-ApolloModels.xcconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#include "../Shared/Workspace-Universal-Framework.xcconfig"

INFOPLIST_FILE = Sources/ApolloModels/Info.plist
AnthonyMDev marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 1 addition & 1 deletion .../Apollo/Apollo-Target-ApolloCore.xcconfig → ...Apollo/Apollo-Target-ApolloUtils.xcconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
#include "../Shared/Workspace-Universal-Framework.xcconfig"

INFOPLIST_FILE = Sources/ApolloCore/Info.plist
INFOPLIST_FILE = Sources/ApolloUtils/Info.plist
73 changes: 42 additions & 31 deletions Package.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,35 +4,38 @@
import PackageDescription

let package = Package(
name: "Apollo",
platforms: [
.iOS(.v12),
.macOS(.v10_14),
.tvOS(.v12),
.watchOS(.v5)
],
products: [
.library(
name: "ApolloCore",
targets: ["ApolloCore"]),
name: "Apollo",
platforms: [
.iOS(.v12),
.macOS(.v10_14),
.tvOS(.v12),
.watchOS(.v5)
],
products: [
.library(
name: "Apollo",
targets: ["Apollo"]),
.library(
name: "Apollo-Dynamic",
type: .dynamic,
targets: ["Apollo"]),
name: "ApolloModels",
targets: ["ApolloModels"]),
.library(
name: "ApolloUtils",
targets: ["ApolloUtils"]),
.library(
name: "Apollo-Dynamic",
type: .dynamic,
targets: ["Apollo"]),
.library(
name: "ApolloCodegenLib",
targets: ["ApolloCodegenLib"]),
.library(
name: "ApolloSQLite",
targets: ["ApolloSQLite"]),
name: "ApolloSQLite",
targets: ["ApolloSQLite"]),
.library(
name: "ApolloWebSocket",
targets: ["ApolloWebSocket"]),
],
dependencies: [
name: "ApolloWebSocket",
targets: ["ApolloWebSocket"]),
],
dependencies: [
.package(
url: "https://github.com/stephencelis/SQLite.swift.git",
.upToNextMinor(from: "0.12.2")),
Expand All @@ -45,26 +48,34 @@ let package = Package(
.package(
url: "https://github.com/apollographql/InflectorKit",
.upToNextMinor(from: "0.0.2")),
],
targets: [
],
targets: [
.target(
name: "ApolloCore",
dependencies: [],
name: "Apollo",
dependencies: [
"ApolloModels",
"ApolloUtils"
],
exclude: [
"Info.plist"
]),
.target(
name: "Apollo",
dependencies: [
"ApolloCore",
],
name: "ApolloModels",
dependencies: [],
exclude: [
"Info.plist",
"CodegenV1"
]),
.target(
name: "ApolloUtils",
dependencies: [],
exclude: [
"Info.plist"
]),
.target(
name: "ApolloCodegenLib",
dependencies: [
"ApolloCore",
"ApolloUtils",
.product(name: "InflectorKit", package: "InflectorKit"),
.product(name: "Stencil", package: "Stencil"),
],
Expand All @@ -89,11 +100,11 @@ let package = Package(
name: "ApolloWebSocket",
dependencies: [
"Apollo",
"ApolloCore",
"ApolloUtils",
.product(name: "Starscream", package: "Starscream"),
],
exclude: [
"Info.plist"
])
]
]
)
2 changes: 1 addition & 1 deletion Sources/Apollo/Bundle+Helpers.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import Foundation
#if !COCOAPODS
import ApolloCore
import ApolloUtils
#endif

extension Bundle: ApolloCompatible {}
Expand Down
22 changes: 22 additions & 0 deletions Sources/Apollo/CacheReference.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import Foundation

/// A reference to a cache record.
public struct CacheReference {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apollo Android uses CacheReference too. I'd be in for renaming to EntityReference as we're still at a moment where we can rename stuff on Android. Feels like that would be more adequate. It's a reference to an Entity, not to a Cache. Or maybe ObjectReference ? (We don't have entity anywhere in the Android codebase)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@designatednerd What's your opinion on the naming of this? Entity, Object, something else?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@martinbonnin Is your point that it's a reference to an entity within the cache rather than to the cache itself? I think that's valid, but Entity by itself is so overloaded I might go with something like CacheEntityReference.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is your point that it's a reference to an entity within the cache rather than to the cache itself?

Yes it was! Rereading that, I'm certainly overthinking this. CacheReference sounds reasonable.
And now back to overthinking this... What's the difference between a CacheReference and a CacheKey ? Could we merge both somehow?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My recollection is that the key is the key used to access stored data vs the reference is the actual stored data - @AnthonyMDev is that about right?

Copy link
Contributor

@martinbonnin martinbonnin Jun 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got curious and tried to merge CacheKey and CacheReference. Turns out it's working well: apollographql/apollo-kotlin#3157

The only reason I can think of having two different data structures in Kotlin is if we want to make CacheKey a value class that is inlined to a String for performance reasons. If we do that, we can't check the type of CacheKey anymore which means everything becomes a string and we can't serialize the CacheKey to their special String value.

But besides that (which we don't optimize at the moment and I feel should be an implementation details if we ever decide to) I actually like the reduced mental load and API surface.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmmmm. I'm going to look into this. Thanks for that @martijnwalraven

My recollection is that the key is the key used to access stored data vs the reference is the actual stored data - @AnthonyMDev is that about right?

No, the reference isn't the actual stored data. It's just a wrapper type for the CacheKey as a String. It indicates that the raw String data isn't just a regular raw a String property on the object, but is a String representing the reference to the cache key of another object.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gooooot it, thank you for clarifying

public let key: String

public init(key: String) {
self.key = key
}
}

extension CacheReference: Equatable {
public static func ==(lhs: CacheReference, rhs: CacheReference) -> Bool {
return lhs.key == rhs.key
}
}

extension CacheReference: CustomStringConvertible {
public var description: String {
return "-> #\(key)"
}
}
2 changes: 1 addition & 1 deletion Sources/Apollo/Decoding.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ private func decode(groupedFields: GroupedFields,
fieldEntries.append((responseName, fieldEntry))
}

return ResultMap(fieldEntries)
return ResultMap(fieldEntries, uniquingKeysWith: { (_, last) in last })
}

/// Before execution, the selection set is converted to a grouped field set. Each entry in the grouped field set is a list of fields that share a response key. This ensures all fields with the same response key (alias or field name) included via referenced fragments are executed at the same time.
Expand Down
9 changes: 0 additions & 9 deletions Sources/Apollo/Dictionary+Helpers.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,3 @@ public extension Dictionary {
lhs.merge(rhs) { (_, new) in new }
}
}

extension Dictionary {
init<S: Sequence>(_ entries: S) where S.Iterator.Element == Element {
self = Dictionary(minimumCapacity: entries.underestimatedCount)
for (key, value) in entries {
self[key] = value
}
}
}
2 changes: 1 addition & 1 deletion Sources/Apollo/DispatchQueue+Optional.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import Foundation
#if !COCOAPODS
import ApolloCore
import ApolloUtils
#endif

extension DispatchQueue: ApolloCompatible {}
Expand Down
6 changes: 3 additions & 3 deletions Sources/Apollo/GraphQLExecutor.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import Foundation
#if !COCOAPODS
import ApolloCore
import ApolloUtils
#endif

/// A field resolver is responsible for resolving a value for a field.
Expand All @@ -10,7 +10,7 @@ typealias GraphQLFieldResolver = (_ object: JSONObject, _ info: GraphQLResolveIn
/// Because data may be loaded from a database, these loads are batched for performance reasons.
/// By returning a `PossiblyDeferred` wrapper, we allow `ApolloStore` to use a `DataLoader` that
/// will defer loading the next batch of records from the cache until they are needed.
typealias ReferenceResolver = (Reference) -> PossiblyDeferred<JSONObject>
typealias ReferenceResolver = (CacheReference) -> PossiblyDeferred<JSONObject>

struct GraphQLResolveInfo {
let variables: GraphQLMap?
Expand Down Expand Up @@ -312,7 +312,7 @@ final class GraphQLExecutor {
try accumulator.accept(list: $0, info: info)
}
case .object:
if let reference = value as? Reference, let resolveReference = resolveReference {
if let reference = value as? CacheReference, let resolveReference = resolveReference {
return resolveReference(reference).flatMap {
self.complete(value: $0,
ofType: returnType,
Expand Down
2 changes: 1 addition & 1 deletion Sources/Apollo/GraphQLGETTransformer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import Foundation
#if !COCOAPODS
import ApolloCore
import ApolloUtils
#endif

public struct GraphQLGETTransformer {
Expand Down
2 changes: 1 addition & 1 deletion Sources/Apollo/GraphQLQueryWatcher.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import Foundation
#if !COCOAPODS
import ApolloCore
import ApolloUtils
#endif

/// A `GraphQLQueryWatcher` is responsible for watching the store, and calling the result handler with a new result whenever any of the data the previous result depends on changes.
Expand Down
4 changes: 1 addition & 3 deletions Sources/Apollo/GraphQLResponseGenerator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import Foundation

final class GraphQLResponseGenerator: GraphQLResultAccumulator {
func accept(scalar: JSONValue, info: GraphQLResolveInfo) -> JSONValue {
return scalar
Expand All @@ -18,7 +16,7 @@ final class GraphQLResponseGenerator: GraphQLResultAccumulator {
}

func accept(fieldEntries: [(key: String, value: JSONValue)], info: GraphQLResolveInfo) -> JSONValue {
return JSONObject(fieldEntries)
return JSONObject(fieldEntries, uniquingKeysWith: { (_, last) in last })
}

func finish(rootValue: JSONValue, info: GraphQLResolveInfo) throws -> JSONObject {
Expand Down
6 changes: 2 additions & 4 deletions Sources/Apollo/GraphQLResultNormalizer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import Foundation

final class GraphQLResultNormalizer: GraphQLResultAccumulator {
private var records: RecordSet = [:]

Expand All @@ -22,10 +20,10 @@ final class GraphQLResultNormalizer: GraphQLResultAccumulator {
func accept(fieldEntries: [(key: String, value: JSONValue)], info: GraphQLResolveInfo) throws -> JSONValue {
let cachePath = info.cachePath.joined

let object = JSONObject(fieldEntries)
let object = JSONObject(fieldEntries, uniquingKeysWith: { (_, last) in last })
records.merge(record: Record(key: cachePath, object))

return Reference(key: cachePath)
return CacheReference(key: cachePath)
}

func finish(rootValue: JSONValue, info: GraphQLResolveInfo) throws -> RecordSet {
Expand Down
10 changes: 8 additions & 2 deletions Sources/Apollo/GraphQLSelectionSet.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#if !COCOAPODS
import ApolloCore
import ApolloModels
#endif

public typealias ResultMap = [String: Any?]
Expand Down Expand Up @@ -29,6 +29,12 @@ extension GraphQLSelectionSet {
}
}

/// For backwards compatibility with legacy codegen.
/// The `GraphQLVariable` class has been replaced by `InputValue.variable`
public func GraphQLVariable(_ name: String) -> InputValue {
return .variable(name)
}

public protocol GraphQLSelection {
}

Expand Down Expand Up @@ -71,7 +77,7 @@ public struct FieldArguments: ExpressibleByDictionaryLiteral {
let arguments: InputValue

public init(dictionaryLiteral elements: (String, InputValue)...) {
arguments = .object(Dictionary(elements))
arguments = .object(Dictionary(elements, uniquingKeysWith: { (_, last) in last }))
}

public func evaluate(with variables: [String: JSONEncodable]?) throws -> JSONValue {
Expand Down
Loading