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

issue-24093-Create_background_task_that_converts_fat_content_into_json_content #24615

Conversation

jgambarios
Copy link
Contributor

@jgambarios jgambarios commented Apr 12, 2023

  • Added logic to schedule the job to allow multiple executions in case of errors, if everything goes well, the job is removed.
  • Fixed code to avoid overriding the "title" set by the contentlet json with the data coming from the db.
  • Fixed code to ignore File Assets metadata legacy fields, migrating the data of metadata legacy fields depending on how old the dotCMS instance is, can cause parse issues as the metadata format varies, and either way the metadata is ignored as it is calculated directly from the asset, the metadata legacy field data won't be used.

🤖 Generated by Copilot at cfa302b

Summary

🧪🛠️⏱️

This pull request improves the JSON conversion and population of contentlets in dotCMS. It refactors the ContentletTransformer and PopulateContentletAsJSONJob classes to fix some issues and simplify the logic. It also adds a new integration test class PopulateContentletAsJSONJobTest to verify the functionality.

Oh we are the coders of the dotCMS crew
We write the tests and refactor too
We heave and ho on the count of three
To make the JSON and the Quartz API

Walkthrough

  • Refactor the PopulateContentletAsJSONJob class to use the standard Quartz API instead of the custom DotStatefulJob API (link, link, link, link)
  • Modify the run method to use the jobDataMap to get the excludingAssetSubType parameter and to remove the job from the scheduler after execution (link, link)
  • Modify the fireJob method to create and schedule the job using a JobDetail and a SimpleTrigger object, and to check if the job already exists before scheduling it (link)
  • Add three helper methods: removeJob, getJobName and getJobGroupName to support the new logic (link)
  • Remove the unused import of the HostAssetsJobProxy class and add the imports of the classes and packages used in the new logic (link)
  • Refactor the ContentletTransformer class to improve the performance and consistency of the JSON generation (link, link, link, link)
    • Remove the line that sets the title property of the contentlet from the ContentletTransformer class, since it is not part of the contentlet table (link)
    • Add a line that sets the title property of the contentlet from the original map that contains the contentlet fields, since it is needed for the JSON conversion (link)
    • Add an else-if branch to the switch statement to handle the case of metadata fields of file asset content types, and set their value to an empty map, since the metadata will be generated from the asset (link)
    • Add an empty line to separate the setIdentifier and setInode methods for readability (link)
  • Add a new test class PopulateContentletAsJSONJobTest to the integration-test package to test the logic of the PopulateContentletAsJSONJob class (link)

@github-actions
Copy link

github-actions bot commented Apr 12, 2023

Unit Tests Report

0 tests   - 1 423   0 ✔️  - 1 413   0s ⏱️ - 3m 52s
0 suites  -    140   0 💤  -      10 
0 files    -    140   0 ±       0 

Results for commit 1df7726. ± Comparison against base commit cffb861.

♻️ This comment has been updated with latest results.

…background_task_that_converts_fat_content_into_json_content
@jgambarios jgambarios marked this pull request as ready for review April 12, 2023 14:33
@github-actions
Copy link

github-actions bot commented Apr 12, 2023

Postman Tests Report

     66 files  ±0  1 412 suites  ±0   3h 36m 13s ⏱️ - 5m 28s
   632 tests ±0     628 ✔️ ±0  0 💤 ±0  4 ±0 
2 348 runs  ±0  2 341 ✔️ ±0  0 💤 ±0  7 ±0 

For more details on these failures, see this check.

Results for commit 1df7726. ± Comparison against base commit cffb861.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Apr 12, 2023

Integration Tests [postgres] Report

   414 files  +1     414 suites  +1   1h 34m 29s ⏱️ - 5m 11s
3 973 tests +1  3 950 ✔️ +2  23 💤 ±0  0  - 1 
3 992 runs  +1  3 969 ✔️ +2  23 💤 ±0  0  - 1 

Results for commit 1df7726. ± Comparison against base commit cffb861.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Apr 12, 2023

Integration Tests [mssql] Report

0 tests  ±0   0 ✔️ ±0   0s ⏱️ ±0s
0 suites ±0   0 💤 ±0 
0 files   ±0   0 ±0 

Results for commit 1df7726. ± Comparison against base commit cffb861.

♻️ This comment has been updated with latest results.

@jgambarios jgambarios marked this pull request as draft April 13, 2023 16:43
…d could cause issues and the value either way is always ignored, metadata is calculated directly from the asset.
…background_task_that_converts_fat_content_into_json_content
@dotcms-sonarqube
Copy link

SonarQube Quality Gate

Quality Gate failed

Failed condition 0.0% 0.0% Coverage on New Code (is less than 80%)

See analysis details on SonarQube

@jgambarios jgambarios marked this pull request as ready for review April 14, 2023 14:04
@nollymar nollymar merged commit 7e18d4c into master Apr 14, 2023
14 of 21 checks passed
@nollymar nollymar deleted the issue-24093-Create_background_task_that_converts_fat_content_into_json_content branch April 14, 2023 20:09
@nollymar nollymar linked an issue Apr 14, 2023 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create background task that converts fat content into json content
3 participants