Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Bug: SqlTopicRepository: Implicitly IsDirty() topics not being saved #73

Closed
JeremyCaney opened this issue Mar 30, 2021 · 1 comment
Closed

Bug: SqlTopicRepository: Implicitly IsDirty() topics not being saved #73

JeremyCaney opened this issue Mar 30, 2021 · 1 comment
Assignees
@JeremyCaney
Labels
Area: Repositories Relates to the `ITopicRepository` interface or one of its implementations. Priority: 1 Severity 2: Major Status 5: Complete Task is considered complete, and ready for deployment. Type: Bug Behavior that is inconsistent with documented or expected behavior.
Milestone
OnTopic 5.1.0

Comments

@JeremyCaney
Copy link
Member

If a Topic contains an AttributeRecord that is marked as IsExtendedAttribute, but which corresponds to a AttributeDescriptor that is not marked as IsExtendedAttribute—or vice versa!—then it is treated as IsDirty() implicitly by the TopicRepository.GetAttributes() method, even if the AttributeRecord is otherwise clean. This is important as it allows attributes that have been toggled between indexed and extended storage to be updated during Save(). This functionality is well-established and covered by extensive unit tests.

Unfortunately, this rule is not being honored by the SqlTopicRepository implementation, which does not have good unit test coverage due to the challenges of mocking the SQL interfaces (e.g., SqlConnection, SqlCommand, &c.). Looking at the code for Save(), it appears that this rule is being honored when evaluating indexed attributes that have been moved to extended attributes, but not the other direction.
@JeremyCaney JeremyCaney added Type: Bug Behavior that is inconsistent with documented or expected behavior. Area: Repositories Relates to the `ITopicRepository` interface or one of its implementations. Severity 2: Major Priority: 1 Status 2: Scheduled Planned for an upcoming release. labels Mar 30, 2021
@JeremyCaney JeremyCaney added this to the OnTopic 5.1.0 milestone Mar 30, 2021
@JeremyCaney JeremyCaney self-assigned this Mar 30, 2021
JeremyCaney added a commit that referenced this issue Mar 30, 2021
@JeremyCaney
Ensure that implicitly dirty attributes are Save()d
c272e52 
Previously, the `SqlTopicRepository.Save()` would save an otherwise clean `Topic` if any of its `Attributes` were marked as indexed, even though their corresponding `AttributeDescriptor` marked them as extended. That is correct behavior. The opposite scenario, however, was not being tripped. As a result, attributes reconfigured from extended to indexed storage would not be picked up as part of `Save()` unless the `Topic` was _explicitly_ marked as `IsDirty()`. This fixes that issue, and resolves #73.
JeremyCaney added a commit that referenced this issue Mar 30, 2021
@JeremyCaney
Merge branch 'bugfix/SqlTopicRepository.Save-implicit-IsDirty' into d…
4e9ec50 
…evelop

This resolves a bug in which attributes which had been reconfigured from extended to indexed storage weren't being saved by `SqlTopicRepository.Save()` unless the `Topic` was explicitly marked as `IsDirty()`. This fix ensures that topics that are _implicitly_ dirty due to `IsExtendedAttribute` mismatches are treated as `IsDirty()` as far as `Save()` is concerned. This was already supported for indexed attributes configured as extended attributes, but not the other way around.

This resolves Issue #73.
@JeremyCaney JeremyCaney added Status 5: Complete Task is considered complete, and ready for deployment. and removed Status 2: Scheduled Planned for an upcoming release. labels Mar 30, 2021
@JeremyCaney
Copy link
Member Author

This was resolved in 4e9ec50. This is not validated by unit tests, since the main SqlTopicRepository is not yet covered due to complexities of mocking SqlConnection, SqlCommand, &c. (This remains the one hole in our unit testing coverage—though the base logic, stored procedures, and extension methods themselves remain well covered.)

@JeremyCaney JeremyCaney mentioned this issue Apr 7, 2021
Merged
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees

@JeremyCaney JeremyCaney

Labels
Area: Repositories Relates to the `ITopicRepository` interface or one of its implementations. Priority: 1 Severity 2: Major Status 5: Complete Task is considered complete, and ready for deployment. Type: Bug Behavior that is inconsistent with documented or expected behavior.
Projects
None yet
Milestone
OnTopic 5.1.0
Development

No branches or pull requests
1 participant
@JeremyCaney