Skip to content

Commit

Permalink
CUMULUS-3243:Updated granule delete logic (#3338)
Browse files Browse the repository at this point in the history
* CUMULUS-3243:Updated granule delete logic to delete granule which is not in DynamoDB

* add unit tests
  • Loading branch information
jennyhliu authored Apr 28, 2023
1 parent 60f9950 commit 4c85ec3
Show file tree
Hide file tree
Showing 8 changed files with 155 additions and 15 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).

## Unreleased

## [v15.0.3] 2023-04-28

### Fixed

- **CUMULUS-3243**
- Updated granule delete logic to delete granule which is not in DynamoDB
- Updated granule unpublish logic to handle granule which is not in DynamoDB and/or CMR

## [v15.0.2] 2023-04-25

### Fixed
Expand Down
2 changes: 1 addition & 1 deletion example/lambdas/python-reference-task/package.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ cp ./*.py ./dist/

cd ./dist || exit 1

find . -type f -print0 | xargs -0 node ../../../../bin/zip.js lambda.zip $(ls | grep -v lambda.zip)
node ../../../../bin/zip.js lambda.zip $(ls | grep -v lambda.zip)

cd .. || exit 1

Expand Down
8 changes: 5 additions & 3 deletions packages/api/lib/granule-remove-from-cmr.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
const { GranuleNotPublished } = require('@cumulus/errors');
const { CMR } = require('@cumulus/cmr-client');
const log = require('@cumulus/common/log');
const {
Expand Down Expand Up @@ -29,15 +28,18 @@ const models = require('../models');
const _removeGranuleFromCmr = async (granule, collectionId) => {
log.info(`granules.removeGranuleFromCmrByGranule granule_id: ${granule.granule_id}, colletion_id: ${collectionId}`);
if (!granule.published || !granule.cmr_link) {
throw new GranuleNotPublished(`Granule ${granule.granule_id} in Collection ${collectionId} is not published to CMR, so cannot be removed from CMR`);
log.warn(`Granule ${granule.granule_id} in Collection ${collectionId} is not published to CMR, so cannot be removed from CMR`);
return;
}

const cmrSettings = await cmrjsCmrUtils.getCmrSettings();
const cmr = new CMR(cmrSettings);
const metadata = await cmr.getGranuleMetadata(granule.cmr_link);

// Use granule UR to delete from CMR
await cmr.deleteGranule(metadata.title, collectionId);
if (metadata) {
await cmr.deleteGranule(metadata.title, collectionId);
}
};

/**
Expand Down
14 changes: 14 additions & 0 deletions packages/api/models/granules.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const {
DeletePublishedGranule,
ValidationError,
} = require('@cumulus/errors');
const { RecordDoesNotExist } = require('@cumulus/errors');
const {
generateMoveFileParams,
} = require('@cumulus/ingest/granule');
Expand Down Expand Up @@ -482,6 +483,19 @@ class Granule extends Manager {
}
return executionDescription;
}

async update(itemKeys, updates = {}, fieldsToDelete = []) {
let granule;
try {
granule = await super.update(itemKeys, updates, fieldsToDelete);
} catch (error) {
if (!(error instanceof RecordDoesNotExist)) {
throw error;
}
logger.info(`Granule record ${JSON.stringify(itemKeys)} does not exist.`);
}
return granule;
}
}

module.exports = Granule;
10 changes: 6 additions & 4 deletions packages/api/src/lib/granule-delete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,13 @@ const deleteGranuleAndFiles = async (params: {
await granulePgModel.delete(trx, {
cumulus_id: pgGranule.cumulus_id,
});
await granuleModelClient.delete(dynamoGranule);
if (dynamoGranule) {
await granuleModelClient.delete(dynamoGranule);
}
await deleteGranule({
esClient,
granuleId: dynamoGranule.granuleId,
collectionId: dynamoGranule.collectionId,
granuleId: pgGranule.granule_id,
collectionId: granuleToPublishToSns.collectionId,
index: process.env.ES_INDEX,
ignore: [404],
});
Expand All @@ -145,7 +147,7 @@ const deleteGranuleAndFiles = async (params: {
deletedFiles: files,
};
} catch (error) {
logger.debug(`Error deleting granule with ID ${pgGranule.granule_id} or S3 files ${JSON.stringify(dynamoGranule.files)}: ${JSON.stringify(error)}`);
logger.debug(`Error deleting granule with ID ${pgGranule.granule_id} or S3 files ${JSON.stringify(files)}: ${JSON.stringify(error)}`);
// Delete is idempotent, so there may not be a DynamoDB
// record to recreate
if (dynamoGranule) {
Expand Down
56 changes: 56 additions & 0 deletions packages/api/tests/lib/test-granule-delete.js
Original file line number Diff line number Diff line change
Expand Up @@ -604,3 +604,59 @@ test.serial('deleteGranuleAndFiles() does not require a PostgreSQL granule', asy
s3Buckets.public.name,
]));
});

test.serial('deleteGranuleAndFiles() does not require a DynamoDB granule', async (t) => {
const {
newPgGranule,
files,
s3Buckets,
} = await createGranuleAndFiles({
dbClient: t.context.knex,
collectionId: t.context.collectionId,
collectionCumulusId: t.context.collectionCumulusId,
granuleParams: { published: false },
esClient: t.context.esClient,
});

await granuleModel.delete({ granuleId: newPgGranule.granule_id });

const details = await deleteGranuleAndFiles({
knex: t.context.knex,
pgGranule: newPgGranule,
esClient: t.context.esClient,
});

t.truthy(details.deletionTime);
t.like(details, {
collection: t.context.collectionId,
deletedGranuleId: newPgGranule.granule_id,
});
t.is(details.deletedFiles.length, files.length);

t.false(await granulePgModel.exists(
t.context.knex,
{
granule_id: newPgGranule.granule_id,
collection_cumulus_id: newPgGranule.collection_cumulus_id,
}
));
t.false(
await t.context.esGranulesClient.exists(
newPgGranule.granule_id,
t.context.collectionId
)
);

// Verify files were deleted from S3 and Postgres
await Promise.all(
files.map(async (file) => {
t.false(await s3ObjectExists({ Bucket: file.bucket, Key: file.key }));
t.false(await filePgModel.exists(t.context.knex, { bucket: file.bucket, key: file.key }));
})
);

t.teardown(() => deleteS3Buckets([
s3Buckets.protected.name,
s3Buckets.public.name,
]));
});
43 changes: 36 additions & 7 deletions packages/api/tests/lib/test-granule-remove-from-cmr.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,21 +113,50 @@ test.after.always(async (t) => {
});
});

test('unpublishGranule() removing a granule from CMR fails if the granule is not in CMR', async (t) => {
test('unpublishGranule() removing a granule from CMR succeeds if the granule is not published to CMR', async (t) => {
const {
originalDynamoGranule,
originalPgGranule,
pgGranuleCumulusId,
collectionId,
} = await createGranuleInDynamoAndPG(t, {
published: false,
cmrLink: undefined,
});
try {
await unpublishGranule({ knex: t.context.knex, pgGranuleRecord: originalPgGranule });
} catch (error) {
t.is(error.message, `Granule ${originalPgGranule.granule_id} in Collection ${collectionId} is not published to CMR, so cannot be removed from CMR`);
}
await unpublishGranule({ knex: t.context.knex, pgGranuleRecord: originalPgGranule });

t.like(
await t.context.granulesModel.get({ granuleId: originalDynamoGranule.granuleId }),
{
published: false,
cmrLink: undefined,
}
);

t.like(
await t.context.granulePgModel.get(t.context.knex, { cumulus_id: pgGranuleCumulusId }),
{
published: false,
cmr_link: null,
}
);
});

test.serial('unpublishGranule() removing a granule from CMR succeeds if the granule is not in CMR', async (t) => {
const {
originalDynamoGranule,
originalPgGranule,
pgGranuleCumulusId,
} = await createGranuleInDynamoAndPG(t, {
published: true,
cmrLink: 'example.com',
});

const cmrMetadataStub = sinon.stub(CMR.prototype, 'getGranuleMetadata').resolves(undefined);
t.teardown(() => {
cmrMetadataStub.restore();
});

await unpublishGranule({ knex: t.context.knex, pgGranuleRecord: originalPgGranule });

t.like(
await t.context.granulesModel.get({ granuleId: originalDynamoGranule.granuleId }),
Expand Down
29 changes: 29 additions & 0 deletions packages/api/tests/models/granules/test-granules-model.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const test = require('ava');
const sinon = require('sinon');
const omit = require('lodash/omit');

const awsServices = require('@cumulus/aws-client/services');
const s3Utils = require('@cumulus/aws-client/S3');
Expand Down Expand Up @@ -622,3 +623,31 @@ test('_getMutableFieldNames() returns correct fields for completed status', (t)

t.deepEqual(updateFields, Object.keys(item));
});

test('update() updates granule', async (t) => {
const { granuleModel } = t.context;
const granule = fakeGranuleFactoryV2({ published: true, cmrLink: randomString() });
await granuleModel.create(granule);

const updatedGranule = await granuleModel.update(
{ granuleId: granule.granuleId },
{ published: false },
['cmrLink']
);

t.false(updatedGranule.published);
t.falsy(updatedGranule.cmrLink);
const omitList = ['cmrLink', 'published', 'updatedAt'];
t.deepEqual(omit(updatedGranule, omitList), omit(granule, omitList));
});

test('update() returns undefined if granule does not exist', async (t) => {
const { granuleModel } = t.context;
const updatedGranule = await granuleModel.update(
{ granuleId: randomString() },
{ published: false },
['cmrLink']
);

t.deepEqual(updatedGranule, undefined);
});

0 comments on commit 4c85ec3

Please sign in to comment.