From 324bbd15940b0c25415f9ceaf01de5aa684bc2ec Mon Sep 17 00:00:00 2001 From: Nikolay Tverdokhlebov Date: Sat, 12 Aug 2023 00:09:12 +0300 Subject: [PATCH] feat: optimize author obtaining --- src/models.ts | 5 + src/services/authors.ts | 48 +++-- src/services/metadata.ts | 60 ++++-- src/vcs-connector/github.ts | 85 +++----- .../services/metadataAuthors.test.ts | 18 +- tests/units/services/authors.test.ts | 183 +++++++++++++----- 6 files changed, 233 insertions(+), 166 deletions(-) diff --git a/src/models.ts b/src/models.ts index aa1540b0..ae3ecfe4 100644 --- a/src/models.ts +++ b/src/models.ts @@ -240,3 +240,8 @@ export type YandexCloudTranslateGlossaryPair = { sourceText: string; translatedText: string; }; + +export type CommitInfo = { + email: string; + hashCommit: string; +}; diff --git a/src/services/authors.ts b/src/services/authors.ts index 3764b19b..2b07abe8 100644 --- a/src/services/authors.ts +++ b/src/services/authors.ts @@ -1,35 +1,40 @@ -import {replaceDoubleToSingleQuotes, сarriage} from '../utils'; -import {REGEXP_AUTHOR} from '../constants'; +import {replaceDoubleToSingleQuotes} from '../utils'; import {VCSConnector} from '../vcs-connector/connector-models'; -async function updateAuthorMetadataString( - defaultMetadata = '', +async function updateAuthorMetadataStringByAuthorLogin( + authorLogin: string, vcsConnector?: VCSConnector, - filePath?: string | null, ): Promise { if (!vcsConnector) { - return defaultMetadata; + return ''; } - const matchAuthor = defaultMetadata.match(REGEXP_AUTHOR); + const user = await getAuthorDetails(vcsConnector, authorLogin); - if (matchAuthor && matchAuthor?.length > 0) { - const authorLogin = matchAuthor[0]; - const user = await getAuthorDetails(vcsConnector, authorLogin); + if (user) { + return user; + } - if (user) { - return defaultMetadata.replace(authorLogin, user); - } - } else if (filePath) { - const user = vcsConnector.getExternalAuthorByPath(filePath); + return ''; +} - if (user) { - const author = replaceDoubleToSingleQuotes(JSON.stringify(user)); - return `${defaultMetadata}${сarriage}author: ${author}`; - } + +async function updateAuthorMetadataStringByFilePath( + filePath: string, + vcsConnector?: VCSConnector, +): Promise { + if (!vcsConnector) { + return ''; + } + + const user = vcsConnector.getExternalAuthorByPath(filePath); + + if (user) { + const author = replaceDoubleToSingleQuotes(JSON.stringify(user)); + return author; } - return defaultMetadata; + return ''; } async function getAuthorDetails(vcsConnector: VCSConnector, author: string | object): Promise { @@ -53,6 +58,7 @@ async function getAuthorDetails(vcsConnector: VCSConnector, author: string | obj } export { - updateAuthorMetadataString, + updateAuthorMetadataStringByAuthorLogin, + updateAuthorMetadataStringByFilePath, getAuthorDetails, }; diff --git a/src/services/metadata.ts b/src/services/metadata.ts index 76b38754..0832b561 100644 --- a/src/services/metadata.ts +++ b/src/services/metadata.ts @@ -2,11 +2,15 @@ import {dump} from 'js-yaml'; import {VCSConnector} from '../vcs-connector/connector-models'; import {Metadata, MetaDataOptions, Resources} from '../models'; -import {getAuthorDetails, updateAuthorMetadataString} from './authors'; +import { + getAuthorDetails, + updateAuthorMetadataStringByAuthorLogin, + updateAuthorMetadataStringByFilePath, +} from './authors'; import {getFileContributorsMetadata, getFileContributorsString} from './contributors'; import {isObject} from './utils'; import {сarriage} from '../utils'; -import {metadataBorder} from '../constants'; +import {metadataBorder, REGEXP_AUTHOR} from '../constants'; import {dirname, relative, resolve} from 'path'; import {ArgvService} from './index'; @@ -78,26 +82,54 @@ async function getContentWithUpdatedDynamicMetadata( return fileContent; } + let fileMetadata: string | undefined, fileMainContent: string | undefined; const matches = matchMetadata(fileContent); + if (matches && matches.length > 0) { + const [, matchedFileMetadata, , matchedFileMainContent] = matches; + fileMetadata = matchedFileMetadata; + fileMainContent = matchedFileMainContent; + } + const newMetadatas: string[] = []; const {isContributorsEnabled} = options; if (isContributorsEnabled) { const contributorsMetaData = await getContributorsMetadataString(options, fileContent); - if (contributorsMetaData) { newMetadatas.push(contributorsMetaData); } + + let authorMetadata = ''; + if (fileMetadata) { + const matchAuthor = fileMetadata.match(REGEXP_AUTHOR); + if (matchAuthor) { + const matchedAuthor = matchAuthor[0]; + authorMetadata = await updateAuthorMetadataStringByAuthorLogin(matchedAuthor, options.vcsConnector); + } + } + + if (!authorMetadata) { + const {fileData: {tmpInputFilePath, inputFolderPathLength}} = options; + const relativeFilePath = tmpInputFilePath.substring(inputFolderPathLength); + authorMetadata = await updateAuthorMetadataStringByFilePath(relativeFilePath, options.vcsConnector); + } + + if (authorMetadata) { + newMetadatas.push(`author: ${authorMetadata}`); + } } - if (matches && matches.length > 0) { - const [, fileMetadata, , fileMainContent] = matches; - let updatedDefaultMetadata = ''; + if (fileMetadata && fileMainContent) { + let updatedFileMetadata = fileMetadata; + const matchAuthor = fileMetadata.match(REGEXP_AUTHOR); - updatedDefaultMetadata = await getAuthorMetadataString(options, fileMetadata); + const isNewMetadataIncludesAuthor = newMetadatas.some((item) => /^author: /.test(item)); + if (matchAuthor && isNewMetadataIncludesAuthor) { + updatedFileMetadata = updatedFileMetadata.replace(`author: ${matchAuthor[0]}`, ''); + } - return `${getUpdatedMetadataString(newMetadatas, updatedDefaultMetadata)}${fileMainContent}`; + return `${getUpdatedMetadataString(newMetadatas, updatedFileMetadata)}${fileMainContent}`; } return `${getUpdatedMetadataString(newMetadatas)}${fileContent}`; @@ -142,18 +174,6 @@ async function getContributorsMetadataString( return undefined; } -async function getAuthorMetadataString(options: MetaDataOptions, fileMetadata: string) { - const {fileData: {tmpInputFilePath, inputFolderPathLength}} = options; - - const relativeFilePath = tmpInputFilePath.substring(inputFolderPathLength); - - return await updateAuthorMetadataString( - fileMetadata, - options.vcsConnector, - relativeFilePath, - ); -} - function getUpdatedMetadataString(newMetadatas: string[], defaultMetadata = ''): string { const newMetadata = newMetadatas.join(сarriage) + (newMetadatas.length ? сarriage : ''); const preparedDefaultMetadata = defaultMetadata.trimRight(); diff --git a/src/vcs-connector/github.ts b/src/vcs-connector/github.ts index 10c829e9..0757cad5 100644 --- a/src/vcs-connector/github.ts +++ b/src/vcs-connector/github.ts @@ -1,12 +1,12 @@ import {Octokit} from '@octokit/core'; import {join, normalize} from 'path'; import simpleGit, {SimpleGitOptions} from 'simple-git'; -import {asyncify, mapLimit} from 'async'; import {minimatch} from 'minimatch'; import github from './client/github'; import {ArgvService} from '../services'; import { + CommitInfo, Contributor, Contributors, ContributorsByPathFunction, @@ -28,11 +28,8 @@ import {addSlashPrefix, logger} from '../utils'; import {validateConnectorFields} from './connector-validator'; import process from 'process'; -const MAX_CONCURRENCY = 99; - const authorByGitEmail: Map = new Map(); const authorByPath: Map = new Map(); -const authorAlreadyCheckedForPath: Map = new Map(); const contributorsByPath: Map = new Map(); const contributorsData: Map = new Map(); @@ -109,7 +106,17 @@ async function getAllContributorsTocFiles(httpClientByToken: Octokit): Promise = { - baseDir: join(rootInput, masterDir), - }; - - const normalizePath = normalize(addSlashPrefix(path)); - - if (authorAlreadyCheckedForPath.has(normalizePath)) { - return null; - } - - const commitData = await simpleGit(options).raw( - 'log', - `${FIRST_COMMIT_FROM_ROBOT_IN_GITHUB}..HEAD`, - '--diff-filter=A', - '--pretty=format:%ae;%an;%H', - '--', - path, - ); - - const [email, name, hashCommit] = commitData.split(';'); - if (!(email && hashCommit)) { - return null; - } - - authorAlreadyCheckedForPath.set(normalizePath, true); - - if (shouldAuthorBeIgnored({email, name})) { - return null; - } - - return {hashCommit, normalizePath, email}; -} - type ShouldAuthorBeIgnoredArgs = { email?: string; name?: string; diff --git a/tests/integrations/services/metadataAuthors.test.ts b/tests/integrations/services/metadataAuthors.test.ts index 478377c7..674beeae 100644 --- a/tests/integrations/services/metadataAuthors.test.ts +++ b/tests/integrations/services/metadataAuthors.test.ts @@ -1,6 +1,6 @@ import {readFileSync} from 'fs'; import {REGEXP_AUTHOR} from '../../../src/constants'; -import {replaceDoubleToSingleQuotes} from '../../../src/utils/markup'; +import {replaceDoubleToSingleQuotes, сarriage} from '../../../src/utils/markup'; import {MetaDataOptions} from 'models'; import {getContentWithUpdatedMetadata} from 'services/metadata'; import {VCSConnector} from 'vcs-connector/connector-models'; @@ -103,6 +103,7 @@ describe('getContentWithUpdatedMetadata (Authors)', () => { metaDataOptions.vcsConnector = { ...defaultVCSConnector, getUserByLogin: () => Promise.resolve(null), + getExternalAuthorByPath: () => null, }; const fileContent = readFileSync(authorAliasInMetadataFilePath, 'utf8'); @@ -117,18 +118,11 @@ describe('getContentWithUpdatedMetadata (Authors)', () => { const fileContent = readFileSync(simpleMetadataFilePath, 'utf8'); const updatedFileContent = await getContentWithUpdatedMetadata(fileContent, metaDataOptions); + const lastMetadataRow = 'editable: false'; + const expectedFileContent = fileContent + .replace(lastMetadataRow, replaceDoubleToSingleQuotes(`${lastMetadataRow}${сarriage}author: ${replaceDoubleToSingleQuotes(JSON.stringify(expectedAuthorData))}`)); - expect(updatedFileContent).toEqual(fileContent); - }); - - test('if metadata does not have author', async () => { - metaDataOptions.isContributorsEnabled = true; - metaDataOptions.vcsConnector = defaultVCSConnector; - const fileContent = readFileSync(simpleMetadataFilePath, 'utf8'); - - const updatedFileContent = await getContentWithUpdatedMetadata(fileContent, metaDataOptions); - - expect(updatedFileContent).toEqual(fileContent); + expect(updatedFileContent).toEqual(expectedFileContent); }); }); }); diff --git a/tests/units/services/authors.test.ts b/tests/units/services/authors.test.ts index 7007a714..796fddfb 100644 --- a/tests/units/services/authors.test.ts +++ b/tests/units/services/authors.test.ts @@ -1,7 +1,13 @@ import * as units from 'utils/markup'; -import {REGEXP_AUTHOR} from '../../../src/constants'; -import {getAuthorDetails, updateAuthorMetadataString} from 'services/authors'; +import { + getAuthorDetails, + updateAuthorMetadataStringByAuthorLogin, + updateAuthorMetadataStringByFilePath, +} from 'services/authors'; import {VCSConnector} from 'vcs-connector/connector-models'; +import {Contributor} from 'models'; + +const filepath = 'index.md'; const author = { avatar: 'https://example.ru/logo.png', @@ -11,18 +17,23 @@ const author = { login: 'alias', }; +const authorByPath: Map = new Map(); + const defaultVCSConnector: VCSConnector = { - addNestedContributorsForPath: () => { }, + addNestedContributorsForPath: () => {}, getContributorsByPath: () => Promise.resolve(null), - getUserByLogin: () => Promise.resolve(null), - getExternalAuthorByPath: () => null, + getUserByLogin: () => Promise.resolve(author), + getExternalAuthorByPath: (path) => authorByPath.get(path), }; describe('getAuthorDetails returns author details', () => { let spyReplaceDoubleToSingleQuotes: jest.SpyInstance; beforeAll(() => { - spyReplaceDoubleToSingleQuotes = jest.spyOn(units, 'replaceDoubleToSingleQuotes'); + spyReplaceDoubleToSingleQuotes = jest.spyOn( + units, + 'replaceDoubleToSingleQuotes' + ); }); beforeEach(() => { @@ -30,7 +41,6 @@ describe('getAuthorDetails returns author details', () => { }); afterEach(() => { - defaultVCSConnector.getUserByLogin = () => Promise.resolve(null); expect(spyReplaceDoubleToSingleQuotes).toHaveBeenCalled(); expect(spyReplaceDoubleToSingleQuotes).toHaveBeenCalledTimes(2); }); @@ -40,27 +50,40 @@ describe('getAuthorDetails returns author details', () => { }); test('when author is object', async () => { - const expectedAuthorDetails = units.replaceDoubleToSingleQuotes(JSON.stringify(author)); + const expectedAuthorDetails = units.replaceDoubleToSingleQuotes( + JSON.stringify(author) + ); - const authorDetails = await getAuthorDetails(defaultVCSConnector, author); + const authorDetails = await getAuthorDetails( + defaultVCSConnector, + author + ); expect(authorDetails).toEqual(expectedAuthorDetails); }); test('when author is stringified object', async () => { const stringifiedObject = JSON.stringify(author); - const expectedAuthorDetails = units.replaceDoubleToSingleQuotes(stringifiedObject); + const expectedAuthorDetails = + units.replaceDoubleToSingleQuotes(stringifiedObject); - const authorDetails = await getAuthorDetails(defaultVCSConnector, stringifiedObject); + const authorDetails = await getAuthorDetails( + defaultVCSConnector, + stringifiedObject + ); expect(authorDetails).toEqual(expectedAuthorDetails); }); test('when author is alias and "getUserByLogin" returns author data by alias', async () => { - const expectedAuthorDetails = units.replaceDoubleToSingleQuotes(JSON.stringify(author)); - defaultVCSConnector.getUserByLogin = () => Promise.resolve(author); + const expectedAuthorDetails = units.replaceDoubleToSingleQuotes( + JSON.stringify(author) + ); - const authorDetails = await getAuthorDetails(defaultVCSConnector, author.login); + const authorDetails = await getAuthorDetails( + defaultVCSConnector, + author.login + ); expect(authorDetails).toEqual(expectedAuthorDetails); }); @@ -73,65 +96,125 @@ describe('getAuthorDetails does not return author details', () => { test('when author is alias and "getUserByLogin" does not return author data by alias', async () => { const expectedAuthorDetails = null; - defaultVCSConnector.getUserByLogin = () => Promise.resolve(null); - const spyReplaceDoubleToSingleQuotes = jest.spyOn(units, 'replaceDoubleToSingleQuotes'); - - const authorDetails = await getAuthorDetails(defaultVCSConnector, author.login); + const vcsConnector = { + ...defaultVCSConnector, + getUserByLogin: () => Promise.resolve(null), + }; + const spyReplaceDoubleToSingleQuotes = jest.spyOn( + units, + 'replaceDoubleToSingleQuotes' + ); + + const authorDetails = await getAuthorDetails( + vcsConnector, + author.login + ); expect(authorDetails).toEqual(expectedAuthorDetails); expect(spyReplaceDoubleToSingleQuotes).not.toHaveBeenCalled(); }); }); -describe('updateAuthorMetadataString', () => { +describe('update author metadata by authorLogin', () => { afterAll(() => { jest.clearAllMocks(); }); - describe('should return default metadata', () => { - test('when "vcsConnector" is undefined', async () => { - const expectedMetadata = 'Some metadata'; + test('returns empty strring when "vcsConnector" is undefined', async () => { + const expectedMetadata = ''; - const updatedMetadata = await updateAuthorMetadataString(expectedMetadata); + const updatedMetadata = await updateAuthorMetadataStringByAuthorLogin( + author.login + ); - expect(updatedMetadata).toEqual(expectedMetadata); - }); + expect(updatedMetadata).toEqual(expectedMetadata); + }); + + test('returns empty strring when "getUserByLogin" returns null', async () => { + const expectedMetadata = ''; + const vcsConnector = { + ...defaultVCSConnector, + getUserByLogin: () => Promise.resolve(null), + }; - test('when "defaultMetadata" is empty', async () => { - const expectedMetadata = ''; + const authorDetails = await updateAuthorMetadataStringByAuthorLogin( + author.login, + vcsConnector + ); - const authorDetails = await updateAuthorMetadataString(expectedMetadata, defaultVCSConnector); + expect(authorDetails).toEqual(expectedMetadata); + }); - expect(authorDetails).toEqual(expectedMetadata); - }); + test('returns full author metadata', async () => { + const expectedMetadata = units.replaceDoubleToSingleQuotes( + JSON.stringify(author) + ); - test('when "getAuthorDetails" does not return author data', async () => { - const expectedMetadata = `--- - author: alias - ---`; + const updatedMetadata = await updateAuthorMetadataStringByAuthorLogin( + author.login, + defaultVCSConnector + ); - defaultVCSConnector.getUserByLogin = () => Promise.resolve(null); + expect(updatedMetadata).toEqual(expectedMetadata); + }); +}); - const updatedMetadata = await updateAuthorMetadataString(expectedMetadata, defaultVCSConnector); +describe('update author metadata by filePath', () => { + beforeAll(() => { + authorByPath.set(filepath, author); + }); - expect(updatedMetadata).toEqual(expectedMetadata); - }); + afterAll(() => { + jest.clearAllMocks(); }); - describe('should return updated metadata', () => { - test('when "getAuthorDetails" returns author data', async () => { - const defaultMetadata = `--- - author: alias - ---`; - const authorDetails = units.replaceDoubleToSingleQuotes(JSON.stringify(author)); - defaultVCSConnector.getUserByLogin = () => Promise.resolve(author); + test('returns empty strring when "vcsConnector" is undefined', async () => { + const expectedMetadata = ''; + + const updatedMetadata = await updateAuthorMetadataStringByFilePath( + filepath + ); + + expect(updatedMetadata).toEqual(expectedMetadata); + }); + + test('returns empty strring when "getExternalAuthorByPath" returns null', async () => { + const expectedMetadata = ''; + const vcsConnector = { + ...defaultVCSConnector, + getExternalAuthorByPath: () => null, + }; + + const authorDetails = await updateAuthorMetadataStringByFilePath( + filepath, + vcsConnector + ); + + expect(authorDetails).toEqual(expectedMetadata); + }); + + test('returns empty strring when there is no author for path', async () => { + const expectedMetadata = ''; + const filepathWithoutAuthor = 'utils.md'; + + const authorDetails = await updateAuthorMetadataStringByFilePath( + filepathWithoutAuthor, + defaultVCSConnector + ); + + expect(authorDetails).toEqual(expectedMetadata); + }); - const updatedMetadata = await updateAuthorMetadataString(defaultMetadata, defaultVCSConnector); + test('returns full author metadata', async () => { + const expectedMetadata = units.replaceDoubleToSingleQuotes( + JSON.stringify(author) + ); - const matchAuthor = defaultMetadata.match(REGEXP_AUTHOR); - const expectedMetadata = defaultMetadata.replace(matchAuthor[0], authorDetails); + const updatedMetadata = await updateAuthorMetadataStringByFilePath( + filepath, + defaultVCSConnector + ); - expect(updatedMetadata).toEqual(expectedMetadata); - }); + expect(updatedMetadata).toEqual(expectedMetadata); }); });