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.

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

Use new Database validation for Unique Fields when a Contentlets is saved or updated #30279

Closed
Tracked by #29459
freddyDOTCMS opened this issue Oct 7, 2024 · 6 comments · Fixed by #30466
Closed
Tracked by #29459

Comments

@freddyDOTCMS
Copy link
Contributor

freddyDOTCMS commented Oct 7, 2024

Parent Issue

#29459

Problem Statement

Describe here #29459

We need to use the new API method created here #30277 to do the unique field validation any time a Contentlet is created, updated or imported, but we need to create a new config properties to enabled/disabled this new behavior if it is disabled then the old validation using ES is going to be used.

Steps to Reproduce

  • Create a new Content Type with 2 text field: title and unique, mark 'unique' as a unique field
  • Used curl or postman to run the follow query at least 100 times:

PUT: http://localhost:8082/api/v1/workflow/actions/default/fire/PUBLISH
Body:

{ 
    "contentlet" : {
        "title" : "content_{{unique}}",
        "unique": "PPP",
        "languageId" : 1,
        "stInode": "[New Content Type ID]",
        "host": "[Host Id]"
    }
}

Pre-request script:

const generateRandomString = (length) => {
    const characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
    let result = '';
    const charactersLength = characters.length;
    for (let i = 0; i < length; i++) {
        result += characters.charAt(Math.floor(Math.random() * charactersLength));
    }
    return result;
};

pm.collectionVariables.set("unique", generateRandomString(10));

Repeat this 5 times and check if you get any duplication, in the spike I got DUPLICATION every 5 running.

We need to send this change disabled it means dotCMS needs to keep using the ES Validation.

Acceptance Criteria

  • Implemented the new approach using the Database.
  • Have a way to disabled this new implementation and used the old one instead.

dotCMS Version

latest

Proposed Objective

Core Features

Proposed Priority

Priority 2 - Important

External Links... Slack Conversations, Support Tickets, Figma Designs, etc.

No response

Assumptions & Initiation Needs

No response

Quality Assurance Notes & Workarounds

No response

Sub-Tasks & Estimates

No response

@freddyDOTCMS freddyDOTCMS changed the title Use new Databse validation for Unique Fields when a Contentlets is saved or updated Use new Database validation for Unique Fields when a Contentlets is saved or updated Oct 7, 2024
@freddyDOTCMS freddyDOTCMS moved this from New to Next 1-3 Sprints in dotCMS - Product Planning Oct 7, 2024
@freddyDOTCMS freddyDOTCMS self-assigned this Oct 11, 2024
@freddyDOTCMS freddyDOTCMS moved this from Next 1-3 Sprints to In Progress in dotCMS - Product Planning Oct 11, 2024
freddyDOTCMS added a commit that referenced this issue Oct 25, 2024
freddyDOTCMS added a commit that referenced this issue Oct 25, 2024
@freddyDOTCMS
Copy link
Contributor Author

PR: #30466

@freddyDOTCMS
Copy link
Contributor Author

Note to QA: To test it we need

  • Set the DOT_ FEATURE_FLAG_DB_UNIQUE_FIELD_VALIDATION to TRUE
  • Run the Task Upgrade 'Task241007CreateUniqueFieldsTable' using the End point

URL: /v1/upgradetask
Method POST
Body: {
"upgradeTaskClass": "com.dotmarketing.startup.runonce.Task241007CreateUniqueFieldsTable"
}

  • Create a new Content Type with at least one unique FIeld
  • Try to create 2 Contentlet with the same unique field

Also test another cases like using the uniquePerSite FIeld Variable set to true, or try to create a Variant version with the same unique value used by a DEFAULT Variant version, etc.

@freddyDOTCMS freddyDOTCMS removed their assignment Oct 30, 2024
@dsilvam dsilvam self-assigned this Oct 31, 2024
@jcastro-dotcms
Copy link
Contributor

INTERNAL QA: FAILED

  • Docker image: trunk_7408571

This is what I did:

  • Started dotCMS, and followed the Steps to Reproduce posted in the ticket's description.
  • Ran the Postman request with the pre-request script as described, and got the duplicate error after the second or third time of running it, which is expected.
  • Stopped dotCMS, and set the expected FF as described in this comment: Use new Database validation for Unique Fields when a Contentlets is saved or updated #30279 (comment)
  • Then, I executed the upgrade task to create the table.
  • Un-published/archived/deleted the contents that I had created in the second step.
  • Tried to create more contentlets, got the unique field error since the beginning. I cannot create contents of such a type now.

@jcastro-dotcms jcastro-dotcms removed their assignment Nov 1, 2024
@jcastro-dotcms jcastro-dotcms moved this from Internal QA to In Progress in dotCMS - Product Planning Nov 1, 2024
@freddyDOTCMS
Copy link
Contributor Author

INTERNAL QA: FAILED

  • Docker image: trunk_7408571

This is what I did:

  • Started dotCMS, and followed the Steps to Reproduce posted in the ticket's description.
  • Ran the Postman request with the pre-request script as described, and got the duplicate error after the second or third time of running it, which is expected.
  • Stopped dotCMS, and set the expected FF as described in this comment: Use new Database validation for Unique Fields when a Contentlets is saved or updated #30279 (comment)
  • Then, I executed the upgrade task to create the table.
  • Un-published/archived/deleted the contents that I had created in the second step.
  • Tried to create more contentlets, got the unique field error since the beginning. I cannot create contents of such a type now.

This is expected because we are not still cleaning up the extra table, it is going to work after this card #30285 is done
For now we must test without deleting Contentlet.

Also you can not change the uniquePerSIte properties after create the first Contentlet because it is going to work after this other card #30281

@jcastro-dotcms
Copy link
Contributor

jcastro-dotcms commented Nov 1, 2024

After clarification from @freddyDOTCMS , I can confirm that the fix is working as expected. As he said, the cleanup process for the new table will be developed in another ticket.

Also, keep in mind that the validation process when the DB check is enabled may be slower than when using ES for that. This has already been discussed, and approved.

@josemejias11
Copy link
Contributor

Approved: Tested on trunk_7a2d6bc, Docker, macOS 14.5, FF v126.0.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants