From 5e32afc36a1574a728a59bcf4f4c10d5cef8487f Mon Sep 17 00:00:00 2001 From: Ben Helleman Date: Tue, 17 Sep 2024 16:22:43 -0400 Subject: [PATCH] fix: SITES-23133 [Importer] Report on the progress of a import job via a new jobs endpoint --- .../docs/schema.json | 4 ++ .../src/index.d.ts | 11 ---- .../src/models/importer/import-constants.js | 6 --- .../src/models/importer/import-job.js | 22 -------- .../models/importer/import-model-mapper.js | 54 ------------------- .../src/service/import-job/accessPatterns.js | 46 ++-------------- .../src/service/import-url/accessPatterns.js | 4 +- .../test/it/db.test.js | 14 ----- .../unit/models/importer/import-job.test.js | 10 ---- .../importer/import-model-mapper.test.js | 44 --------------- .../unit/service/import-job/index.test.js | 5 -- 11 files changed, 9 insertions(+), 211 deletions(-) delete mode 100644 packages/spacecat-shared-data-access/src/models/importer/import-model-mapper.js delete mode 100644 packages/spacecat-shared-data-access/test/unit/models/importer/import-model-mapper.test.js diff --git a/packages/spacecat-shared-data-access/docs/schema.json b/packages/spacecat-shared-data-access/docs/schema.json index 6e6f92af..1771cb03 100644 --- a/packages/spacecat-shared-data-access/docs/schema.json +++ b/packages/spacecat-shared-data-access/docs/schema.json @@ -975,6 +975,10 @@ "AttributeName": "successCount", "AttributeType": "N" }, + { + "AttributeName": "redirectCount", + "AttributeType": "N" + }, { "AttributeName": "urlCount", "AttributeType": "N" diff --git a/packages/spacecat-shared-data-access/src/index.d.ts b/packages/spacecat-shared-data-access/src/index.d.ts index 96361b4c..ed1bdc2e 100644 --- a/packages/spacecat-shared-data-access/src/index.d.ts +++ b/packages/spacecat-shared-data-access/src/index.d.ts @@ -530,17 +530,6 @@ export interface ImportJob { * Retrieves the initiatedBy metadata (name, imsOrgId, imsUserId, userAgent) of the import job. */ getInitiatedBy: () => object; - - /** - * Retrieves the progress of the import job. - */ - getProgress: () => { - completed: number; - failed: number; - pending: number; - running: number; - redirect: number; - }; } export interface ImportUrl { diff --git a/packages/spacecat-shared-data-access/src/models/importer/import-constants.js b/packages/spacecat-shared-data-access/src/models/importer/import-constants.js index 1153d836..4166bb92 100644 --- a/packages/spacecat-shared-data-access/src/models/importer/import-constants.js +++ b/packages/spacecat-shared-data-access/src/models/importer/import-constants.js @@ -16,12 +16,6 @@ export const ImportOptions = { ENABLE_JAVASCRIPT: 'enableJavascript', PAGE_LOAD_TIMEOUT: 'pageLoadTimeout', - - // the following 4 options need to be removed - SAVE_AS_DOCX: 'saveAsDocx', - HAS_CUSTOM_HEADERS: 'hasCustomHeaders', - HAS_CUSTOM_IMPORT_JS: 'hasCustomImportJs', - SCROLL_TO_BOTTOM: 'scrollToBottom', }; /** diff --git a/packages/spacecat-shared-data-access/src/models/importer/import-job.js b/packages/spacecat-shared-data-access/src/models/importer/import-job.js index 8b548782..b63d26ff 100644 --- a/packages/spacecat-shared-data-access/src/models/importer/import-job.js +++ b/packages/spacecat-shared-data-access/src/models/importer/import-job.js @@ -16,19 +16,6 @@ import { import { Base } from '../base.js'; import { ImportJobStatus, ImportOptions } from './import-constants.js'; -export const jobToApiMap = [ - ['id', 'id'], - ['baseURL', 'baseURL'], - ['options', 'options'], - ['startTime', 'startTime'], - ['endTime', 'endTime'], - ['duration', 'duration'], - ['status', 'status'], - ['urlCount', 'urlCount'], - ['initiatedBy', 'initiatedBy'], - ['progress', 'progress'], -]; - /** * Creates a new ImportJob object. * @@ -50,15 +37,6 @@ const ImportJob = (data) => { self.getImportQueueId = () => self.state.importQueueId; self.getInitiatedBy = () => self.state.initiatedBy; - // the progress is a derived property from querying the status of the import urls table - self.getProgress = () => self.state.progress || { - running: 0, - failed: 0, - completed: 0, - pending: 0, - redirect: 0, - }; - /** * Updates the state of the ImportJob. * @param key - The key to update. diff --git a/packages/spacecat-shared-data-access/src/models/importer/import-model-mapper.js b/packages/spacecat-shared-data-access/src/models/importer/import-model-mapper.js deleted file mode 100644 index b0cad9e8..00000000 --- a/packages/spacecat-shared-data-access/src/models/importer/import-model-mapper.js +++ /dev/null @@ -1,54 +0,0 @@ -/* - * Copyright 2024 Adobe. All rights reserved. - * This file is licensed to you under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. You may obtain a copy - * of the License at http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under - * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS - * OF ANY KIND, either express or implied. See the License for the specific language - * governing permissions and limitations under the License. - */ - -/** - * Map the properties of the source object into the target object based on the mapping array. - * - * @param {object} sourceObj The source object. - * @param {array} mapping An array of arrays in the format of either - * ['modelPropertyKey', ['apiPropertyKey'] or ['modelPropertyKey', modelToApiFn()] - */ -function mappingReducer(sourceObj, mapping) { - const sourceMapIndex = 0; - const targetMapIndex = 1; - - return mapping.reduce((targetObj, mapEntry) => { - if (sourceObj[mapEntry[sourceMapIndex]] === undefined) { - return targetObj; - } - - if (typeof mapEntry[targetMapIndex] === 'function') { - const result = mapEntry[targetMapIndex](sourceObj[mapEntry[sourceMapIndex]]); - Object.assign(targetObj, result); - } else { - // eslint-disable-next-line no-param-reassign - targetObj[mapEntry[targetMapIndex]] = sourceObj[mapEntry[sourceMapIndex]]; - } - - return targetObj; - }, {}); -} - -/** - * Map the model object to a plain old object based on the model map. If the source - * object does not contain a key that is specified in the mapping array, that key - * will not be included in the resulting object. - * - * @param {object} model The model to convert to a POJO - * @param {array} modelMap Map mapping array that contains multiple arrays. The nested arrays - * containing key mappings between objects. - * Inner array format can be either ['modelPropertyKey', ['apiPropertyKey'] or - * ['modelPropertyKey', modelToApiFn()] - */ -export function map(model, modelMap) { - return mappingReducer(model, modelMap); -} diff --git a/packages/spacecat-shared-data-access/src/service/import-job/accessPatterns.js b/packages/spacecat-shared-data-access/src/service/import-job/accessPatterns.js index 28203017..369a761e 100644 --- a/packages/spacecat-shared-data-access/src/service/import-job/accessPatterns.js +++ b/packages/spacecat-shared-data-access/src/service/import-job/accessPatterns.js @@ -10,11 +10,9 @@ * governing permissions and limitations under the License. */ -import { isArray, isObject } from '@adobe/spacecat-shared-utils'; +import { isObject } from '@adobe/spacecat-shared-utils'; import { ImportJobDto } from '../../dto/import-job.js'; import { createImportJob } from '../../models/importer/import-job.js'; -import { getImportUrlsByJobId } from '../import-url/accessPatterns.js'; -import { ImportUrlStatus } from '../../models/importer/import-constants.js'; /** * Get all Import Jobs within a specific date range @@ -50,50 +48,12 @@ export const getImportJobsByDateRange = async (dynamoClient, config, log, startD * @returns {Promise | null} */ export const getImportJobByID = async (dynamoClient, config, log, id) => { - const jobEntity = await dynamoClient.getItem( + const item = await dynamoClient.getItem( config.tableNameImportJobs, { id }, ); - if (!jobEntity) { - return null; - } - - const jobModel = ImportJobDto.fromDynamoItem(jobEntity); - - const importUrls = await getImportUrlsByJobId(dynamoClient, config, log, id); - - if (isArray(importUrls)) { - jobModel.state.progress = importUrls.reduce((acc, importUrl) => { - // eslint-disable-next-line default-case - switch (importUrl.state.status) { - case ImportUrlStatus.PENDING: - acc.pending += 1; - break; - case ImportUrlStatus.REDIRECT: - acc.redirect += 1; - break; - case ImportUrlStatus.RUNNING: - acc.running += 1; - break; - case ImportUrlStatus.COMPLETE: - acc.completed += 1; - break; - case ImportUrlStatus.FAILED: - acc.failed += 1; - break; - } - return acc; - }, { - pending: 0, - redirect: 0, - running: 0, - completed: 0, - failed: 0, - }); - } - - return jobModel; + return item ? ImportJobDto.fromDynamoItem(item) : null; }; /** diff --git a/packages/spacecat-shared-data-access/src/service/import-url/accessPatterns.js b/packages/spacecat-shared-data-access/src/service/import-url/accessPatterns.js index 9b33756c..cbd19fb1 100644 --- a/packages/spacecat-shared-data-access/src/service/import-url/accessPatterns.js +++ b/packages/spacecat-shared-data-access/src/service/import-url/accessPatterns.js @@ -98,7 +98,7 @@ export const getImportUrlsByJobIdAndStatus = async (dynamoClient, config, log, j }; /** - * Get Import Urls by Job ID + * Get Import Urls by Job ID, if no urls exist an empty array is returned. * @param {DynamoClient} dynamoClient * @param {Object} config * @param {Logger} log @@ -115,5 +115,5 @@ export const getImportUrlsByJobId = async (dynamoClient, config, log, jobId) => }, }); - return items ? items.map(ImportUrlDto.fromDynamoItem) : null; + return items ? items.map((item) => ImportUrlDto.fromDynamoItem(item)) : []; }; diff --git a/packages/spacecat-shared-data-access/test/it/db.test.js b/packages/spacecat-shared-data-access/test/it/db.test.js index db92e761..3b84f634 100644 --- a/packages/spacecat-shared-data-access/test/it/db.test.js +++ b/packages/spacecat-shared-data-access/test/it/db.test.js @@ -1055,13 +1055,6 @@ describe('DynamoDB Integration Test', async () => { expect(updatedJob.getDuration()).to.be.equal(1234); expect(updatedJob.getUrlCount()).to.be.equal(100); expect(updatedJob.getImportQueueId()).to.be.equal('Q-456'); - expect(updatedJob.getProgress()).to.deep.equal({ - running: 0, - failed: 0, - completed: 0, - pending: 0, - redirect: 0, - }); expect(updatedJob.getOptions()).to.deep.equal({ [ImportOptions.ENABLE_JAVASCRIPT]: true, }); @@ -1079,13 +1072,6 @@ describe('DynamoDB Integration Test', async () => { const job = await createNewImportJob(); const jobEntry = await dataAccess.getImportJobByID(job.getId()); expect(job.getId()).to.be.equal(jobEntry.getId()); - expect(job.getProgress()).to.deep.equal({ - running: 0, - failed: 0, - completed: 0, - pending: 0, - redirect: 0, - }); }); it('Verify getImportJobsByDateRange', async () => { diff --git a/packages/spacecat-shared-data-access/test/unit/models/importer/import-job.test.js b/packages/spacecat-shared-data-access/test/unit/models/importer/import-job.test.js index 8d34cb9f..d994e96c 100644 --- a/packages/spacecat-shared-data-access/test/unit/models/importer/import-job.test.js +++ b/packages/spacecat-shared-data-access/test/unit/models/importer/import-job.test.js @@ -183,15 +183,5 @@ describe('ImportJob Model tests', () => { it('retrieves the startTime of the import job', () => { expect(importJob.getStartTime()).to.equal('2024-05-29T14:26:00.000Z'); }); - - it('retrieves the progress of the job', () => { - expect(importJob.getProgress()).to.deep.equal({ - running: 0, - failed: 0, - completed: 0, - pending: 0, - redirect: 0, - }); - }); }); }); diff --git a/packages/spacecat-shared-data-access/test/unit/models/importer/import-model-mapper.test.js b/packages/spacecat-shared-data-access/test/unit/models/importer/import-model-mapper.test.js deleted file mode 100644 index 2d71319f..00000000 --- a/packages/spacecat-shared-data-access/test/unit/models/importer/import-model-mapper.test.js +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Copyright 2024 Adobe. All rights reserved. - * This file is licensed to you under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. You may obtain a copy - * of the License at http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under - * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS - * OF ANY KIND, either express or implied. See the License for the specific language - * governing permissions and limitations under the License. - */ -/* eslint-env mocha */ - -import { expect } from 'chai'; -import { map } from '../../../../src/models/importer/import-model-mapper.js'; - -describe('import-model-mapper', () => { - let model; - let modelMap; - - beforeEach(() => { - model = { - id: '123', - name: 'Test Import', - func: false, - }; - - modelMap = [ - ['id', 'identification'], - ['name', 'name'], - ['func', () => ({ func: true })], - ['nonExistent', 'nonExistent'], - ]; - }); - - it('should map a model to an API response object', () => { - const result = map(model, modelMap); - expect(result).to.deep.equal({ - identification: '123', - name: 'Test Import', - func: true, - }); - }); -}); diff --git a/packages/spacecat-shared-data-access/test/unit/service/import-job/index.test.js b/packages/spacecat-shared-data-access/test/unit/service/import-job/index.test.js index 42d24185..969a8b36 100644 --- a/packages/spacecat-shared-data-access/test/unit/service/import-job/index.test.js +++ b/packages/spacecat-shared-data-access/test/unit/service/import-job/index.test.js @@ -85,11 +85,6 @@ describe('Import Job Tests', () => { const result = await exportedFunctions.getImportJobByID('test-id'); expect(result.state.id).to.equal('test-id'); - expect(result.state.progress.pending).to.equal(1); - expect(result.state.progress.redirect).to.equal(1); - expect(result.state.progress.running).to.equal(1); - expect(result.state.progress.completed).to.equal(1); - expect(result.state.progress.failed).to.equal(1); }); it('should return null if item is not found', async () => {