Fix: Preserve CSV-imported device data during sync #3295
There are no files selected for viewing
Empty file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -450,5 +450,131 @@ public async Task Execute_MissingModelId_ShouldNotFail() | |
| // Assert | ||
| MockRepository.VerifyAll(); | ||
| } | ||
|
|
||
| [Test] | ||
| public async Task Execute_ExistingLorawanDeviceWithMissingTwinProperties_DatabaseValuesPreserved() | ||
| { | ||
| // Arrange | ||
| var mockJobExecutionContext = MockRepository.Create<IJobExecutionContext>(); | ||
|
|
||
| var expectedDeviceModel = Fixture.Create<DeviceModel>(); | ||
| expectedDeviceModel.SupportLoRaFeatures = true; | ||
|
|
||
| // Twin with only core properties and AppKey/AppEUI (OTAA) | ||
| var expectedTwinDevice = new Twin | ||
| { | ||
| DeviceId = Fixture.Create<string>(), | ||
| Tags = new TwinCollection | ||
| { | ||
| ["modelId"] = expectedDeviceModel.Id, | ||
| ["deviceName"] = "UpdatedDeviceName" | ||
| }, | ||
| Version = 2 | ||
| }; | ||
| expectedTwinDevice.Properties.Desired["AppKey"] = "NewAppKey"; | ||
| expectedTwinDevice.Properties.Desired["AppEUI"] = "NewAppEUI"; | ||
|
|
||
| // Existing device with additional LoRaWAN properties that should be preserved | ||
| var existingLorawanDevice = new LorawanDevice | ||
| { | ||
| Id = expectedTwinDevice.DeviceId, | ||
| Name = "OldDeviceName", | ||
| Version = 1, | ||
| AppKey = "OldAppKey", | ||
| AppEUI = "OldAppEUI", | ||
| UseOTAA = false, // Should be updated when AppEUI is in Twin | ||
| AlreadyLoggedInOnce = true, // Should be preserved (DevAddr not in reported properties) | ||
| SensorDecoder = "ExistingSensorDecoder", // Should be preserved | ||
| ClassType = ClassType.C, // Should be preserved | ||
| Deduplication = DeduplicationMode.Mark, // Should be preserved | ||
| RX1DROffset = 5, // Should be preserved | ||
| KeepAliveTimeout = 120, // Should be preserved | ||
| GatewayID = "ExistingGatewayID", // Should be preserved (not in reported properties) | ||
| Tags = new List<DeviceTagValue> | ||
| { | ||
| new() | ||
| { | ||
| Id = Fixture.Create<string>(), | ||
| Name = "existingTag", | ||
| Value = "existingValue" | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| _ = this.mockDeviceService | ||
| .Setup(x => x.GetAllDevice( | ||
| It.IsAny<string>(), | ||
| It.IsAny<string>(), | ||
| It.Is<string>(x => x == "LoRa Concentrator"), | ||
| It.IsAny<string>(), | ||
| It.IsAny<bool?>(), | ||
| It.IsAny<bool?>(), | ||
| It.IsAny<Dictionary<string, string>>(), | ||
| It.Is<int>(x => x == 100))) | ||
| .ReturnsAsync(new PaginationResult<Twin> | ||
| { | ||
| Items = new List<Twin> | ||
| { | ||
| expectedTwinDevice | ||
| }, | ||
| TotalItems = 1 | ||
| }); | ||
|
|
||
| _ = this.mockDeviceModelRepository | ||
| .Setup(x => x.GetByIdAsync(expectedDeviceModel.Id)) | ||
| .ReturnsAsync(expectedDeviceModel); | ||
|
|
||
| _ = this.mockLorawanDeviceRepository.Setup(repository => repository.GetByIdAsync(expectedTwinDevice.DeviceId, d => d.Tags)) | ||
| .ReturnsAsync(existingLorawanDevice); | ||
|
|
||
| this.mockDeviceTagValueRepository.Setup(repository => repository.Delete(It.IsAny<string>())) | ||
| .Verifiable(); | ||
|
|
||
| this.mockLorawanDeviceRepository.Setup(repository => repository.Update(It.Is<LorawanDevice>(d => | ||
| // Verify core properties are updated | ||
| d.Name == "UpdatedDeviceName" && | ||
| d.Version == 2 && | ||
| // Verify Twin properties are updated | ||
| d.AppKey == "NewAppKey" && | ||
| d.AppEUI == "NewAppEUI" && | ||
| d.UseOTAA == true && // Updated because AppEUI is in Twin | ||
|
There was a problem hiding this comment. The expression 'A == true' can be simplified to 'A'. |
||
| // Verify database-only properties are preserved | ||
| d.SensorDecoder == "ExistingSensorDecoder" && | ||
| d.ClassType == ClassType.C && | ||
| d.Deduplication == DeduplicationMode.Mark && | ||
| d.RX1DROffset == 5 && | ||
| d.KeepAliveTimeout == 120 && | ||
| d.GatewayID == "ExistingGatewayID" && | ||
| d.AlreadyLoggedInOnce == true // Preserved because DevAddr not in reported properties | ||
|
There was a problem hiding this comment. The expression 'A == true' can be simplified to 'A'. |
||
| ))) | ||
| .Verifiable(); | ||
|
|
||
| _ = this.mockDeviceRepository.Setup(x => x.GetAllAsync(It.IsAny<Expression<Func<Device, bool>>>(), It.IsAny<CancellationToken>())) | ||
| .ReturnsAsync(new List<Device> | ||
| { | ||
| new Device | ||
| { | ||
| Id = expectedTwinDevice.DeviceId | ||
| } | ||
| }); | ||
|
|
||
| _ = this.mockLorawanDeviceRepository.Setup(x => x.GetAllAsync(It.IsAny<Expression<Func<LorawanDevice, bool>>>(), It.IsAny<CancellationToken>())) | ||
| .ReturnsAsync(new List<LorawanDevice> | ||
| { | ||
| new LorawanDevice | ||
| { | ||
| Id = expectedTwinDevice.DeviceId | ||
| } | ||
| }); | ||
|
|
||
| _ = this.mockUnitOfWork.Setup(work => work.SaveAsync()) | ||
| .Returns(Task.CompletedTask); | ||
|
|
||
| // Act | ||
| await this.syncDevicesJob.Execute(mockJobExecutionContext.Object); | ||
|
|
||
| // Assert | ||
| MockRepository.VerifyAll(); | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mapper's Twin->LorawanDevice mapping uses helper methods like GetDesiredPropertyAsIntegerValue and GetDesiredPropertyAsEnum that return nullable values. When these properties are not present in the Twin, the mapper would set them to null or default values. However, the new implementation only updates properties if they're present in the Twin. This means that if a property WAS in the Twin before but is now removed, it won't be cleared from the database - it will keep the old value. Consider whether properties should be explicitly set to null/default when they're removed from the Twin, or document this as expected behavior.