Skip to content

feat(ContentNavigator): added more detailed path information #1597

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

feat(ContentNavigator): added more detailed path information #1597

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c0622a9
feat(ContentNavigator): added more detailed path information
Aug 14, 2025
64965fe
feat(ContentNavigator): added optimization with parent caching, also …
Aug 14, 2025
0eb18fc
DCO Remediation Commit for Kishan Patel <[email protected]>
Aug 14, 2025
05fb7f2
feat(ContentNavigator): changed uri to have a full path to allow for …
Aug 15, 2025
fced7a6
DCO Remediation Commit for Kishan Patel <[email protected]>
Aug 15, 2025
1e5a9b6
wip(ContentNavigator): Partial working modification
Aug 15, 2025
6e8b68c
wip(ContentNavigator): Partial working modification
Aug 15, 2025
7a2bd53
wip(ContentNavigator) refactored enrichment logic
kpatl1 Aug 22, 2025
1545dc9
feat(ContentNavigator): improved caching
kpatl1 Aug 27, 2025
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
11 changes: 10 additions & 1 deletion client/src/components/ContentNavigator/ContentDataProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,15 @@ class ContentDataProvider
public async getTreeItem(item: ContentItem): Promise<TreeItem> {
const isContainer = getIsContainer(item);
const uri = await this.model.getUri(item, false);

let tooltip = item.name;
const canCopyPath = item.contextValue?.includes("copyPath");

if (canCopyPath && uri.path && uri.path !== `/${item.name}`) {
const fullPath = uri.path.startsWith("/")
? uri.path.substring(1)
: uri.path;
tooltip = fullPath;
}
return {
collapsibleState: isContainer
? TreeItemCollapsibleState.Collapsed
Expand All @@ -230,6 +238,7 @@ class ContentDataProvider
id: item.uid,
label: item.name,
resourceUri: uri,
tooltip: tooltip,
};
}

Expand Down
147 changes: 126 additions & 21 deletions client/src/connection/rest/RestContentAdapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ class RestContentAdapter implements ContentAdapter {
[id: string]: { etag: string; lastModified: string; contentType: string };
};
private contextMenuProvider: ContextMenuProvider;
private pathCache: Map<string, string> = new Map();
private pathPromiseCache: Map<string, Promise<string>> = new Map();

public constructor() {
this.rootFolders = {};
Expand Down Expand Up @@ -118,7 +120,13 @@ class RestContentAdapter implements ContentAdapter {
}
const { data } = await this.connection.get(ancestorsLink.uri);
if (data && data.length > 0) {
return this.enrichWithDataProviderProperties(data[0]);
const enrichedItem = this.enrichWithDataProviderProperties(data[0]);

if (enrichedItem.contextValue?.includes("copyPath")) {
await this.updateUriWithFullPath(enrichedItem);
}

return enrichedItem;
}
}

Expand All @@ -142,19 +150,27 @@ class RestContentAdapter implements ContentAdapter {
? []
: await this.getChildItems(myFavoritesFolder);

const items = result.items.map(
(childItem: ContentItem, index): ContentItem => {
const favoriteUri = fetchFavoriteUri(childItem);
return {
...childItem,
uid: `${parentItem.uid}/${index}`,
...this.enrichWithDataProviderProperties(childItem, {
isInRecycleBin,
isInMyFavorites: parentIdIsFavoritesFolder || !!favoriteUri,
favoriteUri,
}),
};
},
const items = await Promise.all(
result.items.map(
async (childItem: ContentItem, index): Promise<ContentItem> => {
const favoriteUri = fetchFavoriteUri(childItem);
const enrichedItem = {
...childItem,
uid: `${parentItem.uid}/${index}`,
...this.enrichWithDataProviderProperties(childItem, {
isInRecycleBin,
isInMyFavorites: parentIdIsFavoritesFolder || !!favoriteUri,
favoriteUri,
}),
};

if (enrichedItem.contextValue?.includes("copyPath")) {
await this.updateUriWithFullPath(enrichedItem);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the only time we don't call updateUriWithFullPath is when...

  • getting root items
  • moving items

I'd advocate for moving this call inside enrichWithDataProviderProperties (I know that make things a tiny bit more complicated since this function doesn't return a promise)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree. I will go ahead and refactor to allow for this.


return enrichedItem;
},
),
);

return items;
Expand Down Expand Up @@ -186,21 +202,59 @@ class RestContentAdapter implements ContentAdapter {
if (!item) {
return "";
}
const baseKey = item.uri || item.id || item.name;
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like a lot. I'd probably update things such that we're setting the values here, and updating in calling functions where we know the full path might change (rename, move, delete, restore). So the general outline might be something like...

// In a function that changes the value
this.pathCache.set(item.id, await this.calculatePathOfItem(item));

then the implementation would use the id for the cache key since that's mostly unchanging...

if (this.pathCache.has(item.id)) {
      return this.pathCache.get(item.id)!;
    }

    try {
      const path = await this.calculatePathOfItem(item, folderPathOnly);
      this.pathCache.set(item.id, path);
      return path;
    } catch {
      return "";
    }

Would something like this work?

const cacheKey = `${baseKey}_${folderPathOnly || false}`;

if (this.pathCache.has(cacheKey)) {
return this.pathCache.get(cacheKey)!;
}

if (this.pathPromiseCache.has(cacheKey)) {
return this.pathPromiseCache.get(cacheKey)!;
}

const pathPromise = this.calculatePathOfItem(item, folderPathOnly);
this.pathPromiseCache.set(cacheKey, pathPromise);

try {
const path = await pathPromise;
this.pathCache.set(cacheKey, path);
this.pathPromiseCache.delete(cacheKey);

if (!folderPathOnly && path.includes("/")) {
const parentPath = path.substring(0, path.lastIndexOf("/")) || "/";
const parentKey = `${item.parentFolderUri}_true`;
if (!this.pathCache.has(parentKey)) {
this.pathCache.set(parentKey, parentPath);
}
}

return path;
} catch {
this.pathPromiseCache.delete(cacheKey);
this.pathCache.set(cacheKey, "");
return "";
}
}

private async calculatePathOfItem(
item: ContentItem,
folderPathOnly?: boolean,
): Promise<string> {
const filePathParts = [];
let currentContentItem: Pick<ContentItem, "parentFolderUri" | "name"> =
item;
if (!folderPathOnly) {
filePathParts.push(currentContentItem.name);
}

do {
try {
const { data: parentData } = await this.connection.get(
currentContentItem.parentFolderUri,
);
currentContentItem = parentData;
// eslint-disable-next-line @typescript-eslint/no-unused-vars
} catch (e) {
} catch {
return "";
}
if (currentContentItem.name) {
Expand Down Expand Up @@ -319,7 +373,13 @@ class RestContentAdapter implements ContentAdapter {
const response = await this.connection.get(id);
this.updateFileMetadata(id, response);

return this.enrichWithDataProviderProperties(response.data);
const enrichedItem = this.enrichWithDataProviderProperties(response.data);

if (enrichedItem.contextValue?.includes("copyPath")) {
await this.updateUriWithFullPath(enrichedItem);
}

return enrichedItem;
}

public async getItemOfUri(uri: Uri): Promise<ContentItem> {
Expand Down Expand Up @@ -357,7 +417,15 @@ class RestContentAdapter implements ContentAdapter {
`/folders/folders?parentFolderUri=${parentFolderUri}`,
{ name: folderName },
);
return this.enrichWithDataProviderProperties(createFolderResponse.data);
const enrichedItem = this.enrichWithDataProviderProperties(
createFolderResponse.data,
);

if (enrichedItem.contextValue?.includes("copyPath")) {
await this.updateUriWithFullPath(enrichedItem);
}

return enrichedItem;
// eslint-disable-next-line @typescript-eslint/no-unused-vars
} catch (error) {
return;
Expand All @@ -371,9 +439,11 @@ class RestContentAdapter implements ContentAdapter {
item.flags = flags;
item.permission = getPermission(item);

const contextValue = this.contextMenuProvider.availableActions(item);

return {
...item,
contextValue: this.contextMenuProvider.availableActions(item),
contextValue,
fileStat: {
ctime: item.creationTimeStamp,
mtime: item.modifiedTimeStamp,
Expand All @@ -398,6 +468,27 @@ class RestContentAdapter implements ContentAdapter {
}
}

private async updateUriWithFullPath(item: ContentItem): Promise<void> {
try {
const typeName = getTypeName(item);
if (typeName === TRASH_FOLDER_TYPE || !item.parentFolderUri) {
return;
}

const fullPath = await this.getPathOfItem(item);
if (fullPath && fullPath !== item.name) {
item.vscUri = getSasContentUri(
item,
item.flags?.isInRecycleBin || false,
fullPath,
);
}
} catch {
// If path retrieval fails, keep the original URI
return;
}
}

public async renameItem(
item: ContentItem,
newName: string,
Expand Down Expand Up @@ -443,7 +534,15 @@ class RestContentAdapter implements ContentAdapter {
return await this.getItemOfId(item.uri);
}

return this.enrichWithDataProviderProperties(patchResponse.data);
const enrichedItem = this.enrichWithDataProviderProperties(
patchResponse.data,
);

if (enrichedItem.contextValue?.includes("copyPath")) {
await this.updateUriWithFullPath(enrichedItem);
}

return enrichedItem;
// eslint-disable-next-line @typescript-eslint/no-unused-vars
} catch (error) {
return;
Expand Down Expand Up @@ -488,7 +587,13 @@ class RestContentAdapter implements ContentAdapter {
return;
}

return this.enrichWithDataProviderProperties(createdResource);
const enrichedItem = this.enrichWithDataProviderProperties(createdResource);

if (enrichedItem.contextValue?.includes("copyPath")) {
await this.updateUriWithFullPath(enrichedItem);
}

return enrichedItem;
}

public async addChildItem(
Expand Down
12 changes: 9 additions & 3 deletions client/src/connection/rest/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,18 @@ export const getResourceIdFromItem = (item: ContentItem): string | null => {
return getLink(item.links, "GET", "self")?.uri || null;
};

export const getSasContentUri = (item: ContentItem, readOnly?: boolean): Uri =>
export const getSasContentUri = (
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of having a nested ternary here, could we just always pass in fullPath. So the function would become something like

getSasContentUri(item: ContentItem, path: string, readOnly?: boolean)

and we'd just always use path

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Would this not cause some of the times that this method is called to change into an async function because we would have to get the path of the item? I am just unsure why we cannot keep it as an optional parameter?

item: ContentItem,
readOnly?: boolean,
fullPath?: string,
): Uri =>
Uri.parse(
`${readOnly ? `${ContentSourceType.SASContent}ReadOnly` : ContentSourceType.SASContent}:/${
item.name
? item.name.replace(/#/g, "%23").replace(/\?/g, "%3F")
fullPath
? fullPath.replace(/#/g, "%23").replace(/\?/g, "%3F")
: item.name
? item.name.replace(/#/g, "%23").replace(/\?/g, "%3F")
: item.name
}?id=${getResourceIdFromItem(item)}`,
);

Expand Down
Loading