fix dataspace elements with package (finos#3487)

.changeset/great-mayflies-complain.md

@@ -0,0 +1,6 @@
+---
+'@finos/legend-extension-dsl-data-space': patch
+'@finos/legend-graph': patch
+---
+
+Fix resolving elements in packages for fixing in dataspace elements render.

.changeset/little-pigs-invite.md

@@ -0,0 +1,5 @@
+---
+'@finos/legend-extension-dsl-data-space': patch
+'@finos/legend-query-builder': patch
+'@finos/legend-graph': patch
+---

packages/legend-extension-dsl-data-space/src/stores/query-builder/DataSpaceQueryBuilderState.ts

@@ -23,17 +23,17 @@ import {
   QueryBuilderState,
 } from '@finos/legend-query-builder';
 import {
+  type Class,
   type GraphManagerState,
+  getMappingCompatibleClasses,
+  RuntimePointer,
   type QueryExecutionContext,
   type Runtime,
   type Mapping,
-  getMappingCompatibleClasses,
-  RuntimePointer,
-  Class,
-  getDescendantsOfPackage,
   Package,
   QueryDataSpaceExecutionContext,
   Service,
+  elementBelongsToPackage,
 } from '@finos/legend-graph';
 import {
   type DepotServerClient,
@@ -47,7 +47,6 @@ import {
   assertErrorThrown,
   getNullableFirstEntry,
   filterByType,
-  uniq,
 } from '@finos/legend-shared';
 import { action, flow, makeObservable, observable } from 'mobx';
 import { renderDataSpaceQueryBuilderSetupPanelContent } from '../../components/query-builder/DataSpaceQueryBuilder.js';
@@ -56,6 +55,7 @@ import {
   type DataSpaceExecutable,
   DataSpace,
   DataSpacePackageableElementExecutable,
+  type DataSpaceElement,
 } from '../../graph/metamodel/pure/model/packageableElements/dataSpace/DSL_DataSpace_DataSpace.js';
 import { DATA_SPACE_ELEMENT_CLASSIFIER_PATH } from '../../graph-manager/protocol/pure/DSL_DataSpace_PureProtocolProcessorPlugin.js';
 import { DataSpaceAdvancedSearchState } from '../query/DataSpaceAdvancedSearchState.js';
@@ -67,23 +67,44 @@ import {
 import type { ProjectGAVCoordinates } from '@finos/legend-storage';
 import { generateDataSpaceTemplateQueryCreatorRoute } from '../../__lib__/to-delete/DSL_DataSpace_LegendQueryNavigation_to_delete.js';
 
+const matchesDataElement = (
+  _class: Class,
+  element: DataSpaceElement,
+): boolean => {
+  if (_class === element) {
+    return true;
+  }
+  if (element instanceof Package) {
+    return elementBelongsToPackage(_class, element);
+  }
+  return false;
+};
+
 export const resolveUsableDataSpaceClasses = (
   dataSpace: DataSpace,
   mapping: Mapping,
   graphManagerState: GraphManagerState,
 ): Class[] => {
+  const compatibleClasses = getMappingCompatibleClasses(
+    mapping,
+    graphManagerState.usableClasses,
+  );
   if (dataSpace.elements?.length) {
-    const dataSpaceElements = dataSpace.elements.map((ep) => ep.element.value);
-    return uniq([
-      ...dataSpaceElements.filter(filterByType(Class)),
-      ...dataSpaceElements
-        .filter(filterByType(Package))
-        .map((_package) => Array.from(getDescendantsOfPackage(_package)))
-        .flat()
-        .filter(filterByType(Class)),
-    ]);
+    const elements = dataSpace.elements;
+    return compatibleClasses.filter((_class) => {
+      const _classElements = elements
+        .filter((e) => matchesDataElement(_class, e.element.value))
+        // we sort because we respect the closest definition to the element.
+        .sort(
+          (a, b) => b.element.value.path.length - a.element.value.path.length,
+        );
+      if (!_classElements.length) {
+        return false;
+      }
+      return !_classElements[0]?.exclude;
+    });
   }
-  return getMappingCompatibleClasses(mapping, graphManagerState.usableClasses);
+  return compatibleClasses;
 };
 export interface DataSpaceQuerySDLC extends QuerySDLC {
   groupId: string;

packages/legend-graph/src/graph/DependencyManager.ts

@@ -18,6 +18,7 @@ import {
   type Clazz,
   guaranteeNonNullable,
   IllegalStateError,
+  isNonNullable,
 } from '@finos/legend-shared';
 import type { PackageableElement } from '../graph/metamodel/pure/packageableElements/PackageableElement.js';
 import type { Enumeration } from '../graph/metamodel/pure/packageableElements/domain/Enumeration.js';
@@ -331,6 +332,12 @@ export class DependencyManager {
     return model?.getOwnNullableElement(path, includePackage);
   }
 
+  getPackages(path: string): Package[] {
+    return this.dependencyGraphs
+      .map((dep) => dep.getNullablePackage(path))
+      .filter(isNonNullable);
+  }
+
   getElementOrigin(element: PackageableElement): string | undefined {
     const model = this.dependencyGraphs.find((dep) =>
       Boolean(dep.getOwnNullableElement(element.path)),

packages/legend-graph/src/graph/PureModel.ts

@@ -25,6 +25,7 @@ import {
   guaranteeType,
   returnUndefOnError,
   IllegalStateError,
+  isNonNullable,
 } from '@finos/legend-shared';
 import { PrimitiveType } from '../graph/metamodel/pure/packageableElements/domain/PrimitiveType.js';
 import { Enumeration } from '../graph/metamodel/pure/packageableElements/domain/Enumeration.js';
@@ -579,6 +580,15 @@ export class PureModel extends BasicModel {
     return element;
   }
 
+  getPackages(path: string): Package[] {
+    return [
+      this.getNullablePackage(path),
+      ...this.dependencyManager.getPackages(path),
+      this.generationModel.getNullablePackage(path),
+      this.systemModel.getNullablePackage(path),
+    ].filter(isNonNullable);
+  }
+
   getMultiplicity(
     lowerBound: number,
     upperBound: number | undefined,

packages/legend-graph/src/graph/helpers/DomainHelper.ts

@@ -88,9 +88,7 @@ export const deleteElementFromPackage = (
   );
 };
 
-export const getDescendantsOfPackage = (
-  parent: Package,
-): Set<PackageableElement> => {
+const getDescendantsOfPackage = (parent: Package): Set<PackageableElement> => {
   const descendants: Set<PackageableElement> = new Set<PackageableElement>();
   parent.children.forEach((c) => {
     if (c instanceof Package) {
@@ -102,6 +100,32 @@ export const getDescendantsOfPackage = (
   return descendants;
 };
 
+export const getAllDescendantsOfPackage = (
+  parent: Package,
+  graph: PureModel,
+): Set<PackageableElement> =>
+  new Set(
+    graph
+      .getPackages(parent.path)
+      .map((p) => [...getDescendantsOfPackage(p)])
+      .flat(),
+  );
+
+export const elementBelongsToPackage = (
+  element: PackageableElement,
+  parent: Package,
+): boolean => {
+  const elementPackage = element instanceof Package ? element : element.package;
+  if (!elementPackage) {
+    return false;
+  }
+  const elementPackagePath = elementPackage.path;
+  const parentPackage = parent.path;
+  return (elementPackagePath + ELEMENT_PATH_DELIMITER).startsWith(
+    parentPackage + ELEMENT_PATH_DELIMITER,
+  );
+};
+
 export const getElementRootPackage = (element: PackageableElement): Package =>
   !element.package
     ? guaranteeType(element, Package)

packages/legend-manual-tests/src/__tests__/extensions/DSL_DataSpace/DSL_DataSpaceTest.test.tsx

@@ -0,0 +1,180 @@
+/**
+ * Copyright (c) 2020-present, Goldman Sachs
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { expect, test } from '@jest/globals';
+import { resolve } from 'path';
+import { createGraphManagerStateFromGrammar } from '../../utils/testUtils.js';
+import { integrationTest } from '@finos/legend-shared/test';
+import { PackageableElementExplicitReference } from '@finos/legend-graph';
+import { guaranteeNonNullable, guaranteeType } from '@finos/legend-shared';
+import {
+  DataSpace,
+  DataSpaceElementPointer,
+  resolveUsableDataSpaceClasses,
+} from '@finos/legend-extension-dsl-data-space/graph';
+
+test(integrationTest('TEST_DATA_Dataspace-Executables'), async () => {
+  const { modelFileDir, modelFilePath } = {
+    modelFileDir: 'model',
+    modelFilePath: 'TEST_DATA_Dataspace-Executables.pure',
+  };
+  const graphManagerState = await createGraphManagerStateFromGrammar(
+    resolve(__dirname, modelFileDir),
+    modelFilePath,
+  );
+  const graph = graphManagerState.graph;
+  const element = graphManagerState.graph.getElement(
+    'showcase::northwind::dataspace::NorthwindDataSpaceWithExecutables',
+  );
+  expect(element instanceof DataSpace).toEqual(true);
+  const dataspace = guaranteeType(element, DataSpace);
+  expect(dataspace.executables).toHaveLength(3);
+  const defaultMapping = dataspace.defaultExecutionContext.mapping.value;
+  expect(defaultMapping.path).toEqual(
+    'showcase::northwind::mapping::NorthwindMapping',
+  );
+  let usableClasses = resolveUsableDataSpaceClasses(
+    dataspace,
+    defaultMapping,
+    graphManagerState,
+  );
+  let expectedClasses = [
+    'showcase::northwind::model::inventory::Product',
+    'showcase::northwind::model::geography::SalesRegion',
+    'showcase::northwind::model::crm::Employee',
+    'showcase::northwind::model::geography::USState',
+    'showcase::northwind::model::inventory::ProductCategory',
+    'showcase::northwind::model::inventory::Supplier',
+    'showcase::northwind::model::OrderLineItem',
+    'showcase::northwind::model::crm::ShippingCompany',
+    'showcase::northwind::model::crm::Customer',
+    'showcase::northwind::model::geography::SalesTerritory',
+    'showcase::northwind::model::Order',
+  ];
+
+  expect(expectedClasses.sort()).toEqual(
+    usableClasses.map((e) => e.path).sort(),
+  );
+  // 1. include package
+  const filterPackage = guaranteeNonNullable(
+    graph.getNullablePackage('showcase::northwind::model::inventory'),
+  );
+  const elementPointer = new DataSpaceElementPointer();
+  elementPointer.element =
+    PackageableElementExplicitReference.create(filterPackage);
+  dataspace.elements = [elementPointer];
+  usableClasses = resolveUsableDataSpaceClasses(
+    dataspace,
+    defaultMapping,
+    graphManagerState,
+  );
+  expectedClasses = [
+    'showcase::northwind::model::inventory::Product',
+    'showcase::northwind::model::inventory::ProductCategory',
+    'showcase::northwind::model::inventory::Supplier',
+  ];
+  expect(expectedClasses.sort()).toEqual(
+    usableClasses.map((e) => e.path).sort(),
+  );
+  // 2. include package and class
+  const _moreClass = graphManagerState.graph.getClass(
+    'showcase::northwind::model::geography::SalesTerritory',
+  );
+  const classPointer = new DataSpaceElementPointer();
+  classPointer.element = PackageableElementExplicitReference.create(_moreClass);
+  dataspace.elements.push(classPointer);
+  expectedClasses.push('showcase::northwind::model::geography::SalesTerritory');
+  usableClasses = resolveUsableDataSpaceClasses(
+    dataspace,
+    defaultMapping,
+    graphManagerState,
+  );
+  expect(expectedClasses.sort()).toEqual(
+    usableClasses.map((e) => e.path).sort(),
+  );
+  // 3. include package and exclude class
+  classPointer.exclude = true;
+  usableClasses = resolveUsableDataSpaceClasses(
+    dataspace,
+    defaultMapping,
+    graphManagerState,
+  );
+  expectedClasses = [
+    'showcase::northwind::model::inventory::Product',
+    'showcase::northwind::model::inventory::ProductCategory',
+    'showcase::northwind::model::inventory::Supplier',
+  ];
+  expect(expectedClasses.sort()).toEqual(
+    usableClasses.map((e) => e.path).sort(),
+  );
+  // 4 filter by model package, then add a package included in the node as an exclude, then add a class as includes. We respect the more explicit declaration here.
+  const modelPackage = guaranteeNonNullable(
+    graph.getNullablePackage('showcase::northwind::model'),
+  );
+  const modelPackagePointer = new DataSpaceElementPointer();
+  modelPackagePointer.element =
+    PackageableElementExplicitReference.create(modelPackage);
+  dataspace.elements = [modelPackagePointer];
+  usableClasses = resolveUsableDataSpaceClasses(
+    dataspace,
+    defaultMapping,
+    graphManagerState,
+  );
+  expectedClasses = [
+    'showcase::northwind::model::inventory::Product',
+    'showcase::northwind::model::geography::SalesRegion',
+    'showcase::northwind::model::crm::Employee',
+    'showcase::northwind::model::geography::USState',
+    'showcase::northwind::model::inventory::ProductCategory',
+    'showcase::northwind::model::inventory::Supplier',
+    'showcase::northwind::model::OrderLineItem',
+    'showcase::northwind::model::crm::ShippingCompany',
+    'showcase::northwind::model::crm::Customer',
+    'showcase::northwind::model::geography::SalesTerritory',
+    'showcase::northwind::model::Order',
+  ];
+
+  expect(expectedClasses.sort()).toEqual(
+    usableClasses.map((e) => e.path).sort(),
+  );
+  const inventoryPackage = guaranteeNonNullable(
+    graph.getNullablePackage('showcase::northwind::model::inventory'),
+  );
+  const inventoryPackagePointer = new DataSpaceElementPointer();
+  inventoryPackagePointer.element =
+    PackageableElementExplicitReference.create(inventoryPackage);
+  inventoryPackagePointer.exclude = true;
+  dataspace.elements.push(inventoryPackagePointer);
+  usableClasses = resolveUsableDataSpaceClasses(
+    dataspace,
+    defaultMapping,
+    graphManagerState,
+  );
+  expect(usableClasses).toHaveLength(8);
+  const productCategory = graphManagerState.graph.getClass(
+    'showcase::northwind::model::inventory::ProductCategory',
+  );
+  const productCategoryPtr = new DataSpaceElementPointer();
+  productCategoryPtr.element =
+    PackageableElementExplicitReference.create(productCategory);
+  dataspace.elements.push(productCategoryPtr);
+  usableClasses = resolveUsableDataSpaceClasses(
+    dataspace,
+    defaultMapping,
+    graphManagerState,
+  );
+  expect(usableClasses).toHaveLength(9);
+});