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

Jump to bottom

[mongodb] Upgrade DB driver, add more type handlings, fix QuantityType handling #16333

Merged
merged 14 commits into from Feb 17, 2024
Merged

[mongodb] Upgrade DB driver, add more type handlings, fix QuantityType handling #16333

merged 14 commits into from Feb 17, 2024

Conversation

ulbi
Copy link
Contributor

@ulbi ulbi commented Jan 28, 2024

[mongodb] Upgrade DB driver, add more type handlings, fix QuanitityType handling

Description

This pull request addresses issues #16308 and #16310, introducing significant updates and improvements to the system:

PR Classification

  • Improvement: Updates the MongoDB driver for enhanced compatibility and adds comprehensive unit testing.
  • Bugfix: Fixes handling for various item types, ensuring consistent behavior across different MongoDB versions.

Goals and Motivation

The main objective is to modernize the MongoDB driver used within our system from the outdated mongo-java-driver 2.13.1 (2015) to the current mongo-driver-sync 4.11.1. This upgrade not only extends support to MongoDB versions 3.6 through 6.0 but also addresses compatibility issues with MongoDB 5.0 and above, which is no longer adequately supported by the old driver.

Noteworthy Changes for End Users

  • Database Driver Upgrade: Transition to mongo-driver-sync 4.11.1, enhancing support for MongoDB versions 3.6 to 6.0 and ensuring better system performance and reliability.
  • Enhanced Item Support: Improved handling for NumberItem with QuantityTypes and added support for ColorItem, LocationItem, ImageItem, PlayerItem, and CallItem, along with their respective state types, enhancing the system's versatility and user experience.

Unit Testing Enhancements

  • Added unit tests to validate type storage and querying, ensuring robustness and reliability.
  • Expanded testing to cover all supported MongoDB versions (3.6, 4.4, 5.0, 6.0), guaranteeing compatibility across different database versions.

Technical Improvements

  • For consistency with InfluxDB and compatibility with charting tools like Grafana, QuantityType handling now stores only the number without the unit. This adjustment allows for more seamless integration with external tools that may not support string-based data types.

Documentation and Guidelines

  • Updated documentation to reflect these changes, ensuring users and developers can easily understand and adopt the new features and improvements.

J-N-K
J-N-K reviewed Jan 28, 2024
View reviewed changes
Copy link
Member

@J-N-K J-N-K left a comment

Choose a reason for hiding this comment

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

Regarding the change: I fear that all stored quantity-type will be lost, is that correct? Do we have a chance to convert them or a "compatibilty mode" for accessing them?

bundles/org.openhab.persistence.mongodb/pom.xml Outdated Show resolved Hide resolved
bundles/org.openhab.persistence.mongodb/pom.xml Outdated Show resolved Hide resolved
@ulbi
Copy link
Contributor Author

ulbi commented Jan 28, 2024 •
edited

@J-N-K @jlaur just created this PR to address a couple of challenges with the current mongodb driver. I created this as draft in the first place, since I plan two more things:

  1. add code to also handle the data stored by the current mongodb persistence as fas as possible e.g. QuantityType (currently stored as string, but not restored properly) and all other where "toString" provides the full context (ColorItem, LocationItem, ...) - this will not work for ImageItem, since a toString of RawType does not store everything
  2. add exception handling if the ImageItem or anything else tries to store more than 16 MB (document size limit from MongoDB)
  3. store the Unit for QuantityType if provided in a separate field and then restore it from there.
  4. Address your comments ;-)

Once done, this should cover a couple of issues.

@ulbi
Copy link
Contributor Author

ulbi commented Jan 28, 2024

Regarding the change: I fear that all stored quantity-type will be lost, is that correct? Do we have a chance to convert them or a "compatibilty mode" for accessing them?

You are too fast! I will add something for this as well, just wanted to create the PR already, since there are quite some changes already. And just figured out, that the online build environment does not contain docker, so all tests using "Testcontainers" do fail - too bad, this is a great way to verify the different mongodb versions.

@ulbi ulbi force-pushed the 16308-fix branch 2 times, most recently from e243747 to 386cebb Compare January 28, 2024 17:16
@ulbi
openhab#16308 Commit to share
8050753 
Signed-off-by: René Ulbricht <rene_ulbricht@outlook.com>
@ulbi
openhab#16308 fixed the pom.xml and finalized the dependencies
6520826 
Signed-off-by: René Ulbricht <rene_ulbricht@outlook.com>
@ulbi
openhab#16308 openhab#16310 Upgraded MongoDB driver, added initial un…
352e861 
…it tests

Signed-off-by: René Ulbricht <rene_ulbricht@outlook.com>
@ulbi
openhab#16308 openhab#16310 Refactored the MongoDBPersistence adding …
0bc4b8f 
…helper, fixing type handling for HSBType, RawType and QuantityType

Signed-off-by: René Ulbricht <rene_ulbricht@outlook.com>
@ulbi
openhab#16308 openhab#16310 Removed unrequired dependencies and skip …
e9b9e29 
…some tests if docker is missing.

Signed-off-by: René Ulbricht <rene_ulbricht@outlook.com>
@ulbi
openhab#16308 Added backwardcompatibility for the old way of writting…
a29b43e 
… the data where possible

Signed-off-by: René Ulbricht <rene_ulbricht@outlook.com>
@ulbi
openhab#16308 Added test for larger ImageItems and the limit of 16 MB
9ad72c3 
Signed-off-by: René Ulbricht <rene_ulbricht@outlook.com>
@ulbi
openhab#16308 Added unit storage in DB and fixed bug in HistoricItem.…
9bcf925 
…toString

Signed-off-by: René Ulbricht <rene_ulbricht@outlook.com>
@ulbi ulbi marked this pull request as ready for review January 29, 2024 21:25
@ulbi ulbi requested a review from a team as a code owner January 29, 2024 21:25
@ulbi
Copy link
Contributor Author

ulbi commented Feb 8, 2024

@J-N-K Just a quick question, anything else required to get this merged? I'm asking because the changes required for #16338 are ready as well, should I put it in the PR as well?

@jlaur jlaur changed the title [mongodb] Upgrade DB driver, add more type handlings, fix QuanitityType handling [mongodb] Upgrade DB driver, add more type handlings, fix QuantityType handling Feb 9, 2024
@lsiepel
Copy link
Contributor

lsiepel commented Feb 10, 2024

@J-N-K Just a quick question, anything else required to get this merged? I'm asking because the changes required for #16338 are ready as well, should I put it in the PR as well?

As I don’t expect many users to test this persistence prior to stable. Maybe good to integrate it and get some mileage on both changes.

J-N-K
J-N-K requested changes Feb 10, 2024
View reviewed changes
...ongodb/src/main/java/org/openhab/persistence/mongodb/internal/MongoDBPersistenceService.java Outdated Show resolved Hide resolved
...e.mongodb/src/main/java/org/openhab/persistence/mongodb/internal/MongoDBTypeConversions.java Outdated Show resolved Hide resolved
...e.mongodb/src/main/java/org/openhab/persistence/mongodb/internal/MongoDBTypeConversions.java Outdated Show resolved Hide resolved
...e.mongodb/src/main/java/org/openhab/persistence/mongodb/internal/MongoDBTypeConversions.java Outdated Show resolved Hide resolved
...e.mongodb/src/main/java/org/openhab/persistence/mongodb/internal/MongoDBTypeConversions.java Outdated Show resolved Hide resolved
...ce.mongodb/src/test/java/org/openhab/persistence/mongodb/internal/DatabaseTestContainer.java Show resolved Hide resolved
...tence.mongodb/src/test/java/org/openhab/persistence/mongodb/internal/DataCreationHelper.java Show resolved Hide resolved
...e.mongodb/src/main/java/org/openhab/persistence/mongodb/internal/MongoDBTypeConversions.java Show resolved Hide resolved
....persistence.mongodb/src/test/java/org/openhab/persistence/mongodb/internal/SetupResult.java Show resolved Hide resolved
...tence.mongodb/src/test/java/org/openhab/persistence/mongodb/internal/VerificationHelper.java Show resolved Hide resolved
@ulbi
Copy link
Contributor Author

ulbi commented Feb 10, 2024

@lsiepel Can do it, this will fix a couple of the requested changes as well, since I fixed most of the compiler warnings in there. Will just put it in here with the changes requested by @J-N-K .

ulbi and others added 6 commits February 10, 2024 15:40
@ulbi @J-N-K
Apply suggestions from code review
50be513 
Co-authored-by: J-N-K <github@klug.nrw>
Signed-off-by: ulbi <rene_ulbricht@outlook.com>
@ulbi @J-N-K
Update bundles/org.openhab.persistence.mongodb/src/test/java/org/open…
4d25852 
…hab/persistence/mongodb/internal/DataCreationHelper.java

Co-authored-by: J-N-K <github@klug.nrw>
Signed-off-by: ulbi <rene_ulbricht@outlook.com>
@ulbi
Closes openhab#16338 Implemented ModifiablePersistenceServices and fi…
8e360d0 
…xed compiler warnings

Signed-off-by: René Ulbricht <rene_ulbricht@outlook.com>
@ulbi
Closes openhab#16338 Implemented ModifiablePersistenceServices and fi…
f487539 
…xed compiler warnings

Signed-off-by: René Ulbricht <rene_ulbricht@outlook.com>
@ulbi
Closes openhab#16338 Implemented ModifiablePersistenceServices and fi…
775e079 
…xed compiler warnings

Signed-off-by: René Ulbricht <rene_ulbricht@outlook.com>
@ulbi
Closes openhab#16308, openhab#16310, openhab#16338 Refactored the cod…
8ed69ad 
…e to be more robust and implemented the ModifiablePersistenceService

Signed-off-by: René Ulbricht <rene_ulbricht@outlook.com>
@ulbi
Copy link
Contributor Author

ulbi commented Feb 10, 2024

@J-N-K @lsiepel I should have covered all comments and added the enhancement for #16338 as well.

@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/mongo-db-cannot-be-cast-to-class-java-lang-number/153019/2

J-N-K
J-N-K approved these changes Feb 17, 2024
View reviewed changes
Copy link
Member

@J-N-K J-N-K left a comment

Choose a reason for hiding this comment

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

Thanks!

@J-N-K J-N-K merged commit 956b8e4 into openhab:main Feb 17, 2024
3 checks passed
@J-N-K J-N-K added the enhancement An enhancement or new feature for an existing add-on label Feb 17, 2024
@J-N-K J-N-K added this to the 4.2 milestone Feb 17, 2024
This was linked to issues Feb 17, 2024
[mongodb] persistence query gives ClassCastException for QuantityTypes #16308
Closed
[mongodb] Upgrade MongoDB support from 4.x (max) to 6.0 #16310
Closed
[mongodb] Should implement ModifiablePersistenceService #16338
Closed
austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
@ulbi @austvik
[mongodb] Upgrade DB driver, add more type handlings, fix QuantityTyp…
9a2c41b 
…e handling (openhab#16333)

* openhab#16308 openhab#16310 Upgraded MongoDB driver, added initial unit tests
* openhab#16308 openhab#16310 Refactored the MongoDBPersistence adding helper, fixing type handling for HSBType, RawType and QuantityType
* openhab#16308 Added backwardcompatibility for the old way of writting the data where possible
* openhab#16308 Added test for larger ImageItems and the limit of 16 MB

Signed-off-by: René Ulbricht <rene_ulbricht@outlook.com>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@J-N-K J-N-K J-N-K approved these changes

Assignees
No one assigned
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Milestone
4.2
4 participants
@ulbi @lsiepel @openhab-bot @J-N-K