Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4352] Add support to range column filters in table #4353

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

=== Improvements


- https://github.com/eclipse-sirius/sirius-web/issues/4352[#4352] [table] Add support to range column filters in table

== v2025.1.0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import java.net.URLDecoder;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -56,10 +55,30 @@ public Map<String, List<String>> getParameterValues(String kind) {
@Override
public List<String> getParameterEntries(String value) {
if (value.startsWith("[") && value.endsWith("]") && value.length() >= 3) {
var rawValue = value.substring(1);
rawValue = rawValue.substring(0, rawValue.indexOf("]"));
return Arrays.stream(rawValue.split(",")).toList();
var rawValue = value.substring(1, value.length() - 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain me what this change of behavior supposed to do? It's a bit complex to understand just from a Quick Look at the code, what is the difference in the old vs new behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the filter values of a column can be a list, we end up with uri of this type [467ec1c3-8ba7-32dc-9d72-71a2ccad161b:["0","1"],4752c1c3-8ba7-32dc-9d72-71a2ccad161b:["1","2"]]
The previous implementation of getParameterEntries split each , whiche end up giving something like 467ec1c3-8ba7-32dc-9d72-71a2ccad161b:["0" ,"1"] ...
In the new approach, I consider the level of [ to only split the first list.

return this.parseEntries(rawValue);
}
return List.of();
}

private List<String> parseEntries(String rawValue) {
List<String> entries = new ArrayList<>();
int start = 0;
int level = 0;

for (int i = 0; i < rawValue.length(); i++) {
char c = rawValue.charAt(i);
if (c == '[') {
level++;
} else if (c == ']') {
level--;
} else if (c == ',' && level == 0) {
entries.add(rawValue.substring(start, i).trim());
start = i + 1;
}
}
entries.add(rawValue.substring(start).trim());

return entries;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*******************************************************************************
* Copyright (c) 2024 CEA LIST.
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v2.0
* which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Obeo - initial API and implementation
*******************************************************************************/
package org.eclipse.sirius.components.core;

import static org.assertj.core.api.Assertions.assertThat;

import java.util.List;

import org.junit.jupiter.api.Test;

/**
* Tests of the URLParser.
*
* @author frouene
*/
public class URLParserTests {

@Test
public void testGetParameterEntriesWithNestedLists() {
URLParser parser = new URLParser();
String value = "[467ec1c3-8ba7-32dc-9d72-71a2ccad161b:[\"1\",\"\"],4752c1c3-8ba7-32dc-9d72-71a2ccad161b:[\"1\",\"2\"]]";
List<String> expected = List.of(
"467ec1c3-8ba7-32dc-9d72-71a2ccad161b:[\"1\",\"\"]",
"4752c1c3-8ba7-32dc-9d72-71a2ccad161b:[\"1\",\"2\"]"
);
List<String> result = parser.getParameterEntries(value);
assertThat(expected).isEqualTo(result);
}

@Test
public void testGetParameterEntriesWithEmptyList() {
URLParser parser = new URLParser();
String value = "[]";
List<String> expected = List.of();
List<String> result = parser.getParameterEntries(value);
assertThat(expected).isEqualTo(result);
}

@Test
public void testGetParameterEntriesWithSingleElement() {
URLParser parser = new URLParser();
String value = "[467ec1c3-8ba7-32dc-9d72-71a2ccad161b:[\"1\",\"\"]]";
List<String> expected = List.of("467ec1c3-8ba7-32dc-9d72-71a2ccad161b:[\"1\",\"\"]");
List<String> result = parser.getParameterEntries(value);
assertThat(expected).isEqualTo(result);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,14 @@
import org.eclipse.sirius.components.core.api.IEditingContextRepresentationDescriptionProvider;
import org.eclipse.sirius.components.core.api.IIdentityService;
import org.eclipse.sirius.components.core.api.ILabelService;
import org.eclipse.sirius.components.papaya.AnnotableElement;
import org.eclipse.sirius.components.papaya.PapayaFactory;
import org.eclipse.sirius.components.papaya.PapayaPackage;
import org.eclipse.sirius.components.papaya.Type;
import org.eclipse.sirius.components.papaya.spec.PackageSpec;
import org.eclipse.sirius.components.representations.IRepresentationDescription;
import org.eclipse.sirius.components.representations.VariableManager;
import org.eclipse.sirius.components.tables.ColumnFilter;
import org.eclipse.sirius.components.tables.ColumnFilterMapped;
import org.eclipse.sirius.components.tables.descriptions.ColumnDescription;
import org.eclipse.sirius.components.tables.descriptions.ICellDescription;
import org.eclipse.sirius.components.tables.descriptions.IconLabelCellDescription;
Expand Down Expand Up @@ -124,7 +125,7 @@ private PaginatedData getSemanticElements(VariableManager variableManager) {
var direction = variableManager.get(TableRenderer.PAGINATION_DIRECTION, String.class).orElse(null);
var size = variableManager.get(TableRenderer.PAGINATION_SIZE, Integer.class).orElse(0);
var globalFilter = variableManager.get(TableRenderer.GLOBAL_FILTER_DATA, String.class).orElse(null);
List<ColumnFilter> columnFilters = variableManager.get(TableRenderer.COLUMN_FILTERS, List.class).orElse(List.of());
List<ColumnFilterMapped> columnFilters = variableManager.get(TableRenderer.COLUMN_FILTERS, List.class).orElse(List.of());

Predicate<EObject> predicate = eObject -> {
boolean isValidCandidate = eObject instanceof Type && EcoreUtil.isAncestor(self, eObject);
Expand All @@ -137,10 +138,28 @@ private PaginatedData getSemanticElements(VariableManager variableManager) {
isValidCandidate = isValidCandidate || type.getAnnotations().stream().anyMatch(annotation -> annotation.getName().contains(globalFilter));
}
isValidCandidate = isValidCandidate && columnFilters.stream().allMatch(columnFilter -> {
if (columnFilter.id().equals("papaya.NamedElement#name")) {
return type.getName() != null && type.getName().contains(columnFilter.value());
boolean isValidColumFilterCandidat = true;
if (columnFilter.id().equals("papaya.NamedElement#name") && !columnFilter.values().isEmpty()) {
isValidColumFilterCandidat = type.getName() != null && type.getName().contains(columnFilter.values().get(0));
} else if (columnFilter.id().equals("papaya.Type#nbAnnotation") && columnFilter.values().size() == 2) {
int nbAnnotation = type.getAnnotations().size();
if (columnFilter.values().get(0) != null && !columnFilter.values().get(0).isBlank()) {
try {
int minValue = Integer.parseInt(columnFilter.values().get(0));
isValidColumFilterCandidat = minValue <= nbAnnotation;
} catch (NumberFormatException ignored) {
}
}
if (columnFilter.values().get(1) != null && !columnFilter.values().get(1).isBlank()) {
try {
int maxValue = Integer.parseInt(columnFilter.values().get(1));
isValidColumFilterCandidat = isValidColumFilterCandidat && maxValue >= nbAnnotation;
} catch (NumberFormatException e) {
isValidColumFilterCandidat = true;
}
}
}
return true;
return isValidColumFilterCandidat;

});
}
Expand All @@ -166,6 +185,18 @@ private List<ColumnDescription> getColumnDescriptions() {
.filterVariantProvider(variableManager -> "text")
.build();

ColumnDescription nbAnnotationsColumn = ColumnDescription.newColumnDescription(UUID.nameUUIDFromBytes("nbAnnotation".getBytes()).toString())
.semanticElementsProvider(variableManager -> List.of("NbAnnotationColumn"))
.headerLabelProvider(variableManager -> "Annotation number")
.headerIconURLsProvider(variableManager -> List.of("/icons/svg/Default.svg"))
.headerIndexLabelProvider(variableManager -> "")
.targetObjectIdProvider(variableManager -> "papaya.Type#nbAnnotation")
.targetObjectKindProvider(variableManager -> "")
.initialWidthProvider(variableManager -> 250)
.isResizablePredicate(variableManager -> false)
.filterVariantProvider(variableManager -> "range")
.build();

Function<VariableManager, String> headerLabelProvider = variableManager -> variableManager.get(VariableManager.SELF, EStructuralFeature.class)
.map(featureToDisplayName::get)
.orElse("");
Expand All @@ -189,7 +220,7 @@ private List<ColumnDescription> getColumnDescriptions() {
.isResizablePredicate(variableManager -> true)
.filterVariantProvider(variableManager -> "text")
.build();
return List.of(iconColumnDescription, columnDescription);
return List.of(iconColumnDescription, columnDescription, nbAnnotationsColumn);
}

private List<ICellDescription> getCellDescriptions() {
Expand Down Expand Up @@ -240,6 +271,25 @@ private List<ICellDescription> getCellDescriptions() {
.cellValueProvider((variableManager, columnTargetObject) -> "")
.cellIconURLsProvider(iconLabelCellIconURLsProvider)
.build());

Predicate<VariableManager> nbAnnotationCellPredicate = variableManager -> variableManager.get(ColumnDescription.COLUMN_TARGET_OBJECT, Object.class)
.filter(String.class::isInstance)
.map(String.class::cast)
.filter(value -> value.equals("NbAnnotationColumn"))
.isPresent();

cellDescriptions.add(IconLabelCellDescription.newIconLabelCellDescription("nbAnnotationCells")
.canCreatePredicate(nbAnnotationCellPredicate)
.targetObjectIdProvider(variableManager -> "")
.targetObjectKindProvider(variableManager -> "")
.cellValueProvider((variableManager, columnTargetObject) -> variableManager.get(VariableManager.SELF, Object.class)
.filter(AnnotableElement.class::isInstance)
.map(AnnotableElement.class::cast)
.map(annotableElement -> annotableElement.getAnnotations().size())
.map(String::valueOf)
.orElse("NA"))
.cellIconURLsProvider((variableManager, o) -> List.of())
.build());
return cellDescriptions;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public void givenTableWhenColumnResizeMutationTriggeredThenTheRepresentationIsRe
.map(TableRefreshedEventPayload::table)
.ifPresentOrElse(table -> {
assertThat(table).isNotNull();
assertThat(table.getColumns()).hasSize(5);
assertThat(table.getColumns()).hasSize(6);
columnRef.set(table.getColumns().get(1));
assertThat(table.getColumns().get(1).getWidth()).isEqualTo(180);
tableId.set(table.getId());
Expand All @@ -131,7 +131,7 @@ public void givenTableWhenColumnResizeMutationTriggeredThenTheRepresentationIsRe
.map(TableRefreshedEventPayload::table)
.ifPresentOrElse(table -> {
assertThat(table).isNotNull();
assertThat(table.getColumns()).hasSize(5);
assertThat(table.getColumns()).hasSize(6);
assertThat(table.getColumns().get(1).getWidth()).isEqualTo(50);
}, () -> fail(MISSING_TABLE));

Expand Down Expand Up @@ -162,7 +162,7 @@ public void givenTableWhenColumnVisibilityChangesMutationTriggeredThenTheReprese
.map(TableRefreshedEventPayload::table)
.ifPresentOrElse(table -> {
assertThat(table).isNotNull();
assertThat(table.getColumns()).hasSize(5);
assertThat(table.getColumns()).hasSize(6);
columnNameRef.set(table.getColumns().get(0));
columnDescRef.set(table.getColumns().get(1));
columnAnnotationRef.set(table.getColumns().get(2));
Expand Down Expand Up @@ -195,7 +195,7 @@ public void givenTableWhenColumnVisibilityChangesMutationTriggeredThenTheReprese
.map(TableRefreshedEventPayload::table)
.ifPresentOrElse(table -> {
assertThat(table).isNotNull();
assertThat(table.getColumns()).hasSize(5);
assertThat(table.getColumns()).hasSize(6);
assertThat(table.getColumns().get(0).isHidden()).isTrue();
assertThat(table.getColumns().get(1).isHidden()).isFalse();
assertThat(table.getColumns().get(2).isHidden()).isTrue();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public void givenTableRepresentationWhenWeSubscribeToItsEventThenTheRepresentati
.map(TableRefreshedEventPayload::table)
.ifPresentOrElse(table -> {
assertThat(table).isNotNull();
assertThat(table.getColumns()).hasSize(5);
assertThat(table.getColumns()).hasSize(6);
assertThat(table.getLines()).hasSize(2);
}, () -> fail(MISSING_TABLE));

Expand All @@ -138,7 +138,7 @@ public void givenTableWhenRefreshTriggeredThenTableIsRefreshed() {
.map(TableRefreshedEventPayload::table)
.ifPresentOrElse(table -> {
assertThat(table).isNotNull();
assertThat(table.getColumns()).hasSize(5);
assertThat(table.getColumns()).hasSize(6);
assertThat(table.getLines()).hasSize(2);

tableId.set(table.getId());
Expand Down Expand Up @@ -189,12 +189,13 @@ public void givenTableRepresentationWithColumnHeaderWhenWeSubscribeToItsEventThe
.map(TableRefreshedEventPayload::table)
.ifPresentOrElse(table -> {
assertThat(table).isNotNull();
assertThat(table.getColumns()).hasSize(5);
assertThat(table.getColumns()).hasSize(6);
assertThat(table.getColumns().get(0).getHeaderIndexLabel()).isEqualTo("");
assertThat(table.getColumns().get(1).getHeaderIndexLabel()).isEqualTo("A");
assertThat(table.getColumns().get(2).getHeaderIndexLabel()).isEqualTo("B");
assertThat(table.getColumns().get(3).getHeaderIndexLabel()).isEqualTo("C");
assertThat(table.getColumns().get(4).getHeaderIndexLabel()).isEqualTo("D");
assertThat(table.getColumns().get(5).getHeaderIndexLabel()).isEqualTo("");
}, () -> fail(MISSING_TABLE));

StepVerifier.create(flux)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,14 +178,12 @@ public void givenPapayaPackageWhenWeSubscribeToTableWithIconLabelCellThenURLOfIt
assertThat(typename).isEqualTo(TableRefreshedEventPayload.class.getSimpleName());

List<List<String>> iconLabelCellIconURLs = JsonPath.read(body, "$.data.tableEvent.table.lines[*].cells[*].iconURLs");
assertThat(iconLabelCellIconURLs)
assertThat(iconLabelCellIconURLs.stream().filter(iconURL -> !iconURL.isEmpty()).toList())
.isNotEmpty()
.allSatisfy(iconURLs -> {
assertThat(iconURLs)
.isNotEmpty()
.hasSize(2)
.allSatisfy(iconURL -> assertThat(iconURL).startsWith(URLConstants.IMAGE_BASE_PATH));
});
.allSatisfy(iconURLs -> assertThat(iconURLs)
.isNotEmpty()
.hasSize(2)
.allSatisfy(iconURL -> assertThat(iconURL).startsWith(URLConstants.IMAGE_BASE_PATH)));
}, () -> fail("Missing table"));

StepVerifier.create(flux)
Expand Down
6 changes: 5 additions & 1 deletion packages/tables/backend/sirius-components-tables/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@
<version>2025.1.0</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
</dependency>
Comment on lines +59 to +62
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not reviewed the whole PR but a change to a pom.xml is always something important so I've only looked at this file.

sirius-components-tables like most very low level parts of Sirius Components should not have any dependency to anything unless it is absolutely critical. The job done with this code should not be the responsibility of this project. It is not the responsibility of sirius-components-tables to support the fact that the table range column filters are serialized over the network in this way.

</dependencies>

<build>
Expand Down Expand Up @@ -113,4 +117,4 @@
</plugin>
</plugins>
</build>
</project>
</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*******************************************************************************
* Copyright (c) 2024 CEA LIST.
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v2.0
* which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Obeo - initial API and implementation
*******************************************************************************/
package org.eclipse.sirius.components.tables;

import java.util.List;
import java.util.Objects;

/**
* Data representing the filter of a column with column mapped to the targetObjectId.
*
* @author frouene
*/
public record ColumnFilterMapped(String id, List<String> values) {

public ColumnFilterMapped {
Objects.requireNonNull(id);
Objects.requireNonNull(values);
}
}
Loading
Loading