Merge pull request DSpace#1912 from atmire/Listable-object-decorator-refactor

Listable-object decorator refactor

Author: tdonohue

src/app/shared/object-collection/shared/listable-object/listable-object.decorator.spec.ts

@@ -1,7 +1,7 @@
   /* eslint-disable max-classes-per-file */
 import { Item } from '../../../../core/shared/item.model';
 import { ViewMode } from '../../../../core/shared/view-mode.model';
-import { getListableObjectComponent, listableObjectComponent } from './listable-object.decorator';
+import { DEFAULT_VIEW_MODE, getListableObjectComponent, listableObjectComponent } from './listable-object.decorator';
 import { Context } from '../../../../core/shared/context.model';
 import { environment } from '../../../../../environments/environment';
 
@@ -13,6 +13,10 @@ describe('ListableObject decorator function', () => {
   const type3 = 'TestType3';
   const typeAncestor = 'TestTypeAncestor';
   const typeUnthemed = 'TestTypeUnthemed';
+  const typeLowPriority = 'TypeLowPriority';
+  const typeLowPriority2 = 'TypeLowPriority2';
+  const typeMidPriority = 'TypeMidPriority';
+  const typeHighPriority = 'TypeHighPriority';
 
   class Test1List {
   }
@@ -38,6 +42,21 @@ describe('ListableObject decorator function', () => {
   class TestUnthemedComponent {
   }
 
+  class TestDefaultLowPriorityComponent {
+  }
+
+  class TestLowPriorityComponent {
+  }
+
+  class TestDefaultMidPriorityComponent {
+  }
+
+  class TestMidPriorityComponent {
+  }
+
+  class TestHighPriorityComponent {
+  }
+
   /* eslint-enable max-classes-per-file */
 
   beforeEach(() => {
@@ -54,6 +73,15 @@ describe('ListableObject decorator function', () => {
     listableObjectComponent(typeAncestor, ViewMode.ListElement, Context.Any, 'ancestor')(TestAncestorComponent);
     listableObjectComponent(typeUnthemed, ViewMode.ListElement, Context.Any)(TestUnthemedComponent);
 
+    // Register component with different priorities for expected parameters:
+    // ViewMode.DetailedListElement, Context.Search, 'custom'
+    listableObjectComponent(typeLowPriority, DEFAULT_VIEW_MODE, undefined, undefined)(TestDefaultLowPriorityComponent);
+    listableObjectComponent(typeLowPriority, DEFAULT_VIEW_MODE, Context.Search, 'custom')(TestLowPriorityComponent);
+    listableObjectComponent(typeLowPriority2, DEFAULT_VIEW_MODE, undefined, undefined)(TestDefaultLowPriorityComponent);
+    listableObjectComponent(typeMidPriority, ViewMode.DetailedListElement, undefined, undefined)(TestDefaultMidPriorityComponent);
+    listableObjectComponent(typeMidPriority, ViewMode.DetailedListElement, undefined, 'custom')(TestMidPriorityComponent);
+    listableObjectComponent(typeHighPriority, ViewMode.DetailedListElement, Context.Search, undefined)(TestHighPriorityComponent);
+
     ogEnvironmentThemes = environment.themes;
   });
 
@@ -81,7 +109,7 @@ describe('ListableObject decorator function', () => {
     });
   });
 
-  describe('If there isn\'nt an exact match', () => {
+  describe('If there isn\'t an exact match', () => {
     describe('If there is a match for one of the entity types and the view mode', () => {
       it('should return the class with the matching entity type and view mode and default context', () => {
         const component = getListableObjectComponent([type3], ViewMode.ListElement, Context.Workspace);
@@ -152,4 +180,45 @@ describe('ListableObject decorator function', () => {
       });
     });
   });
+
+  describe('priorities', () => {
+    beforeEach(() => {
+      environment.themes = [
+        {
+          name: 'custom',
+        }
+      ];
+    });
+
+    describe('If a component with default ViewMode contains specific context and/or theme', () => {
+      it('requesting a specific ViewMode should return the one with the requested context and/or theme', () => {
+        const component = getListableObjectComponent([typeLowPriority], ViewMode.DetailedListElement, Context.Search, 'custom');
+        expect(component).toEqual(TestLowPriorityComponent);
+      });
+    });
+
+    describe('If a component with default Context contains specific ViewMode and/or theme', () => {
+      it('requesting a specific Context should return the one with the requested view-mode and/or theme', () => {
+        const component = getListableObjectComponent([typeMidPriority], ViewMode.DetailedListElement, Context.Search, 'custom');
+        expect(component).toEqual(TestMidPriorityComponent);
+      });
+    });
+
+    describe('If multiple components exist, each containing a different default value for one of the requested parameters', () => {
+      it('the component with the latest default value in the list should be returned', () => {
+        let component = getListableObjectComponent([typeMidPriority, typeLowPriority], ViewMode.DetailedListElement, Context.Search, 'custom');
+        expect(component).toEqual(TestMidPriorityComponent);
+
+        component = getListableObjectComponent([typeLowPriority, typeMidPriority, typeHighPriority], ViewMode.DetailedListElement, Context.Search, 'custom');
+        expect(component).toEqual(TestHighPriorityComponent);
+      });
+    });
+
+    describe('If two components exist for two different types, both configured for the same view-mode, but one for a specific context and/or theme', () => {
+      it('requesting a component for that specific context and/or theme while providing both types should return the most relevant one', () => {
+        const component = getListableObjectComponent([typeLowPriority2, typeLowPriority], ViewMode.DetailedListElement, Context.Search, 'custom');
+        expect(component).toEqual(TestLowPriorityComponent);
+      });
+    });
+  });
 });

src/app/shared/object-collection/shared/listable-object/listable-object.decorator.ts

@@ -11,6 +11,53 @@ export const DEFAULT_VIEW_MODE = ViewMode.ListElement;
 export const DEFAULT_CONTEXT = Context.Any;
 export const DEFAULT_THEME = '*';
 
+/**
+ * A class used to compare two matches and their relevancy to determine which of the two gains priority over the other
+ *
+ * "level" represents the index of the first default value that was used to find the match with:
+ * ViewMode being index 0, Context index 1 and theme index 2. Examples:
+ * - If a default value was used for context, but not view-mode and theme, the "level" will be 1
+ * - If a default value was used for view-mode and context, but not for theme, the "level" will be 0
+ * - If no default value was used for any of the fields, the "level" will be 3
+ *
+ * "relevancy" represents the amount of values that didn't require a default value to fall back on. Examples:
+ * - If a default value was used for theme, but not view-mode and context, the "relevancy" will be 2
+ * - If a default value was used for view-mode and context, but not for theme, the "relevancy" will be 1
+ * - If a default value was used for all fields, the "relevancy" will be 0
+ * - If no default value was used for any of the fields, the "relevancy" will be 3
+ *
+ * To determine which of two MatchRelevancies is the most relevant, we compare "level" and "relevancy" in that order.
+ * If any of the two is higher than the other, that match is most relevant. Examples:
+ * - { level: 1, relevancy: 1 } is more relevant than { level: 0, relevancy: 2 }
+ * - { level: 1, relevancy: 1 } is less relevant than { level: 1, relevancy: 2 }
+ * - { level: 1, relevancy: 1 } is more relevant than { level: 1, relevancy: 0 }
+ * - { level: 1, relevancy: 1 } is less relevant than { level: 2, relevancy: 0 }
+ * - { level: 1, relevancy: 1 } is more relevant than null
+ */
+class MatchRelevancy {
+  constructor(public match: any,
+              public level: number,
+              public relevancy: number) {
+  }
+
+  isMoreRelevantThan(otherMatch: MatchRelevancy): boolean {
+    if (hasNoValue(otherMatch)) {
+      return true;
+    }
+    if (otherMatch.level > this.level) {
+      return false;
+    }
+    if (otherMatch.level === this.level && otherMatch.relevancy > this.relevancy) {
+      return false;
+    }
+    return true;
+  }
+
+  isLessRelevantThan(otherMatch: MatchRelevancy): boolean {
+    return !this.isMoreRelevantThan(otherMatch);
+  }
+}
+
 /**
  * Factory to allow us to inject getThemeConfigFor so we can mock it in tests
  */
@@ -48,47 +95,70 @@ export function listableObjectComponent(objectType: string | GenericConstructor<
 
 /**
  * Getter to retrieve the matching listable object component
+ *
+ * Looping over the provided types, it'll attempt to find the best match depending on the {@link MatchRelevancy} returned by getMatch()
+ * The most relevant match between types is kept and eventually returned
+ *
  * @param types The types of which one should match the listable component
  * @param viewMode The view mode that should match the components
  * @param context The context that should match the components
  * @param theme The theme that should match the components
  */
 export function getListableObjectComponent(types: (string | GenericConstructor<ListableObject>)[], viewMode: ViewMode, context: Context = DEFAULT_CONTEXT, theme: string = DEFAULT_THEME) {
-  let bestMatch;
-  let bestMatchValue = 0;
+  let currentBestMatch: MatchRelevancy = null;
   for (const type of types) {
     const typeMap = map.get(type);
     if (hasValue(typeMap)) {
-      const typeModeMap = typeMap.get(viewMode);
-      if (hasValue(typeModeMap)) {
-        const contextMap = typeModeMap.get(context);
-        if (hasValue(contextMap)) {
-          const match = resolveTheme(contextMap, theme);
-          if (hasValue(match)) {
-            return match;
-          }
-          if (bestMatchValue < 3 && hasValue(contextMap.get(DEFAULT_THEME))) {
-            bestMatchValue = 3;
-            bestMatch = contextMap.get(DEFAULT_THEME);
-          }
-        }
-        if (bestMatchValue < 2 &&
-          hasValue(typeModeMap.get(DEFAULT_CONTEXT)) &&
-          hasValue(typeModeMap.get(DEFAULT_CONTEXT).get(DEFAULT_THEME))) {
-          bestMatchValue = 2;
-          bestMatch = typeModeMap.get(DEFAULT_CONTEXT).get(DEFAULT_THEME);
-        }
+      const match = getMatch(typeMap, [viewMode, context, theme], [DEFAULT_VIEW_MODE, DEFAULT_CONTEXT, DEFAULT_THEME]);
+      if (hasNoValue(currentBestMatch) || currentBestMatch.isLessRelevantThan(match)) {
+        currentBestMatch = match;
+      }
+    }
+  }
+  return hasValue(currentBestMatch) ? currentBestMatch.match : null;
+}
+
+/**
+ * Find an object within a nested map, matching the provided keys as best as possible, falling back on defaults wherever
+ * needed.
+ *
+ * Starting off with a Map, it loops over the provided keys, going deeper into the map until it finds a value
+ * If at some point, no value is found, it'll attempt to use the default value for that index instead
+ * If the default value exists, the index is stored in the "level"
+ * If no default value exists, 1 is added to "relevancy"
+ * See {@link MatchRelevancy} what these represent
+ *
+ * @param typeMap         a multi-dimensional map
+ * @param keys            the keys of the multi-dimensional map to loop over. Each key represents a level within the map
+ * @param defaults        the default values to use for each level, in case no value is found for the key at that index
+ * @returns matchAndLevel a {@link MatchRelevancy} object containing the match and its level of relevancy
+ */
+function getMatch(typeMap: Map<any, any>, keys: any[], defaults: any[]): MatchRelevancy {
+  let currentMap = typeMap;
+  let level = -1;
+  let relevancy = 0;
+  for (let i = 0; i < keys.length; i++) {
+    // If we're currently checking the theme, resolve it first to take extended themes into account
+    let currentMatch = defaults[i] === DEFAULT_THEME ? resolveTheme(currentMap, keys[i]) : currentMap.get(keys[i]);
+    if (hasNoValue(currentMatch)) {
+      currentMatch = currentMap.get(defaults[i]);
+      if (level === -1) {
+        level = i;
       }
-      if (bestMatchValue < 1 &&
-        hasValue(typeMap.get(DEFAULT_VIEW_MODE)) &&
-        hasValue(typeMap.get(DEFAULT_VIEW_MODE).get(DEFAULT_CONTEXT)) &&
-        hasValue(typeMap.get(DEFAULT_VIEW_MODE).get(DEFAULT_CONTEXT).get(DEFAULT_THEME))) {
-        bestMatchValue = 1;
-        bestMatch = typeMap.get(DEFAULT_VIEW_MODE).get(DEFAULT_CONTEXT).get(DEFAULT_THEME);
+    } else {
+      relevancy++;
+    }
+    if (hasValue(currentMatch)) {
+      if (currentMatch instanceof Map) {
+        currentMap = currentMatch as Map<any, any>;
+      } else {
+        return new MatchRelevancy(currentMatch, level > -1 ? level : i + 1, relevancy);
       }
+    } else {
+      return null;
     }
   }
-  return bestMatch;
+  return null;
 }
 
 /**