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

Make deviceDisplayName optional in DeviceRegistration #487

Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ data class AltBeaconDeviceRegistration(
* This value is constrained from -127 to 0.
*/
val referenceRssi: Short,
@Required
override val deviceDisplayName: String? = null, // TODO: We could map known manufacturerId's to display names.
override val additionalSpecifications: ApplicationData? = null
) : DeviceRegistration()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import kotlin.js.JsExport
@JsExport
data class BLESerialNumberDeviceRegistration(
val serialNumber: String,
@Required
override val deviceDisplayName: String? = null,
override val additionalSpecifications: ApplicationData? = null
) : DeviceRegistration()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import kotlin.js.JsExport
@Serializable
@JsExport
data class DefaultDeviceRegistration(
@Required
override val deviceDisplayName: String? = null,
override val additionalSpecifications: ApplicationData? = null,
@Required
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ abstract class DeviceRegistration
* An optional concise textual representation for display purposes describing the key specifications of the device.
* E.g., device manufacturer, name, and operating system version.
*/
@Required
abstract val deviceDisplayName: String?

@Required
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import kotlin.js.JsExport
@JsExport
data class MACAddressDeviceRegistration(
val macAddress: MACAddress,
@Required
override val deviceDisplayName: String? = null,
override val additionalSpecifications: ApplicationData? = null
) : DeviceRegistration()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ data class WebsiteDeviceRegistration(
* The HTTP User-Agent header of the user agent which made the HTTP request to [url].
*/
val userAgent: String,
@Required
override val deviceDisplayName: String? = userAgent,
override val additionalSpecifications: ApplicationData? = null
) : DeviceRegistration()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ data class CustomDeviceRegistration internal constructor(
@Serializable
private data class BaseMembers(
override val deviceId: String,
override val deviceDisplayName: String?,
override val deviceDisplayName: String? = null,
override val additionalSpecifications: ApplicationData? = null
) : DeviceRegistration()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,17 @@ class ApiJsonObjectMigrationBuilder(
json[ fieldName ] = newJson
}

/**
* Retrieve the object identified by [fieldName] in this object, and if present, run the specified [migration].
*/
fun updateOptionalObject( fieldName: String, migration: ApiJsonObjectMigrationBuilder.() -> Unit )
{
val o = json[ fieldName ] as? JsonObject ?: return
val newJson: JsonObject = ApiJsonObjectMigrationBuilder( o, minimumMinorVersion, targetMinorVersion )
.apply( migration ).build()
json[ fieldName ] = newJson
}

/**
* Retrieve the array identified by [fieldName] in this object, and run the specified [migration].
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,17 @@ class ApiJsonObjectMigrationBuilderTest
@Serializable
class MigrateInner( val c: Int )

@Serializable
class MigrateOptional( val optionalInner: MigrateInner? = null ) : ToMigrate

private val toMigrateSerializer = PolymorphicSerializer( ToMigrate::class )

private val json = createDefaultJSON(
SerializersModule {
polymorphic( ToMigrate::class )
{
subclass( Migrate::class )
subclass( MigrateOptional::class )
}
}
)
Expand Down Expand Up @@ -84,6 +88,53 @@ class ApiJsonObjectMigrationBuilderTest
assertEquals( toSet, inner[ MigrateInner::c.name ] )
}

@Test
fun updateObject_fails_if_object_not_present()
{
val toMigrate = MigrateOptional( optionalInner = null )
val toMigrateJson = json.encodeToJsonElement( toMigrateSerializer, toMigrate ).jsonObject

migrate( toMigrateJson ) {
assertFailsWith<IllegalArgumentException>
{
updateObject( MigrateOptional::optionalInner.name ) { }
}
}
}

@Test
fun updateOptionalObject_if_object_present_succeeds()
{
val toMigrate = MigrateOptional( optionalInner = MigrateInner( 0 ) )
val toMigrateJson = json.encodeToJsonElement( toMigrateSerializer, toMigrate ).jsonObject

val toSet = JsonPrimitive( 42 )
val migrated = migrate( toMigrateJson ) {
updateOptionalObject( MigrateOptional::optionalInner.name ) {
json[ MigrateInner::c.name ] = toSet
}
}

val inner = assertNotNull( migrated[ MigrateOptional::optionalInner.name ]?.jsonObject )
assertEquals( toSet, inner[ MigrateInner::c.name ] )
}

@Test
fun updateOptionalObject_if_object_not_present_does_nothing()
{
val toMigrate = MigrateOptional( optionalInner = null )
val toMigrateJson = json.encodeToJsonElement( toMigrateSerializer, toMigrate ).jsonObject

val toSet = JsonPrimitive( 42 )
val migrated = migrate( toMigrateJson ) {
updateOptionalObject( MigrateOptional::optionalInner.name ) {
json[ MigrateInner::c.name ] = toSet
}
}

assertEquals( toMigrateJson.toString(), migrated.toString() )
}

@Test
fun updateArray_succeeds()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import kotlinx.serialization.*
*/
interface DeploymentService : ApplicationService<DeploymentService, DeploymentService.Event>
{
companion object { val API_VERSION = ApiVersion( 1, 1 ) }
companion object { val API_VERSION = ApiVersion( 1, 3 ) }

@Serializable
sealed class Event( override val aggregateId: String? ) : IntegrationEvent<DeploymentService>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
package dk.cachet.carp.deployments.infrastructure.versioning

import dk.cachet.carp.common.application.services.ApiVersion
import dk.cachet.carp.common.infrastructure.versioning.ApiResponse
import dk.cachet.carp.common.infrastructure.versioning.ApplicationServiceApiMigrator
import dk.cachet.carp.common.infrastructure.versioning.Major1Minor0To1Migration
import dk.cachet.carp.common.infrastructure.versioning.*
import dk.cachet.carp.deployments.application.DeploymentService
import dk.cachet.carp.deployments.infrastructure.DeploymentServiceInvoker
import dk.cachet.carp.deployments.infrastructure.DeploymentServiceRequest
import kotlinx.serialization.json.JsonObject
import kotlinx.serialization.json.*


private val major1Minor0To1Migration =
Expand All @@ -29,10 +27,47 @@ private val major1Minor0To1Migration =
}


private val major1Minor1To3Migration =
@Suppress( "MagicNumber" )
object : ApiMigration( 1, 3 )
{
override fun migrateRequest( request: JsonObject ) = request

override fun migrateResponse( request: JsonObject, response: ApiResponse, targetVersion: ApiVersion ) =
when ( request.getType( ) )
{
"dk.cachet.carp.deployments.infrastructure.DeploymentServiceRequest.GetDeviceDeploymentFor" ->
{
val responseObject = (response.response as? JsonObject)?.migrate {
requiredDeviceDisplayName( "registration" )

updateOptionalObject( "connectedDeviceRegistrations" )
{
for ( connectedDevice in json.keys ) requiredDeviceDisplayName( connectedDevice )
}
}
ApiResponse( responseObject, response.ex )
}
else -> response
}

/**
* The `deviceDisplayName` field in `DeviceRegistration`, with default value `null`, was changed from required
* into optional. So, if it isn't set, it needs to bet set to `null` explicitly.
*/
private fun ApiJsonObjectMigrationBuilder.requiredDeviceDisplayName( fieldName: String ) =
updateObject( fieldName ) {
json.getOrPut( "deviceDisplayName" ) { JsonNull }
}

override fun migrateEvent( event: JsonObject ) = event
}


val DeploymentServiceApiMigrator = ApplicationServiceApiMigrator(
DeploymentService.API_VERSION,
DeploymentServiceInvoker,
DeploymentServiceRequest.Serializer,
DeploymentService.Event.serializer(),
listOf( major1Minor0To1Migration )
listOf( major1Minor0To1Migration, major1Minor1To3Migration )
)
Loading
Loading