fix issue where meta tags wouldn't be updated properly based on the route

src/app/+collection-page/collection-page.component.ts

@@ -1,6 +1,11 @@
 import { ChangeDetectionStrategy, Component, OnInit } from '@angular/core';
 import { ActivatedRoute, Router } from '@angular/router';
-import { BehaviorSubject, combineLatest as observableCombineLatest, Observable, Subject } from 'rxjs';
+import {
+  BehaviorSubject,
+  combineLatest as observableCombineLatest,
+  Observable,
+  Subject
+} from 'rxjs';
 import { filter, map, mergeMap, startWith, switchMap, take } from 'rxjs/operators';
 import { PaginatedSearchOptions } from '../shared/search/paginated-search-options.model';
 import { SearchService } from '../core/shared/search/search.service';
@@ -8,8 +13,6 @@ import { SortDirection, SortOptions } from '../core/cache/models/sort-options.mo
 import { CollectionDataService } from '../core/data/collection-data.service';
 import { PaginatedList } from '../core/data/paginated-list.model';
 import { RemoteData } from '../core/data/remote-data';
-
-import { MetadataService } from '../core/metadata/metadata.service';
 import { Bitstream } from '../core/shared/bitstream.model';
 
 import { Collection } from '../core/shared/collection.model';
@@ -65,7 +68,6 @@ export class CollectionPageComponent implements OnInit {
   constructor(
     private collectionDataService: CollectionDataService,
     private searchService: SearchService,
-    private metadata: MetadataService,
     private route: ActivatedRoute,
     private router: Router,
     private authService: AuthService,
@@ -122,10 +124,6 @@ export class CollectionPageComponent implements OnInit {
       getAllSucceededRemoteDataPayload(),
       map((collection) => getCollectionPageRoute(collection.id))
     );
-
-    this.route.queryParams.pipe(take(1)).subscribe((params) => {
-      this.metadata.processRemoteData(this.collectionRD$);
-    });
   }
 
   isNotEmpty(object: any) {

src/app/+item-page/full/full-item-page.component.ts

@@ -11,8 +11,6 @@ import { ItemDataService } from '../../core/data/item-data.service';
 import { RemoteData } from '../../core/data/remote-data';
 import { Item } from '../../core/shared/item.model';
 
-import { MetadataService } from '../../core/metadata/metadata.service';
-
 import { fadeInOut } from '../../shared/animations/fade';
 import { hasValue } from '../../shared/empty.util';
 import { AuthService } from '../../core/auth/auth.service';
@@ -35,8 +33,8 @@ export class FullItemPageComponent extends ItemPageComponent implements OnInit {
 
   metadata$: Observable<MetadataMap>;
 
-  constructor(route: ActivatedRoute, router: Router, items: ItemDataService, metadataService: MetadataService, authService: AuthService) {
-    super(route, router, items, metadataService, authService);
+  constructor(route: ActivatedRoute, router: Router, items: ItemDataService, authService: AuthService) {
+    super(route, router, items, authService);
   }
 
   /*** AoT inheritance fix, will hopefully be resolved in the near future **/

src/app/+item-page/simple/item-page.component.ts

@@ -8,8 +8,6 @@ import { RemoteData } from '../../core/data/remote-data';
 
 import { Item } from '../../core/shared/item.model';
 
-import { MetadataService } from '../../core/metadata/metadata.service';
-
 import { fadeInOut } from '../../shared/animations/fade';
 import { getAllSucceededRemoteDataPayload, redirectOn4xx } from '../../core/shared/operators';
 import { ViewMode } from '../../core/shared/view-mode.model';
@@ -54,7 +52,6 @@ export class ItemPageComponent implements OnInit {
     private route: ActivatedRoute,
     private router: Router,
     private items: ItemDataService,
-    private metadataService: MetadataService,
     private authService: AuthService,
   ) { }
 
@@ -66,7 +63,6 @@ export class ItemPageComponent implements OnInit {
       map((data) => data.dso as RemoteData<Item>),
       redirectOn4xx(this.router, this.authService)
     );
-    this.metadataService.processRemoteData(this.itemRD$);
     this.itemPageRoute$ = this.itemRD$.pipe(
       getAllSucceededRemoteDataPayload(),
       map((item) => getItemPageRoute(item))

src/app/core/metadata/metadata.service.spec.ts

@@ -1,82 +1,30 @@
-import { CommonModule, Location } from '@angular/common';
-import { HttpClient } from '@angular/common/http';
-import { Component, CUSTOM_ELEMENTS_SCHEMA } from '@angular/core';
-import { ComponentFixture, fakeAsync, TestBed, tick } from '@angular/core/testing';
-import { Meta, MetaDefinition, Title } from '@angular/platform-browser';
-import { ActivatedRoute, Router } from '@angular/router';
-import { RouterTestingModule } from '@angular/router/testing';
+import { fakeAsync, tick } from '@angular/core/testing';
+import { Meta, Title } from '@angular/platform-browser';
+import { Router, NavigationEnd } from '@angular/router';
 
-import { Store, StoreModule } from '@ngrx/store';
-
-import { TranslateLoader, TranslateModule, TranslateService } from '@ngx-translate/core';
-import { EmptyError, Observable, of } from 'rxjs';
+import { TranslateService } from '@ngx-translate/core';
+import { Observable, of } from 'rxjs';
 
 import { RemoteData } from '../data/remote-data';
 import { Item } from '../shared/item.model';
 
+import { ItemMock, MockBitstream1, MockBitstream3, } from '../../shared/mocks/item.mock';
 import {
-  ItemMock, MockBitstream1, MockBitstream2, MockBitstream3, MockBitstreamFormat1, MockBitstreamFormat2,
-  MockBitstreamFormat3,
-  MockOriginalBundle,
-} from '../../shared/mocks/item.mock';
-import { TranslateLoaderMock } from '../../shared/mocks/translate-loader.mock';
-import { NotificationsService } from '../../shared/notifications/notifications.service';
-import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils';
-import { FollowLinkConfig } from '../../shared/utils/follow-link-config.model';
-import { AuthService } from '../auth/auth.service';
-import { BrowseService } from '../browse/browse.service';
-import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service';
-import { ObjectCacheService } from '../cache/object-cache.service';
-
-import { CoreState } from '../core.reducers';
-import { BitstreamDataService } from '../data/bitstream-data.service';
-import { BitstreamFormatDataService } from '../data/bitstream-format-data.service';
-import { CommunityDataService } from '../data/community-data.service';
-import { DefaultChangeAnalyzer } from '../data/default-change-analyzer.service';
-import { DSOChangeAnalyzer } from '../data/dso-change-analyzer.service';
-
-import { ItemDataService } from '../data/item-data.service';
-import { buildPaginatedList, PaginatedList } from '../data/paginated-list.model';
-import { FindListOptions } from '../data/request.models';
-import { RequestService } from '../data/request.service';
-import { BitstreamFormat } from '../shared/bitstream-format.model';
+  createSuccessfulRemoteDataObject$,
+  createSuccessfulRemoteDataObject
+} from '../../shared/remote-data.utils';
+import { PaginatedList } from '../data/paginated-list.model';
 import { Bitstream } from '../shared/bitstream.model';
-import { HALEndpointService } from '../shared/hal-endpoint.service';
 import { MetadataValue } from '../shared/metadata.models';
-import { PageInfo } from '../shared/page-info.model';
-import { UUIDService } from '../shared/uuid.service';
 
 import { MetadataService } from './metadata.service';
-import { storeModuleConfig } from '../../app.reducer';
 import { RootDataService } from '../data/root-data.service';
-import { Root } from '../data/root.model';
 import { Bundle } from '../shared/bundle.model';
-import { BundleDataService } from '../data/bundle-data.service';
 import { createPaginatedList } from '../../shared/testing/utils.test';
+import { getMockTranslateService } from '../../shared/mocks/translate.service.mock';
+import { DSONameService } from '../breadcrumbs/dso-name.service';
 import { HardRedirectService } from '../services/hard-redirect.service';
 
-/* tslint:disable:max-classes-per-file */
-@Component({
-  template: `
-    <router-outlet></router-outlet>`
-})
-class TestComponent {
-  constructor(private metadata: MetadataService) {
-    metadata.listenForRouteChange();
-  }
-}
-
-@Component({ template: '' })
-class DummyItemComponent {
-  constructor(private route: ActivatedRoute, private items: ItemDataService, private metadata: MetadataService) {
-    this.route.params.subscribe((params) => {
-      this.metadata.processRemoteData(this.items.findById(params.id));
-    });
-  }
-}
-
-/* tslint:enable:max-classes-per-file */
-
 describe('MetadataService', () => {
   let metadataService: MetadataService;
 
@@ -84,281 +32,306 @@ describe('MetadataService', () => {
 
   let title: Title;
 
-  let store: Store<CoreState>;
+  let dsoNameService: DSONameService;
 
-  let objectCacheService: ObjectCacheService;
-  let requestService: RequestService;
-  let uuidService: UUIDService;
-  let remoteDataBuildService: RemoteDataBuildService;
-  let itemDataService: ItemDataService;
   let bundleDataService;
   let bitstreamDataService;
-  let authService: AuthService;
   let rootService: RootDataService;
   let translateService: TranslateService;
   let hardRedirectService: HardRedirectService;
 
-  let location: Location;
   let router: Router;
-  let fixture: ComponentFixture<TestComponent>;
-
-  let tagStore: Map<string, MetaDefinition[]>;
 
   beforeEach(() => {
-
-    store = new Store<CoreState>(undefined, undefined, undefined);
-    spyOn(store, 'dispatch');
-
-    objectCacheService = new ObjectCacheService(store, undefined);
-    uuidService = new UUIDService();
-    requestService = new RequestService(objectCacheService, uuidService, store, undefined);
-    remoteDataBuildService = new RemoteDataBuildService(objectCacheService, undefined, requestService);
-    bitstreamDataService = {
-      findAllByItemAndBundleName(item: Item, bundleName: string, options?: FindListOptions, ...linksToFollow: FollowLinkConfig<Bitstream>[]): Observable<RemoteData<PaginatedList<Bitstream>>> {
-        if (item.equals(ItemMock)) {
-          return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), [MockBitstream1, MockBitstream2]));
-        } else {
-          return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), []));
-        }
-      },
-      findAllByHref: jasmine.createSpy(),
-    };
-    const mockBitstreamFormatDataService = {
-      findByBitstream(bitstream: Bitstream): Observable<RemoteData<BitstreamFormat>> {
-        switch (bitstream) {
-          case MockBitstream1:
-            return createSuccessfulRemoteDataObject$(MockBitstreamFormat1);
-            break;
-          case MockBitstream2:
-            return createSuccessfulRemoteDataObject$(MockBitstreamFormat2);
-            break;
-          case MockBitstream3:
-            return createSuccessfulRemoteDataObject$(MockBitstreamFormat3);
-            break;
-          default:
-            return createSuccessfulRemoteDataObject$(new BitstreamFormat());
-        }
-      }
-    };
-    bundleDataService = jasmine.createSpyObj('bundleDataService', {
-      findByItemAndName: createSuccessfulRemoteDataObject$(MockOriginalBundle),
+    rootService = jasmine.createSpyObj({
+      findRoot: createSuccessfulRemoteDataObject$({ dspaceVersion: 'mock-dspace-version' })
     });
-    rootService = jasmine.createSpyObj('rootService', {
-      findRoot: createSuccessfulRemoteDataObject$(Object.assign(new Root(), {
-        dspaceVersion: 'mock-dspace-version'
-      }))
+    bitstreamDataService = jasmine.createSpyObj({
+      findAllByHref: createSuccessfulRemoteDataObject$(createPaginatedList([MockBitstream3]))
     });
-    hardRedirectService = jasmine.createSpyObj('hardRedirectService', {
+    bundleDataService = jasmine.createSpyObj({
+      findByItemAndName: mockBundleRD$([MockBitstream3])
+    });
+    translateService = getMockTranslateService();
+    meta = jasmine.createSpyObj({
+      addTag: {},
+      removeTag: {}
+    });
+    title = jasmine.createSpyObj({
+      setTitle: {}
+    });
+    dsoNameService = jasmine.createSpyObj({
+      getName: ItemMock.firstMetadataValue('dc.title')
+    });
+    router = {
+      url: '/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357',
+      events: of(new NavigationEnd(1, '', '')),
+      routerState: {
+        root: {}
+      }
+    } as any as Router;
+    hardRedirectService = jasmine.createSpyObj( {
       getRequestOrigin: 'https://request.org',
     });
-
-    TestBed.configureTestingModule({
-      imports: [
-        CommonModule,
-        StoreModule.forRoot({}, storeModuleConfig),
-        TranslateModule.forRoot({
-          loader: {
-            provide: TranslateLoader,
-            useClass: TranslateLoaderMock
-          }
-        }),
-        RouterTestingModule.withRoutes([
-          { path: 'items/:id', component: DummyItemComponent, pathMatch: 'full' },
-          {
-            path: 'other',
-            component: DummyItemComponent,
-            pathMatch: 'full',
-            data: { title: 'Dummy Title', description: 'This is a dummy item component for testing!' }
-          }
-        ])
-      ],
-      declarations: [
-        TestComponent,
-        DummyItemComponent
-      ],
-      providers: [
-        { provide: ObjectCacheService, useValue: objectCacheService },
-        { provide: RequestService, useValue: requestService },
-        { provide: RemoteDataBuildService, useValue: remoteDataBuildService },
-        { provide: HALEndpointService, useValue: {} },
-        { provide: AuthService, useValue: {} },
-        { provide: NotificationsService, useValue: {} },
-        { provide: HttpClient, useValue: {} },
-        { provide: DSOChangeAnalyzer, useValue: {} },
-        { provide: CommunityDataService, useValue: {} },
-        { provide: DefaultChangeAnalyzer, useValue: {} },
-        { provide: BitstreamFormatDataService, useValue: mockBitstreamFormatDataService },
-        { provide: BitstreamDataService, useValue: bitstreamDataService },
-        { provide: BundleDataService, useValue: bundleDataService },
-        { provide: RootDataService, useValue: rootService },
-        { provide: HardRedirectService, useValue: hardRedirectService },
-        Meta,
-        Title,
-        // tslint:disable-next-line:no-empty
-        { provide: ItemDataService, useValue: { findById: () => {} } },
-        BrowseService,
-        MetadataService
-      ],
-      schemas: [CUSTOM_ELEMENTS_SCHEMA]
-    });
-    meta = TestBed.inject(Meta);
-    title = TestBed.inject(Title);
-    itemDataService = TestBed.inject(ItemDataService);
-    metadataService = TestBed.inject(MetadataService);
-    authService = TestBed.inject(AuthService);
-    translateService = TestBed.inject(TranslateService);
-
-    router = TestBed.inject(Router);
-    location = TestBed.inject(Location);
-
-    fixture = TestBed.createComponent(TestComponent);
-
-    tagStore = metadataService.getTagStore();
+    metadataService = new MetadataService(
+      router,
+      translateService,
+      meta,
+      title,
+      dsoNameService,
+      bundleDataService,
+      bitstreamDataService,
+      undefined,
+      rootService,
+      hardRedirectService
+    );
   });
 
   it('items page should set meta tags', fakeAsync(() => {
-    spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(ItemMock));
-    router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']);
+    (metadataService as any).processRouteChange({
+      data: {
+        value: {
+          dso: createSuccessfulRemoteDataObject(ItemMock),
+        }
+      }
+    });
     tick();
-    expect(title.getTitle()).toEqual('Test PowerPoint Document');
-    expect(tagStore.get('citation_title')[0].content).toEqual('Test PowerPoint Document');
-    expect(tagStore.get('citation_author')[0].content).toEqual('Doe, Jane');
-    expect(tagStore.get('citation_publication_date')[0].content).toEqual('1650-06-26');
-    expect(tagStore.get('citation_issn')[0].content).toEqual('123456789');
-    expect(tagStore.get('citation_language')[0].content).toEqual('en');
-    expect(tagStore.get('citation_keywords')[0].content).toEqual('keyword1; keyword2; keyword3');
+    expect(title.setTitle).toHaveBeenCalledWith('Test PowerPoint Document');
+    expect(meta.addTag).toHaveBeenCalledWith({
+      property: 'citation_title',
+      content: 'Test PowerPoint Document'
+    });
+    expect(meta.addTag).toHaveBeenCalledWith({ property: 'citation_author', content: 'Doe, Jane' });
+    expect(meta.addTag).toHaveBeenCalledWith({
+      property: 'citation_publication_date',
+      content: '1650-06-26'
+    });
+    expect(meta.addTag).toHaveBeenCalledWith({ property: 'citation_issn', content: '123456789' });
+    expect(meta.addTag).toHaveBeenCalledWith({ property: 'citation_language', content: 'en' });
+    expect(meta.addTag).toHaveBeenCalledWith({
+      property: 'citation_keywords',
+      content: 'keyword1; keyword2; keyword3'
+    });
   }));
 
   it('items page should set meta tags as published Thesis', fakeAsync(() => {
-    spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(mockPublisher(mockType(ItemMock, 'Thesis'))));
-    router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']);
+    (metadataService as any).processRouteChange({
+      data: {
+        value: {
+          dso: createSuccessfulRemoteDataObject(mockPublisher(mockType(ItemMock, 'Thesis'))),
+        }
+      }
+    });
     tick();
-    expect(tagStore.get('citation_dissertation_name')[0].content).toEqual('Test PowerPoint Document');
-    expect(tagStore.get('citation_pdf_url')[0].content).toEqual('https://request.org/bitstreams/99b00f3c-1cc6-4689-8158-91965bee6b28/download');
+    expect(meta.addTag).toHaveBeenCalledWith({
+      property: 'citation_dissertation_name',
+      content: 'Test PowerPoint Document'
+    });
+    expect(meta.addTag).toHaveBeenCalledWith({
+      property: 'citation_pdf_url',
+      content: 'https://request.org/bitstreams/4db100c1-e1f5-4055-9404-9bc3e2d15f29/download'
+    });
   }));
 
   it('items page should set meta tags as published Technical Report', fakeAsync(() => {
-    spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(mockPublisher(mockType(ItemMock, 'Technical Report'))));
-    router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']);
-    tick();
-    expect(tagStore.get('citation_technical_report_institution')[0].content).toEqual('Mock Publisher');
-  }));
-
-  it('other navigation should add title, description and Generator', fakeAsync(() => {
-    spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(ItemMock));
-    spyOn(translateService, 'get').and.returnValues(of('DSpace :: '), of('Dummy Title'), of('This is a dummy item component for testing!'));
-    router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']);
-    tick();
-    expect(tagStore.size).toBeGreaterThan(0);
-    router.navigate(['/other']);
-    tick();
-    expect(tagStore.size).toEqual(3);
-    expect(title.getTitle()).toEqual('DSpace :: Dummy Title');
-    expect(tagStore.get('title')[0].content).toEqual('DSpace :: Dummy Title');
-    expect(tagStore.get('description')[0].content).toEqual('This is a dummy item component for testing!');
-    expect(tagStore.get('Generator')[0].content).toEqual('mock-dspace-version');
-  }));
-
-  describe('when the item has no bitstreams', () => {
-
-    beforeEach(() => {
-      // this.bitstreamDataService.findAllByItemAndBundleName(this.item, 'ORIGINAL')
-      // spyOn(MockItem, 'getFiles').and.returnValue(observableOf([]));
+    (metadataService as any).processRouteChange({
+      data: {
+        value: {
+          dso: createSuccessfulRemoteDataObject(mockPublisher(mockType(ItemMock, 'Technical Report'))),
+        }
+      }
     });
-
-    it('processRemoteData should not produce an EmptyError', fakeAsync(() => {
-      spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(ItemMock));
-      spyOn(metadataService, 'processRemoteData').and.callThrough();
-      router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']);
     tick();
-      expect(metadataService.processRemoteData).not.toThrow(new EmptyError());
+    expect(meta.addTag).toHaveBeenCalledWith({
+      property: 'citation_technical_report_institution',
+      content: 'Mock Publisher'
+    });
   }));
 
+  it('other navigation should add title and description', fakeAsync(() => {
+    (translateService.get as jasmine.Spy).and.returnValues(of('DSpace :: '), of('Dummy Title'), of('This is a dummy item component for testing!'));
+    (metadataService as any).processRouteChange({
+      data: {
+        value: {
+          title: 'Dummy Title',
+          description: 'This is a dummy item component for testing!'
+        }
+      }
+    });
+    tick();
+    expect(title.setTitle).toHaveBeenCalledWith('DSpace :: Dummy Title');
+    expect(meta.addTag).toHaveBeenCalledWith({
+      property: 'title',
+      content: 'DSpace :: Dummy Title'
+    });
+    expect(meta.addTag).toHaveBeenCalledWith({
+      property: 'description',
+      content: 'This is a dummy item component for testing!'
+    });
+  }));
+
+  describe(`listenForRouteChange`, () => {
+    it(`should call processRouteChange`, fakeAsync(() => {
+      spyOn(metadataService as any, 'processRouteChange').and.callFake(() => undefined);
+      metadataService.listenForRouteChange();
+      tick();
+      expect((metadataService as any).processRouteChange).toHaveBeenCalled();
+    }));
+    it(`should add Generator`, fakeAsync(() => {
+      spyOn(metadataService as any, 'processRouteChange').and.callFake(() => undefined);
+      metadataService.listenForRouteChange();
+      tick();
+      expect(meta.addTag).toHaveBeenCalledWith({
+        property: 'Generator',
+        content: 'mock-dspace-version'
+      });
+    }));
   });
 
   describe('citation_abstract_html_url', () => {
     it('should use dc.identifier.uri if available', fakeAsync(() => {
-      spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(mockUri(ItemMock, 'https://ddg.gg')));
-      router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']);
+      (metadataService as any).processRouteChange({
+        data: {
+          value: {
+            dso: createSuccessfulRemoteDataObject(mockUri(ItemMock, 'https://ddg.gg')),
+          }
+        }
+      });
       tick();
-      expect(tagStore.get('citation_abstract_html_url')[0].content).toEqual('https://ddg.gg');
+      expect(meta.addTag).toHaveBeenCalledWith({
+        property: 'citation_abstract_html_url',
+        content: 'https://ddg.gg'
+      });
     }));
 
     it('should use current route as fallback', fakeAsync(() => {
-      spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(mockUri(ItemMock)));
-      router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']);
+      (metadataService as any).processRouteChange({
+        data: {
+          value: {
+            dso: createSuccessfulRemoteDataObject(mockUri(ItemMock)),
+          }
+        }
+      });
       tick();
-       expect(tagStore.get('citation_abstract_html_url')[0].content).toEqual('https://request.org/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357');
+      expect(meta.addTag).toHaveBeenCalledWith({
+        property: 'citation_abstract_html_url',
+        content: 'https://request.org/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357'
+      });
     }));
   });
 
   describe('citation_*_institution / citation_publisher', () => {
     it('should use citation_dissertation_institution tag for dissertations', fakeAsync(() => {
-      spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(mockPublisher(mockType(ItemMock, 'Thesis'))));
-      router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']);
+      (metadataService as any).processRouteChange({
+        data: {
+          value: {
+            dso: createSuccessfulRemoteDataObject(mockPublisher(mockType(ItemMock, 'Thesis'))),
+          }
+        }
+      });
       tick();
-      expect(tagStore.get('citation_dissertation_institution')[0].content).toEqual('Mock Publisher');
-      expect(tagStore.get('citation_technical_report_institution')).toBeFalsy();
-      expect(tagStore.get('citation_publisher')).toBeFalsy();
+      expect(meta.addTag).toHaveBeenCalledWith({
+        property: 'citation_dissertation_institution',
+        content: 'Mock Publisher'
+      });
+      expect(meta.addTag).not.toHaveBeenCalledWith(jasmine.objectContaining({ property: 'citation_technical_report_institution' }));
+      expect(meta.addTag).not.toHaveBeenCalledWith(jasmine.objectContaining({ property: 'citation_publisher' }));
     }));
 
     it('should use citation_tech_report_institution tag for tech reports', fakeAsync(() => {
-      spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(mockPublisher(mockType(ItemMock, 'Technical Report'))));
-      router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']);
+      (metadataService as any).processRouteChange({
+        data: {
+          value: {
+            dso: createSuccessfulRemoteDataObject(mockPublisher(mockType(ItemMock, 'Technical Report'))),
+          }
+        }
+      });
       tick();
-      expect(tagStore.get('citation_dissertation_institution')).toBeFalsy();
-      expect(tagStore.get('citation_technical_report_institution')[0].content).toEqual('Mock Publisher');
-      expect(tagStore.get('citation_publisher')).toBeFalsy();
+      expect(meta.addTag).not.toHaveBeenCalledWith(jasmine.objectContaining({ property: 'citation_dissertation_institution' }));
+      expect(meta.addTag).toHaveBeenCalledWith({
+        property: 'citation_technical_report_institution',
+        content: 'Mock Publisher'
+      });
+      expect(meta.addTag).not.toHaveBeenCalledWith(jasmine.objectContaining({ property: 'citation_publisher' }));
     }));
 
     it('should use citation_publisher for other item types', fakeAsync(() => {
-      spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(mockPublisher(mockType(ItemMock, 'Some Other Type'))));
-      router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']);
+      (metadataService as any).processRouteChange({
+        data: {
+          value: {
+            dso: createSuccessfulRemoteDataObject(mockPublisher(mockType(ItemMock, 'Some Other Type'))),
+          }
+        }
+      });
       tick();
-      expect(tagStore.get('citation_dissertation_institution')).toBeFalsy();
-      expect(tagStore.get('citation_technical_report_institution')).toBeFalsy();
-      expect(tagStore.get('citation_publisher')[0].content).toEqual('Mock Publisher');
+      expect(meta.addTag).not.toHaveBeenCalledWith(jasmine.objectContaining({ property: 'citation_dissertation_institution' }));
+      expect(meta.addTag).not.toHaveBeenCalledWith(jasmine.objectContaining({ property: 'citation_technical_report_institution' }));
+      expect(meta.addTag).toHaveBeenCalledWith({
+        property: 'citation_publisher',
+        content: 'Mock Publisher'
+      });
     }));
   });
 
   describe('citation_pdf_url', () => {
     it('should link to primary Bitstream URL regardless of format', fakeAsync(() => {
-      spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(ItemMock));
-      bundleDataService.findByItemAndName.and.returnValue(mockBundleRD$([], MockBitstream3));
-      router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']);
+      (bundleDataService.findByItemAndName as jasmine.Spy).and.returnValue(mockBundleRD$([], MockBitstream3));
+
+      (metadataService as any).processRouteChange({
+        data: {
+          value: {
+            dso: createSuccessfulRemoteDataObject(ItemMock),
+          }
+        }
+      });
       tick();
-      expect(tagStore.get('citation_pdf_url')[0].content).toEqual('https://request.org/bitstreams/4db100c1-e1f5-4055-9404-9bc3e2d15f29/download');
+      expect(meta.addTag).toHaveBeenCalledWith({
+        property: 'citation_pdf_url',
+        content: 'https://request.org/bitstreams/4db100c1-e1f5-4055-9404-9bc3e2d15f29/download'
+      });
     }));
 
     describe('no primary Bitstream', () => {
       it('should link to first and only Bitstream regardless of format', fakeAsync(() => {
-        spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(ItemMock));
-        bundleDataService.findByItemAndName.and.returnValue(mockBundleRD$([MockBitstream3]));
-        router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']);
+        (bundleDataService.findByItemAndName as jasmine.Spy).and.returnValue(mockBundleRD$([MockBitstream3]));
+
+        (metadataService as any).processRouteChange({
+          data: {
+            value: {
+              dso: createSuccessfulRemoteDataObject(ItemMock),
+            }
+          }
+        });
         tick();
-        expect(tagStore.get('citation_pdf_url')[0].content).toEqual('https://request.org/bitstreams/4db100c1-e1f5-4055-9404-9bc3e2d15f29/download');
+        expect(meta.addTag).toHaveBeenCalledWith({
+          property: 'citation_pdf_url',
+          content: 'https://request.org/bitstreams/4db100c1-e1f5-4055-9404-9bc3e2d15f29/download'
+        });
       }));
 
       it('should link to first Bitstream with allowed format', fakeAsync(() => {
-        spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(ItemMock));
-
         const bitstreams = [MockBitstream3, MockBitstream3, MockBitstream1];
-        bundleDataService.findByItemAndName.and.returnValue(mockBundleRD$(bitstreams));
-        bitstreamDataService.findAllByHref.and.returnValues(
+        (bundleDataService.findByItemAndName as jasmine.Spy).and.returnValue(mockBundleRD$(bitstreams));
+        (bitstreamDataService.findAllByHref as jasmine.Spy).and.returnValues(
           ...mockBitstreamPages$(bitstreams).map(bp => createSuccessfulRemoteDataObject$(bp)),
         );
 
-        router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']);
+        (metadataService as any).processRouteChange({
+          data: {
+            value: {
+              dso: createSuccessfulRemoteDataObject(ItemMock),
+            }
+          }
+        });
         tick();
-        expect(tagStore.get('citation_pdf_url')[0].content).toEqual('https://request.org/bitstreams/cf9b0c8e-a1eb-4b65-afd0-567366448713/download');
+        expect(meta.addTag).toHaveBeenCalledWith({
+          property: 'citation_pdf_url',
+          content: 'https://request.org/bitstreams/cf9b0c8e-a1eb-4b65-afd0-567366448713/download'
+        });
       }));
     });
   });
 
-  const mockRemoteData = (mockItem: Item): Observable<RemoteData<Item>> => {
-    return createSuccessfulRemoteDataObject$(mockItem);
-  };
-
   const mockType = (mockItem: Item, type: string): Item => {
     const typedMockItem = Object.assign(new Item(), mockItem) as Item;
     typedMockItem.metadata['dc.type'] = [{ value: type }] as MetadataValue[];

src/app/core/metadata/metadata.service.ts

@@ -6,11 +6,10 @@ import { ActivatedRoute, NavigationEnd, Router } from '@angular/router';
 import { TranslateService } from '@ngx-translate/core';
 
 import { BehaviorSubject, combineLatest, Observable, of as observableOf, EMPTY } from 'rxjs';
-import { distinctUntilKeyChanged, filter, map, take, switchMap, expand } from 'rxjs/operators';
+import { filter, map, take, switchMap, expand } from 'rxjs/operators';
 
 import { hasValue, hasNoValue } from '../../shared/empty.util';
 import { DSONameService } from '../breadcrumbs/dso-name.service';
-import { CacheableObject } from '../cache/object-cache.reducer';
 import { BitstreamDataService } from '../data/bitstream-data.service';
 import { BitstreamFormatDataService } from '../data/bitstream-format-data.service';
 
@@ -35,11 +34,9 @@ import { HardRedirectService } from '../services/hard-redirect.service';
 @Injectable()
 export class MetadataService {
 
-  private initialized: boolean;
-
   private tagStore: Map<string, MetaDefinition[]>;
 
-  private currentObject: BehaviorSubject<DSpaceObject>;
+  private currentObject: BehaviorSubject<DSpaceObject> = new BehaviorSubject<DSpaceObject>(undefined);
 
   /**
    * When generating the citation_pdf_url meta tag for Items with more than one Bitstream (and no primary Bitstream),
@@ -70,11 +67,13 @@ export class MetadataService {
   ) {
     // TODO: determine what open graph meta tags are needed and whether
     // the differ per route. potentially add image based on DSpaceObject
-    this.initialized = false;
     this.tagStore = new Map<string, MetaDefinition[]>();
   }
 
   public listenForRouteChange(): void {
+    // This never changes, set it only once
+    this.setGenerator();
+
     this.router.events.pipe(
       filter((event) => event instanceof NavigationEnd),
       map(() => this.router.routerState.root),
@@ -86,22 +85,9 @@ export class MetadataService {
     });
   }
 
-  public processRemoteData(remoteData: Observable<RemoteData<CacheableObject>>): void {
-    remoteData.pipe(map((rd: RemoteData<CacheableObject>) => rd.payload),
-      filter((co: CacheableObject) => hasValue(co)),
-      take(1))
-      .subscribe((dspaceObject: DSpaceObject) => {
-        if (!this.initialized) {
-          this.initialize(dspaceObject);
-        }
-        this.currentObject.next(dspaceObject);
-      });
-  }
-
   private processRouteChange(routeInfo: any): void {
-    if (routeInfo.params.value.id === undefined) {
     this.clearMetaTags();
-    }
+
     if (routeInfo.data.value.title) {
       const titlePrefix = this.translate.get('repository.title.prefix');
       const title = this.translate.get(routeInfo.data.value.title, routeInfo.data.value);
@@ -116,15 +102,10 @@ export class MetadataService {
       });
     }
 
-    this.setGenerator();
+    if (hasValue(routeInfo.data.value.dso) && hasValue(routeInfo.data.value.dso.payload)) {
+      this.currentObject.next(routeInfo.data.value.dso.payload);
+      this.setDSOMetaTags();
     }
-
-  private initialize(dspaceObject: DSpaceObject): void {
-    this.currentObject = new BehaviorSubject<DSpaceObject>(dspaceObject);
-    this.currentObject.asObservable().pipe(distinctUntilKeyChanged('uuid')).subscribe(() => {
-      this.setMetaTags();
-    });
-    this.initialized = true;
   }
 
   private getCurrentRoute(route: ActivatedRoute): ActivatedRoute {
@@ -134,9 +115,7 @@ export class MetadataService {
     return route;
   }
 
-  private setMetaTags(): void {
-
-    this.clearMetaTags();
+  private setDSOMetaTags(): void {
 
     this.setTitleTag();
     this.setDescriptionTag();
@@ -415,7 +394,7 @@ export class MetadataService {
    */
   private setGenerator(): void {
     this.rootService.findRoot().pipe(getFirstSucceededRemoteDataPayload()).subscribe((root) => {
-      this.addMetaTag('Generator', root.dspaceVersion);
+      this.meta.addTag({ property: 'Generator', content: root.dspaceVersion });
     });
   }