Skip to content

Commit

Permalink
[RND-562] Rename "documentId" to "meadowlarkId" for all PostgreSQL co…
Browse files Browse the repository at this point in the history
…lumns (#255)

* [RND-562] Rename "documentId" to "meadowlarkId" for all PostgreSQL columns

Replace uses of name id to use meadowlarkId: document, alias, references and functions.
Update test cases.

Mongodb:
- Rename functions to add documentUuid or meadowlarkId to clarify the parameters.
- Update variables to use meadowlarkId instead of just id.
- Rename field aliasIds to aliasMeadowlarkId.

Postgres:
- Rename functions to add documentUuid or meadowlarkId to clarify the parameters.
- Update database scripts and queries to replace document_id by meadowlark_id and other columns using meadowlarkId.

* Remove test timeout
  • Loading branch information
jleiva-gap authored Jun 13, 2023
1 parent d19a7c0 commit ec091dd
Show file tree
Hide file tree
Showing 35 changed files with 366 additions and 326 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,19 @@ import { getSharedClient, closeSharedConnection } from './repository/Db';

// DocumentStore implementation
export async function deleteDocumentById(request: DeleteRequest): Promise<DeleteResult> {
return Delete.deleteDocumentById(request, await getSharedClient());
return Delete.deleteDocumentByDocumentUuid(request, await getSharedClient());
}

export async function getDocumentById(request: GetRequest): Promise<GetResult> {
return Get.getDocumentById(request, await getSharedClient());
return Get.getDocumentByDocumentUuid(request, await getSharedClient());
}

export async function upsertDocument(request: UpsertRequest): Promise<UpsertResult> {
return Upsert.upsertDocument(request, await getSharedClient());
}

export async function updateDocumentById(request: UpdateRequest): Promise<UpdateResult> {
return Update.updateDocumentById(request, await getSharedClient());
return Update.updateDocumentByDocumentUuid(request, await getSharedClient());
}

export async function securityMiddleware(middlewareModel: MiddlewareModel): Promise<MiddlewareModel> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export interface MeadowlarkDocument extends MeadowlarkDocumentId {

/**
* An array of ids this document will satisfy when reference validation performs existence checks.
* This array always includes the document id itself. If this document is a subclass, the array will also
* This array always includes the document meadowlark id itself. If this document is a subclass, the array will also
* contain the id of this document in superclass form (superclass name and project, identity property
* naming differences - like schoolId versus educationOrganizationId - accounted for). Such an id is an alias.
*
Expand All @@ -78,7 +78,7 @@ export interface MeadowlarkDocument extends MeadowlarkDocumentId {
* a reference from ClassPeriod to a School with schoolId=123, as well as a reference from
* Assessment to EducationOrganization with educationOrganizationId=123.
*/
aliasIds: MeadowlarkId[];
aliasMeadowlarkIds: MeadowlarkId[];

/**
* True if this document has been reference and descriptor validated.
Expand Down Expand Up @@ -135,9 +135,9 @@ export function meadowlarkDocumentFrom(
createdAt: number,
lastModifiedAt: number,
): MeadowlarkDocument {
const aliasIds: MeadowlarkId[] = [meadowlarkId];
const aliasMeadowlarkIds: MeadowlarkId[] = [meadowlarkId];
if (documentInfo.superclassInfo != null) {
aliasIds.push(getMeadowlarkIdForSuperclassInfo(documentInfo.superclassInfo));
aliasMeadowlarkIds.push(getMeadowlarkIdForSuperclassInfo(documentInfo.superclassInfo));
}

return {
Expand All @@ -149,7 +149,7 @@ export function meadowlarkDocumentFrom(
documentUuid,
_id: meadowlarkId,
edfiDoc,
aliasIds,
aliasMeadowlarkIds,
outboundRefs: referencedDocumentIdsFrom(documentInfo),
validated: validate,
createdBy,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export async function getNewClient(): Promise<MongoClient> {
.collection(DOCUMENT_COLLECTION_NAME);
await documentCollection.createIndex({ documentUuid: 1 });
await documentCollection.createIndex({ outboundRefs: 1 });
await documentCollection.createIndex({ aliasIds: 1 });
await documentCollection.createIndex({ aliasMeadowlarkIds: 1 });

// Create authorizations collection if not exists
const authorizationCollection: Collection<AuthorizationDocument> = newClient
Expand Down Expand Up @@ -102,7 +102,7 @@ export async function writeLockReferencedDocuments(
session: ClientSession,
): Promise<void> {
await mongoCollection.updateMany(
{ aliasIds: { $in: referencedMeadowlarkIds } },
{ aliasMeadowlarkIds: { $in: referencedMeadowlarkIds } },
{ $set: { lock: new ObjectId() } },
{ session },
);
Expand All @@ -118,7 +118,10 @@ export const onlyReturnDocumentUuid = (session: ClientSession): FindOptions => (
});

// MongoDB FindOption to return only the aliasId
export const onlyReturnAliasId = (session: ClientSession): FindOptions => ({ projection: { 'aliasIds.$': 1 }, session });
export const onlyReturnAliasMeadowlarkId = (session: ClientSession): FindOptions => ({
projection: { 'aliasMeadowlarkIds.$': 1 },
session,
});

// MongoDB ReplaceOption that enables upsert (insert if not exists)
export const asUpsert = (session: ClientSession): ReplaceOptions => ({ upsert: true, session });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ async function checkForReferencesToDocument(
mongoCollection: Collection<MeadowlarkDocument>,
session: ClientSession,
): Promise<DeleteResult | null> {
// Read for aliasIds to validate against
// Read for aliasMeadowlarkIds to validate against
const deleteCandidate: WithId<MeadowlarkDocument> | null = await mongoCollection.findOne(
{ documentUuid },
onlyReturnAliasIds(session),
Expand All @@ -38,21 +38,21 @@ async function checkForReferencesToDocument(

// Check for any references to the document to be deleted
const anyReferences: WithId<MeadowlarkDocument> | null = await mongoCollection.findOne(
onlyDocumentsReferencing(deleteCandidate.aliasIds),
onlyDocumentsReferencing(deleteCandidate.aliasMeadowlarkIds),
onlyReturnId(session),
);

if (!anyReferences) return null;

// Abort on validation failure
Logger.debug(
`${moduleName}.checkForReferencesToDocument: Deleting document uuid ${documentUuid} failed due to existing references`,
`${moduleName}.checkForReferencesToDocument: Deleting DocumentUuid ${documentUuid} failed due to existing references`,
traceId,
);

// Get the information of up to five blocking documents for failure message purposes
const referringDocuments: WithId<MeadowlarkDocument>[] = await mongoCollection
.find(onlyDocumentsReferencing(deleteCandidate.aliasIds), limitFive(session))
.find(onlyDocumentsReferencing(deleteCandidate.aliasMeadowlarkIds), limitFive(session))
.toArray();

const blockingDocuments: BlockingDocument[] = referringDocuments.map((document) => ({
Expand All @@ -72,7 +72,7 @@ async function checkForReferencesToDocument(
*
* This function expects Session to have an active transaction. Aborting the transaction on error is left to the caller.
*/
export async function deleteDocumentByIdTransaction(
export async function deleteDocumentByMeadowlarkIdTransaction(
{ documentUuid, validateNoReferencesToDocument, traceId }: DeleteRequest,
mongoCollection: Collection<MeadowlarkDocument>,
session: ClientSession,
Expand All @@ -88,14 +88,17 @@ export async function deleteDocumentByIdTransaction(
}

// Perform the document delete
Logger.debug(`${moduleName}.deleteDocumentByIdTransaction: Deleting document documentUuid ${documentUuid}`, traceId);
Logger.debug(
`${moduleName}.deleteDocumentByMeadowlarkIdTransaction: Deleting document documentUuid ${documentUuid}`,
traceId,
);

const { acknowledged, deletedCount } = await mongoCollection.deleteOne({ documentUuid }, { session });

if (!acknowledged) {
const msg =
'mongoCollection.deleteOne returned acknowledged: false, indicating a problem with write concern configuration';
Logger.error(`${moduleName}.deleteDocumentByIdTransaction`, traceId, msg);
Logger.error(`${moduleName}.deleteDocumentByMeadowlarkIdTransaction`, traceId, msg);
return { response: 'UNKNOWN_FAILURE', failureMessage: '' };
}

Expand All @@ -108,7 +111,10 @@ export async function deleteDocumentByIdTransaction(
* Takes a DeleteRequest and MongoClient from the BackendFacade and performs a delete by documentUuid
* and returns the DeleteResult.
*/
export async function deleteDocumentById(deleteRequest: DeleteRequest, client: MongoClient): Promise<DeleteResult> {
export async function deleteDocumentByDocumentUuid(
deleteRequest: DeleteRequest,
client: MongoClient,
): Promise<DeleteResult> {
const session: ClientSession = client.startSession();
let deleteResult: DeleteResult = { response: 'UNKNOWN_FAILURE', failureMessage: '' };
try {
Expand All @@ -119,7 +125,7 @@ export async function deleteDocumentById(deleteRequest: DeleteRequest, client: M
await retry(
async () => {
await session.withTransaction(async () => {
deleteResult = await deleteDocumentByIdTransaction(deleteRequest, mongoCollection, session);
deleteResult = await deleteDocumentByMeadowlarkIdTransaction(deleteRequest, mongoCollection, session);
if (deleteResult.response !== 'DELETE_SUCCESS') {
await session.abortTransaction();
}
Expand All @@ -129,14 +135,14 @@ export async function deleteDocumentById(deleteRequest: DeleteRequest, client: M
retries: numberOfRetries,
onRetry: () => {
Logger.warn(
`${moduleName}.deleteDocumentById got write conflict error for documentUuid ${deleteRequest.documentUuid}. Retrying...`,
`${moduleName}.deleteDocumentByDocumentUuid got write conflict error for documentUuid ${deleteRequest.documentUuid}. Retrying...`,
deleteRequest.traceId,
);
},
},
);
} catch (e) {
Logger.error(`${moduleName}.deleteDocumentById`, deleteRequest.traceId, e);
Logger.error(`${moduleName}.deleteDocumentByDocumentUuid`, deleteRequest.traceId, e);

let response: DeleteResult = { response: 'UNKNOWN_FAILURE', failureMessage: e.message };

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,11 @@ import { getDocumentCollection } from './Db';

const moduleName: string = 'mongodb.repository.get';

export async function getDocumentById({ documentUuid, traceId }: GetRequest, client: MongoClient): Promise<GetResult> {
Logger.debug(`${moduleName}.getDocumentById ${documentUuid}`, traceId);
export async function getDocumentByDocumentUuid(
{ documentUuid, traceId }: GetRequest,
client: MongoClient,
): Promise<GetResult> {
Logger.debug(`${moduleName}.getDocumentByDocumentUuid ${documentUuid}`, traceId);

const mongoCollection: Collection<MeadowlarkDocument> = getDocumentCollection(client);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { MeadowlarkDocument } from '../model/MeadowlarkDocument';
import { SecurityResult } from '../security/SecurityResult';
import { getDocumentCollection } from './Db';

function extractIdIfUpsert(frontendRequest: FrontendRequest): MeadowlarkId | null {
function extractMeadowlarkIdIfUpsert(frontendRequest: FrontendRequest): MeadowlarkId | null {
if (frontendRequest.action !== 'upsert') return null;

return meadowlarkIdForDocumentIdentity(
Expand Down Expand Up @@ -54,7 +54,7 @@ export async function rejectByOwnershipSecurity(
Logger.debug(`${functionName} - document found for documentUuid ${documentUuid}`, frontendRequest.traceId);
} else {
// security by meadowlarkId if it's upsert - document body with no documentUuid
const meadowlarkId: MeadowlarkId | null = extractIdIfUpsert(frontendRequest);
const meadowlarkId: MeadowlarkId | null = extractMeadowlarkIdIfUpsert(frontendRequest);

if (meadowlarkId == null) {
Logger.error(`${functionName} - no documentUuid or meadowlarkId to secure against`, frontendRequest.traceId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ import {
MeadowlarkId,
} from '@edfi/meadowlark-core';
import { Logger } from '@edfi/meadowlark-utilities';
import { onlyReturnAliasId, onlyReturnId } from './Db';
import { onlyReturnAliasMeadowlarkId, onlyReturnId } from './Db';
import { MeadowlarkDocument } from '../model/MeadowlarkDocument';

/**
* Finds whether the given reference ids are actually documents in the db. Uses the aliasIds
* Finds whether the given reference ids are actually documents in the db. Uses the aliasMeadowlarkIds
* array for this existence check.
*
* @param referenceMeadowlarkIds The reference ids to check for existence in the db
Expand All @@ -30,7 +30,7 @@ async function findReferencedMeadowlarkIdsByMeadowlarkId(
findOptions: FindOptions,
): Promise<MeadowlarkDocument[]> {
const referencedDocuments: MeadowlarkDocument[] = await mongoDocuments
.find({ aliasIds: { $in: referenceMeadowlarkIds } }, findOptions)
.find({ aliasMeadowlarkIds: { $in: referenceMeadowlarkIds } }, findOptions)
.toArray();
return referencedDocuments as MeadowlarkDocument[];
}
Expand All @@ -45,18 +45,20 @@ async function findReferencedMeadowlarkIdsByMeadowlarkId(
*/
export function findMissingReferences(
refsInDb: MeadowlarkDocument[],
documentOutboundRefs: string[],
documentOutboundRefs: MeadowlarkId[],
documentReferences: DocumentReference[],
): MissingIdentity[] {
// eslint-disable-next-line no-underscore-dangle
const idsOfRefsInDb: MeadowlarkId[] = refsInDb.map((outRef) =>
const meadowlarkIdsOfRefsInDb: MeadowlarkId[] = refsInDb.map((outRef) =>
// eslint-disable-next-line no-underscore-dangle
outRef.aliasIds.length > 0 ? outRef.aliasIds[0] : outRef._id,
outRef.aliasMeadowlarkIds.length > 0 ? outRef.aliasMeadowlarkIds[0] : outRef._id,
);
const outRefIdsNotInDb: string[] = R.difference(documentOutboundRefs, idsOfRefsInDb);
const outRefMeadowlarkIdsNotInDb: MeadowlarkId[] = R.difference(documentOutboundRefs, meadowlarkIdsOfRefsInDb);

// Gets the array indexes of the missing references, for the documentOutboundRefs array
const arrayIndexesOfMissing: number[] = outRefIdsNotInDb.map((outRefId) => documentOutboundRefs.indexOf(outRefId));
const arrayIndexesOfMissing: number[] = outRefMeadowlarkIdsNotInDb.map((outRefMeadowlarkId) =>
documentOutboundRefs.indexOf(outRefMeadowlarkId),
);

// Pick out the DocumentReferences of the missing from the entire array of DocumentReferences,
const pickedDocumentReferencesOfMissing: DocumentReference[] = R.props(
Expand All @@ -70,15 +72,15 @@ export function findMissingReferences(
}));
}

// MongoDB FindOption to return only the aliasIds field
// MongoDB FindOption to return only the aliasMeadowlarkIds field
export const onlyReturnAliasIds = (session: ClientSession): FindOptions => ({
projection: { aliasIds: 1 },
projection: { aliasMeadowlarkIds: 1 },
session,
});

// MongoDB Filter on documents with the given aliasIds in their outboundRefs list
export const onlyDocumentsReferencing = (aliasIds: MeadowlarkId[]): Filter<MeadowlarkDocument> => ({
outboundRefs: { $in: aliasIds },
// MongoDB Filter on documents with the given aliasMeadowlarkIds in their outboundRefs list
export const onlyDocumentsReferencing = (aliasMeadowlarkIds: MeadowlarkId[]): Filter<MeadowlarkDocument> => ({
outboundRefs: { $in: aliasMeadowlarkIds },
});

/**
Expand All @@ -104,30 +106,30 @@ export async function validateReferences(
getMeadowlarkIdForDocumentReference(dr),
) as MeadowlarkId[];

const referenceIdsInDb: MeadowlarkDocument[] = await findReferencedMeadowlarkIdsByMeadowlarkId(
const referenceMeadowlarkIdsInDb: MeadowlarkDocument[] = await findReferencedMeadowlarkIdsByMeadowlarkId(
referencedMeadowlarkIds,
mongoDocuments,
onlyReturnAliasId(session),
onlyReturnAliasMeadowlarkId(session),
);

if (referencedMeadowlarkIds.length !== referenceIdsInDb.length) {
if (referencedMeadowlarkIds.length !== referenceMeadowlarkIdsInDb.length) {
Logger.debug('mongodb.repository.WriteHelper.validateReferences: documentReferences not found', traceId);
failures.push(...findMissingReferences(referenceIdsInDb, referencedMeadowlarkIds, documentReferences));
failures.push(...findMissingReferences(referenceMeadowlarkIdsInDb, referencedMeadowlarkIds, documentReferences));
}

const descriptorReferenceIds: MeadowlarkId[] = descriptorReferences.map((dr: DocumentReference) =>
const descriptorReferenceMeadowlarkIds: MeadowlarkId[] = descriptorReferences.map((dr: DocumentReference) =>
getMeadowlarkIdForDocumentReference(dr),
) as MeadowlarkId[];

const descriptorsInDb = await findReferencedMeadowlarkIdsByMeadowlarkId(
descriptorReferenceIds,
descriptorReferenceMeadowlarkIds,
mongoDocuments,
onlyReturnId(session),
);

if (descriptorReferenceIds.length !== descriptorsInDb.length) {
if (descriptorReferenceMeadowlarkIds.length !== descriptorsInDb.length) {
Logger.debug('mongodb.repository.WriteHelper.validateReferences: descriptorReferences not found', traceId);
failures.push(...findMissingReferences(descriptorsInDb, descriptorReferenceIds, descriptorReferences));
failures.push(...findMissingReferences(descriptorsInDb, descriptorReferenceMeadowlarkIds, descriptorReferences));
}

return failures;
Expand Down
Loading

0 comments on commit ec091dd

Please sign in to comment.