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.

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

Fix js + enable js tests + code clean #45

Merged
merged 19 commits into from
Jun 20, 2022
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
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: 1 addition & 2 deletions .github/workflows/pull_request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ jobs:
- name: Assemble
run: ./gradlew assemble
- name: Run JS Tests
if: ${{ false }}
run: ./gradlew cleanTest jsTest
run: ./gradlew cleanTest jsLegacyTest
- name: Upload JS test artifact
uses: actions/upload-artifact@v2
if: failure()
Expand Down
26 changes: 25 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ The Firebase Kotlin SDK uses Kotlin serialization to read and write custom class
```groovy
plugins {
kotlin("multiplatform") // or kotlin("jvm") or any other kotlin plugin
kotlin("plugin.serialization") version "1.5.30"
kotlin("plugin.serialization") version "1.6.10"
}
```

Expand Down Expand Up @@ -96,6 +96,30 @@ You can also omit the serializer but this is discouraged due to a [current limit
data class Post(val timestamp: Double = ServerValue.TIMESTAMP)
```

Alternatively, `firebase-firestore` also provides a [Timestamp] class which could be used:
```kotlin
@Serializable
data class Post(val timestamp: Timestamp = Timestamp.serverValue())
```

In addition `firebase-firestore` provides [GeoPoint] and [DocumentReference] classes which allow persisting
geo points and document references in a native way:

```kotlin
@Serializable
data class PointOfInterest(
val reference: DocumentReference,
val location: GeoPoint,
val timestamp: Timestamp
)

val document = PointOfInterest(
reference = Firebase.firestore.collection("foo").document("bar"),
location = GeoPoint(51.939, 4.506),
timestamp = Timestamp.now()
)
```

<h3><a href="https://kotlinlang.org/docs/reference/functions.html#default-arguments">Default arguments</a></h3>

To reduce boilerplate, default arguments are used in the places where the Firebase Android SDK employs the builder pattern:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,8 @@ actual fun FirebaseEncoder.structureEncoder(descriptor: SerialDescriptor): Compo
.let { FirebaseCompositeEncoder(shouldEncodeElementDefault, positiveInfinity) { _, index, value -> it.add(index, value) } }
StructureKind.MAP -> mutableListOf<Any?>()
.let { FirebaseCompositeEncoder(shouldEncodeElementDefault, positiveInfinity, { value = it.chunked(2).associate { (k, v) -> k to v } }) { _, _, value -> it.add(value) } }
StructureKind.CLASS -> mutableMapOf<Any?, Any?>()
StructureKind.CLASS, StructureKind.OBJECT -> mutableMapOf<Any?, Any?>()
.also { value = it }
.let { FirebaseCompositeEncoder(shouldEncodeElementDefault, positiveInfinity) { _, index, value -> it[descriptor.getElementName(index)] = value } }
StructureKind.OBJECT -> FirebaseCompositeEncoder(shouldEncodeElementDefault, positiveInfinity) { _, _, obj -> value = obj }
vpodlesnyak marked this conversation as resolved.
Show resolved Hide resolved
else -> TODO("Not implemented ${descriptor.kind}")
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,17 +71,19 @@ class EncodersTest {
@Test
fun testEncodeDecodedSealedClass() {
val test = SealedClass.Test("Foo")
val encoded = encode(test, false)
val decoded = decode(encoded) as? SealedClass.Test
val serializer = SealedClass.Test.serializer() // has to be used because of JS issue
Copy link
Author

Choose a reason for hiding this comment

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

legacy JS is not finding a serializer in serializer<T>() call (see issue) and it seems unlikely to be fixed. the upstream repo is using legacy JS and not IR JS, so I don't consider migrating at the moment

val encoded = encode(serializer, test, false)
val decoded = decode(serializer, encoded)
assertEquals(test, decoded)
}

@Test
fun testEncodeDecodeGenericClass() {
val test = SealedClass.Test("Foo")
val generic = GenericClass(test)
val encoded = encode(generic, false)
val decoded = decode(encoded) as? GenericClass<SealedClass.Test>
val serializer = GenericClass.serializer(SealedClass.Test.serializer())
val encoded = encode(serializer, generic, false)
val decoded = decode(serializer, encoded)
assertEquals(generic, decoded)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,8 @@ actual fun FirebaseEncoder.structureEncoder(descriptor: SerialDescriptor): Compo
.let { FirebaseCompositeEncoder(shouldEncodeElementDefault, positiveInfinity) { _, index, value -> it.add(index, value) } }
StructureKind.MAP -> mutableListOf<Any?>()
.let { FirebaseCompositeEncoder(shouldEncodeElementDefault, positiveInfinity, { value = it.chunked(2).associate { (k, v) -> k to v } }) { _, _, value -> it.add(value) } }
StructureKind.CLASS -> mutableMapOf<Any?, Any?>()
StructureKind.CLASS, StructureKind.OBJECT -> mutableMapOf<Any?, Any?>()
.also { value = it }
.let { FirebaseCompositeEncoder(shouldEncodeElementDefault, positiveInfinity) { _, index, value -> it[descriptor.getElementName(index)] = value } }
StructureKind.OBJECT -> FirebaseCompositeEncoder(shouldEncodeElementDefault, positiveInfinity) { _, _, obj -> value = obj }
else -> TODO("Not implemented ${descriptor.kind}")
}
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,8 @@ external object firebase {
fun update(field: FieldPath, value: Any?, vararg moreFieldsAndValues: Any?): Promise<Unit>
fun delete(): Promise<Unit>
fun onSnapshot(next: (snapshot: DocumentSnapshot) -> Unit, error: (error: Error) -> Unit): ()->Unit

fun isEqual(other: DocumentReference): Boolean
}

open class WriteBatch {
Expand All @@ -442,13 +444,15 @@ external object firebase {
fun delete(documentReference: DocumentReference): Transaction
}

open class Timestamp(seconds: Long, nanoseconds: Int) {
open class Timestamp(seconds: Double, nanoseconds: Double) {
companion object {
fun now(): Timestamp
}

val seconds: Long
val nanoseconds: Int
val seconds: Double
val nanoseconds: Double

fun isEqual(other: Timestamp): Boolean
}
open class FieldPath(vararg fieldNames: String) {
companion object {
Expand All @@ -459,6 +463,8 @@ external object firebase {
open class GeoPoint(latitude: Double, longitude: Double) {
val latitude: Double
val longitude: Double

fun isEqual(other: GeoPoint): Boolean
Copy link
Author

Choose a reason for hiding this comment

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

unfortunately interops doesn't map it to equals

}

abstract class FieldValue {
Expand All @@ -468,6 +474,7 @@ external object firebase {
fun arrayUnion(vararg elements: Any): FieldValue
fun serverTimestamp(): FieldValue
}
fun isEqual(other: FieldValue): Boolean
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,9 @@
@file:JvmName("tests")
package dev.gitlive.firebase.firestore

import dev.gitlive.firebase.*
import kotlinx.serialization.Serializable
import androidx.test.platform.app.InstrumentationRegistry
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.runBlocking
import kotlin.test.*

actual val emulatorHost: String = "10.0.2.2"

Expand All @@ -19,64 +16,4 @@ actual val context: Any = InstrumentationRegistry.getInstrumentation().targetCon
actual fun runTest(test: suspend CoroutineScope.() -> Unit) = runBlocking { test() }

actual fun encodedAsMap(encoded: Any?): Map<String, Any?> = encoded as Map<String, Any?>

class FirebaseFirestoreAndroidTest {

@BeforeTest
fun initializeFirebase() {
Firebase
.takeIf { Firebase.apps(context).isEmpty() }
?.apply {
initialize(
context,
FirebaseOptions(
applicationId = "1:846484016111:ios:dd1f6688bad7af768c841a",
apiKey = "AIzaSyCK87dcMFhzCz_kJVs2cT2AVlqOTLuyWV0",
databaseUrl = "https://fir-kotlin-sdk.firebaseio.com",
storageBucket = "fir-kotlin-sdk.appspot.com",
projectId = "fir-kotlin-sdk"
)
)
Firebase.firestore.useEmulator(emulatorHost, 8080)
}
}

@Serializable
data class TestDataWithDocumentReference(
Copy link
Author

Choose a reason for hiding this comment

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

moved to common tests

val uid: String,
@Serializable(with = FirebaseDocumentReferenceSerializer::class)
val reference: DocumentReference,
@Serializable(with = FirebaseReferenceNullableSerializer::class)
val ref: FirebaseReference?
)

@Test
fun encodeDocumentReferenceObject() = runTest {
val doc = Firebase.firestore.document("a/b")
val item = TestDataWithDocumentReference("123", doc, FirebaseReference.Value(doc))
val encoded = encode(item, shouldEncodeElementDefault = false) as Map<String, Any?>
assertEquals("123", encoded["uid"])
assertEquals(doc.android, encoded["reference"])
assertEquals(doc.android, encoded["ref"])
}

@Test
fun encodeDeleteDocumentReferenceObject() = runTest {
val doc = Firebase.firestore.document("a/b")
val item = TestDataWithDocumentReference("123", doc, FirebaseReference.ServerDelete)
val encoded = encode(item, shouldEncodeElementDefault = false) as Map<String, Any?>
assertEquals("123", encoded["uid"])
assertEquals(doc.android, encoded["reference"])
assertEquals(FieldValue.delete, encoded["ref"])
}

@Test
fun decodeDocumentReferenceObject() = runTest {
val doc = Firebase.firestore.document("a/b")
val obj = mapOf("uid" to "123", "reference" to doc.android, "ref" to doc.android)
val decoded: TestDataWithDocumentReference = decode(obj)
assertEquals("123", decoded.uid)
assertEquals(doc.path, decoded.reference.path)
assertEquals(doc.path, decoded.ref?.reference?.path)
}
}
actual fun rawEncoded(vararg pairs: Pair<String, Any?>): Any = pairs.toMap()

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,7 +1,18 @@
package dev.gitlive.firebase.firestore

actual typealias GeoPoint = com.google.firebase.firestore.GeoPoint
import kotlinx.serialization.Serializable

actual fun geoPointWith(latitude: Double, longitude: Double) = GeoPoint(latitude, longitude)
actual val GeoPoint.latitude: Double get() = latitude
actual val GeoPoint.longitude: Double get() = longitude
/** A class representing a platform specific Firebase GeoPoint. */
actual typealias PlatformGeoPoint = com.google.firebase.firestore.GeoPoint

/** A class representing a Firebase GeoPoint. */
@Serializable(with = GeoPointSerializer::class)
actual class GeoPoint internal actual constructor(internal actual val platformValue: PlatformGeoPoint) {
Copy link
Author

Choose a reason for hiding this comment

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

wrapper class allows using @Serializable and overriding equals for JS

actual constructor(latitude: Double, longitude: Double) : this(PlatformGeoPoint(latitude, longitude))
actual val latitude: Double = platformValue.latitude
actual val longitude: Double = platformValue.longitude
override fun equals(other: Any?): Boolean =
this === other || other is GeoPoint && platformValue == other.platformValue
override fun hashCode(): Int = platformValue.hashCode()
override fun toString(): String = platformValue.toString()
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,33 @@
package dev.gitlive.firebase.firestore

actual typealias Timestamp = com.google.firebase.Timestamp
import kotlinx.serialization.Serializable

actual fun timestampNow(): Timestamp = Timestamp.now()
actual fun timestampWith(seconds: Long, nanoseconds: Int) = Timestamp(seconds, nanoseconds)
actual val Timestamp.seconds: Long get() = seconds
actual val Timestamp.nanoseconds: Int get() = nanoseconds
/** A class representing a platform specific Firebase Timestamp. */
actual typealias PlatformTimestamp = com.google.firebase.Timestamp

/** A base class that could be used to combine [Timestamp] and [Timestamp.ServerTimestamp] in the same field. */
@Serializable(with = BaseTimestampSerializer::class)
actual sealed class BaseTimestamp
Copy link
Author

@vpodlesnyak vpodlesnyak May 18, 2022

Choose a reason for hiding this comment

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

Interesting find. I have to use expect / actual sealed class because kotlin common and JS specific code are treated as different modules. this could be caused by the fact JS doesn't support packages.
unfortunately android studio check warns you to about exhaustive whens in this case

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thats weird...


/** A class representing a Firebase Timestamp. */
@Serializable(with = TimestampSerializer::class)
actual class Timestamp internal actual constructor(
internal actual val platformValue: PlatformTimestamp
): BaseTimestamp() {
actual constructor(seconds: Long, nanoseconds: Int) : this(PlatformTimestamp(seconds, nanoseconds))

actual val seconds: Long = platformValue.seconds
actual val nanoseconds: Int = platformValue.nanoseconds

override fun equals(other: Any?): Boolean =
this === other || other is Timestamp && platformValue == other.platformValue
override fun hashCode(): Int = platformValue.hashCode()
override fun toString(): String = platformValue.toString()

actual companion object {
actual fun now(): Timestamp = Timestamp(PlatformTimestamp.now())
}

/** A server time timestamp. */
actual object ServerTimestamp: BaseTimestamp()
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
package dev.gitlive.firebase.firestore

import com.google.firebase.Timestamp
import com.google.firebase.firestore.FieldValue
import com.google.firebase.firestore.GeoPoint

actual fun isSpecialValue(value: Any) = when(value) {
is FieldValue,
is GeoPoint,
is Timestamp,
is DocumentReference -> true
is PlatformGeoPoint,
is PlatformTimestamp,
is PlatformDocumentReference -> true
else -> false
}
Loading