Skip to content

Commit

Permalink
[RND-552] [RND-540] Introduce total ordering to document insert/updat…
Browse files Browse the repository at this point in the history
…es - MongoDB and PostgreSQL (#289)

* [RND-552] Send timestamp to backend

* [RND-552] fix read tests

* [NRD-552] tests for ca/lma

* [RND-552] timestamp tests

* [RND-552] upsert stale timestamp

* [RND-522] add postgresql

* get returns lastmodifiedate

* part way through test updates

* test updates complete
  • Loading branch information
bradbanister authored Aug 22, 2023
1 parent 0da9e6d commit efd2693
Show file tree
Hide file tree
Showing 70 changed files with 1,300 additions and 443 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
DeleteRequest,
DeleteResult,
} from '@edfi/meadowlark-core';
import { DocumentUuid, MeadowlarkId, TraceId } from '@edfi/meadowlark-core/src/model/BrandedTypes';
import { DocumentUuid, MeadowlarkId, TraceId } from '@edfi/meadowlark-core/src/model/IdTypes';
import { generateDocumentUuid } from '@edfi/meadowlark-core/src/model/DocumentIdentity';
import { Client } from '@elastic/elasticsearch';
import { queryDocuments } from '../../src/repository/QueryElasticsearch';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,19 +96,14 @@ export interface MeadowlarkDocument extends MeadowlarkDocumentId {
createdBy: string;

/*
* Creation date as as Unix timestamp.
* Creation date as as Unix timestamp, or null if this is a document to be updated and we don't know the create time yet
*/
createdAt?: number | undefined;
createdAt: number | null;

/*
* Last modified date as as Unix timestamp.
* Last modified date as as Unix timestamp.
*/
lastModifiedAt?: number | undefined;

/*
* Last modified date (UTC Format).
*/
_lastModifiedDate?: string | undefined;
lastModifiedAt: number;

/**
* An ObjectId managed by Meadowlark transactions for read locking. Optional because it does not need to be
Expand All @@ -124,17 +119,27 @@ function referencedDocumentIdsFrom(documentInfo: DocumentInfo): MeadowlarkId[] {
];
}

export function meadowlarkDocumentFrom(
resourceInfo: ResourceInfo,
documentInfo: DocumentInfo,
documentUuid: DocumentUuid,
meadowlarkId: MeadowlarkId,
edfiDoc: object,
validate: boolean,
createdBy: string,
createdAt: number,
lastModifiedAt: number,
): MeadowlarkDocument {
export function meadowlarkDocumentFrom({
resourceInfo,
documentInfo,
documentUuid,
meadowlarkId,
edfiDoc,
validate,
createdBy,
createdAt,
lastModifiedAt,
}: {
resourceInfo: ResourceInfo;
documentInfo: DocumentInfo;
documentUuid: DocumentUuid;
meadowlarkId: MeadowlarkId;
edfiDoc: object;
validate: boolean;
createdBy: string;
createdAt: number | null;
lastModifiedAt: number;
}): MeadowlarkDocument {
const aliasMeadowlarkIds: MeadowlarkId[] = [meadowlarkId];
if (documentInfo.superclassInfo != null) {
aliasMeadowlarkIds.push(getMeadowlarkIdForSuperclassInfo(documentInfo.superclassInfo));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,15 @@ export async function writeLockReferencedDocuments(
// MongoDB FindOption to return only the indexed _id field, making this a covered query (MongoDB will optimize)
export const onlyReturnId = (session: ClientSession): FindOptions => ({ projection: { _id: 1 }, session });

// MongoDB FindOption to return only the indexed documentUuid field, making this a covered query (MongoDB will optimize)
export const onlyReturnDocumentUuid = (session: ClientSession): FindOptions => ({
projection: { documentUuid: 1, createdAt: 1 },
// MongoDB FindOption to return only createdAt and lastModifiedAt
export const onlyReturnTimestamps = (session: ClientSession): FindOptions => ({
projection: { createdAt: 1, lastModifiedAt: 1 },
session,
});

// MongoDB FindOption to return the indexed documentUuid and the createdAt and lastModifiedAt fields
export const onlyReturnDocumentUuidAndTimestamps = (session: ClientSession): FindOptions => ({
projection: { documentUuid: 1, createdAt: 1, lastModifiedAt: 1 },
session,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,18 @@ export async function getDocumentByDocumentUuid(

try {
const result: WithId<MeadowlarkDocument> | null = await mongoCollection.findOne({ documentUuid });
if (result === null) return { response: 'GET_FAILURE_NOT_EXISTS', document: {} };
// eslint-disable-next-line no-underscore-dangle
const documentLastModifiedDate: string | null = result.lastModifiedAt
? new Date(result.lastModifiedAt).toISOString()
: null;
if (result === null) {
return { response: 'GET_FAILURE_NOT_EXISTS', edfiDoc: {}, documentUuid, lastModifiedDate: 0 };
}

return {
response: 'GET_SUCCESS',
document: { id: documentUuid, ...result.edfiDoc, _lastModifiedDate: documentLastModifiedDate },
edfiDoc: result.edfiDoc,
documentUuid,
lastModifiedDate: result.lastModifiedAt,
};
} catch (e) {
Logger.error(`${moduleName}.getDocumentById exception`, traceId, e);
return { response: 'UNKNOWN_FAILURE', document: {} };
return { response: 'UNKNOWN_FAILURE', edfiDoc: {}, documentUuid, lastModifiedDate: 0 };
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { Logger, Config } from '@edfi/meadowlark-utilities';
import { Collection, ClientSession, MongoClient, WithId } from 'mongodb';
import retry from 'async-retry';
import { MeadowlarkDocument, meadowlarkDocumentFrom } from '../model/MeadowlarkDocument';
import { getDocumentCollection, limitFive, onlyReturnId, writeLockReferencedDocuments } from './Db';
import { getDocumentCollection, limitFive, onlyReturnTimestamps, writeLockReferencedDocuments } from './Db';
import { deleteDocumentByMeadowlarkIdTransaction } from './Delete';
import { onlyDocumentsReferencing, validateReferences } from './ReferenceValidation';
import { upsertDocumentTransaction } from './Upsert';
Expand Down Expand Up @@ -91,11 +91,13 @@ async function tryUpdateByReplacement(
mongoCollection: Collection<MeadowlarkDocument>,
session: ClientSession,
): Promise<UpdateResult | null> {
// Try to update - for a matching documentUuid and matching identity (via meadowlarkId)
// Try to update - for a matching documentUuid and matching identity (via meadowlarkId),
// where the update request is not stale
const { acknowledged, matchedCount } = await mongoCollection.updateOne(
{
_id: meadowlarkId,
documentUuid,
lastModifiedAt: { $lt: document.lastModifiedAt },
},
{
$set: {
Expand Down Expand Up @@ -145,7 +147,7 @@ async function updateAllowingIdentityChange(
): Promise<UpdateResult> {
const { documentUuid, resourceInfo, traceId, security } = updateRequest;

// Optimize by trying a replacement update, which will succeed if there is no identity change
// Optimize happy path by trying a replacement update, which will succeed if there is no identity change
const tryUpdateByReplacementResult: UpdateResult | null = await tryUpdateByReplacement(
document,
updateRequest,
Expand All @@ -159,8 +161,31 @@ async function updateAllowingIdentityChange(
return tryUpdateByReplacementResult;
}

// Either the documentUuid doesn't exist or the identity has changed.
// The following delete attempt will catch if documentUuid does not exist
// Failure to match means either:
// 1) document not there (invalid documentUuid)
// 2) this is an attempt to change the document identity (mismatched meadowlarkId)
// 3) the requestTimestamp for the update is too old

// See if the document is there
const documentByUuid: WithId<MeadowlarkDocument> | null = await mongoCollection.findOne(
{ documentUuid: updateRequest.documentUuid },
onlyReturnTimestamps(session),
);

if (documentByUuid == null) {
// The document is not there
return { response: 'UPDATE_FAILURE_NOT_EXISTS' };
}

if (documentByUuid.lastModifiedAt >= document.lastModifiedAt) {
// The update request is stale
return { response: 'UPDATE_FAILURE_WRITE_CONFLICT' };
}

// Get the createdAt from the original document before deleting
document.createdAt = documentByUuid.createdAt;

// Attempt to delete document before insert. Will work only if meadowlarkIds match between old and new
const deleteResult = await deleteDocumentByMeadowlarkIdTransaction(
{ documentUuid, resourceInfo, security, validateNoReferencesToDocument: true, traceId },
mongoCollection,
Expand All @@ -173,8 +198,11 @@ async function updateAllowingIdentityChange(
// document was deleted, so we can insert the new version
return insertUpdatedDocument(updateRequest, mongoCollection, session, document);
case 'DELETE_FAILURE_NOT_EXISTS':
// document was not found on delete
return { response: 'UPDATE_FAILURE_NOT_EXISTS' };
// document was not found on delete, which shouldn't happen
return {
response: 'UNKNOWN_FAILURE',
failureMessage: `Document uuid ${documentUuid} should have been found but was not`,
};
case 'DELETE_FAILURE_REFERENCE':
// We have an update cascade scenario
//
Expand Down Expand Up @@ -219,14 +247,26 @@ async function updateDisallowingIdentityChange(

if (tryUpdateByReplacementResult != null) return tryUpdateByReplacementResult;

// Failure to match means either document not there (invalid documentUuid) or this is an attempt to change
// the document identity (mismatched meadowlarkId). See if the document is there.
// Failure to match means either:
// 1) document not there (invalid documentUuid)
// 2) this is an attempt to change the document identity (mismatched meadowlarkId)
// 3) the requestTimestamp for the update is too old

// See if the document is there
const documentByUuid: WithId<MeadowlarkDocument> | null = await mongoCollection.findOne(
{ documentUuid: updateRequest.documentUuid },
onlyReturnId(session),
onlyReturnTimestamps(session),
);

if (documentByUuid == null) return { response: 'UPDATE_FAILURE_NOT_EXISTS' };
if (documentByUuid == null) {
// The document is not there
return { response: 'UPDATE_FAILURE_NOT_EXISTS' };
}

if (documentByUuid.lastModifiedAt >= document.lastModifiedAt) {
// The update request is stale
return { response: 'UPDATE_FAILURE_WRITE_CONFLICT' };
}

// The identity of the new document is different from the existing document
return { response: 'UPDATE_FAILURE_IMMUTABLE_IDENTITY' };
Expand Down Expand Up @@ -283,8 +323,6 @@ async function updateDocumentByDocumentUuidTransaction(
): Promise<UpdateResult> {
const { meadowlarkId, documentUuid, resourceInfo, documentInfo, edfiDoc, validateDocumentReferencesExist, security } =
updateRequest;
// last modified date as an Unix timestamp.
const lastModifiedAt: number = Date.now();
if (validateDocumentReferencesExist) {
const invalidReferenceResult: UpdateResult | null = await checkForInvalidReferences(
updateRequest,
Expand All @@ -295,17 +333,17 @@ async function updateDocumentByDocumentUuidTransaction(
return invalidReferenceResult;
}
}
const document: MeadowlarkDocument = meadowlarkDocumentFrom(
const document: MeadowlarkDocument = meadowlarkDocumentFrom({
resourceInfo,
documentInfo,
documentUuid,
meadowlarkId,
edfiDoc,
validateDocumentReferencesExist,
security.clientId,
Date.now(),
lastModifiedAt,
);
validate: validateDocumentReferencesExist,
createdBy: security.clientId,
createdAt: null, // null because this is a document update, createdAt must come from existing document
lastModifiedAt: documentInfo.requestTimestamp,
});
if (resourceInfo.allowIdentityUpdates) {
return updateAllowingIdentityChange(document, updateRequest, mongoCollection, session);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,13 @@ import {
import { Logger, Config } from '@edfi/meadowlark-utilities';
import retry from 'async-retry';
import { MeadowlarkDocument, meadowlarkDocumentFrom } from '../model/MeadowlarkDocument';
import { writeLockReferencedDocuments, asUpsert, limitFive, getDocumentCollection, onlyReturnDocumentUuid } from './Db';
import {
writeLockReferencedDocuments,
asUpsert,
limitFive,
getDocumentCollection,
onlyReturnDocumentUuidAndTimestamps,
} from './Db';
import { onlyDocumentsReferencing, validateReferences } from './ReferenceValidation';

const moduleName: string = 'mongodb.repository.Upsert';
Expand All @@ -32,15 +38,18 @@ export async function upsertDocumentTransaction(
// Check whether this document exists in the db
const existingDocument: WithId<MeadowlarkDocument> | null = await mongoCollection.findOne(
{ _id: meadowlarkId },
onlyReturnDocumentUuid(session),
onlyReturnDocumentUuidAndTimestamps(session),
);

// If there is an existing document, ensure this request is not stale
if (existingDocument != null && existingDocument.lastModifiedAt >= documentInfo.requestTimestamp) {
// The upsert request is stale
return { response: 'UPSERT_FAILURE_WRITE_CONFLICT' };
}

// the documentUuid of the existing document if this is an update, or a new one if this is an insert
const documentUuid: DocumentUuid | null = existingDocument?.documentUuid ?? generateDocumentUuid();
// Unix timestamp
const createdAt: number = existingDocument?.createdAt ?? Date.now();
// last modified date as an Unix timestamp.
const lastModifiedAt: number = Date.now();

// Check whether this is an insert or update
const isInsert: boolean = existingDocument == null;

Expand Down Expand Up @@ -114,17 +123,17 @@ export async function upsertDocumentTransaction(

const document: MeadowlarkDocument =
documentFromUpdate ??
meadowlarkDocumentFrom(
meadowlarkDocumentFrom({
resourceInfo,
documentInfo,
documentUuid,
meadowlarkId,
edfiDoc,
validateDocumentReferencesExist,
security.clientId,
createdAt,
lastModifiedAt,
);
validate: validateDocumentReferencesExist,
createdBy: security.clientId,
createdAt: existingDocument?.createdAt ?? documentInfo.requestTimestamp,
lastModifiedAt: documentInfo.requestTimestamp,
});

await writeLockReferencedDocuments(mongoCollection, document.outboundRefs, session);
// Perform the document upsert
Expand Down
Loading

0 comments on commit efd2693

Please sign in to comment.