Issue 30277 create api factory methods to insert in the unique fields table #30466
Conversation
This file contains 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
ref: #29555 Changing link from `/c/dotAI` to `/c/dotai`
…sert-in-the-unique_fields-table
…sert-in-the-unique_fields-table
…-unique_fields-table' of https://github.com/dotCMS/core into issue-30277-Create-API-Factory-methods-to-insert-in-the-unique_fields-table
…sert-in-the-unique_fields-table
…sert-in-the-unique_fields-table
…-unique_fields-table' of https://github.com/dotCMS/core into issue-30277-Create-API-Factory-methods-to-insert-in-the-unique_fields-table
…sert-in-the-unique_fields-table
…API-Factory-methods-to-insert-in-the-unique_fields-table
…sert-in-the-unique_fields-table
…-unique_fields-table' of https://github.com/dotCMS/core into issue-30277-Create-API-Factory-methods-to-insert-in-the-unique_fields-table
jdotcms
reviewed
Oct 28, 2024
...java/com/dotcms/contenttype/business/uniquefields/UniqueFieldValidationStrategyResolver.java
Outdated
Show resolved
Hide resolved
jdotcms
reviewed
Oct 28, 2024
...in/java/com/dotcms/contenttype/business/uniquefields/extratable/UniqueFieldDataBaseUtil.java
Outdated
Show resolved
Hide resolved
dsilvam
reviewed
Oct 28, 2024
dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java
Outdated
Show resolved
Hide resolved
dsilvam
reviewed
Oct 28, 2024
...ms/contenttype/business/uniquefields/extratable/ExtraTableUniqueFieldValidationStrategy.java
Outdated
Show resolved
Hide resolved
jcastro-dotcms
requested changes
Oct 28, 2024
dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java
Outdated
Show resolved
Hide resolved
dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java
Outdated
Show resolved
Hide resolved
...c/main/java/com/dotcms/contenttype/business/uniquefields/extratable/UniqueFieldCriteria.java
Outdated
Show resolved
Hide resolved
dsilvam
reviewed
Oct 28, 2024
…sert-in-the-unique_fields-table
…-unique_fields-table' of https://github.com/dotCMS/core into issue-30277-Create-API-Factory-methods-to-insert-in-the-unique_fields-table
jcastro-dotcms
approved these changes
Oct 30, 2024
jdotcms
approved these changes
Oct 30, 2024
dsilvam
approved these changes
Oct 30, 2024
…sert-in-the-unique_fields-table
|
freddyDOTCMS
deleted the
issue-30277-Create-API-Factory-methods-to-insert-in-the-unique_fields-table
branch
dsolistorres
pushed a commit
that referenced
this pull request
Nov 5, 2024
… table (#30466) We use a Lucene query to check for unique field values, but this approach has issues due to a race condition. The ElasticSearch data isn’t updated immediately after the Contentlet database update, so if another Contentlet with the same unique values is saved before ElasticSearch is refreshed, duplicates can occur. This issue is particularly likely during high-volume imports, such as when importing hundreds of Contentlets at once. I was able to reproduce this error locally by directly creating Contentlets via the API endpoint, sending 100 simultaneous requests using Postman. The new approach for validating unique fields involves using an additional table. This table will have a primary key created from a hash, which combines the following elements: the ContentType's variable name, language, Field's variable name, Field's value, and—if the uniquePerSite field variable is set to TRUE—the site ID. ### Proposed Changes * We don't want to remove the old approach with the ES validation, we want to keep it if we need to come back to it to avoid any unexpected problem with the new approach, so I am going to create a Strategy to switch between this 2 approaches using a config property. https://github.com/dotCMS/core/pull/30466/files#diff-fa1ceaa19618a6b2bbc30e24c6f930b4971f417db50babb748c2e2837ba9eb82R237 https://github.com/dotCMS/core/pull/30466/files#diff-fa1ceaa19618a6b2bbc30e24c6f930b4971f417db50babb748c2e2837ba9eb82R7654 https://github.com/dotCMS/core/pull/30466/files#diff-445f8d01aa4de058eaaf883e573d17ef1123b1e74a9a98120ac156b78f4c6522R17 * For our new Extra table approach we ned to save the register in the extra table with the Contentlet's ID, the Contentlet's ID is not used for the hash calculation but is going to be sued later to clean up the table when a Contentlet is deleted, that is why a afterSaved method is included in the Strategy, this method is going to be called after saved the Contentlet https://github.com/dotCMS/core/pull/30466/files#diff-fa1ceaa19618a6b2bbc30e24c6f930b4971f417db50babb748c2e2837ba9eb82R5528 - I am going to remove all the ES validation code, and add it in the new ESUniqueFieldValidationStrategy class https://github.com/dotCMS/core/pull/30466/files#diff-fa1ceaa19618a6b2bbc30e24c6f930b4971f417db50babb748c2e2837ba9eb82L7635-L7725 https://github.com/dotCMS/core/pull/30466/files#diff-e3b9fd8560a668db0c37d00715b112bfa3fdfddc778b5dd1c4bac3b73f1fdd72R45 - I need to create a new ExtraTableUniqueFieldValidationStrategy for the Extra table validation Strategy implementation https://github.com/dotCMS/core/pull/30466/files#diff-efdeeb8f5e148f60415043882bbe26bafc2d643378abdb58e7b8fc944087fe52R40 - Create a Singleton util Class to provide all the method to work with the new extra table, I don't create a Factory because in this case really we don't have a API to match the Factory https://github.com/dotCMS/core/pull/30466/files#diff-b0aed2eddcc36be2a2e2f46624345a83c8698db238fe4dc228c0fb13a11e344fR16 ### Checklist - [ ] Tests - [ ] Translations - [ ] Security Implications Contemplated (add notes if applicable) ### Additional Info ** any additional useful context or info ** ### Screenshots Original | Updated :-------------------------:|:-------------------------: ** original screenshot ** | ** updated screenshot ** --------- Co-authored-by: Will Ezell <[email protected]>
spbolton
pushed a commit
that referenced
this pull request
Nov 11, 2024
… table (#30466) We use a Lucene query to check for unique field values, but this approach has issues due to a race condition. The ElasticSearch data isn’t updated immediately after the Contentlet database update, so if another Contentlet with the same unique values is saved before ElasticSearch is refreshed, duplicates can occur. This issue is particularly likely during high-volume imports, such as when importing hundreds of Contentlets at once. I was able to reproduce this error locally by directly creating Contentlets via the API endpoint, sending 100 simultaneous requests using Postman. The new approach for validating unique fields involves using an additional table. This table will have a primary key created from a hash, which combines the following elements: the ContentType's variable name, language, Field's variable name, Field's value, and—if the uniquePerSite field variable is set to TRUE—the site ID. ### Proposed Changes * We don't want to remove the old approach with the ES validation, we want to keep it if we need to come back to it to avoid any unexpected problem with the new approach, so I am going to create a Strategy to switch between this 2 approaches using a config property. https://github.com/dotCMS/core/pull/30466/files#diff-fa1ceaa19618a6b2bbc30e24c6f930b4971f417db50babb748c2e2837ba9eb82R237 https://github.com/dotCMS/core/pull/30466/files#diff-fa1ceaa19618a6b2bbc30e24c6f930b4971f417db50babb748c2e2837ba9eb82R7654 https://github.com/dotCMS/core/pull/30466/files#diff-445f8d01aa4de058eaaf883e573d17ef1123b1e74a9a98120ac156b78f4c6522R17 * For our new Extra table approach we ned to save the register in the extra table with the Contentlet's ID, the Contentlet's ID is not used for the hash calculation but is going to be sued later to clean up the table when a Contentlet is deleted, that is why a afterSaved method is included in the Strategy, this method is going to be called after saved the Contentlet https://github.com/dotCMS/core/pull/30466/files#diff-fa1ceaa19618a6b2bbc30e24c6f930b4971f417db50babb748c2e2837ba9eb82R5528 - I am going to remove all the ES validation code, and add it in the new ESUniqueFieldValidationStrategy class https://github.com/dotCMS/core/pull/30466/files#diff-fa1ceaa19618a6b2bbc30e24c6f930b4971f417db50babb748c2e2837ba9eb82L7635-L7725 https://github.com/dotCMS/core/pull/30466/files#diff-e3b9fd8560a668db0c37d00715b112bfa3fdfddc778b5dd1c4bac3b73f1fdd72R45 - I need to create a new ExtraTableUniqueFieldValidationStrategy for the Extra table validation Strategy implementation https://github.com/dotCMS/core/pull/30466/files#diff-efdeeb8f5e148f60415043882bbe26bafc2d643378abdb58e7b8fc944087fe52R40 - Create a Singleton util Class to provide all the method to work with the new extra table, I don't create a Factory because in this case really we don't have a API to match the Factory https://github.com/dotCMS/core/pull/30466/files#diff-b0aed2eddcc36be2a2e2f46624345a83c8698db238fe4dc228c0fb13a11e344fR16 ### Checklist - [ ] Tests - [ ] Translations - [ ] Security Implications Contemplated (add notes if applicable) ### Additional Info ** any additional useful context or info ** ### Screenshots Original | Updated :-------------------------:|:-------------------------: ** original screenshot ** | ** updated screenshot ** --------- Co-authored-by: Will Ezell <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
We use a Lucene query to check for unique field values, but this approach has issues due to a race condition. The ElasticSearch data isn’t updated immediately after the Contentlet database update, so if another Contentlet with the same unique values is saved before ElasticSearch is refreshed, duplicates can occur. This issue is particularly likely during high-volume imports, such as when importing hundreds of Contentlets at once.
I was able to reproduce this error locally by directly creating Contentlets via the API endpoint, sending 100 simultaneous requests using Postman.
The new approach for validating unique fields involves using an additional table. This table will have a primary key created from a hash, which combines the following elements: the ContentType's variable name, language, Field's variable name, Field's value, and—if the uniquePerSite field variable is set to TRUE—the site ID.
Proposed Changes
https://github.com/dotCMS/core/pull/30466/files#diff-fa1ceaa19618a6b2bbc30e24c6f930b4971f417db50babb748c2e2837ba9eb82R237
https://github.com/dotCMS/core/pull/30466/files#diff-fa1ceaa19618a6b2bbc30e24c6f930b4971f417db50babb748c2e2837ba9eb82R7654
https://github.com/dotCMS/core/pull/30466/files#diff-445f8d01aa4de058eaaf883e573d17ef1123b1e74a9a98120ac156b78f4c6522R17
https://github.com/dotCMS/core/pull/30466/files#diff-fa1ceaa19618a6b2bbc30e24c6f930b4971f417db50babb748c2e2837ba9eb82R5528
https://github.com/dotCMS/core/pull/30466/files#diff-fa1ceaa19618a6b2bbc30e24c6f930b4971f417db50babb748c2e2837ba9eb82L7635-L7725
https://github.com/dotCMS/core/pull/30466/files#diff-e3b9fd8560a668db0c37d00715b112bfa3fdfddc778b5dd1c4bac3b73f1fdd72R45
https://github.com/dotCMS/core/pull/30466/files#diff-efdeeb8f5e148f60415043882bbe26bafc2d643378abdb58e7b8fc944087fe52R40
https://github.com/dotCMS/core/pull/30466/files#diff-b0aed2eddcc36be2a2e2f46624345a83c8698db238fe4dc228c0fb13a11e344fR16
Checklist
Additional Info
** any additional useful context or info **
Screenshots
This PR fixes: #30277