Revert "DOP-3394: Upsert page documents to updated_documents collection" (#818)

Revert "DOP-3394: Upsert page documents to updated_documents collection (#816)"

This reverts commit f0ea39e.

Author: rayangler

modules/persistence/index.ts

@@ -6,7 +6,7 @@ import AdmZip from 'adm-zip';
 import minimist from 'minimist';
 import * as mongodb from 'mongodb';
 import { teardown as closeDBConnection } from './src/services/connector';
-import { insertAndUpdatePages } from './src/services/pages';
+import { insertPages } from './src/services/pages';
 import {
   insertMetadata,
   insertMergedMetadataEntries,
@@ -37,7 +37,7 @@ const app = async (path: string) => {
     // that only one build will be used per run of this module.
     const buildId = new mongodb.ObjectId();
     const metadata = await metadataFromZip(zip);
-    await Promise.all([insertAndUpdatePages(buildId, zip), insertMetadata(buildId, metadata), upsertAssets(zip)]);
+    await Promise.all([insertPages(buildId, zip), insertMetadata(buildId, metadata), upsertAssets(zip)]);
     await insertMergedMetadataEntries(buildId, metadata);
     // DOP-3447 clean up stale metadata
     await deleteStaleMetadata(metadata);

modules/persistence/package-lock.json

@@ -26,21 +26,20 @@
     "bson": "^4.7.0",
     "config": "^3.3.7",
     "dotenv": "^16.0.2",
-    "fast-deep-equal": "^3.1.3",
     "minimist": "^1.2.6",
     "mongodb": "^4.9.1",
     "ts-jest": "^27.0.5",
     "typescript": "^4.4.3"
   },
   "devDependencies": {
     "@shelf/jest-mongodb": "^4.1.4",
-    "@typescript-eslint/eslint-plugin": "~5.5.0",
-    "@typescript-eslint/parser": "^5.47.0",
     "eslint": "^8.3.0",
     "eslint-config-airbnb-base": "^15.0.0",
     "eslint-plugin-import": "^2.25.3",
     "eslint-plugin-jest": "^22.1.3",
     "jest": "^27.2.4",
-    "jest-environment-node": "^27.1.0"
+    "jest-environment-node": "^27.1.0",
+    "@typescript-eslint/eslint-plugin": "~5.5.0",
+    "@typescript-eslint/parser": "^5.47.0"
   }
 }

modules/persistence/package.json

@@ -1,5 +1,5 @@
 import AdmZip from 'adm-zip';
-import { bulkUpsertAll } from '../connector';
+import { bulkUpsert } from '../connector';
 
 const COLLECTION_NAME = 'assets';
 
@@ -15,7 +15,7 @@ const assetsFromZip = (zip: AdmZip) => {
 export const upsertAssets = async (zip: AdmZip) => {
   try {
     const assets = assetsFromZip(zip);
-    return bulkUpsertAll(assets, COLLECTION_NAME);
+    return bulkUpsert(assets, COLLECTION_NAME);
   } catch (error) {
     console.error(`Error at upsertion time for ${COLLECTION_NAME}: ${error}`);
     throw error;

modules/persistence/src/services/assets/index.ts

@@ -50,32 +50,28 @@ export const insert = async (docs: any[], collection: string, buildId: ObjectId)
   }
 };
 
-export const bulkWrite = async (operations: mongodb.AnyBulkWriteOperation[], collection: string) => {
-  const dbSession = await db();
+// Upsert wrapper, requires an _id field.
+export const bulkUpsert = async (items: Document[], collection: string) => {
+  const upsertSession = await db();
   try {
-    return dbSession.collection(collection).bulkWrite(operations);
+    const operations: mongodb.AnyBulkWriteOperation[] = [];
+    items.forEach((item: Document) => {
+      const op = {
+        updateOne: {
+          filter: { _id: item._id },
+          update: { $set: item },
+          upsert: true,
+        },
+      };
+      operations.push(op);
+    });
+    return upsertSession.collection(collection).bulkWrite(operations);
   } catch (error) {
-    console.error(`Error at bulk write time for ${collection}: ${error}`);
+    console.error(`Error at bulk upsertion time for ${collection}: ${error}`);
     throw error;
   }
 };
 
-// Upsert wrapper, requires an _id field.
-export const bulkUpsertAll = async (items: Document[], collection: string) => {
-  const operations: mongodb.AnyBulkWriteOperation[] = [];
-  items.forEach((item: Document) => {
-    const op = {
-      updateOne: {
-        filter: { _id: item._id },
-        update: { $set: item },
-        upsert: true,
-      },
-    };
-    operations.push(op);
-  });
-  return bulkWrite(operations, collection);
-};
-
 export const deleteDocuments = async (_ids: ObjectId[], collection: string) => {
   const deleteSession = await db();
   try {

modules/persistence/src/services/connector/index.ts

@@ -1,31 +1,9 @@
 import AdmZip from 'adm-zip';
 import { deserialize } from 'bson';
-import isEqual from 'fast-deep-equal';
-import { AnyBulkWriteOperation, Document, FindCursor, ObjectId } from 'mongodb';
-import { bulkWrite, db, insert } from '../connector';
-
-interface StaticAsset {
-  checksum: string;
-  key: string;
-}
-
-interface PageAst {
-  [key: string]: any;
-}
-
-export interface UpdatedPage {
-  page_id: string;
-  filename: string;
-  ast: PageAst;
-  static_assets: StaticAsset[];
-
-  created_at: Date;
-  updated_at: Date;
-  deleted: boolean;
-}
+import { ObjectId } from 'mongodb';
+import { insert } from '../connector';
 
 const COLLECTION_NAME = 'documents';
-const UPDATED_AST_COLL_NAME = 'updated_documents';
 
 // Service responsible for memoization of page level documents.
 // Any extraneous logic performed on page level documents as part of upload should be added here
@@ -37,175 +15,12 @@ const pagesFromZip = (zip: AdmZip) => {
     .map((entry) => deserialize(entry.getData()));
 };
 
-/**
- *
- * Finds the page documents for a given Snooty project name + branch combination.
- * If this is the first build for the Snooty project name + branch, no documents
- * will be found.
- *
- * @param pageIdPrefix - Includes the Snooty project name, user (docsworker-xlarge), and branch
- * @param collection - The collection to perform the find query on
- */
-const findPrevPageDocs = async (pageIdPrefix: string, collection: string) => {
-  const dbSession = await db();
-  const findQuery = {
-    page_id: { $regex: new RegExp(`^${pageIdPrefix}`) },
-    deleted: false,
-  };
-  const projection = {
-    _id: 0,
-    page_id: 1,
-    ast: 1,
-  };
-
-  try {
-    return dbSession.collection<UpdatedPage>(collection).find(findQuery).project(projection);
-  } catch (error) {
-    console.error(
-      `Error trying to find previous page documents using prefix ${pageIdPrefix} in ${collection}}: ${error}`
-    );
-    throw error;
-  }
-};
-
-const createPageAstMapping = async (docsCursor: FindCursor) => {
-  // Create mapping for page id and its AST
-  const mapping: Record<string, object> = {};
-  // Create set of all page ids. To be used for tracking unseen pages in the current build
-  const pageIds = new Set<string>();
-  for await (const doc of docsCursor) {
-    mapping[doc.page_id] = doc.ast;
-    pageIds.add(doc.page_id);
-  }
-  return { mapping, pageIds };
-};
-
-class UpdatedPagesManager {
-  currentPages: Document[];
-  operations: AnyBulkWriteOperation[];
-  prevPageDocsMapping: Record<string, object>;
-  prevPageIds: Set<string>;
-
-  constructor(prevPageDocsMapping: Record<string, object>, prevPagesIds: Set<string>, pages: Document[]) {
-    this.currentPages = pages;
-    this.operations = [];
-    this.prevPageDocsMapping = prevPageDocsMapping;
-    this.prevPageIds = prevPagesIds;
-
-    const updateTime = new Date();
-    this.checkForPageDiffs(updateTime);
-    this.markUnseenPagesAsDeleted(updateTime);
-  }
-
-  /**
-   *
-   * Compares the ASTs of the current pages with the previous pages. New update
-   * operations are added whenever a diff in the page ASTs is found. Page IDs are
-   * removed from `prevPageIds` to signal that the previous page has been "seen"
-   *
-   * @param updateTime - the time to set updates to
-   */
-  checkForPageDiffs(updateTime: Date) {
-    this.currentPages.forEach((page) => {
-      // Filter out rst (non-page) files
-      if (!page.filename.endsWith('.txt')) {
-        return;
-      }
-
-      const currentPageId = page.page_id;
-      this.prevPageIds.delete(currentPageId);
-
-      // Update the document if page's current AST is different from previous build's.
-      // New pages should always count as having a "different" AST
-      if (!isEqual(page.ast, this.prevPageDocsMapping[currentPageId])) {
-        const operation = {
-          updateOne: {
-            filter: { page_id: currentPageId },
-            update: {
-              $set: {
-                page_id: currentPageId,
-                filename: page.filename,
-                ast: page.ast,
-                static_assets: page.static_assets,
-                updated_at: updateTime,
-                deleted: false,
-              },
-              $setOnInsert: {
-                created_at: updateTime,
-              },
-            },
-            upsert: true,
-          },
-        };
-        this.operations.push(operation);
-      }
-    });
-  }
-
-  /**
-   *
-   * Marks any pages from the previous build that were not used as "deleted"
-   *
-   * @param updateTime - the time to set updates to
-   */
-  markUnseenPagesAsDeleted(updateTime: Date) {
-    this.prevPageIds.forEach((unseenPageId) => {
-      const operation = {
-        updateOne: {
-          filter: { page_id: unseenPageId },
-          update: {
-            $set: {
-              deleted: true,
-              updated_at: updateTime,
-            },
-          },
-        },
-      };
-      this.operations.push(operation);
-    });
-  }
-
-  getOperations() {
-    return this.operations;
-  }
-}
-
-/**
- *
- * Upserts pages in separate collection. Copies of a page are created by page_id.
- * Updated pages within the same Snooty project name + branch should only update
- * related page documents.
- *
- * @param pages
- * @param collection
- */
-const updatePages = async (pages: Document[], collection: string) => {
-  if (pages.length === 0) {
-    return;
-  }
-
-  // Find all pages that share the same project name + branch. Expects page IDs
-  // to include these two properties after parse
-  const pageIdPrefix = pages[0].page_id.split('/').slice(0, 3).join('/');
-  const previousPagesCursor = await findPrevPageDocs(pageIdPrefix, collection);
-  const { mapping: prevPageDocsMapping, pageIds: prevPageIds } = await createPageAstMapping(previousPagesCursor);
-
-  const updatedPagesManager = new UpdatedPagesManager(prevPageDocsMapping, prevPageIds, pages);
-  const operations = updatedPagesManager.getOperations();
-
-  if (operations.length > 0) {
-    await bulkWrite(operations, collection);
-  }
-};
-
-export const insertAndUpdatePages = async (buildId: ObjectId, zip: AdmZip) => {
+export const insertPages = async (buildId: ObjectId, zip: AdmZip) => {
   try {
-    const pages = pagesFromZip(zip);
-    return Promise.all([insert(pages, COLLECTION_NAME, buildId), updatePages(pages, UPDATED_AST_COLL_NAME)]);
+    const pages = await pagesFromZip(zip);
+    return insert(pages, COLLECTION_NAME, buildId);
   } catch (error) {
     console.error(`Error at insertion time for ${COLLECTION_NAME}: ${error}`);
     throw error;
   }
 };
-
-export const _updatePages = updatePages;

modules/persistence/src/services/pages/index.ts

@@ -1,5 +1,5 @@
 import { ObjectID } from 'bson';
-import { bulkUpsertAll, db, insert } from '../../src/services/connector';
+import { bulkUpsert, db, insert } from '../../src/services/connector';
 
 const mockConnect = jest.fn();
 const mockDb = jest.fn();
@@ -123,7 +123,7 @@ describe('Connector module', () => {
     const collection = 'metadata';
 
     test('it calls on collection to update one with upsert option true', async () => {
-      await bulkUpsertAll([payload], collection);
+      await bulkUpsert([payload], collection);
       expect(mockCollection).toBeCalledWith(collection);
       expect(mockBulkWrite).toBeCalledWith([
         {
@@ -136,10 +136,10 @@ describe('Connector module', () => {
       ]);
     });
 
-    test('it throws error on bulkWrite error', async () => {
+    test('it throws error on updateone error', async () => {
       mockBulkWrite.mockRejectedValueOnce(new Error('test error') as never);
       try {
-        await bulkUpsertAll([payload], collection);
+        await bulkUpsert([payload], collection);
       } catch (e) {
         expect(e.message).toEqual('test error');
       }