STITCH-1294 v4 iOS SDK Documentation #43

adamchel · 2018-03-30T19:57:18Z

This is most of the docs work that needs to be done for iOS. Remaining work will be distribution (STITCH-1353), and renaming modules and documenting how to get the package in XCode once we work on packaging (STITCH-1293).

To generate the documentation for a particular module, run jazzy -c —config jazzy.json in the root directory of the module. Right now both StitchCore and StitchCore-iOS are documented and have jazzy configurations.

For public facing protocols/classes/etc, I tried to be as comprehensive as possible with documentation markup.

For internal protocols/classes/etc, I tried to explain every symbol, but I did not necessarily use the documentation markup to describe every parameter, return value, or thrown exception, because in many cases it was self-explanatory.

The only significant thing left undocumented is a richer description of each error case in StitchErrorCode. I’m happy to work on it, jbut didn’t think it was worth the time right now.

Also, due to a bug in Jazzy (realm/jazzy#896), I had to put all public-facing function signatures in protocols on one line and add SwiftLint exceptions where that made the line more than 120 characters long. Until the Jazzy bug is fixed, we’ll need to maintain those linter exceptions.

I also made a number of drive-by changes that were mostly refactoring/cleanup. They’re all described here:

Major drive-by code changes:

Moved refresh token related methods in CoreStitchAuth to CoreStitchAuth+StitchAuthRequestClient.swift
Moved the shouldRefreshOnFailure property to StitchAuthRequest from StitchRequest since the property is only relevant for authenticated requests
Removed StitchRequestBuilderError since everything was just using StitchRequestBuilder anyway.
Removed redundant url property in StitchRequest and StitchAuthRequest
Renamed StitchAuthDocRequestBuilderError to StitchDocRequestBuilderError and moved it to StitchRequest.swift since it isn’t exclusive to authenticated requests.
Renamed StitchAuthRoutes.swift to StitchAppRoutes.swift since they weren’t exclusively about auth.
Moved builder and Buildee protocols to their own file in Common/Builder.swift since they’re used by both requests and client configurations
Made StitchErrorCodable internal rather than public.
Made StitchUserProfileImpl internal rather than public.
Renamed StitchClientErrors to StitchClientError
Made StitchCore-iOS a shared scheme in the workspace so jazzy could build it properly from the command line with xcodebuild.
Changed the “args” field passed to the Stitch server in requests to “arguments”

edaniels

Fantastic work Adam. I've a few nits but this looks awesome. This is very detailed and thorough.

edaniels · 2018-03-31T13:01:03Z

StitchCore-iOS/DocsSources/Abstracts/Getting Started.md

+
+#### Executing a Function
+
+1. Once logged in, executing a function happens via the StitchClient's `executeFunction()` method


[typo] executeFunction

edaniels · 2018-03-31T13:04:55Z

StitchCore-iOS/StitchCore-iOS/Core/Auth/Internal/StitchAuthImpl.swift

+     *            specified in the `forProvider` parameter.
+     */
+    public func providerClient<Provider>(forProvider provider: Provider)
+        -> Provider.Client where Provider: AuthProviderClientSupplier {


[q] It's called a AuthProviderClientSupplier but the instances are a clientProvider. Should we change one or the other?

I'll rename to clientSupplier. I think Provider is confusing because of the fact that it's for an authentication provider.

edaniels · 2018-03-31T13:10:12Z

StitchCore-iOS/StitchCore-iOS/Core/Auth/Internal/StitchUserFactoryImpl.swift

 internal final class StitchUserFactoryImpl: StitchUserFactory {
+    /**
+     * The factory function which can produce a `StitchUserImpl` with the provided ID, logged in provider type/name,


[nit] ID -> id.

edaniels · 2018-03-31T13:13:54Z

StitchCore/Tests/stitch-ios-sdk-protoTests/Internal/Common/SynchronizedTests.swift

-                        let value = counter.value
-                        if i % 2 == 0 {
-                            usleep(50)
+        if #available(OSX 10.12, *) {


[q] Why was this added?

It wasn't compiling without it but now when I remove it compiles so I'll just remove it.

edaniels · 2018-03-31T13:16:25Z

StitchCore/Sources/StitchCore/StitchClientConfiguration.swift

    var baseURL: String { get }
+
+    /**
+     * The underlying storage for authentication info.


[nit] It's kind of more than just for auth info. We store device details too. Maybe The underlying storage for persisting authentication and app state.

edaniels · 2018-03-31T13:23:50Z

StitchCore-iOS/StitchCore-iOS/Services/Internal/NamedServiceClientProvider.swift

 }

+/**
+ * A generic wrapper for a `NamedServiceClientProvider`.
+ */
 public struct AnyNamedServiceClientProvider<T> {


[q] Why is this different than how the auth suppliers work?

Not really sure. I didn't write this originally, and I figured we would make usability adjustments here once we actually implement some concrete services. This stuff isn't being used anywhere yet anyway.

edaniels · 2018-03-31T13:26:12Z

StitchCore-iOS/StitchCore-iOS/Services/Internal/StitchServiceImpl.swift

+     *     - error: An error object that indicates why the function call failed, or `nil` if the function call was
+     *              successful.
+     *
+     */
    public func callFunction(withName name: String,
                             withArgs args: BSONArray,


[aside] Won't this be onerous for callers to construct arrays rather than a variadic list of some sort?

I would consider a variadic list of ExtendedJSONRepresentables... we can also make the BSONArray class better.

I just followed what we did in Java. Usage doesn't seem that bad:

stitchClient.callFunction(withName: "echoArg", withArgs: ["Hello world!"]) { (value, error) in // do stuff }

But I'm okay with it if we can do the same in Java, and we can do it without messing with the completion handler pattern we've been using. (If the variadic parameter has to be the last parameter, then it would make the completion handler for callFunction different than every other asynchronous action.

edaniels · 2018-03-31T13:27:37Z

StitchCore/Common/Builder.swift

@@ -0,0 +1,34 @@
+/**


[q] How useful is this abstraction?

This was a nested abstraction I had in core previously– Adam has simply moved it out to its own file.

With the builder pattern we are using, it saves us some repeated code for each builder, which we have a fair amount of. It's also the only way to create conformance with this kind of abstraction.

This was especially helpful with all of the builder extensions we wrote, based off of the Android builders.

The alternative is to do a more "old school" (less Swifty, more Java-y) builder pattern and do a fair amount of copy and pasting.

Going to leave as is then

edaniels · 2018-03-31T13:30:29Z

StitchCore/Sources/StitchCore/Auth/Internal/Models/StoreAuthInfo.swift

    let accessToken: String

+    /**
+     * The permanent refresh token for the user.


[nit] A refresh token has the potential to be invalidated.

edaniels · 2018-03-31T13:31:30Z

StitchCore/Sources/StitchCore/Auth/Providers/Anonymous/AnonymousCredential.swift

    public var providerName: String

+    /**
+     * The type of the provider for this credential.
+     */
    public var providerType: String = "anon-user"


[nit] All of these providerType should be constants.

Agreed. It would be nice if these were part of an enum.

jsflax

A few minors and nits, otherwise, great job!

jsflax · 2018-04-01T23:22:18Z

StitchCore-iOS/StitchCore-iOS/Core/Auth/Internal/StitchAuthImpl.swift

    private struct DelegateWeakRef {
        weak var value: StitchAuthDelegate?
        init(value: StitchAuthDelegate) {
            self.value = value
        }
    }
+
+    /**
+     * An array of weak references to `StitchAuthDelegate`, each of which will be notified when authentication events


[nit] list* not array

jsflax · 2018-04-01T23:24:20Z

StitchCore-iOS/StitchCore-iOS/Core/Auth/Providers/UserPassword/UserPasswordAuthProvider.swift

        dispatcher.run(withCompletionHandler: completionHandler) {
-            let _ = try super.confirmUser(withToken: token, withTokenId: tokenId)
+            _ = try super.confirmUser(withToken: token, withTokenId: tokenId)


[nit][q] Do we need the _ = syntax here? Without a return value, there shouldn't be a warning here.

these methods in super do have a return value (the raw Response), so we have to explicitly ignore it. (and it will properly throw when the response is not a 200-299)

jsflax · 2018-04-01T23:24:37Z

StitchCore-iOS/StitchCore-iOS/Core/Auth/Providers/UserPassword/UserPasswordAuthProvider.swift

        dispatcher.run(withCompletionHandler: completionHandler) {
-            let _ = try super.reset(password: password, withToken: token, withTokenId: tokenId)
+            _ = try super.reset(password: password, withToken: token, withTokenId: tokenId)


[nit][q] Same remark as above.

same response as above

jsflax · 2018-04-01T23:24:45Z

StitchCore-iOS/StitchCore-iOS/Core/Auth/Providers/UserPassword/UserPasswordAuthProvider.swift

    func sendResetPasswordEmail(toEmail email: String, completionHandler: @escaping (Error?) -> Void) {
        dispatcher.run(withCompletionHandler: completionHandler) {
-            let _ = try super.sendResetPasswordEmail(toEmail: email)
+            _ = try super.sendResetPasswordEmail(toEmail: email)


[nit][q] Same remark as above.

same response as above

jsflax · 2018-04-01T23:27:52Z

StitchCore-iOS/StitchCore-iOS/Core/Stitch.swift

@@ -33,67 +39,26 @@ public class Stitch {
    public static func initialize() throws {
        try! StitchCore.sync(self) {


https://jira.mongodb.org/browse/STITCH-1356 I'm going to get on it once this PR is merged in.

jsflax · 2018-04-01T23:28:45Z

StitchCore-iOS/StitchCore-iOS/Core/StitchAppClient.swift

+
+    // MARK: Functions
+
+    // swiftlint:disable line_length


[minor] We should avoid disabling this check at all costs. The method sign below could have newline between parameters.

it's like that because jazzy cannot get the full function signature with newlines in protocols :(

Ah wow, that seems like a pretty huge bug.

jsflax · 2018-04-01T23:31:03Z

StitchCore/Common/Builder.swift

@@ -0,0 +1,34 @@
+/**


This was a nested abstraction I had in core previously– Adam has simply moved it out to its own file.

With the builder pattern we are using, it saves us some repeated code for each builder, which we have a fair amount of. It's also the only way to create conformance with this kind of abstraction.

This was especially helpful with all of the builder extensions we wrote, based off of the Android builders.

The alternative is to do a more "old school" (less Swifty, more Java-y) builder pattern and do a fair amount of copy and pasting.

jsflax · 2018-04-01T23:31:58Z

StitchCore/Sources/StitchCore/Auth/Providers/Anonymous/AnonymousCredential.swift

    public var providerName: String

+    /**
+     * The type of the provider for this credential.
+     */
    public var providerType: String = "anon-user"


Agreed. It would be nice if these were part of an enum.

jsflax · 2018-04-01T23:34:40Z

StitchCore/Sources/StitchCore/Auth/Providers/UserPass/CoreUserPasswordAuthProviderClient.swift

+    /**
+     * Private helper which returns the provided path, appended to the base route for the authentication provider.
+     */
+    private func extensionRoute(forPath path: String) -> String {
        return "\(authRoutes.authProviderLoginRoute(withProviderName: providerName))/\(path)"


We should write a test for this. I believe it should be authProviderRoute, not authProviderLoginRoute, and that should clean this up.

adamchel requested review from edaniels and jsflax March 30, 2018 19:57

edaniels approved these changes Mar 31, 2018

View reviewed changes

jsflax approved these changes Apr 1, 2018

View reviewed changes

adamchel added 3 commits April 2, 2018 16:30

progress on making the ios SDK docs complete

aaaaeb7

completed documentation for v4 iOS SDK

b2ca055

address PR comments

c326e19

adamchel force-pushed the STITCH-1294 branch from b503554 to c326e19 Compare April 2, 2018 20:32

adamchel merged commit 851e314 into mongodb:v4-alpha Apr 2, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

STITCH-1294 v4 iOS SDK Documentation #43

STITCH-1294 v4 iOS SDK Documentation #43

adamchel commented Mar 30, 2018 •

edited

Loading

edaniels left a comment

edaniels Mar 31, 2018

edaniels Mar 31, 2018

adamchel Apr 2, 2018

edaniels Mar 31, 2018

edaniels Mar 31, 2018

adamchel Apr 2, 2018

edaniels Mar 31, 2018

edaniels Mar 31, 2018

adamchel Apr 2, 2018

edaniels Mar 31, 2018

jsflax Apr 2, 2018

adamchel Apr 2, 2018

edaniels Mar 31, 2018

jsflax Apr 1, 2018

adamchel Apr 2, 2018

edaniels Mar 31, 2018

edaniels Mar 31, 2018

jsflax Apr 1, 2018

jsflax left a comment

jsflax Apr 1, 2018

jsflax Apr 1, 2018

adamchel Apr 2, 2018

jsflax Apr 1, 2018

adamchel Apr 2, 2018

jsflax Apr 1, 2018

adamchel Apr 2, 2018

jsflax Apr 1, 2018

jsflax Apr 1, 2018

edaniels Apr 1, 2018

jsflax Apr 1, 2018

jsflax Apr 1, 2018

jsflax Apr 1, 2018

jsflax Apr 1, 2018


		#### Executing a Function

		1. Once logged in, executing a function happens via the StitchClient's `executeFunction()` method

		@@ -33,67 +39,26 @@ public class Stitch {
		public static func initialize() throws {
		try! StitchCore.sync(self) {

STITCH-1294 v4 iOS SDK Documentation #43

STITCH-1294 v4 iOS SDK Documentation #43

Conversation

adamchel commented Mar 30, 2018 • edited Loading

edaniels left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsflax left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamchel commented Mar 30, 2018 •

edited

Loading