From 65b8256bf88263af000020ac5f5c1cc344542c50 Mon Sep 17 00:00:00 2001 From: Andrei Ivanovici Date: Sat, 1 Jun 2024 00:52:24 +0300 Subject: [PATCH] chore: code cleanup Signed-off-by: Andrei Ivanovici --- .../api/{util.test.ts => AzureDevops.test.ts} | 3 +- plugins/catalog-import/src/api/AzureDevops.ts | 27 +++- .../src/api/AzureRepoApiClient.test.ts | 131 +++++++++++------- .../src/api/AzureRepoApiClient.ts | 74 ++++++---- .../src/api/CatalogImportClient.test.ts | 3 +- .../src/api/CatalogImportClient.ts | 10 +- plugins/catalog-import/src/api/util.ts | 40 ------ 7 files changed, 169 insertions(+), 119 deletions(-) rename plugins/catalog-import/src/api/{util.test.ts => AzureDevops.test.ts} (97%) delete mode 100644 plugins/catalog-import/src/api/util.ts diff --git a/plugins/catalog-import/src/api/util.test.ts b/plugins/catalog-import/src/api/AzureDevops.test.ts similarity index 97% rename from plugins/catalog-import/src/api/util.test.ts rename to plugins/catalog-import/src/api/AzureDevops.test.ts index 3a14a2e7ae..4a3dfca42d 100644 --- a/plugins/catalog-import/src/api/util.test.ts +++ b/plugins/catalog-import/src/api/AzureDevops.test.ts @@ -13,7 +13,8 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { parseRepoUrl } from './util'; + +import { parseRepoUrl } from './AzureDevops'; describe('parseRepoUrl', () => { it('parses Azure DevOps Cloud url', async () => { diff --git a/plugins/catalog-import/src/api/AzureDevops.ts b/plugins/catalog-import/src/api/AzureDevops.ts index d028c75129..f7309d01f8 100644 --- a/plugins/catalog-import/src/api/AzureDevops.ts +++ b/plugins/catalog-import/src/api/AzureDevops.ts @@ -17,7 +17,6 @@ import { ScmAuthApi } from '@backstage/integration-react'; import { ConfigApi } from '@backstage/core-plugin-api'; import { getBranchName, getCatalogFilename } from '../components/helpers'; import { createAzurePullRequest } from './AzureRepoApiClient'; -import { parseRepoUrl } from './util'; export interface AzureRepoParts { tenantUrl: string; @@ -77,3 +76,29 @@ export async function submitAzurePrToRepo( location: catalogLocation, }; } + +export function parseRepoUrl(sourceUrl: string) { + const url = new URL(sourceUrl); + + let host = url.host; + let org; + let project; + let repo; + + const parts = url.pathname.split('/').map(part => decodeURIComponent(part)); + if (parts[2] === '_git') { + org = parts[1]; + project = repo = parts[3]; + } else if (parts[3] === '_git') { + org = parts[1]; + project = parts[2]; + repo = parts[4]; + } else if (parts[4] === '_git') { + host = `${host}/${parts[1]}`; + org = parts[2]; + project = parts[3]; + repo = parts[5]; + } + + return { host, org, project, repo }; +} diff --git a/plugins/catalog-import/src/api/AzureRepoApiClient.test.ts b/plugins/catalog-import/src/api/AzureRepoApiClient.test.ts index 5024b8cda4..a8fbe9db76 100644 --- a/plugins/catalog-import/src/api/AzureRepoApiClient.test.ts +++ b/plugins/catalog-import/src/api/AzureRepoApiClient.test.ts @@ -154,6 +154,7 @@ function mockPrEndpoint() { describe('RepoApiClient', () => { const server = setupServer(); setupRequestMockHandlers(server); + const testToken = new Date().toString(); const sut = new RepoApiClient({ project: 'project', tenantUrl: 'https://dev.azure.com/acme', @@ -168,25 +169,28 @@ describe('RepoApiClient', () => { }); describe('getRepository', () => { it('should get an existing repository', async () => { - await expect(sut.getRepository('success')).resolves.toEqual({ + await expect(sut.getRepository('success', testToken)).resolves.toEqual({ id: '01', name: 'success', defaultBranch: 'refs/heads/master', }); }); it('should throw when the repository repository does not exist', async () => { - await expect(sut.getRepository('not-found')).rejects.toThrow( + await expect(sut.getRepository('not-found', testToken)).rejects.toThrow( new Error('repository not found'), ); }); }); describe('getDefaultBranch', () => { it('should return when correct branch', async () => { - const foundRef = await sut.getDefaultBranch({ - name: 'success', - defaultBranch: 'refs/heads/main', - id: '01', - }); + const foundRef = await sut.getDefaultBranch( + { + name: 'success', + defaultBranch: 'refs/heads/main', + id: '01', + }, + testToken, + ); expect(foundRef).toEqual({ name: 'refs/heads/main', objectId: '0000000000000000000000000000000000000000', @@ -194,37 +198,43 @@ describe('RepoApiClient', () => { }); it('should throw when the repository does not exist', async () => { - const promise = sut.getDefaultBranch({ - name: 'fail', - defaultBranch: 'refs/heads/main', - id: '01', - }); + const promise = sut.getDefaultBranch( + { + name: 'fail', + defaultBranch: 'refs/heads/main', + id: '01', + }, + testToken, + ); await expect(promise).rejects.toThrow(new Error('repository not found')); }); it('should throw when the default branch does not exist', async () => { - const promise = sut.getDefaultBranch({ - name: 'success', - defaultBranch: 'refs/heads/missing_branch', - id: '01', - }); + const promise = sut.getDefaultBranch( + { + name: 'success', + defaultBranch: 'refs/heads/missing_branch', + id: '01', + }, + testToken, + ); await expect(promise).rejects.toThrow( new Error(`The requested ref 'heads/missing_branch' was not found`), ); }); }); describe('pushNewBranch', () => { - let sourceBranch: AzureRef; let options: NewBranchOptions; let expectedResult: AzureRefUpdate; beforeEach(() => { - sourceBranch = { - name: 'refs/heads/main', - objectId: '0000000000000000000000000000000000000000', - }; options = { + repoName: 'name', title: 'title', + sourceBranch: { + name: 'refs/heads/main', + objectId: '0000000000000000000000000000000000000000', + }, branchName: 'backstage-integration', fileName: 'catalog-info.yaml', fileContent: 'This is a test', @@ -238,29 +248,43 @@ describe('RepoApiClient', () => { }); it('should create a new branch', async () => { await expect( - sut.pushNewBranch('success', sourceBranch, options), + sut.pushNewBranch( + { + ...options, + repoName: 'success', + }, + testToken, + ), ).resolves.toEqual(expectedResult); }); it('should throw when api call fails', async () => { await expect( - sut.pushNewBranch('error', sourceBranch, options), + sut.pushNewBranch( + { + ...options, + repoName: 'error', + }, + testToken, + ), ).rejects.toThrow(new Error('internal error')); }); }); describe('createPullRequest', () => { - const sourceBranchName = 'refs/heads/main'; - const targetBranchName = 'refs/heads/backstage-integration'; const options: CreatePrOptions = { + repoName: 'repoName', + sourceName: 'refs/heads/main', + targetName: 'refs/heads/backstage-integration', title: 'Title', description: 'Description', }; it('should create a new Pull request', async () => { await expect( sut.createPullRequest( - 'success', - sourceBranchName, - targetBranchName, - options, + { + ...options, + repoName: 'success', + }, + testToken, ), ).resolves.toEqual({ pullRequestId: 'PR01', @@ -272,12 +296,7 @@ describe('RepoApiClient', () => { }); it('should throw when api call fails', async () => { await expect( - sut.createPullRequest( - 'error', - sourceBranchName, - targetBranchName, - options, - ), + sut.createPullRequest({ ...options, repoName: 'error' }, testToken), ).rejects.toThrow(new Error('internal error')); }); }); @@ -297,6 +316,7 @@ describe('createAzurePullRequest', () => { }); it('should create a new Pull request', async () => { + const testToken = new Date().getTime().toString(); const options: AzurePrOptions = { tenantUrl: 'https://dev.azure.com/acme', repository: 'test', @@ -306,7 +326,8 @@ describe('createAzurePullRequest', () => { fileContent: 'content', branchName: 'backstage-integration', description: 'Test Description', - } as any; + token: testToken, + }; const repo: AzureRepo = { name: options.repository, defaultBranch: 'ref/heads/main', @@ -337,18 +358,34 @@ describe('createAzurePullRequest', () => { await expect(createAzurePullRequest(options, client)).resolves.toEqual( expectedResult, ); - expect(clientMock.getRepository).toHaveBeenCalledWith(options.repository); - expect(clientMock.getDefaultBranch).toHaveBeenCalledWith(repo); - expect(clientMock.pushNewBranch).toHaveBeenCalledWith( - repo.name, - defaultBranch, - options, + expect(clientMock.getRepository).toHaveBeenCalledWith( + options.repository, + testToken, ); + expect(clientMock.getDefaultBranch).toHaveBeenCalledWith(repo, testToken); + const expectedBranchOptions: NewBranchOptions = { + repoName: repo.name, + sourceBranch: defaultBranch, + branchName: options.branchName, + fileContent: options.fileContent, + fileName: options.fileName, + title: options.title, + }; + expect(clientMock.pushNewBranch).toHaveBeenCalledWith( + expectedBranchOptions, + testToken, + ); + + const expectedPrOptions: CreatePrOptions = { + repoName: repo.name, + description: options.description, + title: options.title, + sourceName: options.branchName, + targetName: defaultBranch.name, + }; expect(clientMock.createPullRequest).toHaveBeenCalledWith( - repo.name, - options.branchName, - defaultBranch.name, - options, + expectedPrOptions, + testToken, ); }); }); diff --git a/plugins/catalog-import/src/api/AzureRepoApiClient.ts b/plugins/catalog-import/src/api/AzureRepoApiClient.ts index a743159ea8..56be626f5e 100644 --- a/plugins/catalog-import/src/api/AzureRepoApiClient.ts +++ b/plugins/catalog-import/src/api/AzureRepoApiClient.ts @@ -95,7 +95,6 @@ const apiVersions = '6.0'; export interface RepoApiClientOptions { project: string; tenantUrl: string; - token: string; } export interface NewBranchOptions { @@ -103,9 +102,14 @@ export interface NewBranchOptions { fileName: string; title: string; branchName: string; + repoName: string; + sourceBranch: AzureRef; } export interface CreatePrOptions { + repoName: string; + sourceName: string; + targetName: string; description: string; title: string; } @@ -134,11 +138,12 @@ export class RepoApiClient { path: string, version: string, queryParams: Record | undefined = undefined, + token: string, ): Promise { const endpoint = this.createEndpoint(path, version, queryParams); const result = await fetch(endpoint, { headers: { - Authorization: `Bearer ${this._options.token}`, + Authorization: `Bearer ${token}`, }, }); if (!result.ok) { @@ -151,12 +156,13 @@ export class RepoApiClient { path: string, version: string, payload: unknown, + token: string, ): Promise { const endpoint = this.createEndpoint(path, version); const result = await fetch(endpoint, { method: 'POST', headers: { - Authorization: `Bearer ${this._options.token}`, + Authorization: `Bearer ${token}`, 'Content-Type': 'application/json', }, body: JSON.stringify(payload), @@ -167,16 +173,20 @@ export class RepoApiClient { return await result.json(); } - async getRepository(repositoryName: string): Promise { - return this.get(`/${repositoryName}`, apiVersions); + async getRepository( + repositoryName: string, + token: string, + ): Promise { + return this.get(`/${repositoryName}`, apiVersions, undefined, token); } - async getDefaultBranch(repo: AzureRepo): Promise { + async getDefaultBranch(repo: AzureRepo, token: string): Promise { const filter = repo.defaultBranch.replace('refs/', ''); const result: RefQueryResult = await this.get( `/${repo.name}/refs`, apiVersions, { filter }, + token, ); if (!result.value?.length) { return Promise.reject( @@ -187,10 +197,10 @@ export class RepoApiClient { } async pushNewBranch( - repoName: string, - sourceBranch: AzureRef, options: NewBranchOptions, + token: string, ): Promise { + const { sourceBranch, repoName } = options; const push: AzurePush = { refUpdates: [ { @@ -220,16 +230,16 @@ export class RepoApiClient { `/${repoName}/pushes`, apiVersions, push, + token, ); return result.refUpdates[0]; } async createPullRequest( - repoName: string, - sourceName: string, - targetName: string, options: CreatePrOptions, + token: string, ): Promise { + const { repoName, sourceName, targetName } = options; const payload: CreateAzurePr = { title: options.title, description: options.description, @@ -241,6 +251,7 @@ export class RepoApiClient { `/${repoName}/pullrequests`, apiVersions, payload, + token, ); } } @@ -249,18 +260,33 @@ export async function createAzurePullRequest( options: AzurePrOptions, client: RepoApiClient | undefined = undefined, ): Promise { + const { + title, + repository, + token, + fileContent, + fileName, + branchName, + description, + } = options; const actualClient = client ?? new RepoApiClient(options); - const repo = await actualClient.getRepository(options.repository); - const defaultBranch = await actualClient.getDefaultBranch(repo); - const refUpdate = await actualClient.pushNewBranch( - repo.name, - defaultBranch, - options, - ); - return actualClient.createPullRequest( - repo.name, - refUpdate.name, - defaultBranch.name, - options, - ); + const repo = await actualClient.getRepository(repository, token); + const defaultBranch = await actualClient.getDefaultBranch(repo, token); + const branchOptions: NewBranchOptions = { + title: title, + repoName: repo.name, + sourceBranch: defaultBranch, + branchName: branchName, + fileContent: fileContent, + fileName: fileName, + }; + const refUpdate = await actualClient.pushNewBranch(branchOptions, token); + const prOptions: CreatePrOptions = { + title, + description, + repoName: repo.name, + sourceName: refUpdate.name, + targetName: defaultBranch.name, + }; + return actualClient.createPullRequest(prOptions, token); } diff --git a/plugins/catalog-import/src/api/CatalogImportClient.test.ts b/plugins/catalog-import/src/api/CatalogImportClient.test.ts index d4e4eeed54..315b59ea67 100644 --- a/plugins/catalog-import/src/api/CatalogImportClient.test.ts +++ b/plugins/catalog-import/src/api/CatalogImportClient.test.ts @@ -43,6 +43,7 @@ jest.mock('@octokit/rest', () => { return octokit; } } + return { Octokit }; }); @@ -299,7 +300,7 @@ describe('CatalogImportClient', () => { ), ).rejects.toThrow( new Error( - 'This URL was not recognized as a valid GitHub URL because there was no configured integration that matched the given host name. You could try to paste the full URL to a catalog-info.yaml file instead.', + 'This URL was not recognized as a valid git URL because there was no configured integration that matched the given host name. Currently GitHub and Azure DevOps are supported. You could try to paste the full URL to a catalog-info.yaml file instead.', ), ); }); diff --git a/plugins/catalog-import/src/api/CatalogImportClient.ts b/plugins/catalog-import/src/api/CatalogImportClient.ts index fe7fc5c59e..9745bd4a85 100644 --- a/plugins/catalog-import/src/api/CatalogImportClient.ts +++ b/plugins/catalog-import/src/api/CatalogImportClient.ts @@ -91,9 +91,9 @@ export class CatalogImportClient implements CatalogImportApi { } const supportedIntegrations = ['github', 'azure']; const foundIntegration = this.scmIntegrationsApi.byUrl(url); - const iSupported = supportedIntegrations.find( - it => it === foundIntegration?.type, - ); + const iSupported = + !!foundIntegration && + supportedIntegrations.find(it => it === foundIntegration.type); if (!iSupported) { const catalogFilename = getCatalogFilename(this.configApi); @@ -103,7 +103,7 @@ export class CatalogImportClient implements CatalogImportApi { ); } throw new Error( - `This URL was not recognized as a valid GitHub URL because there was no configured integration that matched the given host name. You could try to paste the full URL to a ${catalogFilename} file instead.`, + `This URL was not recognized as a valid git URL because there was no configured integration that matched the given host name. Currently GitHub and Azure DevOps are supported. You could try to paste the full URL to a ${catalogFilename} file instead.`, ); } @@ -146,7 +146,7 @@ export class CatalogImportClient implements CatalogImportApi { return { type: 'repository', - integrationType: foundIntegration!.type, + integrationType: foundIntegration.type, url: url, generatedEntities: analyzation.generateEntities.map(x => x.entity), }; diff --git a/plugins/catalog-import/src/api/util.ts b/plugins/catalog-import/src/api/util.ts deleted file mode 100644 index dc6b4ae2a6..0000000000 --- a/plugins/catalog-import/src/api/util.ts +++ /dev/null @@ -1,40 +0,0 @@ -/* - * Copyright 2024 The Backstage Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -export function parseRepoUrl(sourceUrl: string) { - const url = new URL(sourceUrl); - - let host = url.host; - let org; - let project; - let repo; - - const parts = url.pathname.split('/').map(part => decodeURIComponent(part)); - if (parts[2] === '_git') { - org = parts[1]; - project = repo = parts[3]; - } else if (parts[3] === '_git') { - org = parts[1]; - project = parts[2]; - repo = parts[4]; - } else if (parts[4] === '_git') { - host = `${host}/${parts[1]}`; - org = parts[2]; - project = parts[3]; - repo = parts[5]; - } - - return { host, org, project, repo }; -}