From 6167b8f1b3648b457fbf769c8078b58c01bbd288 Mon Sep 17 00:00:00 2001 From: Valentino Giardino <77643678+valentinogiardino@users.noreply.github.com> Date: Tue, 3 Dec 2024 11:42:51 -0300 Subject: [PATCH 1/2] [Content Import Job Management] Implement the Content Import Get Job Status REST endpoint #30791 (#30818) ### Proposed Changes - Enhanced `ContentImportResource` to include a new endpoint for retrieving the status of a content import job. - Introduced a new class `ResponseEntityJobView` to represent job entities in API responses. - Added validations for workflow action and key fields in `ImportContentletsProcessor`. - Introduced a method `isCsvFile` to check if the uploaded file is a CSV file, with corresponding validation added in `ContentImportParams`. ### Checklist - [x] Tests - [x] Translations - [x] Security Implications Contemplated (add notes if applicable) ### Additional Info - API updates streamline job status tracking and provide clearer data structures for frontend integration. - Refactoring the validation and workflow action handling improves error clarity and ensures robust data validation in the content import process. (This fix IQA findings on #30669) --- .../impl/ImportContentletsProcessor.java | 89 ++- .../dotcms/rest/ResponseEntityJobView.java | 9 + .../dotimport/ContentImportHelper.java | 11 + .../dotimport/ContentImportParams.java | 7 + .../dotimport/ContentImportResource.java | 57 +- ...rtContentletsProcessorIntegrationTest.java | 173 +++++- .../ContentImportResourceIntegrationTest.java | 101 +++- ...tentImportResource.postman_collection.json | 568 ++++++++++++++++-- .../resources/ContentImportResource/test.txt | 1 + 9 files changed, 914 insertions(+), 102 deletions(-) create mode 100644 dotCMS/src/main/java/com/dotcms/rest/ResponseEntityJobView.java create mode 100644 dotcms-postman/src/main/resources/postman/resources/ContentImportResource/test.txt diff --git a/dotCMS/src/main/java/com/dotcms/jobs/business/processor/impl/ImportContentletsProcessor.java b/dotCMS/src/main/java/com/dotcms/jobs/business/processor/impl/ImportContentletsProcessor.java index 04420a9bf468..bf42bf0fc249 100644 --- a/dotCMS/src/main/java/com/dotcms/jobs/business/processor/impl/ImportContentletsProcessor.java +++ b/dotCMS/src/main/java/com/dotcms/jobs/business/processor/impl/ImportContentletsProcessor.java @@ -23,6 +23,7 @@ import com.dotmarketing.exception.DotSecurityException; import com.dotmarketing.portlets.contentlet.action.ImportAuditUtil; import com.dotmarketing.portlets.languagesmanager.model.Language; +import com.dotmarketing.portlets.workflows.model.WorkflowAction; import com.dotmarketing.util.AdminLogger; import com.dotmarketing.util.ImportUtil; import com.dotmarketing.util.Logger; @@ -38,12 +39,7 @@ import java.io.Reader; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; -import java.util.Calendar; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Optional; +import java.util.*; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.LongConsumer; @@ -198,6 +194,13 @@ && getWorkflowActionId(parameters).isEmpty()) { // Make sure the content type exist (will throw an exception if it doesn't) final var contentTypeFound = findContentType(parameters); + // Make sure the workflow action exist (will throw an exception if it doesn't) + findWorkflowAction(parameters); + + // Make sure the fields exist in the content type (will throw an exception if it doesn't) + validateFields(parameters, contentTypeFound); + + // Security measure to prevent invalid attempts to import a host. final ContentType hostContentType = APILocator.getContentTypeAPI( APILocator.systemUser()).find(Host.HOST_VELOCITY_VAR_NAME); @@ -212,6 +215,30 @@ && getWorkflowActionId(parameters).isEmpty()) { } } + /** + * Validates that the fields specified in the job parameters exist in the given content type. + * + *

This method checks each field specified in the job parameters against the fields defined + * in the provided content type. If any field is not found in the content type, a + * {@link JobValidationException} is thrown.

+ * + * @param parameters The job parameters containing the fields to validate + * @param contentTypeFound The content type to validate the fields against + * @throws JobValidationException if any field specified in the parameters is not found in the content type + */ + private void validateFields(final Map parameters, final ContentType contentTypeFound) { + var fields = contentTypeFound.fields(); + for (String field : getFields(parameters)) { + if (fields.stream().noneMatch(f -> Objects.equals(f.variable(), field))) { + final var errorMessage = String.format( + "Field [%s] not found in Content Type [%s].", field, contentTypeFound.variable() + ); + Logger.error(this, errorMessage); + throw new JobValidationException(errorMessage); + } + } + } + /** * Handles cancellation requests for the import operation. When called, it marks the operation * for cancellation. @@ -618,6 +645,56 @@ private ContentType findContentType(final Map parameters) } } + + /** + * Finds and returns a workflow action based on the provided parameters. + * + *

This method retrieves the workflow action ID from the given parameters and attempts to + * find the corresponding workflow action using the Workflow API. + * + * + * @param parameters a map containing parameters required for finding the workflow action, + * including the workflow action ID and user details. + * + * @return the {@link WorkflowAction} corresponding to the workflow action ID. + * + * @throws JobValidationException if the workflow action cannot be found. + * @throws JobProcessingException if an error occurs during user retrieval or + * workflow action lookup. + */ + private WorkflowAction findWorkflowAction(final Map parameters) { + + final var workflowActionId = getWorkflowActionId(parameters); + final User user; + + try { + user = getUser(parameters); + } catch (DotDataException | DotSecurityException e) { + final var errorMessage = "Error retrieving user."; + Logger.error(this.getClass(), errorMessage); + throw new JobProcessingException(errorMessage, e); + } + + try { + var workflowAction = APILocator.getWorkflowAPI() + .findAction(workflowActionId,user); + if(Objects.isNull(workflowAction)){ + final var errorMessage = String.format( + "Workflow Action [%s] not found.", workflowActionId + ); + Logger.error(this.getClass(), errorMessage); + throw new JobValidationException(errorMessage); + } + return workflowAction; + } catch (DotDataException | DotSecurityException e) { + final var errorMessage = String.format( + "Error finding Workflow Action [%s].", workflowActionId + ); + Logger.error(this.getClass(), errorMessage); + throw new JobProcessingException(errorMessage, e); + } + } + /** * Retrieves the existing language based on an id or ISO code. * diff --git a/dotCMS/src/main/java/com/dotcms/rest/ResponseEntityJobView.java b/dotCMS/src/main/java/com/dotcms/rest/ResponseEntityJobView.java new file mode 100644 index 000000000000..1c3436b67579 --- /dev/null +++ b/dotCMS/src/main/java/com/dotcms/rest/ResponseEntityJobView.java @@ -0,0 +1,9 @@ +package com.dotcms.rest; + +import com.dotcms.jobs.business.job.Job; + +public class ResponseEntityJobView extends ResponseEntityView { + public ResponseEntityJobView(Job entity) { + super(entity); + } +} diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/dotimport/ContentImportHelper.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/dotimport/ContentImportHelper.java index 1b086adf1402..8487fdbe5a79 100644 --- a/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/dotimport/ContentImportHelper.java +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/dotimport/ContentImportHelper.java @@ -1,6 +1,7 @@ package com.dotcms.rest.api.v1.content.dotimport; import com.dotcms.jobs.business.api.JobQueueManagerAPI; +import com.dotcms.jobs.business.job.Job; import com.dotcms.rest.api.v1.JobQueueManagerHelper; import com.dotcms.rest.api.v1.temp.DotTempFile; import com.dotmarketing.business.APILocator; @@ -95,6 +96,16 @@ public String createJob( return jobQueueManagerAPI.createJob(queueName, jobParameters); } + /** + * gets a job + * @param jobId The ID of the job + * @return Job + * @throws DotDataException if there's an error fetching the job + */ + Job getJob(final String jobId) throws DotDataException { + return jobQueueManagerAPI.getJob(jobId); + } + /** * Constructs a map of job parameters based on the provided inputs. * diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/dotimport/ContentImportParams.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/dotimport/ContentImportParams.java index 3ac164f1ae14..c5639ad6c07d 100644 --- a/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/dotimport/ContentImportParams.java +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/dotimport/ContentImportParams.java @@ -85,5 +85,12 @@ public void checkValid() { if (contentDisposition == null || contentDisposition.getFileName() == null) { throw new ValidationException("The file must have a valid file name."); } + if (!isCsvFile(contentDisposition.getFileName())) { + throw new ValidationException("The file must be a CSV file."); + } + } + + private boolean isCsvFile(final String fileName) { + return fileName != null && fileName.toLowerCase().endsWith(".csv"); } } \ No newline at end of file diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/dotimport/ContentImportResource.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/dotimport/ContentImportResource.java index 66f7904e10f0..f911bf5899a3 100644 --- a/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/dotimport/ContentImportResource.java +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/dotimport/ContentImportResource.java @@ -1,6 +1,9 @@ package com.dotcms.rest.api.v1.content.dotimport; import com.dotcms.jobs.business.error.JobValidationException; +import com.dotcms.jobs.business.job.Job; +import com.dotcms.repackage.javax.validation.ValidationException; +import com.dotcms.rest.ResponseEntityJobView; import com.dotcms.rest.ResponseEntityStringView; import com.dotcms.rest.ResponseEntityView; import com.dotcms.rest.WebResource; @@ -10,6 +13,7 @@ import com.dotmarketing.util.Logger; import com.fasterxml.jackson.core.JsonProcessingException; import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.Parameter; import io.swagger.v3.oas.annotations.media.Content; import io.swagger.v3.oas.annotations.media.ExampleObject; import io.swagger.v3.oas.annotations.media.Schema; @@ -142,7 +146,7 @@ public Response importContent( // Create the content import job final String jobId = importHelper.createJob(CMD_PUBLISH, IMPORT_QUEUE_NAME, params, initDataObject.getUser(), request); return Response.ok(new ResponseEntityView<>(jobId)).build(); - } catch (JobValidationException e) { + } catch (JobValidationException | ValidationException e) { // Handle validation exception and return appropriate error message return ExceptionMapperUtil.createResponse(null, e.getMessage()); } @@ -230,9 +234,58 @@ public Response validateContentImport( // Create the content import job in preview mode final String jobId = importHelper.createJob(CMD_PREVIEW, IMPORT_QUEUE_NAME, params, initDataObject.getUser(), request); return Response.ok(new ResponseEntityView<>(jobId)).build(); - } catch (JobValidationException e) { + } catch (JobValidationException | ValidationException e) { // Handle validation exception and return appropriate error message return ExceptionMapperUtil.createResponse(null, e.getMessage()); } } + + + + @GET + @Path("/{jobId}") + @Produces(MediaType.APPLICATION_JSON) + @Operation( + operationId = "getJobStatus", + summary = "Retrieves the status of a content import job", + description = "Fetches the current status of a content import job based on the provided job ID.", + tags = {"Content Import"}, + responses = { + @ApiResponse( + responseCode = "200", + description = "Successfully retrieved job status", + content = @Content( + mediaType = "application/json", + schema = @Schema(implementation = ResponseEntityJobView.class) + ) + ), + @ApiResponse(responseCode = "401", description = "Invalid user authentication"), + @ApiResponse(responseCode = "403", description = "Forbidden due to insufficient permissions"), + @ApiResponse(responseCode = "404", description = "Job not found"), + @ApiResponse(responseCode = "500", description = "Internal server error") + } + ) + public ResponseEntityView getJobStatus( + @Context final HttpServletRequest request, + @Context final HttpServletResponse response, + @PathParam("jobId") @Parameter( + required = true, + description = "The ID of the job whose status is to be retrieved", + schema = @Schema(type = "string") + ) final String jobId) + throws DotDataException { + + // Initialize the WebResource and set required user information + final var initDataObject = new WebResource.InitBuilder(webResource) + .requiredBackendUser(true) + .requiredFrontendUser(false) + .requestAndResponse(request, response) + .rejectWhenNoUser(true) + .init(); + + Logger.debug(this, ()->String.format(" user %s is retrieving status of job: %s", initDataObject.getUser().getUserId(), jobId)); + + Job job = importHelper.getJob(jobId); + return new ResponseEntityView<>(job); + } } \ No newline at end of file diff --git a/dotcms-integration/src/test/java/com/dotcms/jobs/business/processor/impl/ImportContentletsProcessorIntegrationTest.java b/dotcms-integration/src/test/java/com/dotcms/jobs/business/processor/impl/ImportContentletsProcessorIntegrationTest.java index 9055a905d33c..fbfd96a170cc 100644 --- a/dotcms-integration/src/test/java/com/dotcms/jobs/business/processor/impl/ImportContentletsProcessorIntegrationTest.java +++ b/dotcms-integration/src/test/java/com/dotcms/jobs/business/processor/impl/ImportContentletsProcessorIntegrationTest.java @@ -1,8 +1,5 @@ package com.dotcms.jobs.business.processor.impl; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; - import com.dotcms.contenttype.model.type.ContentType; import com.dotcms.datagen.TestDataUtils; import com.dotcms.jobs.business.error.JobValidationException; @@ -22,16 +19,15 @@ import java.io.FileInputStream; import java.io.IOException; import java.nio.file.Files; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; +import java.util.*; import javax.servlet.http.HttpServletRequest; import org.jboss.weld.junit5.EnableWeld; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.*; +import static com.dotmarketing.portlets.workflows.business.SystemWorkflowConstants.WORKFLOW_PUBLISH_ACTION_ID; /** * Integration tests for the {@link ImportContentletsProcessor} class. These tests verify the * functionality of content import operations in a real database environment. The tests cover both @@ -108,7 +104,7 @@ void test_process_preview_using_content_type_variable() throws Exception { // Create test job final var testJob = createTestJob( csvFile, "preview", "1", testContentType.variable(), - "b9d89c80-3d88-4311-8365-187323c96436" + WORKFLOW_PUBLISH_ACTION_ID ); // Process the job in preview mode @@ -154,7 +150,7 @@ void test_process_preview_invalid_content_type_variable() throws Exception { // Create test job final var testJob = createTestJob( csvFile, "preview", "1", "doesNotExist", - "b9d89c80-3d88-4311-8365-187323c96436" + WORKFLOW_PUBLISH_ACTION_ID ); try { @@ -199,7 +195,7 @@ void test_process_preview_using_language_iso_code() throws Exception { // Create test job final var testJob = createTestJob( csvFile, "preview", "en-us", testContentType.variable(), - "b9d89c80-3d88-4311-8365-187323c96436" + WORKFLOW_PUBLISH_ACTION_ID ); // Process the job in preview mode @@ -245,7 +241,7 @@ void test_process_preview_invalid_language() throws Exception { // Create test job final var testJob = createTestJob( csvFile, "preview", "12345", "doesNotExist", - "b9d89c80-3d88-4311-8365-187323c96436" + WORKFLOW_PUBLISH_ACTION_ID ); try { @@ -256,6 +252,82 @@ void test_process_preview_invalid_language() throws Exception { } } + + /** + * Scenario: Test the preview mode of the content import process with an invalid workflow action. + *

+ * Expected: A JobValidationException should be thrown. + * + * @throws Exception if there's an error during the test execution + */ + @Test + void test_process_preview_invalid_workflow_action() throws Exception { + ContentType testContentType = null; + + try { + // Initialize processor + final var processor = new ImportContentletsProcessor(); + + // Create test content type + testContentType = createTestContentType(); + + // Create test CSV file + File csvFile = createTestCsvFile(); + + // Create test job + final var testJob = createTestJob( + csvFile, "preview", "en-us", testContentType.variable(), + "doesNotExist" + ); + + // Process the job in preview mode + assertThrows(JobValidationException.class, ()-> processor.validate((testJob.parameters()))); + } finally { + if (testContentType != null) { + // Clean up test content type + APILocator.getContentTypeAPI(systemUser).delete(testContentType); + } + } + } + + + /** + * Scenario: Test the preview mode of the content import process with an invalid key field. + *

+ * Expected: A JobValidationException should be thrown. + * + * @throws Exception if there's an error during the test execution + */ + @Test + void test_process_preview_invalid_key_field() throws Exception { + ContentType testContentType = null; + + try { + // Initialize processor + final var processor = new ImportContentletsProcessor(); + + // Create test content type + testContentType = createTestContentType(); + + // Create test CSV file + File csvFile = createTestCsvFile(); + + // Create test job + final var testJob = createTestJob( + csvFile, "preview", "en-us", testContentType.variable(), + WORKFLOW_PUBLISH_ACTION_ID, List.of("doesNotExist") + ); + + assertThrows(JobValidationException.class, ()-> processor.validate((testJob.parameters()))); + + } finally { + if (testContentType != null) { + // Clean up test content type + APILocator.getContentTypeAPI(systemUser).delete(testContentType); + } + } + } + /** * Tests the preview mode of the content import process. This test: *