Refactor IDService and add unit tests

No longer is a singleton but a normal servicce now again. The singleton 
logic was needed in the past but not anymore.

projects/editor/src/app/components/unit-view/page-view/properties-panel/element-model-properties.component.ts

@@ -83,7 +83,7 @@ export class ElementModelPropertiesComponent {
   }
 
   addRow(question: string): void {
-    const newRow = UnitService.createLikertRow(
+    const newRow = this.unitService.createLikertRow(
       question,
       (this.combinedProperties.columns as LikertColumn[]).length
     );

projects/editor/src/app/services/id.service.spec.ts

+import { TestBed } from '@angular/core/testing';
+import { IdService } from './id.service';
+
+describe('IDService', () => {
+  let service: IdService;
+
+  beforeEach(() => {
+    TestBed.configureTestingModule({ providers: [IdService] });
+    service = TestBed.inject(IdService);
+  });
+
+  it('getNewID should fail on empty string param', () => {
+    expect(() => { service.getNewID(''); }).toThrow(Error('ID-Service: No type given!'));
+  });
+
+  it('getNewID should return first ID', () => {
+    expect(service.getNewID('text')).toBe('text_1');
+  });
+
+  it('getNewID should return different IDs counting up', () => {
+    service.getNewID('text');
+    expect(service.getNewID('text')).toBe('text_2');
+  });
+
+  it('service should return next id when one is already taken', () => {
+    service.addID('text_1');
+    expect(service.getNewID('text')).toBe('text_2');
+  });
+
+  it('isIdAvailable should return false when id is already taken', () => {
+    expect(service.isIdAvailable('text_1')).toBe(true);
+    service.addID('text_1');
+    expect(service.isIdAvailable('text_1')).toBe(false);
+    expect(service.isIdAvailable('text_2')).toBe(true);
+  });
+
+  it('isIdAvailable should return true when ID is returned (freed up)', () => {
+    expect(service.isIdAvailable('text_1')).toBe(true);
+    service.addID('text_1');
+    expect(service.isIdAvailable('text_1')).toBe(false);
+    service.removeId('text_1');
+    expect(service.isIdAvailable('text_1')).toBe(true);
+  });
+});

projects/editor/src/app/services/id.service.ts

-export class IdService {
-  private static instance: IdService;
+import { Injectable } from '@angular/core';
 
+@Injectable({
+  providedIn: 'root'
+})
+export class IdService {
   private givenIDs: string[] = [];
   private idCounter: Record<string, number> = {
     text: 0,
@@ -26,13 +29,6 @@ export class IdService {
     value: 0
   };
 
-  static getInstance(): IdService {
-    if (!IdService.instance) {
-      IdService.instance = new IdService();
-    }
-    return IdService.instance;
-  }
-
   getNewID(type: string): string {
     if (!type) {
       throw Error('ID-Service: No type given!');
@@ -53,15 +49,8 @@ export class IdService {
     return !this.givenIDs.includes(value);
   }
 
-  addId(id: string): void {
-    this.givenIDs.push(id);
-  }
-
   /* Remove ID from givenIDs, so it can be used again. */
   removeId(id: string): void {
-    const index = this.givenIDs.indexOf(id, 0);
-    if (index > -1) {
-      this.givenIDs.splice(index, 1);
-    }
+    this.givenIDs.splice(this.givenIDs.indexOf(id, 0), 1);
   }
 }

projects/editor/src/app/services/unit.service.ts

@@ -37,6 +37,7 @@ export class UnitService {
   pageMoved: Subject<void> = new Subject<void>();
 
   constructor(private selectionService: SelectionService,
+              private idService: IdService,
               private veronaApiService: VeronaAPIService,
               private messageService: MessageService,
               private dialogService: DialogService,
@@ -58,10 +59,10 @@ export class UnitService {
     this.unitModel.pages.forEach((page: Page) => {
       page.sections.forEach((section: Section) => {
         section.elements.forEach((element: UIElement) => {
-          IdService.getInstance().addID(element.id);
+          this.idService.addID(element.id);
           if (element.type === 'drop-list') {
             element.value?.forEach((valueElement: DragNDropValueObject) => {
-              IdService.getInstance().addID(valueElement.id);
+              this.idService.addID(valueElement.id);
             });
           }
         });
@@ -125,7 +126,7 @@ export class UnitService {
   duplicateSection(section: Section, page: Page, sectionIndex: number): void {
     const newSection = new Section(section);
     newSection.elements.forEach((element: UIElement) => {
-      element.id = IdService.getInstance().getNewID(element.type);
+      element.id = this.idService.getNewID(element.type);
     });
     page.sections.splice(sectionIndex + 1, 0, newSection);
 
@@ -165,7 +166,7 @@ export class UnitService {
       }
       newElement = ElementFactory.createElement({
         type: elementType,
-        id: IdService.getInstance().getNewID(elementType),
+        id: this.idService.getNewID(elementType),
         src: mediaSrc,
         positionProps: {
           dynamicPositioning: section.dynamicPositioning
@@ -174,7 +175,7 @@ export class UnitService {
     } else {
       newElement = ElementFactory.createElement({
         type: elementType,
-        id: IdService.getInstance().getNewID(elementType),
+        id: this.idService.getNewID(elementType),
         positionProps: {
           dynamicPositioning: section.dynamicPositioning
         }
@@ -194,21 +195,21 @@ export class UnitService {
   }
 
   deleteElements(elements: UIElement[]): void {
-    UnitService.freeUpIds(elements);
+    this.freeUpIds(elements);
     this.unitModel.pages[this.selectionService.selectedPageIndex].sections.forEach(section => {
       section.deleteElements(elements);
     });
     this.veronaApiService.sendVoeDefinitionChangedNotification();
   }
 
-  private static freeUpIds(elements: UIElement[]): void {
+  private freeUpIds(elements: UIElement[]): void {
     elements.forEach(element => {
       if (element.type === 'drop-list') {
         element.value.forEach((value: DragNDropValueObject) => {
-          IdService.getInstance().removeId(value.id);
+          this.idService.removeId(value.id);
         });
       }
-      IdService.getInstance().removeId(element.id);
+      this.idService.removeId(element.id);
     });
   }
 
@@ -247,14 +248,14 @@ export class UnitService {
     console.log('updateElementProperty', elements, property, value);
     for (const element of elements) {
       if (property === 'id') {
-        if (!IdService.getInstance().isIdAvailable((value as string))) { // prohibit existing IDs
+        if (!this.idService.isIdAvailable((value as string))) { // prohibit existing IDs
           this.messageService.showError(this.translateService.instant('idTaken'));
           return false;
         }
-        IdService.getInstance().removeId(element.id);
-        IdService.getInstance().addId(<string>value);
+        this.idService.removeId(element.id);
+        this.idService.addID(<string>value);
       } else if (property === 'text' && element.type === 'cloze') {
-        element.setProperty('parts', ClozeParser.parse(value as string, IdService.getInstance()));
+        element.setProperty('parts', ClozeParser.parse(value as string, this.idService));
       } else {
         element.setProperty(property, value);
       }
@@ -280,7 +281,7 @@ export class UnitService {
     await this.dialogService.showDropListOptionEditDialog(oldOptions[optionIndex])
       .subscribe((result: DragNDropValueObject) => {
         if (result) {
-          if (result.id !== oldOptions[optionIndex].id && !IdService.getInstance().isIdAvailable(result.id)) {
+          if (result.id !== oldOptions[optionIndex].id && !this.idService.isIdAvailable(result.id)) {
             this.messageService.showError(this.translateService.instant('idTaken'));
             return;
           }
@@ -341,11 +342,11 @@ export class UnitService {
     };
   }
 
-  static createLikertRow(question: string, columnCount: number): LikertElementRow {
+  createLikertRow(question: string, columnCount: number): LikertElementRow {
     return new LikertElementRow(
       {
         type: 'likert_row',
-        id: IdService.getInstance().getNewID('likert_row'),
+        id: this.idService.getNewID('likert_row'),
         text: question,
         columnCount: columnCount
       } as LikertElementRow
@@ -442,6 +443,6 @@ export class UnitService {
   }
 
   getNewValueID(): string {
-    return IdService.getInstance().getNewID('value');
+    return this.idService.getNewID('value');
   }
 }