Fixes for #2891 #2889
References
- Fixes #2891
- Fixes #2889
Description
This is a small follow-up PR to fix the mentioned issues
Checklist
- [x] My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
- [x] My PR passes ESLint validation using
yarn lint - [x] My PR doesn't introduce circular dependencies (verified via
yarn check-circ-deps) - [x] My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
- [x] My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
- [x] If my PR includes new libraries/dependencies (in
package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation. - [x] If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
- [x] If my PR fixes an issue ticket, I've linked them together.
@tdonohue @artlowel
regarding the CodeQL alert, i've added some checks how is described in the documentation but it persists. Do you have any suggestion, could we ignore it or do i need to investigate further?
@atarix83 The suggestion from CodeQL is to turn LazyDataServicesMap into an actual Map object:
in data-services.map.ts:
export const LAZY_DATA_SERVICES = new Map<string, () => Promise<Type<HALDataService<any>>>>()
LAZY_DATA_SERVICES.set(AUTHORIZATION.value, () => import('./data/feature-authorization/authorization-data.service').then(m => m.AuthorizationDataService));
...
in lazy-service pass both the map and the key, so you can write something like:
if (map.has(key)) {
const loader = map.get(key):
loader();
} else {
trow new Error(`The key ${key} was not found in the map ${map}`);
}
where map has the type Map<string, () => Promise<any>> in the method to ensure that loader() is indeed a function that returns a promise. If that doesn't appease CodeQL, you may need to explicitly test that loader is a function as well.
thanks @artlowel for suggestions, now every checks pass and PR is to be reviewed