Skip to content

Commit 1f67284

Browse files
authored
No longer storing dependency list in project properties (#1604)
The list of dependencies can grow quite large (>10KB in non trivial repos) and may surpass the limit allowed or slow down the AzDo UI which loads all properties. This PR removes that code and outputs an warning instead as we research if there is a valid alternative. Consequently, all code that access/modifies project properties is also removed as it is unused.
1 parent 1a4b7ce commit 1f67284

File tree

3 files changed

+6
-143
lines changed

3 files changed

+6
-143
lines changed

extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts

-53
Original file line numberDiff line numberDiff line change
@@ -567,59 +567,6 @@ export class AzureDevOpsWebApiClient {
567567
}
568568
}
569569

570-
/**
571-
* Get project properties
572-
* @param project
573-
* @param valueBuilder
574-
* @returns
575-
*/
576-
public async getProjectProperties(project: string): Promise<Record<string, string> | undefined> {
577-
try {
578-
const core = await this.connection.getCoreApi();
579-
const properties = await core.getProjectProperties(project);
580-
return properties?.map((p) => ({ [p.name]: p.value }))?.reduce((a, b) => ({ ...a, ...b }), {});
581-
} catch (e) {
582-
error(`Failed to get project properties: ${e}`);
583-
console.debug(e); // Dump the error stack trace to help with debugging
584-
return undefined;
585-
}
586-
}
587-
588-
/**
589-
* Update a project property
590-
* @param project
591-
* @param name
592-
* @param valueBuilder
593-
* @returns
594-
*/
595-
public async updateProjectProperty(
596-
project: string,
597-
name: string,
598-
valueBuilder: (existingValue: string | undefined) => string,
599-
): Promise<boolean> {
600-
try {
601-
// Get the existing project property value
602-
const core = await this.connection.getCoreApi();
603-
const properties = await core.getProjectProperties(project);
604-
const propertyValue = properties?.find((p) => p.name === name)?.value;
605-
606-
// Update the project property
607-
await core.setProjectProperties(undefined, project, [
608-
{
609-
op: 'add',
610-
path: '/' + name,
611-
value: valueBuilder(propertyValue),
612-
},
613-
]);
614-
615-
return true;
616-
} catch (e) {
617-
error(`Failed to update project property '${name}': ${e}`);
618-
console.debug(e); // Dump the error stack trace to help with debugging
619-
return false;
620-
}
621-
}
622-
623570
private async restApiGet(
624571
url: string,
625572
params?: Record<string, string>,

extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.test.ts

+1-54
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,7 @@ import { AzureDevOpsWebApiClient } from '../azure-devops/AzureDevOpsWebApiClient
44
import { IPullRequestProperties } from '../azure-devops/interfaces/IPullRequest';
55
import { IDependabotUpdate } from '../dependabot/interfaces/IDependabotConfig';
66
import { ISharedVariables } from '../getSharedVariables';
7-
import {
8-
DependabotDependenciesSchema,
9-
DependabotOutputProcessor,
10-
DependabotStoredDependencyListsSchema,
11-
} from './DependabotOutputProcessor';
7+
import { DependabotDependenciesSchema, DependabotOutputProcessor } from './DependabotOutputProcessor';
128
import { IDependabotUpdateOperation } from './interfaces/IDependabotUpdateOperation';
139

1410
jest.mock('../azure-devops/AzureDevOpsWebApiClient');
@@ -53,23 +49,8 @@ describe('DependabotOutputProcessor', () => {
5349
data = {};
5450
});
5551

56-
it('should skip processing "update_dependency_list" if "storeDependencyList" is false', async () => {
57-
taskInputs.storeDependencyList = false;
58-
prAuthorClient.updateProjectProperty = jest.fn().mockResolvedValue(false);
59-
60-
const result = await processor.process(update, 'update_dependency_list', {
61-
...data,
62-
dependencies: [],
63-
dependency_files: [],
64-
});
65-
66-
expect(result).toBe(true);
67-
expect(prApproverClient.updateProjectProperty).not.toHaveBeenCalled();
68-
});
69-
7052
it('should process "update_dependency_list"', async () => {
7153
taskInputs.storeDependencyList = true;
72-
prAuthorClient.updateProjectProperty = jest.fn().mockResolvedValue(true);
7354

7455
const result = await processor.process(update, 'update_dependency_list', {
7556
...data,
@@ -78,7 +59,6 @@ describe('DependabotOutputProcessor', () => {
7859
});
7960

8061
expect(result).toBe(true);
81-
expect(prAuthorClient.updateProjectProperty).toHaveBeenCalled();
8262
});
8363

8464
it('should skip processing "create_pull_request" if "skipPullRequests" is true', async () => {
@@ -292,38 +272,5 @@ describe('DependabotOutputProcessor', () => {
292272
expect(data['dependencies'][3].requirements[0].requirement).toEqual('8.1.0');
293273
expect(data['dependencies'][3].requirements[0].groups).toEqual(['dependencies']);
294274
});
295-
296-
it('works for an existing value', () => {
297-
const rawNuget = JSON.parse(fs.readFileSync('tests/update_dependency_list/nuget.json', 'utf-8'));
298-
const rawPip = JSON.parse(fs.readFileSync('tests/update_dependency_list/pip.json', 'utf-8'));
299-
const raw = {
300-
['repo1']: {
301-
['pip']: rawPip['data'],
302-
['nuget']: rawNuget['data'],
303-
},
304-
};
305-
const lists = DependabotStoredDependencyListsSchema.parse(raw);
306-
307-
expect(Object.keys(lists)).toEqual(['repo1']);
308-
expect(Object.keys(lists['repo1'])).toEqual(['pip', 'nuget']);
309-
310-
const data = lists['repo1']['nuget'];
311-
expect(data['dependency_files']).toEqual(['/Root.csproj']);
312-
expect(data['dependencies'].length).toEqual(76);
313-
314-
expect(data['dependencies'][0].name).toEqual('Azure.Core');
315-
expect(data['dependencies'][0].version).toEqual('1.35.0');
316-
expect(data['dependencies'][0].requirements.length).toEqual(0);
317-
318-
expect(data['dependencies'][3].name).toEqual('GraphQL.Server.Ui.Voyager');
319-
expect(data['dependencies'][3].version).toEqual('8.1.0');
320-
expect(data['dependencies'][3].requirements.length).toEqual(1);
321-
expect(data['dependencies'][3].requirements[0].file).toEqual('/Root.csproj');
322-
expect(data['dependencies'][3].requirements[0].requirement).toEqual('8.1.0');
323-
expect(data['dependencies'][3].requirements[0].groups).toEqual(['dependencies']);
324-
325-
// TODO: this is about 10KB for one repo, two ecosystems, we should find a better way?
326-
expect(JSON.stringify(raw).length).toEqual(10_000);
327-
});
328275
});
329276
});

extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts

+5-36
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,6 @@ export const DependabotDependenciesSchema = z.object({
3131
'last-updated': z.string().optional(), // e.g. '2021-09-01T00:00:00Z'
3232
});
3333
export type DependabotDependencies = z.infer<typeof DependabotDependenciesSchema>;
34-
export const DependabotStoredDependencyListsSchema = z.record(
35-
z.string({ description: 'Repository name' }),
36-
z.record(z.string({ description: 'Package manager' }), DependabotDependenciesSchema),
37-
);
38-
export type DependabotStoredDependencyLists = z.infer<typeof DependabotStoredDependencyListsSchema>;
3934

4035
/**
4136
* Processes dependabot update outputs using the DevOps API
@@ -49,10 +44,6 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess
4944
private readonly taskInputs: ISharedVariables;
5045
private readonly debug: boolean;
5146

52-
// Custom properties used to store dependabot metadata in projects.
53-
// https://learn.microsoft.com/en-us/rest/api/azure/devops/core/projects/set-project-properties
54-
public static PROJECT_PROPERTY_NAME_DEPENDENCY_LIST = 'Dependabot.DependencyList';
55-
5647
// Custom properties used to store dependabot metadata in pull requests.
5748
// https://learn.microsoft.com/en-us/rest/api/azure/devops/git/pull-request-properties
5849
public static PR_PROPERTY_NAME_PACKAGE_MANAGER = 'Dependabot.PackageManager';
@@ -99,24 +90,12 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess
9990
// See: https://github.com/dependabot/cli/blob/main/internal/model/update.go
10091

10192
case 'update_dependency_list':
102-
// Store the dependency list snapshot in project properties, if configured
103-
const parsedData = DependabotDependenciesSchema.parse(data);
93+
// Store the dependency list snapshot, if configured
94+
const _ = DependabotDependenciesSchema.parse(data);
10495
if (this.taskInputs.storeDependencyList) {
105-
return await this.prAuthorClient.updateProjectProperty(
106-
this.taskInputs.project,
107-
DependabotOutputProcessor.PROJECT_PROPERTY_NAME_DEPENDENCY_LIST,
108-
function (existingValue: string) {
109-
const repoDependencyLists = DependabotStoredDependencyListsSchema.parse(
110-
JSON.parse(existingValue || '{}'),
111-
);
112-
repoDependencyLists[repository] = repoDependencyLists[repository] || {};
113-
repoDependencyLists[repository][packageManager] = {
114-
...parsedData,
115-
'last-updated': new Date().toISOString(),
116-
};
117-
118-
return JSON.stringify(repoDependencyLists);
119-
},
96+
warning(
97+
`Storing dependency list snapshot in project properties is no longer supported
98+
due to size limitations. We are looking for alternative solutions.`,
12099
);
121100
}
122101

@@ -358,16 +337,6 @@ export function buildPullRequestProperties(packageManager: string, dependencies:
358337
];
359338
}
360339

361-
export function parseProjectDependencyListProperty(
362-
properties: Record<string, string>,
363-
repository: string,
364-
packageManager: string,
365-
): any {
366-
const dependencyList = properties?.[DependabotOutputProcessor.PROJECT_PROPERTY_NAME_DEPENDENCY_LIST] || '{}';
367-
const repoDependencyLists = JSON.parse(dependencyList);
368-
return repoDependencyLists[repository]?.[packageManager];
369-
}
370-
371340
export function parsePullRequestProperties(
372341
pullRequests: IPullRequestProperties[],
373342
packageManager: string | null,

0 commit comments

Comments
 (0)