-
Notifications
You must be signed in to change notification settings - Fork 743
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.
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
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
7a1170b
WIP
AnthonyMDev d968a46
WIP: ApolloCore - ApolloUtils + ApolloModels
AnthonyMDev 5755e2d
Create ApolloModels
AnthonyMDev e3e6a38
Fix callsites in ApolloWebSocket and ApolloSQL
AnthonyMDev 562951f
Fix callsites ApolloTestSupport
AnthonyMDev 86ffccb
Move InputValue, GraphQLError, and CacheReference back to Apollo
AnthonyMDev 3494a9d
Fixed callsites after moving JSON back to Apollo
AnthonyMDev 90f1f73
Fix all callsites in tests
AnthonyMDev 013a822
Setup Podspec
AnthonyMDev bb83598
Fix minor error
AnthonyMDev 625886c
Minor cleanup
AnthonyMDev eb22a94
Fix codegen tests
AnthonyMDev 001fd7e
Rename ApolloModels -> ApolloAPI
AnthonyMDev File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Large diffs are not rendered by default.
Oops, something went wrong.
67 changes: 67 additions & 0 deletions
67
Apollo.xcodeproj/xcshareddata/xcschemes/ApolloModels.xcscheme
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 = "ApolloAPI.framework" | ||
BlueprintName = "ApolloAPI" | ||
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 = "ApolloAPI.framework" | ||
BlueprintName = "ApolloAPI" | ||
ReferencedContainer = "container:Apollo.xcodeproj"> | ||
</BuildableReference> | ||
</MacroExpansion> | ||
</ProfileAction> | ||
<AnalyzeAction | ||
buildConfiguration = "Debug"> | ||
</AnalyzeAction> | ||
<ArchiveAction | ||
buildConfiguration = "Release" | ||
revealArchiveInOrganizer = "YES"> | ||
</ArchiveAction> | ||
</Scheme> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 changes: 1 addition & 1 deletion
2
.../Apollo/Apollo-Target-ApolloCore.xcconfig → ...n/Apollo/Apollo-Target-ApolloAPI.xcconfig
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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/ApolloAPI/Info.plist |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
#include "../Shared/Workspace-Universal-Framework.xcconfig" | ||
|
||
INFOPLIST_FILE = Sources/ApolloUtils/Info.plist |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 { | ||
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)" | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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 toEntityReference
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 maybeObjectReference
? (We don't have entity anywhere in the Android codebase)There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 likeCacheEntityReference
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 aCacheKey
? Could we merge both somehow?There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
andCacheReference
. Turns out it's working well: apollographql/apollo-kotlin#3157The 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 aString
for performance reasons. If we do that, we can't check the type ofCacheKey
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.
There was a problem hiding this comment.
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
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 aString
representing the reference to the cache key of another object.There was a problem hiding this comment.
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