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.

Sign up for GitHub

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

488 include deviceregistration in devicedeploymentstatus #489

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

488 include deviceregistration in devicedeploymentstatus #489

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a0ebb3e
Include `DeviceRegistration` in `DeviceDeploymentStatus`
yuanchen233 Oct 23, 2024
570ac77
Add regression test case for `GetStudyDeploymentStatusList` with regi…
yuanchen233 Oct 24, 2024
10da0cc
fixup! Include `DeviceRegistration` in `DeviceDeploymentStatus`
Whathecode Oct 25, 2024
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
2 changes: 1 addition & 1 deletion ....clients.core/src/commonTest/kotlin/dk/cachet/carp/clients/domain/ClientRepositoryTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ interface ClientRepositoryTest
Clock.System.now(),
deploymentId,
listOf(
DeviceDeploymentStatus.Registered( primaryDevice, true, emptySet(), emptySet() )
DeviceDeploymentStatus.Registered( primaryDevice, registration, true, emptySet(), emptySet() )
),
emptyList(),
null
Expand Down
1 change: 1 addition & 0 deletions carp.clients.core/src/commonTest/kotlin/dk/cachet/carp/clients/domain/study/StudyTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ class StudyTest
listOf(
DeviceDeploymentStatus.Registered(
smartphone,
primaryDeviceDeployment.registration,
true,
emptySet(),
connectedDevices.map { it.roleName }.toSet()
Expand Down
25 changes: 20 additions & 5 deletions ...re/src/commonMain/kotlin/dk/cachet/carp/deployments/application/DeviceDeploymentStatus.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package dk.cachet.carp.deployments.application

import dk.cachet.carp.common.application.devices.AnyDeviceConfiguration
import dk.cachet.carp.common.application.devices.DeviceRegistration
import kotlinx.serialization.*
import kotlin.js.JsExport

Expand Down Expand Up @@ -56,6 +57,17 @@ sealed class DeviceDeploymentStatus
abstract val remainingDevicesToRegisterBeforeDeployment: Set<String>
}

/**
* A device deployment status which indicates the device has been registered.
*/
sealed interface HasDeviceRegistration
{
/**
* The registration information of the device, if available.
yuanchen233 marked this conversation as resolved.
Show resolved Hide resolved
*/
val deviceRegistration: DeviceRegistration
}


/**
* Device deployment status for when a device has not been registered.
Expand All @@ -65,7 +77,7 @@ sealed class DeviceDeploymentStatus
override val device: AnyDeviceConfiguration,
override val canBeDeployed: Boolean,
override val remainingDevicesToRegisterToObtainDeployment: Set<String>,
override val remainingDevicesToRegisterBeforeDeployment: Set<String>
override val remainingDevicesToRegisterBeforeDeployment: Set<String>,
) : NotDeployed()

/**
Expand All @@ -74,18 +86,20 @@ sealed class DeviceDeploymentStatus
@Serializable
data class Registered(
override val device: AnyDeviceConfiguration,
override val deviceRegistration: DeviceRegistration,
override val canBeDeployed: Boolean,
override val remainingDevicesToRegisterToObtainDeployment: Set<String>,
override val remainingDevicesToRegisterBeforeDeployment: Set<String>
) : NotDeployed()
) : NotDeployed(), HasDeviceRegistration

/**
* Device deployment status when the device has retrieved its [PrimaryDeviceDeployment] and was able to load all the necessary plugins to execute the study.
*/
@Serializable
data class Deployed(
override val device: AnyDeviceConfiguration
) : DeviceDeploymentStatus()
override val device: AnyDeviceConfiguration,
override val deviceRegistration: DeviceRegistration
) : DeviceDeploymentStatus(), HasDeviceRegistration
{
// All devices that have been deployed necessarily can be deployed.
override val canBeDeployed = true
Expand All @@ -97,9 +111,10 @@ sealed class DeviceDeploymentStatus
@Serializable
data class NeedsRedeployment(
override val device: AnyDeviceConfiguration,
override val deviceRegistration: DeviceRegistration,
override val remainingDevicesToRegisterToObtainDeployment: Set<String>,
override val remainingDevicesToRegisterBeforeDeployment: Set<String>
) : NotDeployed()
) : NotDeployed(), HasDeviceRegistration
{
// All devices that have been deployed necessarily can be deployed.
override val canBeDeployed = true
Expand Down
20 changes: 17 additions & 3 deletions ...ployments.core/src/commonMain/kotlin/dk/cachet/carp/deployments/domain/StudyDeployment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,11 +287,25 @@ class StudyDeployment private constructor(
return when
{
needsRedeployment ->
DeviceDeploymentStatus.NeedsRedeployment( device, toObtainDeployment, beforeDeployment )
DeviceDeploymentStatus.NeedsRedeployment(
device,
_registeredDevices.getValue( device ),
toObtainDeployment,
beforeDeployment
)
isDeployed ->
DeviceDeploymentStatus.Deployed( device )
DeviceDeploymentStatus.Deployed(
device,
_registeredDevices.getValue( device )
)
isRegistered ->
DeviceDeploymentStatus.Registered( device, canBeDeployed, toObtainDeployment, beforeDeployment )
DeviceDeploymentStatus.Registered(
device,
_registeredDevices.getValue(device ),
canBeDeployed,
toObtainDeployment,
beforeDeployment
)
else ->
DeviceDeploymentStatus.Unregistered( device, canBeDeployed, toObtainDeployment, beforeDeployment )
}
Expand Down
37 changes: 35 additions & 2 deletions ...tlin/dk/cachet/carp/deployments/infrastructure/versioning/DeploymentServiceApiMigrator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ private val major1Minor0To1Migration =
}
}


private val major1Minor1To3Migration =
@Suppress( "MagicNumber" )
object : ApiMigration( 1, 3 )
Expand All @@ -48,6 +47,35 @@ private val major1Minor1To3Migration =
}
ApiResponse( responseObject, response.ex )
}
"dk.cachet.carp.deployments.infrastructure.DeploymentServiceRequest.CreateStudyDeployment",
"dk.cachet.carp.deployments.infrastructure.DeploymentServiceRequest.GetStudyDeploymentStatus",
"dk.cachet.carp.deployments.infrastructure.DeploymentServiceRequest.UnregisterDevice",
"dk.cachet.carp.deployments.infrastructure.DeploymentServiceRequest.DeployDevice",
Copy link
Member

Choose a reason for hiding this comment

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

This should be DeviceDeployed. 🤔 Why does it work? I'm not a big TDD fan, but there is something to seeing a test fail before making it pass.

Seems like luck would have it that's exactly where coverage is missing. 😅 Could you add a preceding commit which provides coverage for DeploymentService.deviceDeployed()? And, while at it, verify that all the other endpoints of this service are actually called in the current regression tests.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We only have coverage for CreateStudyDeployment, if I remember correctly that was the only failing test, as Actual testing of the status responses should already be covered adequately in StudyDeployment tests., but the regression test case are generated from DeploymentServiceTest.

Extra test cases seem relevant

Do we only want the generated json request files, or we also want those tests in DeploymentServiceTest? I think I'll include them first anyways to agree on the test cases, and they can be removed later if not desired.

I added it to 1.0 because I felt the test case was needed from the start of this major version, not due to some changes introduced in 1.1. Then simlly forget to add them 1.1 😛. As I generated those json files in ver. 1.0 and see a list of diff with origional ones, wandering what I breaked and realized those are the generated UUIDs difference...

Copy link
Member

@Whathecode Whathecode Oct 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Do we only want the generated json request files, or we also want those tests in DeploymentServiceTest?

I'd always keep them in sync, so that it's just a matter of copying the output of all of them over.

But, obviously the comment I left in code there didn't consider the implications on reduced regression testing of the infrastructure layer (JSON) by not covering all potential input/output. 😅 So albeit valid—there is plenty of functional coverage in StudDeploymentTest—, that doesn't cover serialization and JSON schema definitions.

I think it would make sense to write test cases that try to cover as much as possible, rather than writing them as unit tests. These are more or less integration tests after all. So, I'd focus on longer scenarios with multiple calls reflecting fuller study lifetime to get more coverage for the infrastructure layer.

As I generated those json files in ver. 1.0 and see a list of diff with origional ones, wandering what I breaked and realized those are the generated UUIDs difference...

I actually spotted that. 🤔. I thought I injected some more deterministic GUID and timestamp generator for the creation of these tests. I saw it in some test resources, but not others. I don't remember whether that's expected/normal, or oversight.

"dk.cachet.carp.deployments.infrastructure.DeploymentServiceRequest.Stop",
"dk.cachet.carp.deployments.infrastructure.DeploymentServiceRequest.RegisterDevice" ->
{
val responseObject = response.response?.jsonObject?.migrate {
updateArray( "deviceStatusList" ) {
yuanchen233 marked this conversation as resolved.
Show resolved Hide resolved
objects {
removeDeviceRegistration()
}
}
}
ApiResponse( responseObject, response.ex )
}
"dk.cachet.carp.deployments.infrastructure.DeploymentServiceRequest.GetStudyDeploymentStatusList" ->
{
val responseObject = (response.response as? JsonArray)?.migrate {
objects {
updateArray( "deviceStatusList" ) {
objects {
removeDeviceRegistration()
}
}
}
}
ApiResponse( responseObject, response.ex )
}
else -> response
}

Expand All @@ -60,10 +88,15 @@ private val major1Minor1To3Migration =
json.getOrPut( "deviceDisplayName" ) { JsonNull }
}

/**
* The `deviceRegistration` field was added to the `DeviceStatus` object.
*/
private fun ApiJsonObjectMigrationBuilder.removeDeviceRegistration() =
json.remove( "deviceRegistration" )

override fun migrateEvent( event: JsonObject ) = event
}


yuanchen233 marked this conversation as resolved.
Show resolved Hide resolved
val DeploymentServiceApiMigrator = ApplicationServiceApiMigrator(
DeploymentService.API_VERSION,
DeploymentServiceInvoker,
Expand Down
73 changes: 44 additions & 29 deletions ...vice/1.3/DeploymentServiceTest/createStudyDeployment_registers_preregistered_devices.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@
"request": {
"__type": "dk.cachet.carp.deployments.infrastructure.DeploymentServiceRequest.CreateStudyDeployment",
"apiVersion": "1.3",
"id": "bd9f4116-7c3c-4653-9285-38710660a8af",
"id": "5f8b35d8-2d9e-45ae-87d4-62274b0fe325",
"protocol": {
"id": "cc251597-c835-4120-b238-eca1df37eb9e",
"createdOn": "2024-10-09T08:17:33.754071Z",
"id": "0ef899d4-389b-4001-af50-bc7b50d28419",
"createdOn": "2024-10-23T13:28:04.937081Z",
"version": 0,
"ownerId": "27879e75-ccc1-4866-9ab3-4ece1b735052",
"name": "Test protocol",
Expand All @@ -34,7 +34,7 @@
},
"invitations": [
{
"participantId": "5511bab5-4f93-458a-b51e-d468d9624468",
"participantId": "d79806b7-c100-4617-ac26-75a958765d15",
"assignedRoles": {
"__type": "dk.cachet.carp.common.application.users.AssignedTo.All"
},
Expand All @@ -50,21 +50,21 @@
"connectedDevicePreregistrations": {
"Connected": {
"__type": "dk.cachet.carp.common.application.devices.DefaultDeviceRegistration",
"registrationCreatedOn": "2024-10-09T08:17:33.754108Z",
"deviceId": "03baa630-74f7-4f7b-ab89-aea7b1fa5e2a"
"registrationCreatedOn": "2024-10-23T13:28:04.937108Z",
"deviceId": "11ce9129-4d16-4c29-93e4-2e0ae1cb9a76"
}
}
},
"precedingEvents": [],
"publishedEvents": [
{
"__type": "dk.cachet.carp.deployments.application.DeploymentService.Event.StudyDeploymentCreated",
"aggregateId": "bd9f4116-7c3c-4653-9285-38710660a8af",
"aggregateId": "5f8b35d8-2d9e-45ae-87d4-62274b0fe325",
"apiVersion": "1.3",
"studyDeploymentId": "bd9f4116-7c3c-4653-9285-38710660a8af",
"studyDeploymentId": "5f8b35d8-2d9e-45ae-87d4-62274b0fe325",
"protocol": {
"id": "cc251597-c835-4120-b238-eca1df37eb9e",
"createdOn": "2024-10-09T08:17:33.754071Z",
"id": "0ef899d4-389b-4001-af50-bc7b50d28419",
"createdOn": "2024-10-23T13:28:04.937081Z",
"version": 0,
"ownerId": "27879e75-ccc1-4866-9ab3-4ece1b735052",
"name": "Test protocol",
Expand All @@ -91,7 +91,7 @@
},
"invitations": [
{
"participantId": "5511bab5-4f93-458a-b51e-d468d9624468",
"participantId": "d79806b7-c100-4617-ac26-75a958765d15",
"assignedRoles": {
"__type": "dk.cachet.carp.common.application.users.AssignedTo.All"
},
Expand All @@ -107,16 +107,16 @@
"connectedDevicePreregistrations": {
"Connected": {
"__type": "dk.cachet.carp.common.application.devices.DefaultDeviceRegistration",
"registrationCreatedOn": "2024-10-09T08:17:33.754108Z",
"deviceId": "03baa630-74f7-4f7b-ab89-aea7b1fa5e2a"
"registrationCreatedOn": "2024-10-23T13:28:04.937108Z",
"deviceId": "11ce9129-4d16-4c29-93e4-2e0ae1cb9a76"
}
}
}
],
"response": {
"__type": "dk.cachet.carp.deployments.application.StudyDeploymentStatus.DeployingDevices",
"createdOn": "1970-01-01T00:00:00Z",
"studyDeploymentId": "bd9f4116-7c3c-4653-9285-38710660a8af",
"studyDeploymentId": "5f8b35d8-2d9e-45ae-87d4-62274b0fe325",
"deviceStatusList": [
{
"__type": "dk.cachet.carp.deployments.application.DeviceDeploymentStatus.Unregistered",
Expand All @@ -139,14 +139,19 @@
"__type": "dk.cachet.carp.common.infrastructure.test.StubDeviceConfiguration",
"roleName": "Connected"
},
"deviceRegistration": {
"__type": "dk.cachet.carp.common.application.devices.DefaultDeviceRegistration",
"registrationCreatedOn": "2024-10-23T13:28:04.937108Z",
"deviceId": "11ce9129-4d16-4c29-93e4-2e0ae1cb9a76"
},
"canBeDeployed": false,
"remainingDevicesToRegisterToObtainDeployment": [],
"remainingDevicesToRegisterBeforeDeployment": []
}
],
"participantStatusList": [
{
"participantId": "5511bab5-4f93-458a-b51e-d468d9624468",
"participantId": "d79806b7-c100-4617-ac26-75a958765d15",
"assignedParticipantRoles": {
"__type": "dk.cachet.carp.common.application.users.AssignedTo.All"
},
Expand All @@ -163,37 +168,37 @@
"request": {
"__type": "dk.cachet.carp.deployments.infrastructure.DeploymentServiceRequest.RegisterDevice",
"apiVersion": "1.3",
"studyDeploymentId": "bd9f4116-7c3c-4653-9285-38710660a8af",
"studyDeploymentId": "5f8b35d8-2d9e-45ae-87d4-62274b0fe325",
"deviceRoleName": "Primary",
"registration": {
"__type": "dk.cachet.carp.common.application.devices.DefaultDeviceRegistration",
"registrationCreatedOn": "2024-10-09T08:17:33.754227Z",
"deviceId": "a7b167cb-77d6-4db3-8fc3-cc3d612c82de"
"registrationCreatedOn": "2024-10-23T13:28:04.937259Z",
"deviceId": "199d05be-5e67-4c68-8ec9-c55290841c9a"
}
},
"precedingEvents": [],
"publishedEvents": [
{
"__type": "dk.cachet.carp.deployments.application.DeploymentService.Event.DeviceRegistrationChanged",
"aggregateId": "bd9f4116-7c3c-4653-9285-38710660a8af",
"aggregateId": "5f8b35d8-2d9e-45ae-87d4-62274b0fe325",
"apiVersion": "1.3",
"studyDeploymentId": "bd9f4116-7c3c-4653-9285-38710660a8af",
"studyDeploymentId": "5f8b35d8-2d9e-45ae-87d4-62274b0fe325",
"device": {
"__type": "dk.cachet.carp.common.infrastructure.test.StubPrimaryDeviceConfiguration",
"isPrimaryDevice": true,
"roleName": "Primary"
},
"registration": {
"__type": "dk.cachet.carp.common.application.devices.DefaultDeviceRegistration",
"registrationCreatedOn": "2024-10-09T08:17:33.754227Z",
"deviceId": "a7b167cb-77d6-4db3-8fc3-cc3d612c82de"
"registrationCreatedOn": "2024-10-23T13:28:04.937259Z",
"deviceId": "199d05be-5e67-4c68-8ec9-c55290841c9a"
}
}
],
"response": {
"__type": "dk.cachet.carp.deployments.application.StudyDeploymentStatus.DeployingDevices",
"createdOn": "1970-01-01T00:00:00Z",
"studyDeploymentId": "bd9f4116-7c3c-4653-9285-38710660a8af",
"studyDeploymentId": "5f8b35d8-2d9e-45ae-87d4-62274b0fe325",
"deviceStatusList": [
{
"__type": "dk.cachet.carp.deployments.application.DeviceDeploymentStatus.Registered",
Expand All @@ -202,6 +207,11 @@
"isPrimaryDevice": true,
"roleName": "Primary"
},
"deviceRegistration": {
"__type": "dk.cachet.carp.common.application.devices.DefaultDeviceRegistration",
"registrationCreatedOn": "2024-10-23T13:28:04.937259Z",
"deviceId": "199d05be-5e67-4c68-8ec9-c55290841c9a"
},
"canBeDeployed": true,
"remainingDevicesToRegisterToObtainDeployment": [],
"remainingDevicesToRegisterBeforeDeployment": []
Expand All @@ -212,14 +222,19 @@
"__type": "dk.cachet.carp.common.infrastructure.test.StubDeviceConfiguration",
"roleName": "Connected"
},
"deviceRegistration": {
"__type": "dk.cachet.carp.common.application.devices.DefaultDeviceRegistration",
"registrationCreatedOn": "2024-10-23T13:28:04.937108Z",
"deviceId": "11ce9129-4d16-4c29-93e4-2e0ae1cb9a76"
},
"canBeDeployed": false,
"remainingDevicesToRegisterToObtainDeployment": [],
"remainingDevicesToRegisterBeforeDeployment": []
}
],
"participantStatusList": [
{
"participantId": "5511bab5-4f93-458a-b51e-d468d9624468",
"participantId": "d79806b7-c100-4617-ac26-75a958765d15",
"assignedParticipantRoles": {
"__type": "dk.cachet.carp.common.application.users.AssignedTo.All"
},
Expand All @@ -236,7 +251,7 @@
"request": {
"__type": "dk.cachet.carp.deployments.infrastructure.DeploymentServiceRequest.GetDeviceDeploymentFor",
"apiVersion": "1.3",
"studyDeploymentId": "bd9f4116-7c3c-4653-9285-38710660a8af",
"studyDeploymentId": "5f8b35d8-2d9e-45ae-87d4-62274b0fe325",
"primaryDeviceRoleName": "Primary"
},
"precedingEvents": [],
Expand All @@ -249,8 +264,8 @@
},
"registration": {
"__type": "dk.cachet.carp.common.application.devices.DefaultDeviceRegistration",
"registrationCreatedOn": "2024-10-09T08:17:33.754227Z",
"deviceId": "a7b167cb-77d6-4db3-8fc3-cc3d612c82de"
"registrationCreatedOn": "2024-10-23T13:28:04.937259Z",
"deviceId": "199d05be-5e67-4c68-8ec9-c55290841c9a"
},
"connectedDevices": [
{
Expand All @@ -261,8 +276,8 @@
"connectedDeviceRegistrations": {
"Connected": {
"__type": "dk.cachet.carp.common.application.devices.DefaultDeviceRegistration",
"registrationCreatedOn": "2024-10-09T08:17:33.754108Z",
"deviceId": "03baa630-74f7-4f7b-ab89-aea7b1fa5e2a"
"registrationCreatedOn": "2024-10-23T13:28:04.937108Z",
"deviceId": "11ce9129-4d16-4c29-93e4-2e0ae1cb9a76"
}
}
}
Expand Down
Loading