Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Model for providing branch data provider item IDs #401

Merged
merged 3 commits into from
Oct 11, 2022
Merged

Model for providing branch data provider item IDs #401

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
809cfd5
Propagate IDs from BDP items to RG items.
philliphoff Oct 10, 2022
1989a15
Use resource ID as default for omitted resource models.
philliphoff Oct 11, 2022
ec6f5e8
Updated per PR feedback, add path-->item function.
philliphoff Oct 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/api/v2/v2AzureResourcesApi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ export interface ResourceQuickPickOptions {
}

export interface ResourceModelBase {
// TODO: Should IDs be optional?
Copy link
Contributor

@bwateratmsft bwateratmsft Oct 11, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd love if we could keep this optional. Label is a required-ish result from getTreeItem, perhaps we can use that if nothing else is specified? VSCode uses the label as (at least part of) the ID if it isn't specified, though I've not seen the actual code there.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bonus to keeping this and the other values optional is that the model objects don't necessarily have to be objects. They could be e.g. string. I've really hoped we could keep that true.

(As a related aside, I think when applicable, e.g. when we're checking if the model object has an azureResourceId or azurePortalUrl or whatever, we should first check if the model object is actually an object--make no assumptions about what those models are)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't think of a reason why ID must be specified; i.e. why we can't just choose an arbitrary unique value if omitted. It's obviously better if BDPs use proper IDs, but there are definately cases where it's just unlikely to matter (e.g. generic nodes, error nodes, etc.).

We can't use the tree item ID--at least, not generally--as we don't necessarily have the tree item, as we're working solely with model objects. It would probably make sense, if BDPs give their tree items IDs, that they use the same ID as the model itself. If we let VS Code be the sole interpretor of tree item IDs, then we could leave that as a "best practice" but not necessarily care.

readonly id?: string;
readonly quickPickOptions?: ResourceQuickPickOptions;
readonly azureResourceId?: string;
}
Expand Down
8 changes: 4 additions & 4 deletions src/tree/v2/BranchDataProviderItem.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { randomUUID } from 'crypto';
import * as vscode from 'vscode';
import { BranchDataProvider, ResourceBase, ResourceModelBase, WrappedResourceModel } from '../../api/v2/v2AzureResourcesApi';
import { ResourceGroupsItem } from './ResourceGroupsItem';
import { ResourceGroupsItemCache } from './ResourceGroupsItemCache';

export type BranchDataItemOptions = {
defaultId?: string;
defaults?: vscode.TreeItem;
};

Expand All @@ -21,6 +23,8 @@ export class BranchDataProviderItem implements ResourceGroupsItem, WrappedResour
itemCache.addBranchItem(this.branchItem, this);
}

readonly id: string = this.branchItem.id ?? this?.options?.defaultId ?? randomUUID();

async getChildren(): Promise<ResourceGroupsItem[] | undefined> {
const children = await this.branchDataProvider.getChildren(this.branchItem);

Expand All @@ -41,10 +45,6 @@ export class BranchDataProviderItem implements ResourceGroupsItem, WrappedResour
unwrap<T extends ResourceModelBase>(): T | undefined {
return this.branchItem as T;
}

id: string;
name: string;
type: string;
}

export type BranchDataItemFactory = (branchItem: ResourceModelBase, branchDataProvider: BranchDataProvider<ResourceBase, ResourceModelBase>, options?: BranchDataItemOptions) => BranchDataProviderItem;
Expand Down
2 changes: 2 additions & 0 deletions src/tree/v2/GenericItem.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ export class GenericItem implements ResourceGroupsItem {
constructor(public readonly label: string, private readonly options?: GenericItemOptions) {
}

readonly id: string = this.label;

getChildren(): vscode.ProviderResult<ResourceGroupsItem[]> {
return this.options?.children;
}
Expand Down
5 changes: 3 additions & 2 deletions src/tree/v2/ResourceGroupsItem.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
*--------------------------------------------------------------------------------------------*/

import * as vscode from 'vscode';
import { ResourceModelBase } from '../../api/v2/v2AzureResourcesApi';

export interface ResourceGroupsItem extends ResourceModelBase{
export interface ResourceGroupsItem {
readonly id: string;

getChildren(): vscode.ProviderResult<ResourceGroupsItem[]>;
getTreeItem(): vscode.TreeItem | Thenable<vscode.TreeItem>;
}
35 changes: 34 additions & 1 deletion src/tree/v2/ResourceGroupsItemCache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,23 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { localize } from '../../utils/localize';
import { ResourceGroupsItem } from './ResourceGroupsItem';

export class ResourceGroupsItemCache {
private readonly branchItemToItemCache: Map<unknown, ResourceGroupsItem> = new Map();
private readonly itemToBranchItemCache: Map<ResourceGroupsItem, unknown> = new Map();
private readonly itemToChildrenCache: Map<ResourceGroupsItem, ResourceGroupsItem[]> = new Map();
private readonly itemToParentCache: Map<ResourceGroupsItem, ResourceGroupsItem> = new Map();
private readonly rootItemCache: ResourceGroupsItem[] = [];

addBranchItem(branchItem: unknown, item: ResourceGroupsItem): void {
this.branchItemToItemCache.set(branchItem, item);
this.itemToBranchItemCache.set(item, branchItem);
}

addItem(item: ResourceGroupsItem, children: ResourceGroupsItem[]): void {
addRootItem(item: ResourceGroupsItem, children: ResourceGroupsItem[]): void {
this.rootItemCache.push(item);
this.itemToChildrenCache.set(item, children);
children.forEach(child => this.itemToParentCache.set(child, item));
}
Expand All @@ -26,6 +29,7 @@ export class ResourceGroupsItemCache {
this.itemToBranchItemCache.clear();
this.itemToChildrenCache.clear();
this.itemToParentCache.clear();
this.rootItemCache.length = 0;
}

evictItemChildren(item: ResourceGroupsItem): void {
Expand Down Expand Up @@ -76,6 +80,35 @@ export class ResourceGroupsItemCache {
return this.itemToParentCache.get(item);
}

getPathForItem(item: ResourceGroupsItem): string[] {
const path: string[] = [];

let currentItem: ResourceGroupsItem | undefined = item;

while (currentItem) {
path.push(currentItem.id);
currentItem = this.getParentForItem(currentItem);
}

return path.reverse();
}

getItemForPath(path: string[]): ResourceGroupsItem | undefined {
if (path.length === 0) {
throw new Error(localize('tree.ResourceGroupsItemCache.getItemForPath.invalidPathLength', 'Path must have at least one element.'));
}

let currentItem = this.rootItemCache.find(item => item.id === path[0]);

for (let i = 1; currentItem && i < path.length; i++) {
const children = this.itemToChildrenCache.get(currentItem);

currentItem = children?.find(item => item.id === path[i]);
}

return currentItem;
}

updateItemChildren(item: ResourceGroupsItem, children: ResourceGroupsItem[]): void {
this.itemToChildrenCache.set(item, children);
children.forEach(child => this.itemToParentCache.set(child, item));
Expand Down
2 changes: 1 addition & 1 deletion src/tree/v2/ResourceTreeDataProviderBase.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export abstract class ResourceTreeDataProviderBase extends vscode.Disposable imp
if (element) {
this.itemCache.updateItemChildren(element, children);
} else {
children.forEach(child => this.itemCache.addItem(child, []));
children.forEach(child => this.itemCache.addRootItem(child, []));
}
}

Expand Down
9 changes: 4 additions & 5 deletions src/tree/v2/application/DefaultApplicationResourceBranchDataProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,21 @@

import * as vscode from 'vscode';
import { ApplicationResource, BranchDataProvider } from '../../../api/v2/v2AzureResourcesApi';
import { ResourceGroupsItem } from '../ResourceGroupsItem';
import { DefaultApplicationResourceItem } from './DefaultApplicationResourceItem';

export class DefaultApplicationResourceBranchDataProvider implements BranchDataProvider<ApplicationResource, ResourceGroupsItem> {
getChildren(element: ResourceGroupsItem): vscode.ProviderResult<ResourceGroupsItem[]> {
export class DefaultApplicationResourceBranchDataProvider implements BranchDataProvider<ApplicationResource, DefaultApplicationResourceItem> {
getChildren(element: DefaultApplicationResourceItem): vscode.ProviderResult<DefaultApplicationResourceItem[]> {
return element.getChildren();
}

getResourceItem(element: ApplicationResource): ResourceGroupsItem | Thenable<ResourceGroupsItem> {
getResourceItem(element: ApplicationResource): DefaultApplicationResourceItem | Thenable<DefaultApplicationResourceItem> {
return new DefaultApplicationResourceItem(element);
}

// TODO: Implement change eventing.
// onDidChangeTreeData?: vscode.Event<void | ResourceGroupsItem | null | undefined> | undefined;

getTreeItem(element: ResourceGroupsItem): vscode.TreeItem | Thenable<vscode.TreeItem> {
getTreeItem(element: DefaultApplicationResourceItem): vscode.TreeItem | Thenable<vscode.TreeItem> {
return element.getTreeItem();
}
}
13 changes: 5 additions & 8 deletions src/tree/v2/application/DefaultApplicationResourceItem.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,16 @@
*--------------------------------------------------------------------------------------------*/

import * as vscode from 'vscode';
import { ApplicationResource } from '../../../api/v2/v2AzureResourcesApi';
import { ApplicationResource, ResourceModelBase } from '../../../api/v2/v2AzureResourcesApi';
import { getIconPath } from '../../../utils/azureUtils';
import { ResourceGroupsItem } from '../ResourceGroupsItem';

export class DefaultApplicationResourceItem implements ResourceGroupsItem {
export class DefaultApplicationResourceItem implements ResourceModelBase {
constructor(private readonly resource: ApplicationResource) {
}

getChildren(): vscode.ProviderResult<ResourceGroupsItem[]> {
public readonly id: string = this.resource.id;

getChildren(): vscode.ProviderResult<DefaultApplicationResourceItem[]> {
return undefined;
}

Expand All @@ -23,8 +24,4 @@ export class DefaultApplicationResourceItem implements ResourceGroupsItem {

return treeItem;
}

id: string;
name: string;
type: string;
}
3 changes: 3 additions & 0 deletions src/tree/v2/application/GroupingItem.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ export class GroupingItem implements ResourceGroupsItem {
public readonly resources: ApplicationResource[]) {
}

readonly id: string = this.label;

async getChildren(): Promise<ResourceGroupsItem[] | undefined> {
const sortedResources = this.resources.sort((a, b) => a.name.localeCompare(b.name));
const resourceItems = await Promise.all(sortedResources.map(
Expand All @@ -33,6 +35,7 @@ export class GroupingItem implements ResourceGroupsItem {
const resourceItem = await branchDataProvider.getResourceItem(resource);

const options = {
defaultId: resource.id,
defaults: {
iconPath: getIconPath(resource.resourceType)
}
Expand Down
6 changes: 2 additions & 4 deletions src/tree/v2/application/SubscriptionItem.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ export class SubscriptionItem implements ResourceGroupsItem {
private readonly subscription: ApplicationSubscription) {
}

public readonly id: string = this.subscription.subscriptionId;

async getChildren(): Promise<ResourceGroupsItem[]> {
let resources = await this.resourceProviderManager.getResources(this.subscription);

Expand All @@ -49,8 +51,4 @@ export class SubscriptionItem implements ResourceGroupsItem {

return treeItem;
}

id: string;
name: string;
type: string;
}