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
18 changes: 16 additions & 2 deletions src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,14 @@ import { map } from 'rxjs/operators';

import { IdentifiableDataService } from '../data/base/identifiable-data.service';
import { ItemDataService } from '../data/item-data.service';
import { getDSORoute } from '../router/utils/dso-route.utils';
import {
CUSTOM_URL_VALID_PATTERN,
getDSORoute,
getItemPageRoute,
} from '../router/utils/dso-route.utils';
import { DSpaceObject } from '../shared/dspace-object.model';
import { FollowLinkConfig } from '../shared/follow-link-config.model';
import { Item } from '../shared/item.model';
import {
getFirstCompletedRemoteData,
getRemoteDataPayload,
Expand Down Expand Up @@ -63,7 +68,16 @@ export const DSOBreadcrumbResolverByUuid: (route: ActivatedRouteSnapshot, state:
getRemoteDataPayload(),
map((object: DSpaceObject) => {
if (hasValue(object)) {
return { provider: breadcrumbService, key: object, url: getDSORoute(object) };
// For items, fall back to UUID-based route if the custom URL contains non-latin characters
let url: string;
if (object instanceof Item && object.hasMetadata('dspace.customurl')) {
const customUrl = object.firstMetadataValue('dspace.customurl');
const ignoreCustomUrl = !CUSTOM_URL_VALID_PATTERN.test(customUrl);
url = getItemPageRoute(object as Item, ignoreCustomUrl);
} else {
url = getDSORoute(object);
}
return { provider: breadcrumbService, key: object, url };
} else {
return undefined;
}
Expand Down
3 changes: 3 additions & 0 deletions src/app/core/data/request-error.model.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { PathableObjectError } from './response-state.model';

export class RequestError extends Error {
statusCode: number;
statusText: string;
errors?: PathableObjectError[];
}
9 changes: 7 additions & 2 deletions src/app/core/data/request.actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Action } from '@ngrx/store';
import { type } from '../ngrx/type';
import { HALLink } from '../shared/hal-link.model';
import { UnCacheableObject } from '../shared/uncacheable-object.model';
import { PathableObjectError } from './response-state.model';
import { RestRequest } from './rest-request.model';

/**
Expand Down Expand Up @@ -103,7 +104,8 @@ export class RequestErrorAction extends RequestUpdateAction {
uuid: string,
timeCompleted: number,
statusCode: number,
errorMessage: string
errorMessage: string,
errors?: PathableObjectError[]
};

/**
Expand All @@ -115,14 +117,17 @@ export class RequestErrorAction extends RequestUpdateAction {
* the statusCode of the response
* @param errorMessage
* the error message in the response
* @param errors
* the list of pathable errors
*/
constructor(uuid: string, statusCode: number, errorMessage: string) {
constructor(uuid: string, statusCode: number, errorMessage: string, errors?: PathableObjectError[]) {
super();
this.payload = {
uuid,
timeCompleted: new Date().getTime(),
statusCode,
errorMessage,
errors,
};
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/app/core/data/request.effects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export class RequestEffects {
catchError((error: unknown) => {
if (error instanceof RequestError) {
// if it's an error returned by the server, complete the request
return [new RequestErrorAction(request.uuid, error.statusCode, error.message)];
return [new RequestErrorAction(request.uuid, error.statusCode, error.message, error.errors)];
} else {
// if it's a client side error, throw it
throw error;
Expand Down
1 change: 1 addition & 0 deletions src/app/core/data/request.reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ function completeFailedRequest(storeState: RequestState, action: RequestErrorAct
statusCode: action.payload.statusCode,
payloadLink: null,
errorMessage: action.payload.errorMessage,
errors: action.payload.errors,
},
lastUpdated: action.lastUpdated,
}),
Expand Down
1 change: 1 addition & 0 deletions src/app/core/dspace-rest/dspace-rest.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ export class DspaceRestService {

error.statusCode = err.status;
error.statusText = err.statusText;
error.errors = err?.error || null;

return error;
} else {
Expand Down
16 changes: 12 additions & 4 deletions src/app/core/router/utils/dso-route.utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ import {
getItemModuleRoute,
} from '../core-routing-paths';

/**
* Regex to validate that a custom URL contains only safe latin-compatible characters.
* If the custom URL does not match this pattern, routing falls back to the item UUID.
*/
export const CUSTOM_URL_VALID_PATTERN = /^[.a-zA-Z0-9\-_]+$/;

export function getCollectionPageRoute(collectionId: string) {
return new URLCombiner(getCollectionModuleRoute(), collectionId).toString();
}
Expand All @@ -26,14 +32,16 @@ export function getCommunityPageRoute(communityId: string) {

/**
* Get the route to an item's page
* Depending on the item's entity type, the route will either start with /items or /entities
* @param item The item to retrieve the route for
* Depending on the item's entity type, the route will either start with /items or /entities.
*
* @param item The item to retrieve the route for
* @param ignoreCustomUrl When true, always use the UUID even if a valid custom URL exists
*/
export function getItemPageRoute(item: Item) {
export function getItemPageRoute(item: Item, ignoreCustomUrl = false) {
const type = item.firstMetadataValue('dspace.entity.type');
let url = item.uuid;

if (isNotEmpty(item.metadata) && item.hasMetadata('dspace.customurl')) {
if (isNotEmpty(item.metadata) && item.hasMetadata('dspace.customurl') && !ignoreCustomUrl) {
url = item.firstMetadataValue('dspace.customurl');
}

Expand Down
4 changes: 2 additions & 2 deletions src/app/item-page/item-page-routing-paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import {
import { Item } from '@dspace/core/shared/item.model';
import { URLCombiner } from '@dspace/core/url-combiner/url-combiner';

export function getItemEditRoute(item: Item) {
return new URLCombiner(getItemPageRoute(item), ITEM_EDIT_PATH).toString();
export function getItemEditRoute(item: Item, ignoreCustomUrl = false) {
return new URLCombiner(getItemPageRoute(item, ignoreCustomUrl), ITEM_EDIT_PATH).toString();
}

export function getItemEditVersionhistoryRoute(item: Item) {
Expand Down
57 changes: 52 additions & 5 deletions src/app/item-page/item-page.resolver.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { HardRedirectService } from '@dspace/core/services/hard-redirect.service
import { DSpaceObject } from '@dspace/core/shared/dspace-object.model';
import { MetadataValueFilter } from '@dspace/core/shared/metadata.models';
import { AuthServiceStub } from '@dspace/core/testing/auth-service.stub';
import { NotificationsServiceStub } from '@dspace/core/testing/notifications-service.stub';
import { createSuccessfulRemoteDataObject$ } from '@dspace/core/utilities/remote-data.utils';
import { first } from 'rxjs/operators';

Expand Down Expand Up @@ -35,6 +36,8 @@ describe('itemPageResolver', () => {
let authService: AuthServiceStub;
let platformId: any;
let hardRedirectService: any;
let notificationsService: NotificationsServiceStub;
let translateService: any;

const uuid = '1234-65487-12354-1235';
let item: DSpaceObject;
Expand All @@ -46,6 +49,8 @@ describe('itemPageResolver', () => {
hardRedirectService = jasmine.createSpyObj('hardRedirectService', {
redirect: {},
});
notificationsService = new NotificationsServiceStub();
translateService = { instant: (key: string) => key } as any;
item = Object.assign(new DSpaceObject(), {
uuid: uuid,
firstMetadataValue(_keyOrKeys: string | string[], _valueFilter?: MetadataValueFilter): string {
Expand Down Expand Up @@ -74,6 +79,8 @@ describe('itemPageResolver', () => {
authService,
platformId,
hardRedirectService,
notificationsService,
translateService,
).pipe(first())
.subscribe(
() => {
Expand All @@ -83,7 +90,7 @@ describe('itemPageResolver', () => {
);
});

it('should not redirect if were already on the correct route', (done) => {
it('should not redirect if we\'re already on the correct route', (done) => {
spyOn(item, 'firstMetadataValue').and.returnValue(entityType);
spyOn(router, 'navigateByUrl').and.callThrough();

Expand All @@ -96,6 +103,8 @@ describe('itemPageResolver', () => {
authService,
platformId,
hardRedirectService,
notificationsService,
translateService,
).pipe(first())
.subscribe(
() => {
Expand Down Expand Up @@ -129,6 +138,8 @@ describe('itemPageResolver', () => {
let authService: AuthServiceStub;
let platformId: any;
let hardRedirectService: any;
let notificationsService: NotificationsServiceStub;
let translateService: any;

const uuid = '1234-65487-12354-1235';
let item: DSpaceObject;
Expand All @@ -139,6 +150,8 @@ describe('itemPageResolver', () => {
hardRedirectService = jasmine.createSpyObj('hardRedirectService', {
redirect: {},
});
notificationsService = new NotificationsServiceStub();
translateService = { instant: (key: string) => key } as any;
item = Object.assign(new DSpaceObject(), {
uuid: uuid,
id: uuid,
Expand Down Expand Up @@ -168,7 +181,7 @@ describe('itemPageResolver', () => {
const route = { params: { id: uuid } } as any;
const state = { url: `/entities/person/${uuid}` } as any;

resolver(route, state, router, itemService, store, authService, platformId, hardRedirectService)
resolver(route, state, router, itemService, store, authService, platformId, hardRedirectService, notificationsService, translateService)
.pipe(first())
.subscribe((rd: any) => {
const expectedUrl = `/entities/person/${customUrl}`;
Expand All @@ -183,7 +196,7 @@ describe('itemPageResolver', () => {
const route = { params: { id: customUrl } } as any;
const state = { url: `/entities/person/${customUrl}` } as any;

resolver(route, state, router, itemService, store, authService, platformId, hardRedirectService)
resolver(route, state, router, itemService, store, authService, platformId, hardRedirectService, notificationsService, translateService)
.pipe(first())
.subscribe((rd: any) => {
expect(router.navigateByUrl).not.toHaveBeenCalled();
Expand All @@ -197,7 +210,7 @@ describe('itemPageResolver', () => {
const route = { params: { id: customUrl } } as any;
const state = { url: `/entities/person/${customUrl}/edit` } as any;

resolver(route, state, router, itemService, store, authService, platformId, hardRedirectService)
resolver(route, state, router, itemService, store, authService, platformId, hardRedirectService, notificationsService, translateService)
.pipe(first())
.subscribe(() => {
expect(router.navigateByUrl).toHaveBeenCalledWith(`/entities/person/${uuid}/edit`);
Expand All @@ -211,13 +224,47 @@ describe('itemPageResolver', () => {
const route = { params: { id: customUrl } } as any;
const state = { url: `/entities/person/${customUrl}/full` } as any;

resolver(route, state, router, itemService, store, authService, platformId, hardRedirectService)
resolver(route, state, router, itemService, store, authService, platformId, hardRedirectService, notificationsService, translateService)
.pipe(first())
.subscribe(() => {
expect(router.navigateByUrl).not.toHaveBeenCalled();;
done();
});
});

it('should fall back to UUID and show a warning notification when dspace.customurl contains invalid characters', (done) => {
const invalidCustomUrl = 'café-测试';
const itemWithInvalidUrl = Object.assign(new DSpaceObject(), {
uuid: uuid,
id: uuid,
firstMetadataValue(_keyOrKeys: string | string[], _valueFilter?: MetadataValueFilter): string {
return _keyOrKeys === 'dspace.entity.type' ? 'person' : invalidCustomUrl;
},
hasMetadata(_keyOrKeys: string | string[], _valueFilter?: MetadataValueFilter): boolean {
return true;
},
metadata: { 'dspace.customurl': invalidCustomUrl },
});
const invalidItemService = {
findByIdOrCustomUrl: (_id: string) => createSuccessfulRemoteDataObject$(itemWithInvalidUrl),
};
notificationsService = new NotificationsServiceStub();
translateService = { instant: (key: string) => key } as any;

spyOn(router, 'navigateByUrl').and.callThrough();

const encodedInvalidUrl = encodeURIComponent(invalidCustomUrl);
const route = { params: { id: invalidCustomUrl } } as any;
const state = { url: `/entities/person/${encodedInvalidUrl}` } as any;

resolver(route, state, router, invalidItemService, store, authService, platformId, hardRedirectService, notificationsService, translateService)
.pipe(first())
.subscribe(() => {
expect(router.navigateByUrl).toHaveBeenCalledWith(`/entities/person/${uuid}`);
expect(notificationsService.warning).toHaveBeenCalled();
done();
});
});
});

});
37 changes: 29 additions & 8 deletions src/app/item-page/item-page.resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,13 @@ import {
import { AuthService } from '@dspace/core/auth/auth.service';
import { ItemDataService } from '@dspace/core/data/item-data.service';
import { RemoteData } from '@dspace/core/data/remote-data';
import { NotificationOptions } from '@dspace/core/notification-system/models/notification-options.model';
import { NotificationsService } from '@dspace/core/notification-system/notifications.service';
import { ResolvedAction } from '@dspace/core/resolving/resolver.actions';
import { getItemPageRoute } from '@dspace/core/router/utils/dso-route.utils';
import {
CUSTOM_URL_VALID_PATTERN,
getItemPageRoute,
} from '@dspace/core/router/utils/dso-route.utils';
import { HardRedirectService } from '@dspace/core/services/hard-redirect.service';
import {
redirectOn4xx,
Expand All @@ -26,6 +31,7 @@ import {
import { getFirstCompletedRemoteData } from '@dspace/core/shared/operators';
import { hasValue } from '@dspace/shared/utils/empty.util';
import { Store } from '@ngrx/store';
import { TranslateService } from '@ngx-translate/core';
import { Observable } from 'rxjs';
import { map } from 'rxjs/operators';

Expand All @@ -51,6 +57,8 @@ export const itemPageResolver: ResolveFn<RemoteData<Item>> = (
authService: AuthService = inject(AuthService),
platformId: any = inject(PLATFORM_ID),
hardRedirectService: HardRedirectService = inject(HardRedirectService),
notificationsService: NotificationsService = inject(NotificationsService),
translateService: TranslateService = inject(TranslateService),
): Observable<RemoteData<Item>> => {
const itemRD$ = itemService.findByIdOrCustomUrl(
route.params.id,
Expand All @@ -67,23 +75,36 @@ export const itemPageResolver: ResolveFn<RemoteData<Item>> = (
store.dispatch(new ResolvedAction(state.url, itemRD.payload));
});


return itemRD$.pipe(
map((rd: RemoteData<Item>) => {
if (rd.hasSucceeded && hasValue(rd.payload)) {
let itemRoute;
let itemRoute: string;
if (hasValue(rd.payload.metadata) && rd.payload.hasMetadata('dspace.customurl')) {
const customUrl = rd.payload.firstMetadataValue('dspace.customurl');
const isSubPath = !(state.url.endsWith(customUrl) || state.url.endsWith(rd.payload.id) || state.url.endsWith('/full'));
itemRoute = isSubPath ? state.url : router.parseUrl(getItemPageRoute(rd.payload)).toString();
const isValidCustomUrl = CUSTOM_URL_VALID_PATTERN.test(customUrl);
const decodedStateUrl = decodeURIComponent(state.url);
const isSubPath = !(decodedStateUrl.endsWith(customUrl) || decodedStateUrl.endsWith(rd.payload.id) || decodedStateUrl.endsWith('/full'));
itemRoute = (isSubPath || !isValidCustomUrl) ? state.url : router.parseUrl(getItemPageRoute(rd.payload)).toString();
let newUrl: string;
if (route.params.id !== customUrl && !isSubPath) {
newUrl = itemRoute.replace(route.params.id,rd.payload.firstMetadataValue('dspace.customurl'));
} else if (isSubPath && route.params.id === customUrl) {
if (route.params.id !== customUrl && !isSubPath && isValidCustomUrl) {
newUrl = itemRoute.replace(route.params.id, rd.payload.firstMetadataValue('dspace.customurl'));
} else if ((isSubPath || !isValidCustomUrl) && route.params.id === customUrl) {
// In case of a sub path, we need to ensure we navigate to the edit page of the item ID, not the custom URL
const itemId = rd.payload.uuid;
newUrl = itemRoute.replace(rd.payload.firstMetadataValue('dspace.customurl'), itemId);
newUrl = decodeURIComponent(itemRoute).replace(customUrl, itemId);
if (!isValidCustomUrl && !isSubPath) {
// Notify the user that custom url won't be used as it is malformed
const notificationOptions = new NotificationOptions(-1, true);
notificationsService.warning(
translateService.instant('item-page.resolver.invalid-custom-url.title'),
translateService.instant('item-page.resolver.invalid-custom-url.message'),
notificationOptions,
);
}
}


if (hasValue(newUrl)) {
router.navigateByUrl(newUrl);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<ds-alert [type]="AlertTypeEnum.Error">
<h1 class="h4">{{ 'error.custom-url-conflict.title' | translate }}</h1>
<p>{{ 'error.custom-url-conflict.description' | translate: { customUrl: customUrl } }}</p>
<p>{{ 'error.custom-url-conflict.admin-action' | translate }}</p>
@let items = (conflictingItems$ | async);
@if (items) {
@if (items.length > 0) {
<ul class="mb-2">
@for (item of items; track item.uuid) {
<li>
@if (isAuthenticated$ | async) {
<a class="btn btn-link" [routerLink]="item.editLink">
{{ 'error.custom-url-conflict.edit-link' | translate: { name: item.name, uuid: item.uuid } }}
</a>
} @else {
<span>{{ 'error.custom-url-conflict.edit-link' | translate: { name: item.name, uuid: item.uuid } }}</span>
}
</li>
}
</ul>
} @else {
<p class="text-muted small">{{ 'error.custom-url-conflict.no-items-found' | translate }}</p>
}
} @else {
<ds-loading [showMessage]="false"></ds-loading>
}
</ds-alert>
Loading
Loading