refactor: use the same logic for inserting refactorings

Author: motoki317

backend/src/api/serve/refactorings.ts

@@ -1,41 +1,20 @@
 import { Request, Response } from 'express'
-import { commitPlaceholder, RefactoringMeta, RefactoringType } from '../../../../common/common.js'
-import { commitUrl } from '../../utils.js'
-import { commitsCol, refCol, repoCol } from '../../mongo.js'
-import { mergeCommitMetadataIntoRefactorings, updateCommitRefactoringMetadata } from '../../jobs/metadata.js'
+import { PureRefactoringMeta } from '../../../../common/common.js'
+import { commitsCol, repoCol } from '../../mongo.js'
+import { transformAndInsertRefactorings } from '../../jobs/process.js'
 
 interface PostRequest extends Request {
   body: {
     repository: string
     commit: string
 
-    refactorings: {
-      type: string
-      description: string
-
-      meta: {
-        tool?: string
-      }
-    }[]
+    refactorings: PureRefactoringMeta[]
   }
 }
 
 export const postRefactoringsHandler = async (req: PostRequest, res: Response) => {
   const body = req.body
 
-  const refactorings = body.refactorings.map((r): RefactoringMeta => {
-    return {
-      ...r,
-      type: r.type as RefactoringType, // ignore type-check
-
-      sha1: body.commit,
-      repository: body.repository,
-      url: commitUrl(body.repository, body.commit),
-
-      commit: commitPlaceholder(),
-    }
-  })
-
   {
     // Check repository and commits existence
     const actualCommit = await commitsCol.findOne({ _id: body.commit })
@@ -54,16 +33,7 @@ export const postRefactoringsHandler = async (req: PostRequest, res: Response) =
   }
 
   // Insert
-  const insertRes = await refCol.insertMany(refactorings)
-  if (!insertRes.acknowledged) {
-    return res.status(500).json({
-      message: 'Internal error',
-    })
-  }
-
-  // Update metadata
-  await updateCommitRefactoringMetadata(body.commit)
-  await mergeCommitMetadataIntoRefactorings(body.commit)
+  const insertRes = await transformAndInsertRefactorings(body.repository, body.commit, body.refactorings)
 
   return res.status(200).json({
     message: `Inserted ${insertRes.insertedCount} document(s)`,

backend/src/jobs/process.ts

@@ -1,6 +1,6 @@
 import { JobWithId } from '../jobs.js'
-import { commitsCol } from '../mongo.js'
-import { readAllFromCursor } from '../utils.js'
+import { commitsCol, refCol } from '../mongo.js'
+import { commitUrl, readAllFromCursor } from '../utils.js'
 import { processRMiner, rminerToolName } from './runner/rminer.js'
 import { processRefDiff, refDiffToolName } from './runner/refdiff.js'
 import {
@@ -9,12 +9,12 @@ import {
   updateCommitToolsMetadata,
 } from './metadata.js'
 import { formatTime } from '../../../common/utils.js'
-import { CommitProcessState } from '../../../common/common.js'
-import { JobData } from '../../../common/jobs'
+import { commitPlaceholder, CommitProcessState, PureRefactoringMeta, RefactoringMeta } from '../../../common/common.js'
+import { JobData } from '../../../common/jobs.js'
 
 type CommitId = string
 type ToolName = string
-type Processor = (repoUrl: string, commit: string) => Promise<void>
+type Processor = (repoUrl: string, commit: string) => Promise<PureRefactoringMeta[]>
 const processors: Record<ToolName, Processor> = {
   [rminerToolName]: processRMiner,
   [refDiffToolName]: processRefDiff,
@@ -23,13 +23,14 @@ const processors: Record<ToolName, Processor> = {
 const processCommit = async (repoUrl: string, commitId: CommitId, tools: Record<string, CommitProcessState>, retryError: boolean) => {
   const newTools = Object.assign({}, tools)
 
+  let pureRefs: PureRefactoringMeta[] = []
   for (const [tool, process] of Object.entries(processors)) {
     const toProcess = !(tool in tools) || retryError && tools[tool] === CommitProcessState.NG
     if (!toProcess) continue
 
     try {
       const start = performance.now()
-      await process(repoUrl, commitId)
+      pureRefs.push(...(await process(repoUrl, commitId)))
       newTools[tool] = CommitProcessState.OK
       console.log(` -> ${tool} in ${formatTime(start)}`)
     } catch (e) {
@@ -39,9 +40,31 @@ const processCommit = async (repoUrl: string, commitId: CommitId, tools: Record<
     }
   }
 
-  await updateCommitRefactoringMetadata(commitId)
   await updateCommitToolsMetadata(commitId, newTools)
-  await mergeCommitMetadataIntoRefactorings(commitId)
+  await transformAndInsertRefactorings(repoUrl, commitId, pureRefs)
+}
+
+export const transformAndInsertRefactorings = async (repoUrl: string, commit: string, pureRefs: PureRefactoringMeta[]): Promise<{ insertedCount: number }> => {
+  const refactorings = pureRefs.map((r): RefactoringMeta => {
+    return {
+      ...r,
+      sha1: commit,
+      repository: repoUrl,
+      url: commitUrl(repoUrl, commit),
+      commit: commitPlaceholder(),
+    }
+  })
+
+  let insertedCount = 0
+  if (refactorings.length > 0) { // Running insertMany with empty array results in an error
+    const insertRes = await refCol.insertMany(refactorings)
+    insertedCount = insertRes.insertedCount
+  }
+
+  await updateCommitRefactoringMetadata(commit)
+  await mergeCommitMetadataIntoRefactorings(commit)
+
+  return { insertedCount }
 }
 
 export const processCommits = async (job: JobWithId, jobData: JobData) => {

backend/src/jobs/processor/refdiff.ts

@@ -1,19 +1,20 @@
 import equal from 'fast-deep-equal'
 import {
   ProcessedRefDiffRefactoring,
-  RefDiffCommit,
   RefDiffLocation,
   RefDiffLocationWithLines,
   RefDiffNode,
   RefDiffNodeWithLines,
-  RefDiffOutput,
   RefDiffRefactoring,
 } from '../../../../common/refdiff.js'
-import { commitPlaceholder, RefactoringMeta, RefactoringType, RefactoringTypes } from '../../../../common/common.js'
-import { commitUrl } from '../../utils.js'
+import {
+  PureRefactoringMeta,
+  RefactoringType,
+  RefactoringTypes,
+} from '../../../../common/common.js'
 import { refDiffToolName } from '../runner/refdiff.js'
 
-type R = RefactoringMeta & ProcessedRefDiffRefactoring
+type R = PureRefactoringMeta & ProcessedRefDiffRefactoring
 
 const formatTypeAndDescription = (ref: RefDiffRefactoring): [typ: RefactoringType, desc: string] => {
   switch (ref.type) {
@@ -62,8 +63,8 @@ const formatTypeAndDescription = (ref: RefDiffRefactoring): [typ: RefactoringTyp
   }
 }
 
-const extractedMethods = (c: RefDiffCommit): RefDiffRefactoring[] => {
-  return c.refactorings
+const extractedMethods = (refs: RefDiffRefactoring[]): RefDiffRefactoring[] => {
+  return refs
     .filter((r) => r.type === 'EXTRACT' && r.after.type === 'Method')
 }
 
@@ -83,75 +84,68 @@ const process = (ref: RefDiffRefactoring): ProcessedRefDiffRefactoring => ({
   after: processNode(ref.after),
 })
 
-export const processRefDiffOutput = (repoUrl: string, output: RefDiffOutput): R[] => {
-  return output.map((c): RefDiffCommit => {
-    c.refactorings = c.refactorings.flatMap((ref): RefDiffRefactoring[] => {
-      switch (ref.type) {
-        case 'INTERNAL_MOVE_RENAME':
-          return [
-            { ...ref, type: 'INTERNAL_MOVE' },
-            { ...ref, type: 'RENAME' },
-            ref,
-          ]
-        case 'MOVE_RENAME':
-          return [
-            { ...ref, type: 'MOVE' },
-            { ...ref, type: 'RENAME' },
-            ref,
-          ]
-        case 'EXTRACT_MOVE':
-          return [
-            { ...ref, type: 'EXTRACT' },
-            { ...ref, type: 'MOVE' },
-            ref,
-          ]
-        default:
-          return [ref]
-      }
-    })
-    return c
-  }).flatMap((c): R[] => {
-    const extractMethodRefactorings = extractedMethods(c)
+export const processRefDiffOutput = (refs: RefDiffRefactoring[]): R[] => {
+  // Expand some composite refactorings
+  const refactorings = refs.flatMap((ref): RefDiffRefactoring[] => {
+    switch (ref.type) {
+      case 'INTERNAL_MOVE_RENAME':
+        return [
+          { ...ref, type: 'INTERNAL_MOVE' },
+          { ...ref, type: 'RENAME' },
+          ref,
+        ]
+      case 'MOVE_RENAME':
+        return [
+          { ...ref, type: 'MOVE' },
+          { ...ref, type: 'RENAME' },
+          ref,
+        ]
+      case 'EXTRACT_MOVE':
+        return [
+          { ...ref, type: 'EXTRACT' },
+          { ...ref, type: 'MOVE' },
+          ref,
+        ]
+      default:
+        return [ref]
+    }
+  })
 
-    return c.refactorings.map((ref): R => {
-      const [typ, description] = formatTypeAndDescription(ref)
+  const extractMethodRefactorings = extractedMethods(refs)
 
-      const ret: R = {
-        type: typ,
-        description,
+  return refactorings.map((ref): R => {
+    const [typ, description] = formatTypeAndDescription(ref)
 
-        sha1: c.sha1,
-        repository: repoUrl,
-        url: commitUrl(repoUrl, c.sha1),
+    const ret: R = {
+      type: typ,
+      description,
 
-        meta: {
-          tool: refDiffToolName,
-        },
-        commit: commitPlaceholder(),
+      meta: {
+        tool: refDiffToolName,
+      },
 
-        ...process(ref),
-      }
+      ...process(ref),
+    }
 
-      // Pre-compute needed information
-      if (typ === RefactoringTypes.ExtractMethod) {
-        ret.extractMethod = {
-          // Use-case 1: 重複の処理が無い / あるextract
-          sourceMethodsCount: extractSourceMethodsCount(ref, extractMethodRefactorings),
-          // Use-case 2: 数行のみのextract,  extractする前の行数
-          sourceMethodLines: ret.before.location.lines,
-          extractedLines: ret.after.location.lines,
-        }
+    // Pre-compute needed information
+    if (typ === RefactoringTypes.ExtractMethod) {
+      ret.extractMethod = {
+        // Use-case 1: 重複の処理が無い / あるextract
+        sourceMethodsCount: extractSourceMethodsCount(ref, extractMethodRefactorings),
+        // Use-case 2: 数行のみのextract,  extractする前の行数
+        sourceMethodLines: ret.before.location.lines,
+        extractedLines: ret.after.location.lines,
       }
+    }
 
-      // Use-case 3: 具体的なrenameした単語
-      if (ref.type === 'RENAME') {
-        ret.rename = {
-          from: ref.before.name,
-          to: ref.after.name,
-        }
+    // Use-case 3: 具体的なrenameした単語
+    if (ref.type === 'RENAME') {
+      ret.rename = {
+        from: ref.before.name,
+        to: ref.after.name,
       }
+    }
 
-      return ret
-    })
+    return ret
   })
 }