Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ describe('UploadBitstreamComponent', () => {
const mockItem = Object.assign(new Item(), {
id: 'fake-id',
handle: 'fake/handle',
_links: {
self: {
href: '/api/core/items/fake-id'
}
},
metadata: {
'dc.title': [
{
Expand All @@ -83,6 +88,7 @@ describe('UploadBitstreamComponent', () => {
const restEndpoint = 'fake-rest-endpoint';
const mockItemDataService = jasmine.createSpyObj('mockItemDataService', {
getBitstreamsEndpoint: observableOf(restEndpoint),
getBundlesEndpoint: observableOf('/api/core/items/fake-id/bundles'),
createBundle: createSuccessfulRemoteDataObject$(createdBundle),
getBundles: createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), [bundle])),
});
Expand Down Expand Up @@ -131,6 +137,18 @@ describe('UploadBitstreamComponent', () => {
it('should navigate the user to the next page', () => {
expect(routerStub.navigate).toHaveBeenCalled();
});

it('should clear cached requests for the selected bundle bitstreams endpoint', () => {
expect(requestService.removeByHrefSubstring).toHaveBeenCalledWith(restEndpoint);
});

it('should clear cached requests for the item bundles endpoint', () => {
expect(requestService.removeByHrefSubstring).toHaveBeenCalledWith('/api/core/items/fake-id/bundles');
});

it('should clear cached requests for the item self endpoint', () => {
expect(requestService.removeByHrefSubstring).toHaveBeenCalledWith('/api/core/items/fake-id');
});
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

The new cache invalidation behavior in onCompleteItem (metadatabitstreams byHandle + handle/fileGrpType staling) isn’t asserted in the spec; the current expectations only cover the bundle endpoint, bundles endpoint, and item self link. Add assertions for the additional removeByHrefSubstring/setStaleByHrefSubstring calls so regressions in the refresh behavior are caught.

Suggested change
});
});
it('should clear cached requests for the metadatabitstreams byHandle search endpoint', () => {
expect(requestService.removeByHrefSubstring).toHaveBeenCalledWith(
jasmine.stringContaining('/api/core/metadatabitstreams/search/byHandle'),
);
});
it('should mark handle/fileGrpType-based bitstream listings as stale', () => {
expect(requestService.setStaleByHrefSubstring).toHaveBeenCalledWith(
jasmine.stringMatching(/\/handle\/.+fileGrpType=/),
);
});

Copilot uses AI. Check for mistakes.
});
});

Expand Down
21 changes: 21 additions & 0 deletions src/app/item-page/bitstreams/upload/upload-bitstream.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,27 @@ export class UploadBitstreamComponent implements OnInit, OnDestroy {
this.requestService.removeByHrefSubstring(href);
});

// Clear cached requests for this item's bundles to ensure bundle resolution uses fresh data
this.itemService.getBundlesEndpoint(this.itemId).pipe(take(1)).subscribe((href: string) => {
this.requestService.removeByHrefSubstring(href);
});

// Clear cached requests for this item to ensure breadcrumb navigation resolves a fresh item
this.itemRD$.pipe(
getFirstSucceededRemoteDataPayload(),
take(1),
).subscribe((item: Item) => {
this.requestService.removeByHrefSubstring(item._links.self.href);

// Clear metadatabitstreams search cache used by preview and CLARIN files sections
this.requestService.removeByHrefSubstring('/api/core/metadatabitstreams/search/byHandle');
if (item?.handle) {
this.requestService.removeByHrefSubstring(`handle=${encodeURIComponent(item.handle)}`);
this.requestService.removeByHrefSubstring(`handle=${item.handle}`);
}
this.requestService.removeByHrefSubstring('fileGrpType=ORIGINAL');
Comment on lines +215 to +233
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

New code calls RequestService.removeByHrefSubstring multiple times, but that method is deprecated (it just forwards to setStaleByHrefSubstring). Please switch these new invalidations to setStaleByHrefSubstring (and consider updating the existing call in this method too) to avoid adding new deprecated API usage.

Copilot uses AI. Check for mistakes.
Comment on lines +228 to +233
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

The cache invalidation via very generic substrings (e.g. plain handle=... and fileGrpType=ORIGINAL) can mark unrelated requests stale too (any request whose URL contains those query params). It would be safer to target the metadatabitstreams byHandle search more precisely (path + handle + fileGrpType) so you don’t accidentally stale other handle-based calls like refbox/citations.

Suggested change
this.requestService.removeByHrefSubstring('/api/core/metadatabitstreams/search/byHandle');
if (item?.handle) {
this.requestService.removeByHrefSubstring(`handle=${encodeURIComponent(item.handle)}`);
this.requestService.removeByHrefSubstring(`handle=${item.handle}`);
}
this.requestService.removeByHrefSubstring('fileGrpType=ORIGINAL');
if (item?.handle) {
const byHandleBase = '/api/core/metadatabitstreams/search/byHandle';
const encodedHandle = encodeURIComponent(item.handle);
// Invalidate cached metadatabitstreams byHandle searches for this item and ORIGINAL file group
this.requestService.removeByHrefSubstring(`${byHandleBase}?handle=${encodedHandle}&fileGrpType=ORIGINAL`);
this.requestService.removeByHrefSubstring(`${byHandleBase}?handle=${item.handle}&fileGrpType=ORIGINAL`);
}

Copilot uses AI. Check for mistakes.
});

// Bring over the item ID as a query parameter
const queryParams = { itemId: this.itemId, entityType: this.entityType };
this.router.navigate([getBitstreamModuleRoute(), bitstream.id, 'edit'], { queryParams: queryParams });
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Component, Input, OnInit } from '@angular/core';
import { Component, Input, OnChanges, OnDestroy, OnInit, SimpleChanges } from '@angular/core';
import { Item } from '../../core/shared/item.model';
import { getAllSucceededRemoteListPayload, getFirstSucceededRemoteDataPayload } from '../../core/shared/operators';
import { getItemPageRoute } from '../item-page-routing-paths';
Expand All @@ -7,14 +7,14 @@ import { RegistryService } from '../../core/registry/registry.service';
import { Router } from '@angular/router';
import { HALEndpointService } from '../../core/shared/hal-endpoint.service';
import { ConfigurationDataService } from '../../core/data/configuration-data.service';
import { BehaviorSubject } from 'rxjs';
import { BehaviorSubject, Subscription } from 'rxjs';

@Component({
selector: 'ds-clarin-files-section',
templateUrl: './clarin-files-section.component.html',
styleUrls: ['./clarin-files-section.component.scss']
})
export class ClarinFilesSectionComponent implements OnInit {
export class ClarinFilesSectionComponent implements OnInit, OnChanges, OnDestroy {

/**
* The item to display files for
Expand Down Expand Up @@ -71,6 +71,9 @@ export class ClarinFilesSectionComponent implements OnInit {
*/
downloadZipMinFileCount: BehaviorSubject<number> = new BehaviorSubject<number>(-1);

private currentItemHandle: string;
private filesSubscription?: Subscription;


constructor(protected registryService: RegistryService,
protected router: Router,
Expand All @@ -79,15 +82,18 @@ export class ClarinFilesSectionComponent implements OnInit {
}

ngOnInit(): void {
this.registryService
.getMetadataBitstream(this.itemHandle, 'ORIGINAL')
.pipe(getAllSucceededRemoteListPayload())
.subscribe((data: MetadataBitstream[]) => {
this.listOfFiles.next(data);
this.generateCurlCommand();
});
this.totalFileSizes.next(Number(this.item.firstMetadataValue('local.files.size')));
this.loadDownloadZipConfigProperties();
this.refreshFromInputs(true);
}

ngOnChanges(changes: SimpleChanges): void {
if (changes.item || changes.itemHandle) {
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

As written, refreshFromInputs(true) is invoked in both ngOnInit and ngOnChanges; since ngOnChanges fires before ngOnInit on the first binding, this will create two back-to-back metadatabitstreams requests for the initial render. Consider running the initial fetch in only one lifecycle hook or guarding ngOnChanges with firstChange.

Suggested change
if (changes.item || changes.itemHandle) {
const itemChanged = changes.item && !changes.item.firstChange;
const itemHandleChanged = changes.itemHandle && !changes.itemHandle.firstChange;
if (itemChanged || itemHandleChanged) {

Copilot uses AI. Check for mistakes.
this.refreshFromInputs(true);
}
}

ngOnDestroy(): void {
this.filesSubscription?.unsubscribe();
}

setCommandline() {
Expand Down Expand Up @@ -138,4 +144,29 @@ export class ClarinFilesSectionComponent implements OnInit {
this.downloadZipMinFileSize.next(Number(config.values[0]));
});
}

private refreshFromInputs(force = false): void {
if (this.item) {
this.totalFileSizes.next(Number(this.item.firstMetadataValue('local.files.size')));
}

const handle = this.itemHandle || this.item?.handle;
if (!handle) {
return;
}

if (!force && handle === this.currentItemHandle) {
return;
}

this.currentItemHandle = handle;
this.filesSubscription?.unsubscribe();
this.filesSubscription = this.registryService
.getMetadataBitstream(handle, 'ORIGINAL')
.pipe(getAllSucceededRemoteListPayload())
.subscribe((data: MetadataBitstream[]) => {
this.listOfFiles.next(data);
this.generateCurlCommand();
});
}
}
2 changes: 1 addition & 1 deletion src/app/item-page/item.resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export class ItemResolver implements Resolve<RemoteData<Item>> {
resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<RemoteData<Item>> {
const itemRD$ = this.itemService.findById(route.params.id,
true,
false,
true,
...getItemPageLinksToFollow(),
).pipe(
getFirstCompletedRemoteData(),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Component, Input, OnInit } from '@angular/core';
import { BehaviorSubject } from 'rxjs';
import { Component, Input, OnChanges, OnDestroy, OnInit, SimpleChanges } from '@angular/core';
import { BehaviorSubject, Subscription } from 'rxjs';
import { MetadataBitstream } from 'src/app/core/metadata/metadata-bitstream.model';
import { RegistryService } from 'src/app/core/registry/registry.service';
import { Item } from 'src/app/core/shared/item.model';
Expand All @@ -11,26 +11,54 @@ import { ConfigurationDataService } from '../../../../core/data/configuration-da
templateUrl: './preview-section.component.html',
styleUrls: ['./preview-section.component.scss'],
})
export class PreviewSectionComponent implements OnInit {
export class PreviewSectionComponent implements OnInit, OnChanges, OnDestroy {
@Input() item: Item;

listOfFiles: BehaviorSubject<MetadataBitstream[]> = new BehaviorSubject<MetadataBitstream[]>([] as any);
emailToContact: string;
hasNoFiles: BehaviorSubject<boolean> = new BehaviorSubject<boolean>(true);

private currentItemHandle: string;
private filesSubscription?: Subscription;

constructor(protected registryService: RegistryService,
private configService: ConfigurationDataService) {} // Modified
private configService: ConfigurationDataService) {}

ngOnInit(): void {
this.registryService
.getMetadataBitstream(this.item.handle, 'ORIGINAL')
this.configService.findByPropertyName('lr.help.mail')?.subscribe(remoteData => {
this.emailToContact = remoteData.payload?.values?.[0];
});
this.refreshFiles(true);
}

ngOnChanges(changes: SimpleChanges): void {
if (changes.item) {
this.refreshFiles(true);
}
}

ngOnDestroy(): void {
this.filesSubscription?.unsubscribe();
}

private refreshFiles(force = false): void {
const handle = this.item?.handle;
if (!handle) {
return;
}

if (!force && handle === this.currentItemHandle) {
return;
}

this.currentItemHandle = handle;
this.filesSubscription?.unsubscribe();
this.filesSubscription = this.registryService
.getMetadataBitstream(handle, 'ORIGINAL')
.pipe(getAllSucceededRemoteListPayload())
.subscribe((data: MetadataBitstream[]) => {
this.listOfFiles.next(data);
this.hasNoFiles.next(!Array.isArray(data) || data.length === 0);
});
this.configService.findByPropertyName('lr.help.mail')?.subscribe(remoteData => {
this.emailToContact = remoteData.payload?.values?.[0];
});
}
}
Loading