93803: Stricter typing for dataService decorator & LinkService

The initial idea was to type dataService decorator strictly to BaseDataService.
However, HrefOnlyDataService should not expose methods other than findByHref & findAllByHref, but must still work with LinkService.

To address this we introduce HALDataService: an interface with the minimal requirements for a data service to work with HAL links
- dataService decorator can only decorate a class that implements HALDataService
- services retrieved from DATA_SERVICE_FACTORY should therefore work in LinkService

src/app/core/cache/builders/build-decorators.spec.ts

@@ -2,12 +2,21 @@
 import { HALLink } from '../../shared/hal-link.model';
 import { HALResource } from '../../shared/hal-resource.model';
 import { ResourceType } from '../../shared/resource-type';
-import { dataService, getDataServiceFor, getLinkDefinition, link, } from './build-decorators';
+import { dataService, getDataServiceFor, getLinkDefinition, link } from './build-decorators';
+import { HALDataService } from '../../data/base/hal-data-service.interface';
+import { BaseDataService } from '../../data/base/base-data.service';
 
-class TestService {
+class TestService extends BaseDataService<any> {
 }
 
-class AnotherTestService {
+class AnotherTestService implements HALDataService<any> {
+  public findAllByHref(href$, findListOptions, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow): any {
+    return undefined;
+  }
+
+  public findByHref(href$, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow): any {
+    return undefined;
+  }
 }
 
 class TestHALResource implements HALResource {

src/app/core/cache/builders/build-decorators.ts

@@ -3,20 +3,19 @@ import { hasNoValue, hasValue } from '../../../shared/empty.util';
 import { GenericConstructor } from '../../shared/generic-constructor';
 import { HALResource } from '../../shared/hal-resource.model';
 import { ResourceType } from '../../shared/resource-type';
-import {
+import { getResourceTypeValueFor } from '../object-cache.reducer';
-  getResourceTypeValueFor
-} from '../object-cache.reducer';
 import { InjectionToken } from '@angular/core';
 import { CacheableObject } from '../cacheable-object.model';
 import { TypedObject } from '../typed-object.model';
+import { HALDataService } from '../../data/base/hal-data-service.interface';
 
-export const DATA_SERVICE_FACTORY = new InjectionToken<(resourceType: ResourceType) => GenericConstructor<any>>('getDataServiceFor', {
+export const DATA_SERVICE_FACTORY = new InjectionToken<(resourceType: ResourceType) => GenericConstructor<HALDataService<any>>>('getDataServiceFor', {
   providedIn: 'root',
-  factory: () => getDataServiceFor
+  factory: () => getDataServiceFor,
 });
 export const LINK_DEFINITION_FACTORY = new InjectionToken<<T extends HALResource>(source: GenericConstructor<T>, linkName: keyof T['_links']) => LinkDefinition<T>>('getLinkDefinition', {
   providedIn: 'root',
-  factory: () => getLinkDefinition
+  factory: () => getLinkDefinition,
 });
 export const LINK_DEFINITION_MAP_FACTORY = new InjectionToken<<T extends HALResource>(source: GenericConstructor<T>) => Map<keyof T['_links'], LinkDefinition<T>>>('getLinkDefinitions', {
   providedIn: 'root',
@@ -47,16 +46,15 @@ export function getClassForType(type: string | ResourceType) {
 }
 
 /**
- * A class decorator to indicate that this class is a dataservice
+ * A class decorator to indicate that this class is a data service for a given HAL resource type.
- * for a given resource type.
  *
- * "dataservice" in this context means that it has findByHref and
+ * In most cases, a data service should extend {@link BaseDataService}.
- * findAllByHref methods.
+ * At the very least it must implement {@link HALDataService} in order for it to work with {@link LinkService}.
  *
  * @param resourceType the resource type the class is a dataservice for
  */
-export function dataService(resourceType: ResourceType): any {
+export function dataService(resourceType: ResourceType) {
-  return (target: any) => {
+  return (target: GenericConstructor<HALDataService<any>>): void => {
     if (hasNoValue(resourceType)) {
       throw new Error(`Invalid @dataService annotation on ${target}, resourceType needs to be defined`);
     }
@@ -75,7 +73,7 @@ export function dataService(resourceType: ResourceType): any {
  *
  * @param resourceType the resource type you want the matching dataservice for
  */
-export function getDataServiceFor<T extends CacheableObject>(resourceType: ResourceType) {
+export function getDataServiceFor<T extends CacheableObject>(resourceType: ResourceType): GenericConstructor<HALDataService<any>> {
   return dataServiceMap.get(resourceType.value);
 }
 

src/app/core/cache/builders/link.service.ts

@@ -7,24 +7,26 @@ import {
   DATA_SERVICE_FACTORY,
   LINK_DEFINITION_FACTORY,
   LINK_DEFINITION_MAP_FACTORY,
-  LinkDefinition
+  LinkDefinition,
 } from './build-decorators';
 import { RemoteData } from '../../data/remote-data';
 import { EMPTY, Observable } from 'rxjs';
 import { ResourceType } from '../../shared/resource-type';
+import { HALDataService } from '../../data/base/hal-data-service.interface';
+import { PaginatedList } from '../../data/paginated-list.model';
 
 /**
  * A Service to handle the resolving and removing
  * of resolved {@link HALLink}s on HALResources
  */
 @Injectable({
-  providedIn: 'root'
+  providedIn: 'root',
 })
 export class LinkService {
 
   constructor(
     protected parentInjector: Injector,
-    @Inject(DATA_SERVICE_FACTORY) private getDataServiceFor: (resourceType: ResourceType) => GenericConstructor<any>,
+    @Inject(DATA_SERVICE_FACTORY) private getDataServiceFor: (resourceType: ResourceType) => GenericConstructor<HALDataService<any>>,
     @Inject(LINK_DEFINITION_FACTORY) private getLinkDefinition: <T extends HALResource>(source: GenericConstructor<T>, linkName: keyof T['_links']) => LinkDefinition<T>,
     @Inject(LINK_DEFINITION_MAP_FACTORY) private getLinkDefinitions: <T extends HALResource>(source: GenericConstructor<T>) => Map<keyof T['_links'], LinkDefinition<T>>,
   ) {
@@ -51,7 +53,7 @@ export class LinkService {
    * @param model the {@link HALResource} to resolve the link for
    * @param linkToFollow the {@link FollowLinkConfig} to resolve
    */
-  public resolveLinkWithoutAttaching<T extends HALResource, U extends HALResource>(model, linkToFollow: FollowLinkConfig<T>): Observable<RemoteData<U>> {
+  public resolveLinkWithoutAttaching<T extends HALResource, U extends HALResource>(model, linkToFollow: FollowLinkConfig<T>): Observable<RemoteData<U | PaginatedList<U>>> {
     const matchingLinkDef = this.getLinkDefinition(model.constructor, linkToFollow.name);
 
     if (hasValue(matchingLinkDef)) {
@@ -61,9 +63,9 @@ export class LinkService {
         throw new Error(`The @link() for ${linkToFollow.name} on ${model.constructor.name} models uses the resource type ${matchingLinkDef.resourceType.value.toUpperCase()}, but there is no service with an @dataService(${matchingLinkDef.resourceType.value.toUpperCase()}) annotation in order to retrieve it`);
       }
 
-      const service = Injector.create({
+      const service: HALDataService<any> = Injector.create({
         providers: [],
-        parent: this.parentInjector
+        parent: this.parentInjector,
       }).get(provider);
 
       const link = model._links[matchingLinkDef.linkName];

src/app/core/data/base/base-data.service.ts

@@ -22,6 +22,7 @@ import { FindListOptions } from '../find-list-options.model';
 import { PaginatedList } from '../paginated-list.model';
 import { ObjectCacheEntry } from '../../cache/object-cache.reducer';
 import { ObjectCacheService } from '../../cache/object-cache.service';
+import { HALDataService } from './hal-data-service.interface';
 
 /**
  * Common functionality for data services.
@@ -47,7 +48,7 @@ import { ObjectCacheService } from '../../cache/object-cache.service';
  * }
  * ```
  */
-export class BaseDataService<T extends CacheableObject> {
+export class BaseDataService<T extends CacheableObject> implements HALDataService<T> {
   constructor(
     protected linkPath: string,
     protected requestService: RequestService,

src/app/core/data/base/hal-data-service.interface.ts

@@ -0,0 +1,41 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
+import { Observable } from 'rxjs';
+import { FollowLinkConfig } from '../../../shared/utils/follow-link-config.model';
+import { RemoteData } from '../remote-data';
+import { FindListOptions } from '../find-list-options.model';
+import { PaginatedList } from '../paginated-list.model';
+import { HALResource } from '../../shared/hal-resource.model';
+
+/**
+ * An interface defining the minimum functionality needed for a data service to resolve HAL resources.
+ */
+export interface HALDataService<T extends HALResource> {
+  /**
+   * Returns an Observable of {@link RemoteData} of an object, based on an href,
+   * with a list of {@link FollowLinkConfig}, to automatically resolve {@link HALLink}s of the object
+   *
+   * @param href$                       The url of object we want to retrieve. Can be a string or an Observable<string>
+   * @param useCachedVersionIfAvailable If this is true, the request will only be sent if there's no valid cached version.
+   * @param reRequestOnStale            Whether or not the request should automatically be re-requested after the response becomes stale
+   * @param linksToFollow               List of {@link FollowLinkConfig} that indicate which {@link HALLink}s should be automatically resolved
+   */
+  findByHref(href$: string | Observable<string>, useCachedVersionIfAvailable?: boolean, reRequestOnStale?: boolean, ...linksToFollow: FollowLinkConfig<T>[]): Observable<RemoteData<T>>;
+
+  /**
+   * Returns an Observable of a {@link RemoteData} of a {@link PaginatedList} of objects, based on an href,
+   * with a list of {@link FollowLinkConfig}, to automatically resolve {@link HALLink}s of the object
+   *
+   * @param href$                       The url of list we want to retrieve. Can be a string or an Observable<string>
+   * @param findListOptions             The options for to use for this find list request.
+   * @param useCachedVersionIfAvailable If this is true, the request will only be sent if there's no valid cached version.
+   * @param reRequestOnStale            Whether or not the request should automatically be re-requested after the response becomes stale
+   * @param linksToFollow               List of {@link FollowLinkConfig} that indicate which {@link HALLink}s should be automatically resolved
+   */
+  findAllByHref(href$: string | Observable<string>, findListOptions?: FindListOptions, useCachedVersionIfAvailable?: boolean, reRequestOnStale?: boolean, ...linksToFollow: FollowLinkConfig<T>[]): Observable<RemoteData<PaginatedList<T>>>;
+}

src/app/core/data/href-only-data.service.ts

@@ -14,6 +14,7 @@ import { LICENSE } from '../shared/license.resource-type';
 import { CacheableObject } from '../cache/cacheable-object.model';
 import { FindListOptions } from './find-list-options.model';
 import { BaseDataService } from './base/base-data.service';
+import { HALDataService } from './base/hal-data-service.interface';
 
 /**
  * A DataService with only findByHref methods. Its purpose is to be used for resources that don't
@@ -21,6 +22,15 @@ import { BaseDataService } from './base/base-data.service';
  * for their links to be resolved by the LinkService.
  *
  * an @dataService annotation can be added for any number of these resource types
+ *
+ *
+ * Additionally, this service may be used to retrieve objects by `href` regardless of their type
+ * For example
+ * ```
+ * const items$: Observable<RemoteData<PaginatedList<Item>>> = hrefOnlyDataService.findAllByHref<Item>(href);
+ * const sites$: Observable<RemoteData<PaginatedList<Site>>> = hrefOnlyDataService.findAllByHref<Site>(href);
+ * ```
+ * This means we cannot extend from {@link BaseDataService} directly because the method signatures would not match.
  */
 @Injectable({
   providedIn: 'root',
@@ -28,9 +38,10 @@ import { BaseDataService } from './base/base-data.service';
 @dataService(VOCABULARY_ENTRY)
 @dataService(ITEM_TYPE)
 @dataService(LICENSE)
-export class HrefOnlyDataService {
+export class HrefOnlyDataService implements HALDataService<any> {
   /**
-   * Not all BaseDataService methods should be exposed, so
+   * Works with a {@link BaseDataService} internally, but only exposes two of its methods
+   * with altered signatures to (optionally) constrain the arbitrary return type.
    * @private
    */
   private dataService: BaseDataService<any>;