feat(data): Events WebSocket Connection Logic + Subscribe

[tylerjroach]

• PR title and description conform to Pull Request guidelines.

Issue #, if available:

Description of changes:

• Still need to get Logger approved but I went ahead and built it in.

How did you test these changes?
(Please add a line here how the changes were tested)

Documentation update required?

• No
• Yes (Please include a PR link for the documentation update)

General Checklist

• Added Unit Tests
• Added Integration Tests
• Security oriented best practices and standards are followed (e.g. using input sanitization, principle of least privilege, etc)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[mattcreaser]

Could be a val

[tylerjroach]

@mattcreaser This is suspending. What are you suggesting here?

[mattcreaser]

It's not suspending?

    val existingWebSocket: EventsWebSocket?
        get() = _connectResult.get()?.getOrNull()

But this is just preference, there's no issue with current.

[tylerjroach]

Yep sorry, was looking at wrong value. updated

[mattcreaser]

Should not have underscores in non-backing properties

[tylerjroach]

I was treating these like backing properties because they were private and not exposed outside the class, holding a reference that was accessed for the getConnectedWebSocket() method. I'll go ahead and rename though as this isn't technically a backing property.

[mattcreaser]

Seems this should pass fallbackMessage if message is null

[tylerjroach]

Good catch

[mattcreaser]

Doesn't seem to be any advantage to having this be AtomicBoolean over a regular Boolean (possibly marked Volatile)?

[tylerjroach]

Sure, volatile should be sufficient as the okhttp callback of onFailure or onClosed is the only place the value is updated (which should be from same thread/ownership).

[mattcreaser]

Can this function just be calling send(WebSocketMessage.Send.ConnectionInit())

[mattcreaser]

namespace appears to be unused, is it supposed to be used as the tag of the logcat message?

The way I'd kind of expected this to work is for there to be an interface like:

interface LoggerFactory {
     fun logger(namespace: String): Logger
}

And then a particular events class (say EventsWebSocket) would do something like val logger = loggerFactory.logger("EventsWebSocket") so that it wouldn't have to prepend a TAG to every message.

[tylerjroach]

Currently, I have it set up like this:

private val events = Events(
    endpoint = endpoint,
    connectAuthorizer = apiKeyAuthorizer,
    defaultChannelAuthorizers = ChannelAuthorizers(
        subscribeAuthorizer = apiKeyAuthorizer,
        publishAuthorizer = apiKeyAuthorizer
    ),
    options = Events.Options(AndroidLogger("Events", LogLevel.DEBUG))
)

/**
 * Implementation of Logger interface that logs to Android's LogCat
 */
class AndroidLogger(
    override val namespace: String,
    override val thresholdLevel: LogLevel
) : Logger {

    override fun error(message: String) {
        if (!thresholdLevel.above(LogLevel.ERROR)) {
            Log.e(namespace, message)
        }
    }

    override fun error(message: String, error: Throwable?) {
        if (!thresholdLevel.above(LogLevel.ERROR)) {
            Log.e(namespace, message, error)
        }
    }

    override fun warn(message: String) {
        if (!thresholdLevel.above(LogLevel.WARN)) {
            Log.w(namespace, message)
        }
    }

    override fun warn(message: String, issue: Throwable?) {
        if (!thresholdLevel.above(LogLevel.WARN)) {
            Log.w(namespace, message, issue)
        }
    }

    override fun info(message: String) {
        if (!thresholdLevel.above(LogLevel.INFO)) {
            Log.i(namespace, message)
        }
    }

    override fun debug(message: String) {
        if (!thresholdLevel.above(LogLevel.DEBUG)) {
            Log.d(namespace, message)
        }
    }

    override fun verbose(message: String) {
        if (!thresholdLevel.above(LogLevel.VERBOSE)) {
            Log.v(namespace, message)
        }
    }
}

This was also on customer code side but it would be reasonable for us to include the implementation for AndroidLogger. This is all the same as how our current Amplify Logger works.

[tylerjroach]

Addressed after discussion.