diff --git a/.github/test-and-scan.sh b/.github/test-and-scan.sh index 8798efee22..6555d4dd9f 100755 --- a/.github/test-and-scan.sh +++ b/.github/test-and-scan.sh @@ -40,7 +40,10 @@ _mvn_verify() { echo "~> Running mvn verify with options: $*" fi - mvn -B verify "$@" + # -fae + -Dmaven.test.failure.ignore=true: run the whole reactor and do NOT abort on a test + # failure, so the appended sonar:sonar goal always executes and SonarCloud re-analyzes on every + # commit (otherwise a single failing/flaky test stops the build before the scan runs). + mvn -B -fae -Dmaven.test.failure.ignore=true verify "$@" } _mvn_verify $OPT1 $OPT2 $OPT3 \ @@ -50,4 +53,31 @@ _mvn_verify $OPT1 $OPT2 $OPT3 \ RV="$?" .github/github-tools/mvn.test.report.generate -exit "$RV" + +# --------------------------------------------------------------------------- +# Pipeline gate (kept SEPARATE from the scan). +# +# The mvn run above uses -Dmaven.test.failure.ignore=true so a failing/flaky +# test can never stop the reactor before sonar:sonar -> the scan is ALWAYS +# submitted on every push. We therefore re-enforce the test gate here: +# (a) a non-zero mvn exit = a NON-test failure (compilation, sonar, plugin) -> fail; +# (b) otherwise scan the surefire/failsafe XML reports and fail on any real +# failure/error. Flaky tests that passed on retry are recorded as +# with failures="0"/errors="0", so they do NOT trip this. +# --------------------------------------------------------------------------- +if [ "$RV" -ne 0 ]; then + echo "::error::Maven build failed (exit code $RV) — see the log above." + exit "$RV" +fi + +FAILED_REPORTS=$(find . -type f \( -path '*/surefire-reports/*.xml' -o -path '*/failsafe-reports/*.xml' \) -print0 2>/dev/null \ + | xargs -0 -r grep -lE 'failures="[1-9][0-9]*"|errors="[1-9][0-9]*"' 2>/dev/null) + +if [ -n "$FAILED_REPORTS" ]; then + echo "::error::Test failures/errors detected — failing the pipeline (the Sonar scan was still submitted). Offending reports:" + echo "$FAILED_REPORTS" | sed 's/^/ - /' + exit 1 +fi + +echo "All tests passed and the Sonar scan was submitted." +exit 0 diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 45b87885ad..5dfc1f4a75 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -85,6 +85,8 @@ jobs: steps: - uses: actions/checkout@v4 + with: + fetch-depth: 0 - name: Set up JDK 17 uses: actions/setup-java@v4 diff --git a/README.md b/README.md index e7113c6878..34f719b7dd 100644 --- a/README.md +++ b/README.md @@ -32,8 +32,52 @@ To execute a specific test: mvn clean test -Ppre-deployment-verification -pl -Dtest= ``` -By default the logging output in tests is minimized. -The general log level is controlled by the variable `ROOT_LOG_LEVEL`, that in tests is set to `WARN` by default. +By default the logging output in tests is minimized. See [Logging](#logging) below for how to get +verbose/`DEBUG` output, both for the running webapp and for test runs (they work differently). + +## Logging + +Logging is configured via `engine/src/main/resources/base.xml` (logback) and driven by two environment +variables: +- `ROOT_LOG_LEVEL` — the root logger level. Defaults to `DEBUG` when running the webapp; overridden to + `WARN` when running tests (see `pom.xml` surefire configuration). +- `LOG_LEVEL` — the console (`STDOUT`) appender threshold. Defaults to `WARN`, regardless of + `ROOT_LOG_LEVEL`. + +To run the webapp locally with `DEBUG` logs printed to the console: + +``` +cd webapp/ +LOG_LEVEL=DEBUG mvn package jetty:run-war -Pjetty-local -Dspring.profiles.active=swagger -DskipTests -DskipLicenseDownload -Pderby -Pkeycloak +``` + +`ROOT_LOG_LEVEL` does not need to be set for this, since it already defaults to `DEBUG` outside of tests; +`LOG_LEVEL` is the variable that actually gates what reaches the console. + +Test runs are different: `entando-engine`'s test-jar ships `logback-test.xml`, which every other module +picks up on its test classpath. It hardcodes `` (with explicit per-package `DEBUG` +overrides only for a couple of Spring test loggers), so **`ROOT_LOG_LEVEL`/`LOG_LEVEL` have no effect on +test runs** — only on the running webapp. To get `DEBUG` output from a test run, point Logback at a +throwaway config instead: + +``` +cat > /tmp/logback-debug.xml <<'EOF' + + + %d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n + + + + + +EOF +mvn clean test -Ppre-deployment-verification -pl -Dtest= \ + -DargLine=-Dlogback.configurationFile=/tmp/logback-debug.xml +``` + +(Alternatively, add a one-off `` line to +`engine/src/test/resources/logback-test.xml` before running the test — cheaper for a quick, throwaway +check, but remember to revert it since it's a shared test resource.) ## Environment Variables List | Group | Name | Value [default] | Description | diff --git a/admin-console/src/main/java/com/agiletec/apsadmin/global-messages_en.properties b/admin-console/src/main/java/com/agiletec/apsadmin/global-messages_en.properties index f0023ab965..60df02ebaa 100644 --- a/admin-console/src/main/java/com/agiletec/apsadmin/global-messages_en.properties +++ b/admin-console/src/main/java/com/agiletec/apsadmin/global-messages_en.properties @@ -95,6 +95,8 @@ label.no=No label.true=True label.false=False label.bothYesAndNo=Both +label.any=Any +label.notSet=Not set label.state=Status label.all=All label.confirm=Confirm diff --git a/admin-console/src/main/java/com/agiletec/apsadmin/global-messages_it.properties b/admin-console/src/main/java/com/agiletec/apsadmin/global-messages_it.properties index eb02d85b33..c1e7e9203f 100644 --- a/admin-console/src/main/java/com/agiletec/apsadmin/global-messages_it.properties +++ b/admin-console/src/main/java/com/agiletec/apsadmin/global-messages_it.properties @@ -102,6 +102,8 @@ label.no=No label.true=Vero label.false=Falso label.bothYesAndNo=Indifferente +label.any=Qualsiasi +label.notSet=Non impostato label.state=Stato label.all=Tutti label.confirm=Conferma diff --git a/admin-console/src/main/java/com/agiletec/apsadmin/system/entity/AbstractApsEntityFinderAction.java b/admin-console/src/main/java/com/agiletec/apsadmin/system/entity/AbstractApsEntityFinderAction.java index 54c9992b5e..5e5726f849 100644 --- a/admin-console/src/main/java/com/agiletec/apsadmin/system/entity/AbstractApsEntityFinderAction.java +++ b/admin-console/src/main/java/com/agiletec/apsadmin/system/entity/AbstractApsEntityFinderAction.java @@ -24,6 +24,7 @@ import org.entando.entando.ent.util.EntLogging.EntLogFactory; import com.agiletec.aps.system.common.entity.IEntityManager; +import com.agiletec.aps.system.common.entity.NestedBooleanSearchSupport; import com.agiletec.aps.system.common.entity.model.EntitySearchFilter; import com.agiletec.aps.system.common.entity.model.IApsEntity; import com.agiletec.aps.system.common.entity.model.attribute.AttributeInterface; @@ -181,19 +182,20 @@ public List getSearcheableAttributes() { } public List getSearchableAttributes() { - List searchableAttributes = new ArrayList(); - IApsEntity prototype = this.getEntityPrototype(); - if (null == prototype) { - return searchableAttributes; - } - List contentAttributes = prototype.getAttributeList(); - for (int i=0; i_". + return NestedBooleanSearchSupport.collectSearchable(this.getEntityPrototype()); + } + + /** + * Display labels for {@link #getSearchableAttributes()}, keyed by the attribute's machine key. + * A nested boolean's label is its hierarchy (e.g. {@code "compo > cmp_bool"}) reconstructed from the + * real attribute tree, so the search form renders it verbatim instead of splitting the flattened key + * on '_' - which would mis-segment a name that itself contains '_'. + * @return a map from machine key to display label; never null. + */ + public Map getSearchableAttributeLabels() { + return NestedBooleanSearchSupport.buildSearchLabels(this.getEntityPrototype()); } public List getAttributeRoles() { diff --git a/admin-console/src/main/java/com/agiletec/apsadmin/system/entity/EntityActionHelper.java b/admin-console/src/main/java/com/agiletec/apsadmin/system/entity/EntityActionHelper.java index 86f781e2dc..1f89fe024a 100644 --- a/admin-console/src/main/java/com/agiletec/apsadmin/system/entity/EntityActionHelper.java +++ b/admin-console/src/main/java/com/agiletec/apsadmin/system/entity/EntityActionHelper.java @@ -25,6 +25,7 @@ import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; +import com.agiletec.aps.system.common.entity.NestedBooleanSearchSupport; import com.agiletec.aps.system.common.entity.model.ApsEntity; import com.agiletec.aps.system.common.entity.model.AttributeFieldError; import com.agiletec.aps.system.common.entity.model.AttributeTracer; @@ -37,6 +38,7 @@ import com.agiletec.aps.system.common.entity.model.attribute.DateAttribute; import com.agiletec.aps.system.common.entity.model.attribute.ITextAttribute; import com.agiletec.aps.system.common.entity.model.attribute.NumberAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.ThreeStateAttribute; import com.agiletec.aps.util.CheckFormatUtil; import com.agiletec.aps.util.DateConverter; import com.agiletec.apsadmin.system.BaseActionHelper; @@ -49,10 +51,17 @@ * classes which handle elements built with the "ApsEntity' entries. * @author E.Santoboni */ +// NOTE: java:S2143 ("use the java.time API") is intentionally suppressed. Legacy java.util.Date is used +// only to parse the date-range search form fields (via DateConverter); java.time migration is out of +// scope for ESB-1133 (boolean search) and tracked separately. +@SuppressWarnings("java:S2143") public class EntityActionHelper extends BaseActionHelper implements IEntityActionHelper, BeanFactoryAware { private static final EntLogger _logger = EntLogFactory.getSanitizedLogger(EntityActionHelper.class); - + + /** Form value submitted by the ThreeState search selector for the "Not set" (unset) option. */ + private static final String THREE_STATE_NOT_SET_VALUE = "none"; + @Override public void updateEntity(IApsEntity currentEntity, HttpServletRequest request) { try { @@ -192,7 +201,10 @@ public EntitySearchFilter[] getAttributeFilters(AbstractApsEntityFinderAction en if (null == prototype) { return filters; } - List contentAttributes = prototype.getAttributeList(); + // Same flattened view the search form is built from: searchable top-level attributes plus + // Composite-nested boolean-like attributes keyed by "_". Iterating the + // identical list guarantees the parser resolves exactly the field names the form submitted. + List contentAttributes = NestedBooleanSearchSupport.collectSearchable(prototype); for (int i = 0; i < contentAttributes.size(); i++) { AttributeInterface attribute = contentAttributes.get(i); if (attribute.isActive() && attribute.isSearchable()) { @@ -209,6 +221,15 @@ public EntitySearchFilter[] getAttributeFilters(AbstractApsEntityFinderAction en EntitySearchFilter filterToAdd = new EntitySearchFilter(attribute.getName(), true, dateStart, dateEnd); filters = this.addFilter(filters, filterToAdd); } + } else if (attribute instanceof ThreeStateAttribute) { + // ThreeState (tested before BooleanAttribute, which it extends) has three states: + // "true"/"false" filter by value; "none" ("Not set") matches the unset state, which + // on the DB search path is the ABSENCE of a record (ThreeState writes no row when + // unset) - so it is queried via the null option, not a value; blank means "Any". + EntitySearchFilter filterToAdd = this.buildThreeStateFilter(entityFinderAction, attribute.getName()); + if (null != filterToAdd) { + filters = this.addFilter(filters, filterToAdd); + } } else if (attribute instanceof BooleanAttribute) { String booleanValue = entityFinderAction.getSearchFormFieldValue(attribute.getName() + "_booleanFieldName"); if (null != booleanValue && booleanValue.trim().length() > 0) { @@ -231,6 +252,14 @@ public EntitySearchFilter[] getAttributeFilters(AbstractApsEntityFinderAction en @Override public String[] getAttributeFilterFieldName(ApsEntity prototype, String attrName) { AbstractAttribute attr = (AbstractAttribute) prototype.getAttribute(attrName); + if (null == attr) { + // Not a top-level attribute: it may be a Composite-nested boolean addressed by its + // path key "_". Resolve it so the remembered search round-trips. + attr = (AbstractAttribute) NestedBooleanSearchSupport.resolveNestedBooleanByKey(prototype, attrName); + } + if (null == attr) { + return new String[0]; + } if (attr.isTextAttribute()) { return new String[] {attrName + "_textFieldName"}; } else if (attr instanceof DateAttribute) { @@ -240,9 +269,29 @@ public String[] getAttributeFilterFieldName(ApsEntity prototype, String attrName } else if (attr instanceof BooleanAttribute) { return new String[] {attrName + "_booleanFieldName"}; } - return null; + return new String[0]; } + /** + * Build the search filter for a ThreeState attribute from its {@code _booleanFieldName} form field. + * Blank -> {@code null} ("Any", no filter). {@code "none"} ("Not set") -> a null-option filter, + * because an unset ThreeState leaves no DB search record. {@code "true"}/{@code "false"} -> a value + * filter, as for a plain boolean. + */ + private EntitySearchFilter buildThreeStateFilter(AbstractApsEntityFinderAction entityFinderAction, String attrName) { + String value = entityFinderAction.getSearchFormFieldValue(attrName + "_booleanFieldName"); + if (null == value || value.trim().isEmpty()) { + return null; + } + value = value.trim(); + if (THREE_STATE_NOT_SET_VALUE.equalsIgnoreCase(value)) { + EntitySearchFilter filter = new EntitySearchFilter(attrName, true); + filter.setNullOption(true); + return filter; + } + return new EntitySearchFilter(attrName, true, value, false); + } + private Date getDateSearchFormValue(AbstractApsEntityFinderAction entityFinderAction, String fieldName, String dateFieldNameSuffix, boolean start) { String inputFormName = fieldName + dateFieldNameSuffix; diff --git a/admin-console/src/main/java/com/agiletec/apsadmin/system/entity/type/AbstractBaseEntityAttributeConfigAction.java b/admin-console/src/main/java/com/agiletec/apsadmin/system/entity/type/AbstractBaseEntityAttributeConfigAction.java index 2f6c5560bd..c3010b7bf4 100644 --- a/admin-console/src/main/java/com/agiletec/apsadmin/system/entity/type/AbstractBaseEntityAttributeConfigAction.java +++ b/admin-console/src/main/java/com/agiletec/apsadmin/system/entity/type/AbstractBaseEntityAttributeConfigAction.java @@ -14,6 +14,7 @@ package com.agiletec.apsadmin.system.entity.type; import com.agiletec.aps.system.common.entity.IEntityManager; +import com.agiletec.aps.system.common.entity.NestedBooleanSearchSupport; import com.agiletec.aps.system.common.entity.model.IApsEntity; import com.agiletec.aps.system.common.entity.model.attribute.AttributeInterface; import com.agiletec.aps.system.common.entity.model.attribute.AttributeRole; @@ -47,6 +48,10 @@ * Base action for Configure Entity Attributes. * @author E.Santoboni */ +// NOTE: java:S2143 ("use the java.time API") is intentionally suppressed. This date-range attribute +// config action is inherently built on java.util.Date (range start/end/equal fields); migrating it to +// java.time is out of scope for ESB-1133 (boolean search) and tracked separately. +@SuppressWarnings("java:S2143") public class AbstractBaseEntityAttributeConfigAction extends BaseAction implements BeanFactoryAware { private static final EntLogger _logger = EntLogFactory.getSanitizedLogger(AbstractBaseEntityAttributeConfigAction.class); @@ -266,6 +271,24 @@ public boolean isSearchableOptionSupported(String attributeTypeCode) { } return false; } + + /** + * Whether the given attribute type may be flagged searchable when used as a composite child. + * Only plain boolean children are indexed (under the path key "<composite>_<boolean>") in + * the DB search tables; every other type - including CheckBox and ThreeState - is forced + * non-searchable as a composite child, so the searchable option must not be offered for them. + * @param attributeTypeCode the attribute type code. + * @return true only for the plain boolean type. + */ + public boolean isNestedSearchableOptionSupported(String attributeTypeCode) { + try { + AttributeInterface attribute = this.getAttributePrototype(attributeTypeCode); + return NestedBooleanSearchSupport.isIndexableNestedBoolean(attribute); + } catch (Exception t) { + _logger.error("error in isNestedSearchableOptionSupported", t); + } + return false; + } public AttributeInterface getAttributePrototype(String typeCode) { IEntityManager entityManager = this.getEntityManager(); diff --git a/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/attribute-type-entry-composite-element.jsp b/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/attribute-type-entry-composite-element.jsp index cd92c28cd6..cfb0a7f8da 100644 --- a/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/attribute-type-entry-composite-element.jsp +++ b/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/attribute-type-entry-composite-element.jsp @@ -135,6 +135,19 @@ + +
+
+ + +
+
+ +
+
+ diff --git a/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/entityFinding.jsp b/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/entityFinding.jsp index 4a45517d51..7dc80fe0cd 100644 --- a/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/entityFinding.jsp +++ b/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/entityFinding.jsp @@ -137,14 +137,28 @@ http://localhost:8080/PortalExample/do/Entity/search.action?entityManagerName=ja

- +

-
+

_booleanFieldName + <%-- ThreeState: Any (no filter), Yes, No, Not set (the unset/none state). --%>
    -
    • +
    • +
    • +
    • +
    • +
+ + +

+
+

+ _booleanFieldName + +
    +
diff --git a/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/modules/include/threeStateAttributeInputField.jsp b/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/modules/include/threeStateAttributeInputField.jsp index d95d03e64c..d8e9fffd24 100644 --- a/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/modules/include/threeStateAttributeInputField.jsp +++ b/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/modules/include/threeStateAttributeInputField.jsp @@ -24,6 +24,6 @@ id="none_%{#currentThreestateAttributeNameVar}" value="" checked="%{#attribute.booleanValue == null}"/> - + \ No newline at end of file diff --git a/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/user/user-list.jsp b/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/user/user-list.jsp index 8c391c3110..e5fb26a44c 100644 --- a/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/user/user-list.jsp +++ b/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/user/user-list.jsp @@ -241,16 +241,56 @@ - <%-- Boolean & ThreeState --%> + <%-- ThreeState: Any, Yes, No, Not set (the unset/none state) --%> + + + +
+ + +
+ + +
+
+ + <%-- Boolean & CheckBox --%> + test="#attribute.type == 'Boolean' || #attribute.type == 'CheckBox'">
+ value="%{searchableAttributeLabels[#attribute.name] != null ? searchableAttributeLabels[#attribute.name] : #attribute.name}" />
- +

- + + +
+

+ + _booleanFieldName + + + + <%-- ThreeState has three stored states plus "don't filter": Any (no + filter), Yes (true), No (false) and Not set (the unset/none state). --%> +
    +
  • + +
    • +
  • + + +
    • +
  • + + +
    • +
  • + + +
    • +
+ + +

+ +

@@ -203,16 +260,16 @@
  • -
  • - @@ -222,7 +279,7 @@
  • - @@ -378,7 +435,7 @@ value="%{getSearchFormFieldValue(#numberEndInputFieldName)}" /> + test="#attribute.type == 'Boolean' || #attribute.type == 'ThreeState' || #attribute.type == 'CheckBox'"> _booleanFieldName - + _booleanFieldName diff --git a/cms-plugin/src/test/java/com/agiletec/plugins/jacms/aps/system/services/content/ContentManagerTest.java b/cms-plugin/src/test/java/com/agiletec/plugins/jacms/aps/system/services/content/ContentManagerTest.java index 353e598328..3b3840a9b7 100644 --- a/cms-plugin/src/test/java/com/agiletec/plugins/jacms/aps/system/services/content/ContentManagerTest.java +++ b/cms-plugin/src/test/java/com/agiletec/plugins/jacms/aps/system/services/content/ContentManagerTest.java @@ -20,6 +20,12 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.when; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.spy; import com.agiletec.aps.system.common.entity.model.IApsEntity; import com.agiletec.aps.system.common.entity.parse.IEntityTypeFactory; @@ -37,7 +43,6 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; -import org.mockito.Mockito; import org.mockito.MockitoAnnotations; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.beans.factory.BeanFactory; @@ -71,7 +76,7 @@ class ContentManagerTest { private ContentManager contentManager; @BeforeEach - public void setUp() throws Exception { + void setUp() { MockitoAnnotations.initMocks(this); this.contentManager.setEntityClassName(className); this.contentManager.setConfigItemName(JacmsSystemConstants.CONFIG_ITEM_CONTENT_TYPES); @@ -149,8 +154,8 @@ private IApsEntity createFakeEntity(String typeCode, String viewPage, String def @Test void failGetDefaultModelById() throws Exception { Assertions.assertThrows(EntRuntimeException.class, () -> { - Mockito.lenient().when(this.entityTypeFactory.extractEntityType(Mockito.anyString(), Mockito.any(Class.class), - Mockito.anyString(), Mockito.eq(this.entityTypeDom), Mockito.eq(this.beanName), Mockito.eq(this.entityDom))).thenThrow(EntException.class); + lenient().when(this.entityTypeFactory.extractEntityType(anyString(), any(Class.class), + anyString(), eq(this.entityTypeDom), eq(this.beanName), eq(this.entityDom))).thenThrow(EntException.class); String modelId = this.contentManager.getDefaultModel("ART123"); }); } @@ -159,5 +164,15 @@ void failGetDefaultModelById() throws Exception { void getEntityPrototypeShouldReturnNullForNullTypeCode() { Assertions.assertNull(this.contentManager.getEntityPrototype(null)); } + + @Test + void shouldReturnNullFromModelAccessorsWhenTypeNotFound() { + ContentManager spyManager = spy(this.contentManager); + doReturn(null).when(spyManager).getTypeById(anyString()); + + Assertions.assertNull(spyManager.getViewPage("XYZ123")); + Assertions.assertNull(spyManager.getDefaultModel("XYZ123")); + Assertions.assertNull(spyManager.getListModel("XYZ123")); + } } diff --git a/cms-plugin/src/test/java/com/agiletec/plugins/jacms/aps/system/services/content/TestContentManager.java b/cms-plugin/src/test/java/com/agiletec/plugins/jacms/aps/system/services/content/TestContentManager.java index cbbdb10fa5..090d778c1f 100644 --- a/cms-plugin/src/test/java/com/agiletec/plugins/jacms/aps/system/services/content/TestContentManager.java +++ b/cms-plugin/src/test/java/com/agiletec/plugins/jacms/aps/system/services/content/TestContentManager.java @@ -79,7 +79,7 @@ class TestContentManager extends BaseTestCase { void testSearchContents_1_1() throws Throwable { List contentIds = this._contentManager.searchId(null); assertNotNull(contentIds); - assertEquals(25, contentIds.size()); + assertEquals(29, contentIds.size()); EntitySearchFilter creationOrder = new EntitySearchFilter(IContentManager.CONTENT_CREATION_DATE_FILTER_KEY, false); creationOrder.setOrder(EntitySearchFilter.ASC_ORDER); @@ -163,14 +163,14 @@ void testSearchContents_1_2() throws Throwable { EntitySearchFilter[] filters4 = {versionFilter}; contentIds = this._contentManager.searchId(filters4); assertNotNull(contentIds); - assertEquals(22, contentIds.size()); + assertEquals(26, contentIds.size()); } @Test void testSearchContents_1_4() throws Throwable { List contentIds = this._contentManager.searchId(null); assertNotNull(contentIds); - assertEquals(25, contentIds.size()); + assertEquals(29, contentIds.size()); EntitySearchFilter creationOrder = new EntitySearchFilter(IContentManager.CONTENT_CREATION_DATE_FILTER_KEY, false); creationOrder.setOrder(EntitySearchFilter.ASC_ORDER); @@ -198,7 +198,7 @@ void testSearchContents_1_5() throws Throwable { EntitySearchFilter[] filters1 = {creationOrder, descrFilter}; List contentIds = this._contentManager.searchId(filters1); assertNotNull(contentIds); - String[] expected1 = {"ART1", "RAH1", "ART187", "RAH101", "ART102", "EVN103", "ART104", "ART111", "ART112", "EVN23", "ART120", "ART121", "ART122"}; + String[] expected1 = {"ART1", "RAH1", "ART187", "RAH101", "ART102", "EVN103", "ART104", "ART111", "ART112", "EVN23", "ART120", "ART121", "ART122", "BLT4"}; assertEquals(expected1.length, contentIds.size()); this.verifyOrder(contentIds, expected1); @@ -206,7 +206,7 @@ void testSearchContents_1_5() throws Throwable { EntitySearchFilter[] filters2 = {creationOrder, descrFilter}; contentIds = this._contentManager.searchId(filters2); assertNotNull(contentIds); - String[] expected2 = {"RAH101", "ART102", "EVN103", "ART104", "ART111", "ART112", "EVN23", "ART120", "ART121", "ART122"}; + String[] expected2 = {"RAH101", "ART102", "EVN103", "ART104", "ART111", "ART112", "EVN23", "ART120", "ART121", "ART122", "BLT4"}; assertEquals(expected2.length, contentIds.size()); this.verifyOrder(contentIds, expected2); @@ -236,7 +236,7 @@ void testSearchPaginatedContents_1_5() throws Throwable { EntitySearchFilter[] filters1 = {creationOrder, descrFilter, paginationFilter}; SearcherDaoPaginatedResult paginatedContentsId1 = this._contentManager.getPaginatedWorkContentsId(null, true, filters1, groupCodes); assertNotNull(paginatedContentsId1); - String[] totalExpected1 = {"ART1", "RAH1", "ART187", "RAH101", "ART102", "EVN103", "ART104", "ART111", "ART112", "EVN23", "ART120", "ART121", "ART122"}; + String[] totalExpected1 = {"ART1", "RAH1", "ART187", "RAH101", "ART102", "EVN103", "ART104", "ART111", "ART112", "EVN23", "ART120", "ART121", "ART122", "BLT4"}; assertEquals(totalExpected1.length, paginatedContentsId1.getCount().intValue()); assertEquals(4, paginatedContentsId1.getList().size()); for (int i = 0; i < 4; i++) { @@ -248,7 +248,7 @@ void testSearchPaginatedContents_1_5() throws Throwable { EntitySearchFilter[] filters2 = {creationOrder, descrFilter, paginationFilter2}; SearcherDaoPaginatedResult paginatedContentsId2 = this._contentManager.getPaginatedWorkContentsId(null, true, filters2, groupCodes); assertNotNull(paginatedContentsId2); - String[] totalExpected2 = {"RAH101", "ART102", "EVN103", "ART104", "ART111", "ART112", "EVN23", "ART120", "ART121", "ART122"}; + String[] totalExpected2 = {"RAH101", "ART102", "EVN103", "ART104", "ART111", "ART112", "EVN23", "ART120", "ART121", "ART122", "BLT4"}; assertEquals(totalExpected2.length, paginatedContentsId2.getCount().intValue()); assertEquals(6, paginatedContentsId2.getList().size()); for (int i = 3; i < (6+3); i++) { @@ -295,7 +295,9 @@ void testSearchContents_3() throws Throwable { assertNotNull(contentIds); String[] expected = {"ART187", "ART1", "EVN193", "EVN194", "ART180", "RAH1", "EVN191", "EVN192", "RAH101", "EVN103", "ART104", "ART102", "EVN23", - "EVN24", "EVN25", "EVN41", "EVN20", "EVN21", "ART111", "ART120", "ART121", "ART122", "ART112", "ALL4"}; + "EVN24", "EVN25", "EVN41", "EVN20", "EVN21", "ART111", "ART120", "ART121", "ART122", "ART112", "ALL4", + "BLT1", "BLT2", "BLT3", "BLT4"}; + assertEquals(expected.length, contentIds.size()); this.verifyOrder(contentIds, expected); } @@ -352,7 +354,7 @@ void testSearchWorkContents() throws Throwable { groupCodes.add(Group.ADMINS_GROUP_NAME); contents = this._contentManager.loadWorkContentsId(null, groupCodes); assertNotNull(contents); - assertEquals(25, contents.size()); + assertEquals(29, contents.size()); } @Test @@ -592,7 +594,7 @@ void testLoadFullContent() throws Throwable { @Test void testGetContentTypes() { Map smallContentTypes = _contentManager.getSmallContentTypesMap(); - assertEquals(4, smallContentTypes.size()); + assertEquals(5, smallContentTypes.size()); } @Test @@ -642,7 +644,7 @@ void testGetXML() throws Throwable { @Test void testLoadPublicContents() throws EntException { List contents = _contentManager.loadPublicContentsId(null, null, freeGroup); - assertEquals(15, contents.size()); + assertEquals(19, contents.size()); } @Test @@ -1614,6 +1616,128 @@ private void setBooleanValue(Content content, String[] attributeCodes, Boolean v booleanAttribute.setBooleanValue(value); } } + + /** + * End-to-end DB-search test for a plain Boolean nested inside a Composite: once the child is + * flagged searchable, it must be indexed under the path key "Composite_Boolean" and be filterable + * through both the work ({@code workcontentsearch}) and public ({@code contentsearch}) searchers - + * the Solr-disabled counterpart of the AdvContentSearch nested-boolean capability. + */ + @Test + void testLoadContentsByNestedCompositeBooleanAttribute() throws Throwable { + this.setCompositeChildrenSearchable(true); + List addedContents = new ArrayList<>(); + try { + // After the type-config round-trip, every boolean-like composite child (Boolean, CheckBox, + // ThreeState) keeps its inherited searchable flag; non-boolean children are forced false. + Content reloaded = this._contentManager.createContentType("ALL"); + CompositeAttribute reloadedComposite = (CompositeAttribute) reloaded.getAttribute("Composite"); + assertTrue(reloadedComposite.getAttribute("Boolean").isSearchable()); + assertTrue(reloadedComposite.getAttribute("CheckBox").isSearchable()); + assertTrue(reloadedComposite.getAttribute("ThreeState").isSearchable()); + + // feature: the nested boolean, made searchable purely through the content type, is indexed + // under the path key "Composite_Boolean" and filterable through both DB searchers. + String trueId = this.createAllCloneWithNestedBoolean(Boolean.TRUE, addedContents); + String falseId = this.createAllCloneWithNestedBoolean(Boolean.FALSE, addedContents); + + // work table (workcontentsearch) + List workTrue = this.searchByNestedComposite(true, false); + assertTrue(workTrue.contains(trueId)); + assertFalse(workTrue.contains(falseId)); + List workFalse = this.searchByNestedComposite(false, false); + assertTrue(workFalse.contains(falseId)); + assertFalse(workFalse.contains(trueId)); + + // public table (contentsearch) + List onlineTrue = this.searchByNestedComposite(true, true); + assertTrue(onlineTrue.contains(trueId)); + assertFalse(onlineTrue.contains(falseId)); + List onlineFalse = this.searchByNestedComposite(false, true); + assertTrue(onlineFalse.contains(falseId)); + assertFalse(onlineFalse.contains(trueId)); + } finally { + for (String id : addedContents) { + this._contentManager.deleteContent(id); + assertNull(this._contentManager.loadContent(id, false)); + } + this.setCompositeChildrenSearchable(false); + } + } + + @Test + void testBooleanSearchableConfigParityTopLevelAndComposite() throws Throwable { + // A boolean-like is configured the SAME WAY top-level and as a Composite child: the searchable + // flag set on the content type is reported in the type XML (proven by surviving the + // serialize->reload of updateEntityPrototype) and inherited by content instances - identically + // at both nesting levels. No auto-forcing: an unflagged boolean-like stays non-searchable. + Content prototype = this._contentManager.createContentType("ALL"); + prototype.getAttribute("Boolean").setSearchable(true); + ((CompositeAttribute) prototype.getAttribute("Composite")).getAttribute("Boolean").setSearchable(true); + try { + ((IEntityTypesConfigurer) this._contentManager).updateEntityPrototype(prototype); + this._contentManager.reloadEntitiesReferences("ALL"); + super.waitThreads(ApsEntityManager.RELOAD_REFERENCES_THREAD_NAME_PREFIX); + + // reloaded TYPE keeps the flag at both levels -> the type XML reported searchable="true" + Content reloadedType = this._contentManager.createContentType("ALL"); + CompositeAttribute reloadedComposite = (CompositeAttribute) reloadedType.getAttribute("Composite"); + assertTrue(reloadedType.getAttribute("Boolean").isSearchable(), "top-level boolean searchable preserved"); + assertTrue(reloadedComposite.getAttribute("Boolean").isSearchable(), "composite boolean searchable preserved"); + // parity (negative): a boolean-like left unflagged is non-searchable at both levels + assertFalse(reloadedType.getAttribute("CheckBox").isSearchable()); + assertFalse(reloadedComposite.getAttribute("CheckBox").isSearchable()); + + // a CONTENT instance inherits the flag from the type, at both levels + Content content = this._contentManager.loadContent("ALL4", false); + CompositeAttribute contentComposite = (CompositeAttribute) content.getAttribute("Composite"); + assertTrue(content.getAttribute("Boolean").isSearchable(), "top-level boolean inherited by content"); + assertTrue(contentComposite.getAttribute("Boolean").isSearchable(), "composite boolean inherited by content"); + } finally { + Content restore = this._contentManager.createContentType("ALL"); + restore.getAttribute("Boolean").setSearchable(false); + ((CompositeAttribute) restore.getAttribute("Composite")).getAttribute("Boolean").setSearchable(false); + ((IEntityTypesConfigurer) this._contentManager).updateEntityPrototype(restore); + this._contentManager.reloadEntitiesReferences("ALL"); + super.waitThreads(ApsEntityManager.RELOAD_REFERENCES_THREAD_NAME_PREFIX); + } + } + + private void setCompositeChildrenSearchable(boolean searchable) throws Throwable { + Content prototype = this._contentManager.createContentType("ALL"); + CompositeAttribute composite = (CompositeAttribute) prototype.getAttribute("Composite"); + composite.getAttribute("Boolean").setSearchable(searchable); + composite.getAttribute("CheckBox").setSearchable(searchable); + composite.getAttribute("ThreeState").setSearchable(searchable); + ((IEntityTypesConfigurer) this._contentManager).updateEntityPrototype(prototype); + this._contentManager.reloadEntitiesReferences("ALL"); + super.waitThreads(ApsEntityManager.RELOAD_REFERENCES_THREAD_NAME_PREFIX); + } + + private String createAllCloneWithNestedBoolean(Boolean value, List addedContents) throws Throwable { + Content clone = this._contentManager.loadContent("ALL4", false); + clone.setId(null); + CompositeAttribute composite = (CompositeAttribute) clone.getAttribute("Composite"); + BooleanAttribute nestedBoolean = (BooleanAttribute) composite.getAttribute("Boolean"); + nestedBoolean.setBooleanValue(value); + this._contentManager.saveContent(clone); + addedContents.add(clone.getId()); + this._contentManager.insertOnLineContent(clone); + return clone.getId(); + } + + private List searchByNestedComposite(boolean value, boolean online) throws Exception { + List groups = new ArrayList<>(); + groups.add(Group.ADMINS_GROUP_NAME); + EntitySearchFilter typeFilter = new EntitySearchFilter<>( + IContentManager.ENTITY_TYPE_CODE_FILTER_KEY, false, "ALL", false); + EntitySearchFilter nestedFilter = new EntitySearchFilter<>( + "Composite_Boolean", true, Boolean.toString(value), false); + EntitySearchFilter[] filters = {typeFilter, nestedFilter}; + return online + ? this._contentManager.loadPublicContentsId("ALL", null, filters, groups) + : this._contentManager.loadWorkContentsId(filters, groups); + } private void testBooleanAttribute_test4(String booleanAttribute, String[] nullResults, String[] falseResults, String[] trueResults) throws Exception { EntitySearchFilter filterForTrue = new EntitySearchFilter<>(booleanAttribute, true, "true", false); diff --git a/cms-plugin/src/test/java/com/agiletec/plugins/jacms/aps/system/services/content/entity/TestContentEntityManager.java b/cms-plugin/src/test/java/com/agiletec/plugins/jacms/aps/system/services/content/entity/TestContentEntityManager.java index 5b2e5159dd..cda7a0c689 100644 --- a/cms-plugin/src/test/java/com/agiletec/plugins/jacms/aps/system/services/content/entity/TestContentEntityManager.java +++ b/cms-plugin/src/test/java/com/agiletec/plugins/jacms/aps/system/services/content/entity/TestContentEntityManager.java @@ -41,7 +41,7 @@ class TestContentEntityManager extends BaseTestCase { void testSearchRecords() throws Throwable { List contents = this._contentManager.searchRecords(null); assertNotNull(contents); - assertEquals(25, contents.size()); + assertEquals(29, contents.size()); EntitySearchFilter typeFilter = new EntitySearchFilter(IContentManager.ENTITY_TYPE_CODE_FILTER_KEY, false, "ART", false); EntitySearchFilter[] filters1 = {typeFilter}; diff --git a/cms-plugin/src/test/java/com/agiletec/plugins/jacms/aps/system/services/resource/ResourceManagerTest.java b/cms-plugin/src/test/java/com/agiletec/plugins/jacms/aps/system/services/resource/ResourceManagerTest.java index 7f08d76e0c..08a6319764 100644 --- a/cms-plugin/src/test/java/com/agiletec/plugins/jacms/aps/system/services/resource/ResourceManagerTest.java +++ b/cms-plugin/src/test/java/com/agiletec/plugins/jacms/aps/system/services/resource/ResourceManagerTest.java @@ -15,19 +15,22 @@ import com.agiletec.plugins.jacms.aps.system.services.resource.cache.IResourceManagerCacheWrapper; import com.agiletec.plugins.jacms.aps.system.services.resource.model.AttachResource; +import com.agiletec.plugins.jacms.aps.system.services.resource.model.BaseResourceDataBean; import com.agiletec.plugins.jacms.aps.system.services.resource.model.ImageResource; import com.agiletec.plugins.jacms.aps.system.services.resource.model.ResourceInterface; import java.util.HashMap; import java.util.Map; +import org.entando.entando.ent.exception.EntException; import org.mockito.InjectMocks; import org.mockito.Mock; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; -import org.mockito.Mockito; import static org.mockito.Mockito.when; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mock; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; @@ -48,13 +51,13 @@ class ResourceManagerTest { private ResourceManager resourceManager; @BeforeEach - public void setUp() throws Exception { - AttachResource mockAttachResource = Mockito.mock(AttachResource.class); - Mockito.lenient().when(mockAttachResource.getType()).thenReturn("Attach"); - Mockito.lenient().when(mockAttachResource.getResourcePrototype()).thenReturn(mockAttachResource); - ImageResource mockImageResource = Mockito.mock(ImageResource.class); - Mockito.lenient().when(mockImageResource.getResourcePrototype()).thenReturn(mockImageResource); - Mockito.lenient().when(mockImageResource.getType()).thenReturn("Image"); + void setUp() { + AttachResource mockAttachResource = mock(AttachResource.class); + lenient().when(mockAttachResource.getType()).thenReturn("Attach"); + lenient().when(mockAttachResource.getResourcePrototype()).thenReturn(mockAttachResource); + ImageResource mockImageResource = mock(ImageResource.class); + lenient().when(mockImageResource.getResourcePrototype()).thenReturn(mockImageResource); + lenient().when(mockImageResource.getType()).thenReturn("Image"); Map types = new HashMap<>(); types.put("Image", mockImageResource); types.put("Attach", mockAttachResource); @@ -62,16 +65,34 @@ public void setUp() throws Exception { } @Test - public void status_should_be_ready_on_init() { + void status_should_be_ready_on_init() { when(cacheWrapper.getStatus()).thenReturn(IResourceManager.STATUS_READY); int status = this.resourceManager.getStatus(); assertThat(status, is(IResourceManager.STATUS_READY)); } - public void createResourceType() { + @Test + void createResourceType() { ResourceInterface type = this.resourceManager.createResourceType("Image"); Assertions.assertNotNull(type); Assertions.assertEquals("Image", type.getType()); } + @Test + void updateResourceShouldThrowWhenResourceNotFound() { + BaseResourceDataBean bean = new BaseResourceDataBean(); + bean.setResourceId("missing-id"); + Assertions.assertThrows(EntException.class, () -> this.resourceManager.updateResource(bean)); + } + + @Test + void refreshMasterFileNamesShouldNotThrowWhenResourceNotFound() { + Assertions.assertDoesNotThrow(() -> this.resourceManager.refreshMasterFileNames("missing-id")); + } + + @Test + void refreshResourceInstancesShouldNotThrowWhenResourceNotFound() { + Assertions.assertDoesNotThrow(() -> this.resourceManager.refreshResourceInstances("missing-id")); + } + } diff --git a/cms-plugin/src/test/java/com/agiletec/plugins/jacms/apsadmin/content/TestContentFinderAction.java b/cms-plugin/src/test/java/com/agiletec/plugins/jacms/apsadmin/content/TestContentFinderAction.java index cf1e9b38ec..63666bc8a0 100644 --- a/cms-plugin/src/test/java/com/agiletec/plugins/jacms/apsadmin/content/TestContentFinderAction.java +++ b/cms-plugin/src/test/java/com/agiletec/plugins/jacms/apsadmin/content/TestContentFinderAction.java @@ -14,12 +14,20 @@ package com.agiletec.plugins.jacms.apsadmin.content; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import com.agiletec.aps.system.common.entity.ApsEntityManager; +import com.agiletec.aps.system.common.entity.IEntityTypesConfigurer; +import com.agiletec.aps.system.common.entity.model.attribute.AttributeInterface; +import com.agiletec.aps.system.common.entity.model.attribute.BooleanAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.CompositeAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.ThreeStateAttribute; import com.agiletec.aps.system.services.group.Group; import com.agiletec.plugins.jacms.aps.system.services.content.model.Content; import com.agiletec.plugins.jacms.apsadmin.content.util.AbstractBaseTestContentAction; @@ -36,7 +44,7 @@ void testGetList() throws Throwable { String result = this.executeGetList("admin"); assertEquals(Action.SUCCESS, result); List contents = (List) ((ContentFinderAction)this.getAction()).getContents(); - assertEquals(25, contents.size()); + assertEquals(29, contents.size()); result = this.executeGetList("editorCoach"); assertEquals(Action.SUCCESS, result); @@ -63,7 +71,7 @@ void testPerformSearch_1() throws Throwable { Map params = new HashMap(); this.executeSearch("admin", params); ContentFinderAction action = (ContentFinderAction) this.getAction(); - String[] order1 = {"ALL4", "ART112","ART122","ART121","ART120","ART111","ART179","EVN21", + String[] order1 = {"BLT4", "BLT3", "BLT2", "BLT1", "ALL4", "ART112","ART122","ART121","ART120","ART111","ART179","EVN21", "EVN20","EVN41","EVN25","EVN24","EVN23","ART102","ART104","EVN103", "RAH101","EVN192","EVN191","RAH1","ART180","EVN194","EVN193","ART1","ART187"}; List contents = action.getContents(); @@ -364,6 +372,132 @@ void testGetPaginatedContentsIdAfterLoadingResults() throws Throwable { action.getPaginatedContentsId(10); } + /** + * End-to-end: a Composite-nested boolean made searchable through the content type is (a) offered by + * the search form as a path-keyed criterion and (b) usable to restrict the search - the submitted + * form field "__booleanFieldName" reaches the DB searcher and filters the list. + */ + @Test + void testPerformSearchByNestedCompositeBoolean() throws Throwable { + this.setNestedBooleanSearchable("ALL", true); + List added = new ArrayList<>(); + try { + String trueId = this.createAllCloneWithNestedBoolean(Boolean.TRUE, added); + String falseId = this.createAllCloneWithNestedBoolean(Boolean.FALSE, added); + + // (a) the form now offers the nested boolean under its path key + Map setType = new HashMap<>(); + setType.put("contentType", "ALL"); + this.executeSearch("admin", setType); + ContentFinderAction action = (ContentFinderAction) this.getAction(); + assertTrue(this.offersAttribute(action, "Composite_Boolean"), + "the search form should expose the nested boolean 'Composite_Boolean'"); + + // (b) submitting the nested boolean field restricts the results + Map params = new HashMap<>(); + params.put("contentType", "ALL"); + params.put("Composite_Boolean_booleanFieldName", "true"); + this.executeSearch("admin", params); + List contents = ((ContentFinderAction) this.getAction()).getContents(); + assertTrue(contents.contains(trueId)); + assertFalse(contents.contains(falseId)); + + params.put("Composite_Boolean_booleanFieldName", "false"); + this.executeSearch("admin", params); + contents = ((ContentFinderAction) this.getAction()).getContents(); + assertTrue(contents.contains(falseId)); + assertFalse(contents.contains(trueId)); + } finally { + for (String id : added) { + this.getContentManager().deleteContent(id); + } + this.setNestedBooleanSearchable("ALL", false); + } + } + + private boolean offersAttribute(ContentFinderAction action, String name) { + for (AttributeInterface attribute : action.getSearchableAttributes()) { + if (name.equals(attribute.getName())) { + return true; + } + } + return false; + } + + private void setNestedBooleanSearchable(String typeCode, boolean searchable) throws Throwable { + Content prototype = this.getContentManager().createContentType(typeCode); + ((CompositeAttribute) prototype.getAttribute("Composite")).getAttribute("Boolean").setSearchable(searchable); + ((IEntityTypesConfigurer) this.getContentManager()).updateEntityPrototype(prototype); + this.getContentManager().reloadEntitiesReferences(typeCode); + waitThreads(ApsEntityManager.RELOAD_REFERENCES_THREAD_NAME_PREFIX); + } + + private String createAllCloneWithNestedBoolean(Boolean value, List added) throws Throwable { + Content clone = this.getContentManager().loadContent("ALL4", false); + clone.setId(null); + BooleanAttribute nested = (BooleanAttribute) ((CompositeAttribute) clone.getAttribute("Composite")).getAttribute("Boolean"); + nested.setBooleanValue(value); + this.getContentManager().saveContent(clone); + added.add(clone.getId()); + this.getContentManager().insertOnLineContent(clone); + return clone.getId(); + } + + /** + * End-to-end for the ThreeState "Not set" search option: submitting + * {@code _booleanFieldName=none} restricts the list to contents whose ThreeState is unset + * (no search record), distinct from "true"/"false" and from "Any" (no filter). + */ + @Test + void testPerformSearchByThreeStateNotSet() throws Throwable { + this.setThreeStateSearchable("ALL", true); + List added = new ArrayList<>(); + try { + String trueId = this.createAllCloneWithThreeState(Boolean.TRUE, added); + String falseId = this.createAllCloneWithThreeState(Boolean.FALSE, added); + String unsetId = this.createAllCloneWithThreeState(null, added); + + Map params = new HashMap<>(); + params.put("contentType", "ALL"); + params.put("ThreeState_booleanFieldName", "none"); + this.executeSearch("admin", params); + List contents = ((ContentFinderAction) this.getAction()).getContents(); + assertTrue(contents.contains(unsetId)); + assertFalse(contents.contains(trueId)); + assertFalse(contents.contains(falseId)); + + params.put("ThreeState_booleanFieldName", "true"); + this.executeSearch("admin", params); + contents = ((ContentFinderAction) this.getAction()).getContents(); + assertTrue(contents.contains(trueId)); + assertFalse(contents.contains(unsetId)); + assertFalse(contents.contains(falseId)); + } finally { + for (String id : added) { + this.getContentManager().deleteContent(id); + } + this.setThreeStateSearchable("ALL", false); + } + } + + private void setThreeStateSearchable(String typeCode, boolean searchable) throws Throwable { + Content prototype = this.getContentManager().createContentType(typeCode); + prototype.getAttribute("ThreeState").setSearchable(searchable); + ((IEntityTypesConfigurer) this.getContentManager()).updateEntityPrototype(prototype); + this.getContentManager().reloadEntitiesReferences(typeCode); + waitThreads(ApsEntityManager.RELOAD_REFERENCES_THREAD_NAME_PREFIX); + } + + private String createAllCloneWithThreeState(Boolean value, List added) throws Throwable { + Content clone = this.getContentManager().loadContent("ALL4", false); + clone.setId(null); + ((ThreeStateAttribute) clone.getAttribute("ThreeState")).setBooleanValue(value); + this.getContentManager().saveContent(clone); + added.add(clone.getId()); + this.getContentManager().insertOnLineContent(clone); + return clone.getId(); + } + private void executeSearch(String currentUserName, Map params) throws Throwable { this.initAction("/do/jacms/Content", "search"); this.setUserOnSession(currentUserName); diff --git a/cms-plugin/src/test/java/com/agiletec/plugins/jacms/apsadmin/content/attribute/TestContentLinkAction.java b/cms-plugin/src/test/java/com/agiletec/plugins/jacms/apsadmin/content/attribute/TestContentLinkAction.java index f93804f224..f9c4628852 100644 --- a/cms-plugin/src/test/java/com/agiletec/plugins/jacms/apsadmin/content/attribute/TestContentLinkAction.java +++ b/cms-plugin/src/test/java/com/agiletec/plugins/jacms/apsadmin/content/attribute/TestContentLinkAction.java @@ -51,7 +51,7 @@ void testFindContent_1() throws Throwable { ContentLinkAction action = (ContentLinkAction) this.getAction(); List contentIds = action.getContents(); - assertEquals(15, contentIds.size());//Contenuti pubblici liberi o non liberi con free gruppo extra + assertEquals(19, contentIds.size());//Contenuti pubblici liberi o non liberi con free gruppo extra assertTrue(contentIds.contains("EVN25"));//Contenuto coach abilitato al gruppo free assertTrue(contentIds.contains("ART121"));//Contenuto del gruppo "administrators" abilitato al gruppo free } @@ -68,7 +68,7 @@ void testFindContent_2() throws Throwable { ContentLinkAction action = (ContentLinkAction) this.getAction(); List contentIds = action.getContents(); - assertEquals(24, contentIds.size());//Tutti i contenuti pubblici + assertEquals(28, contentIds.size());//Tutti i contenuti pubblici } @Test @@ -83,7 +83,7 @@ void testFindContent_3() throws Throwable { ContentLinkAction action = (ContentLinkAction) this.getAction(); List contentIds = action.getContents(); - assertEquals(20, contentIds.size());// Contenuti pubblici liberi, o del gruppo customers o altri con customers gruppo extra + assertEquals(24, contentIds.size());// Contenuti pubblici liberi, o del gruppo customers o altri con customers gruppo extra assertTrue(contentIds.contains("ART122"));//Contenuto del gruppo "administrators" abilitato al gruppo customers assertTrue(contentIds.contains("ART121"));//Contenuto del gruppo "administrators" abilitato al gruppo free assertTrue(contentIds.contains("EVN25"));//Contenuto del gruppo "coach" abilitato al gruppo free @@ -102,7 +102,7 @@ void testFindContent_4() throws Throwable { ContentLinkAction action = (ContentLinkAction) this.getAction(); List contentIds = action.getContents(); - assertEquals(20, contentIds.size());// Contenuti pubblici liberi, o del gruppo coach o altri con coach gruppo extra + assertEquals(24, contentIds.size());// Contenuti pubblici liberi, o del gruppo coach o altri con coach gruppo extra assertTrue(contentIds.contains("ART121"));//Contenuto del gruppo "administrators" abilitato al gruppo coach assertTrue(contentIds.contains("ART121"));//Contenuto del gruppo "administrators" abilitato al gruppo free assertTrue(contentIds.contains("EVN25"));//Contenuto del gruppo "coach" abilitato al gruppo free diff --git a/cms-plugin/src/test/java/com/agiletec/plugins/jacms/apsadmin/content/attribute/TestHypertextAttributeAction.java b/cms-plugin/src/test/java/com/agiletec/plugins/jacms/apsadmin/content/attribute/TestHypertextAttributeAction.java index 07f4ce9574..8b5e65c6fd 100644 --- a/cms-plugin/src/test/java/com/agiletec/plugins/jacms/apsadmin/content/attribute/TestHypertextAttributeAction.java +++ b/cms-plugin/src/test/java/com/agiletec/plugins/jacms/apsadmin/content/attribute/TestHypertextAttributeAction.java @@ -38,7 +38,7 @@ void testFindContent_1() throws Throwable { this.initIntroContentLink("admin", "ART1");//Contenuto del gruppo Free ContentLinkAttributeAction action = (ContentLinkAttributeAction) this.getAction(); List contentIds = action.getContents(); - assertEquals(15, contentIds.size());//Contenuti pubblici liberi o non liberi con free gruppo extra + assertEquals(19, contentIds.size());//Contenuti pubblici liberi o non liberi con free gruppo extra assertTrue(contentIds.contains("EVN25"));//Contenuto coach abilitato al gruppo free assertTrue(contentIds.contains("ART121"));//Contenuto del gruppo "administrators" abilitato al gruppo free } @@ -48,7 +48,7 @@ void testFindContent_2() throws Throwable { this.initIntroContentLink("admin", "ART120");//Contenuto del gruppo degli amministratori ContentLinkAttributeAction action = (ContentLinkAttributeAction) this.getAction(); List contentIds = action.getContents(); - assertEquals(24, contentIds.size());//Tutti i contenuti pubblici + assertEquals(28, contentIds.size());//Tutti i contenuti pubblici } @Test @@ -56,7 +56,7 @@ void testFindContent_3() throws Throwable { this.initIntroContentLink("editorCustomers", "ART102");//Contenuto del gruppo customers ContentLinkAttributeAction action = (ContentLinkAttributeAction) this.getAction(); List contentIds = action.getContents(); - assertEquals(20, contentIds.size());// Contenuti pubblici liberi, o del gruppo customers o altri con customers gruppo extra + assertEquals(24, contentIds.size());// Contenuti pubblici liberi, o del gruppo customers o altri con customers gruppo extra assertTrue(contentIds.contains("ART122"));//Contenuto del gruppo "administrators" abilitato al gruppo customers assertTrue(contentIds.contains("ART121"));//Contenuto del gruppo "administrators" abilitato al gruppo free assertTrue(contentIds.contains("EVN25"));//Contenuto del gruppo "coach" abilitato al gruppo free @@ -68,7 +68,7 @@ void testFindContent_4() throws Throwable { this.initIntroContentLink("admin", "EVN25");//Contenuto del gruppo coach ContentLinkAttributeAction action = (ContentLinkAttributeAction) this.getAction(); List contentIds = action.getContents(); - assertEquals(20, contentIds.size());// Contenuti pubblici liberi, o del gruppo coach o altri con coach gruppo extra + assertEquals(24, contentIds.size());// Contenuti pubblici liberi, o del gruppo coach o altri con coach gruppo extra assertTrue(contentIds.contains("ART121"));//Contenuto del gruppo "administrators" abilitato al gruppo coach assertTrue(contentIds.contains("ART121"));//Contenuto del gruppo "administrators" abilitato al gruppo free assertTrue(contentIds.contains("EVN25"));//Contenuto del gruppo "coach" abilitato al gruppo free diff --git a/cms-plugin/src/test/java/com/agiletec/plugins/jacms/apsadmin/portal/specialwidget/viewer/TestContentFinderViewerAction.java b/cms-plugin/src/test/java/com/agiletec/plugins/jacms/apsadmin/portal/specialwidget/viewer/TestContentFinderViewerAction.java index bfe6ac2a68..786c65a99f 100644 --- a/cms-plugin/src/test/java/com/agiletec/plugins/jacms/apsadmin/portal/specialwidget/viewer/TestContentFinderViewerAction.java +++ b/cms-plugin/src/test/java/com/agiletec/plugins/jacms/apsadmin/portal/specialwidget/viewer/TestContentFinderViewerAction.java @@ -37,7 +37,7 @@ void testFindContent_1() throws Throwable { ContentFinderViewerAction action = (ContentFinderViewerAction) this.getAction(); List contentIds = action.getContents(); - assertEquals(15, contentIds.size());//Contenuti pubblici liberi o non liberi con free gruppo extra + assertEquals(19, contentIds.size());//Contenuti pubblici liberi o non liberi con free gruppo extra assertTrue(contentIds.contains("EVN25"));//Contenuto coach abilitato al gruppo free assertTrue(contentIds.contains("ART121"));//Contenuto del gruppo "administrators" abilitato al gruppo free } @@ -49,7 +49,7 @@ void testFindContent_2() throws Throwable { ContentFinderViewerAction action = (ContentFinderViewerAction) this.getAction(); List contentIds = action.getContents(); - assertEquals(24, contentIds.size());//Tutti i contenuti pubblici + assertEquals(28, contentIds.size());//Tutti i contenuti pubblici } @Test @@ -59,7 +59,7 @@ void testFindContent_3() throws Throwable { ContentFinderViewerAction action = (ContentFinderViewerAction) this.getAction(); List contentIds = action.getContents(); - assertEquals(20, contentIds.size());// Contenuti pubblici liberi o non liberi con customers gruppo extra + assertEquals(24, contentIds.size());// Contenuti pubblici liberi o non liberi con customers gruppo extra assertTrue(contentIds.contains("ART122"));//Contenuto del gruppo "administrators" abilitato al gruppo customers assertTrue(contentIds.contains("ART121"));//Contenuto del gruppo "administrators" abilitato al gruppo free assertTrue(contentIds.contains("EVN25"));//Contenuto del gruppo "coach" abilitato al gruppo free @@ -71,7 +71,7 @@ void testPerformSearch() throws Throwable { Map params = new HashMap(); this.executeParametrizedSearchContents("admin", "pagina_11", "1", null);//Pagina Free ContentFinderViewerAction action = (ContentFinderViewerAction) this.getAction(); - String[] order1 = {"ALL4", "ART121", "EVN21", "EVN20", "EVN25", + String[] order1 = {"BLT4", "BLT3", "BLT2", "BLT1", "ALL4", "ART121", "EVN21", "EVN20", "EVN25", "EVN24", "EVN23", "EVN192", "EVN191", "RAH1", "ART180", "EVN194", "EVN193", "ART1", "ART187"}; List contents = action.getContents(); diff --git a/cms-plugin/src/test/java/com/agiletec/plugins/jacms/apsadmin/system/entity/TestJacmsEntityManagersAction.java b/cms-plugin/src/test/java/com/agiletec/plugins/jacms/apsadmin/system/entity/TestJacmsEntityManagersAction.java index 752ff2ffcf..bf4dca44e5 100644 --- a/cms-plugin/src/test/java/com/agiletec/plugins/jacms/apsadmin/system/entity/TestJacmsEntityManagersAction.java +++ b/cms-plugin/src/test/java/com/agiletec/plugins/jacms/apsadmin/system/entity/TestJacmsEntityManagersAction.java @@ -57,13 +57,13 @@ void testGetEntityPrototypes() throws Throwable { IEntityTypesAction action = (IEntityTypesAction) this.getAction(); List entityPrototypes = action.getEntityPrototypes(); assertNotNull(entityPrototypes); - assertEquals(4, entityPrototypes.size()); - + assertEquals(5, entityPrototypes.size()); + IApsEntity firstType = entityPrototypes.get(0); assertEquals("ART", firstType.getTypeCode()); assertEquals("Articolo rassegna stampa", firstType.getTypeDescr()); - - IApsEntity lastType = entityPrototypes.get(3); + + IApsEntity lastType = entityPrototypes.get(4); assertEquals("RAH", lastType.getTypeCode()); assertEquals("Tipo_Semplice", lastType.getTypeDescr()); } diff --git a/cms-plugin/src/test/java/org/entando/entando/plugins/jacms/web/content/ContentControllerIntegrationTest.java b/cms-plugin/src/test/java/org/entando/entando/plugins/jacms/web/content/ContentControllerIntegrationTest.java index e4400fdb98..10c241ca88 100644 --- a/cms-plugin/src/test/java/org/entando/entando/plugins/jacms/web/content/ContentControllerIntegrationTest.java +++ b/cms-plugin/src/test/java/org/entando/entando/plugins/jacms/web/content/ContentControllerIntegrationTest.java @@ -35,6 +35,7 @@ import com.agiletec.aps.system.common.entity.IEntityTypesConfigurer; import com.agiletec.aps.system.common.entity.model.EntitySearchFilter; import com.agiletec.aps.system.common.entity.model.attribute.AttributeInterface; +import com.agiletec.aps.system.common.entity.model.attribute.BooleanAttribute; import com.agiletec.aps.system.common.entity.model.attribute.CompositeAttribute; import com.agiletec.aps.system.common.entity.model.attribute.DateAttribute; import com.agiletec.aps.system.common.entity.model.attribute.ListAttribute; @@ -2332,14 +2333,10 @@ void testUpdateContentsBatch() throws Exception { batchContentStatusRequest.getCodes().add(newContentId3); - batchContentStatusRequest.getCodes().stream().forEach(code -> { - try { - Assertions.assertNotNull(this.contentManager.loadContent(code, false)); - Assertions.assertNull(this.contentManager.loadContent(code, true)); - } catch (Exception e) { - Assertions.fail(); - } - }); + batchContentStatusRequest.getCodes().forEach(code -> Assertions.assertDoesNotThrow(() -> { + Assertions.assertNotNull(this.contentManager.loadContent(code, false)); + Assertions.assertNull(this.contentManager.loadContent(code, true)); + })); result = mockMvc .perform(put("/plugins/cms/contents/status") @@ -2348,14 +2345,10 @@ void testUpdateContentsBatch() throws Exception { .header("Authorization", "Bearer " + accessToken)); result.andExpect(status().isOk()); - batchContentStatusRequest.getCodes().stream().forEach(code -> { - try { - Assertions.assertNotNull(this.contentManager.loadContent(code, false)); - Assertions.assertNotNull(this.contentManager.loadContent(code, true)); - } catch (Exception e) { - Assertions.fail(); - } - }); + batchContentStatusRequest.getCodes().forEach(code -> Assertions.assertDoesNotThrow(() -> { + Assertions.assertNotNull(this.contentManager.loadContent(code, false)); + Assertions.assertNotNull(this.contentManager.loadContent(code, true)); + })); batchContentStatusRequest.setStatus("draft"); @@ -2366,14 +2359,10 @@ void testUpdateContentsBatch() throws Exception { .header("Authorization", "Bearer " + accessToken)); result.andExpect(status().isOk()); - batchContentStatusRequest.getCodes().stream().forEach(code -> { - try { - Assertions.assertNotNull(this.contentManager.loadContent(code, false)); - Assertions.assertNull(this.contentManager.loadContent(code, true)); - } catch (Exception e) { - Assertions.fail(); - } - }); + batchContentStatusRequest.getCodes().forEach(code -> Assertions.assertDoesNotThrow(() -> { + Assertions.assertNotNull(this.contentManager.loadContent(code, false)); + Assertions.assertNull(this.contentManager.loadContent(code, true)); + })); } finally { if (null != newContentId1) { @@ -2715,6 +2704,66 @@ void testGetReturnsList() throws Exception { Assertions.assertEquals(payloadSize2, payloadSize); } + @Test + void testGetContentsFilteredByNestedCompositeBooleanAttribute() throws Exception { + // Solr-disabled end-to-end: a searchable boolean nested in a Composite is filterable via + // GET /plugins/cms/contents using the path key "_" as entityAttr. + String trueId = null; + String falseId = null; + try { + trueId = this.createPublishedAllWithNestedBoolean(Boolean.TRUE); + falseId = this.createPublishedAllWithNestedBoolean(Boolean.FALSE); + UserDetails user = new OAuth2TestUtils.UserBuilder("jack_bauer", "0x24") + .withAuthorization(Group.FREE_GROUP_NAME, "tempRole", Permission.BACKOFFICE).build(); + String accessToken = mockOAuthInterceptor(user); + ResultActions result = mockMvc + .perform(get("/plugins/cms/contents") + .param("status", IContentService.STATUS_ONLINE) + .param("filters[0].entityAttr", "Composite_Boolean") + .param("filters[0].operator", "eq") + .param("filters[0].value", "true") + .param("filters[0].type", "boolean") + .param("pageSize", "50") + .header("Authorization", "Bearer " + accessToken)); + result.andExpect(status().isOk()); + String body = result.andReturn().getResponse().getContentAsString(); + int size = JsonPath.read(body, "$.payload.size()"); + List ids = new ArrayList<>(); + for (int i = 0; i < size; i++) { + ids.add(JsonPath.read(body, "$.payload[" + i + "].id")); + } + Assertions.assertTrue(ids.contains(trueId)); + Assertions.assertFalse(ids.contains(falseId)); + } finally { + this.deletePublishedContent(trueId); + this.deletePublishedContent(falseId); + } + } + + private String createPublishedAllWithNestedBoolean(Boolean value) throws Exception { + Content clone = this.contentManager.loadContent("ALL4", false); + clone.setId(null); + clone.setMainGroup(Group.FREE_GROUP_NAME); + CompositeAttribute composite = (CompositeAttribute) clone.getAttribute("Composite"); + BooleanAttribute nestedBoolean = (BooleanAttribute) composite.getAttribute("Boolean"); + nestedBoolean.setSearchable(true); + nestedBoolean.setBooleanValue(value); + this.contentManager.saveContent(clone); + this.contentManager.insertOnLineContent(clone); + return clone.getId(); + } + + private void deletePublishedContent(String id) throws Exception { + if (null == id) { + return; + } + Content content = this.contentManager.loadContent(id, false); + if (null != content) { + this.contentManager.removeOnLineContent(content); + this.contentManager.deleteContent(content); + } + } + @Test void testLoadPublicEvents_1() throws Exception { UserDetails user = new OAuth2TestUtils.UserBuilder("jack_bauer", "0x24") @@ -4546,8 +4595,8 @@ void testGetContentsStatus() throws Throwable { UserDetails user = new OAuth2TestUtils.UserBuilder("jack_bauer", "0x24") .withAuthorization(Group.FREE_GROUP_NAME, "tempRole", Permission.BACKOFFICE).build(); String accessToken = mockOAuthInterceptor(user); - String lastModified = "2014-03-21 17:10:07"; - this.checkStatus(accessToken, 1, 6, 18, 25, lastModified); + String lastModified = "2026-01-01 00:00:04"; + this.checkStatus(accessToken, 1, 6, 22, 29, lastModified); List newContentIds = new ArrayList(); try { for (int i = 0; i < 10; i++) { @@ -4557,7 +4606,7 @@ void testGetContentsStatus() throws Throwable { newContentIds.add(content.getId()); } String dateString1 = DateConverter.getFormattedDate(new Date(), SystemConstants.API_DATE_FORMAT); - this.checkStatus(accessToken, 1+10, 6, 18, 25+10, dateString1); + this.checkStatus(accessToken, 1+10, 6, 22, 29+10, dateString1); synchronized (this) { this.wait(1000); @@ -4569,7 +4618,7 @@ void testGetContentsStatus() throws Throwable { } String dateString2 = DateConverter.getFormattedDate(new Date(), SystemConstants.API_DATE_FORMAT); Assertions.assertNotEquals(dateString1, dateString2); - this.checkStatus(accessToken, 1, 6, 18+10, 25+10, dateString2); + this.checkStatus(accessToken, 1, 6, 22+10, 29+10, dateString2); synchronized (this) { this.wait(1000); @@ -4584,7 +4633,7 @@ void testGetContentsStatus() throws Throwable { synchronized (this) { this.wait(1000); } - this.checkStatus(accessToken, 1, 6+10, 18, 25+10, dateString3); + this.checkStatus(accessToken, 1, 6+10, 22, 29+10, dateString3); } catch (Exception e) { throw e; } finally { @@ -4594,7 +4643,7 @@ void testGetContentsStatus() throws Throwable { this.contentManager.removeOnLineContent(content); this.contentManager.deleteContent(id); } - this.checkStatus(accessToken, 1, 6, 18, 25, lastModified); + this.checkStatus(accessToken, 1, 6, 22, 29, lastModified); } } diff --git a/contentworkflow-plugin/src/test/java/com/agiletec/plugins/jpcontentworkflow/apsadmin/content/TestContentFinderAction.java b/contentworkflow-plugin/src/test/java/com/agiletec/plugins/jpcontentworkflow/apsadmin/content/TestContentFinderAction.java index 38576ea74b..52f969e3c6 100644 --- a/contentworkflow-plugin/src/test/java/com/agiletec/plugins/jpcontentworkflow/apsadmin/content/TestContentFinderAction.java +++ b/contentworkflow-plugin/src/test/java/com/agiletec/plugins/jpcontentworkflow/apsadmin/content/TestContentFinderAction.java @@ -61,7 +61,7 @@ public void testSearch_1() throws Throwable { this.executeSearch("admin", params); ContentFinderAction action = (ContentFinderAction) this.getAction(); SearcherDaoPaginatedResult result = action.getPaginatedContentsId(10); - assertEquals(25, result.getCount().intValue()); + assertEquals(29, result.getCount().intValue()); assertEquals(10, result.getList().size()); this.executeSearch("editorCoach", params); action = (ContentFinderAction) this.getAction(); @@ -102,7 +102,7 @@ public void testSearch_2() throws Throwable { this.executeSearch("admin", params); ContentFinderAction action = (ContentFinderAction) this.getAction(); List contents = action.getContents(); - assertEquals(25, contents.size()); + assertEquals(29, contents.size()); this.executeSearch("editorCoach", params); action = (ContentFinderAction) this.getAction(); contents = action.getContents(); diff --git a/contentworkflow-plugin/src/test/java/com/agiletec/plugins/jpcontentworkflow/apsadmin/content/TestIntroNewContentAction.java b/contentworkflow-plugin/src/test/java/com/agiletec/plugins/jpcontentworkflow/apsadmin/content/TestIntroNewContentAction.java index 074111b1f9..29c16e50cc 100644 --- a/contentworkflow-plugin/src/test/java/com/agiletec/plugins/jpcontentworkflow/apsadmin/content/TestIntroNewContentAction.java +++ b/contentworkflow-plugin/src/test/java/com/agiletec/plugins/jpcontentworkflow/apsadmin/content/TestIntroNewContentAction.java @@ -67,7 +67,7 @@ public void testOpenNew() throws Throwable { assertEquals(Action.SUCCESS, result); JpCwIntroNewContentAction action = (JpCwIntroNewContentAction) this.getAction(); List contentTypes = action.getContentTypes(); - assertEquals(3, contentTypes.size()); + assertEquals(4, contentTypes.size()); for (int i=0; i infos = currAttribute.getSearchInfos(this.getLangManager().getLangs()); - if (currAttribute.isSearchable() && null != infos) { - for (int i=0; i attributes = entity.getAttributeList(); + for (int i = 0; i < attributes.size(); i++) { + this.addAttributeSearchRecord(id, attributes.get(i), null, false, stat); } stat.executeBatch(); } + + /** + * Recursively add the search records of an attribute. Elementary attributes are indexed exactly as + * before (by their own name, when searchable). Complex attributes are traversed to reach their + * elementary attributes - preserving the historical "flattened" behaviour - with one addition: a + * plain boolean attribute nested inside a Composite is indexed under the path key + * <composite>_<boolean> to avoid name collisions. {@code CheckBoxAttribute} + * and {@code ThreeStateAttribute} are excluded, and a boolean reached through a List/Monolist keeps + * the legacy plain-name behaviour (its path is not built). + * @param id the entity id. + * @param attribute the attribute to process. + * @param path the composite name path accumulated so far ('_'-joined), or null when at top level. + * @param listAncestor true when a List/Monolist is on the ancestry chain (disables path building). + * @param stat the batch statement to fill. + * @throws Throwable in case of error. + */ + private void addAttributeSearchRecord(String id, AttributeInterface attribute, String path, + boolean listAncestor, PreparedStatement stat) throws Throwable { + if (attribute.isSimple()) { + this.addSimpleAttributeSearchRecord(id, attribute, path, listAncestor, stat); + } else { + this.descendComplexAttributeSearchRecords(id, attribute, path, listAncestor, stat); + } + } + + private void addSimpleAttributeSearchRecord(String id, AttributeInterface attribute, String path, + boolean listAncestor, PreparedStatement stat) throws SQLException { + if (!attribute.isSearchable()) { + return; + } + List infos = attribute.getSearchInfos(this.getLangManager().getLangs()); + if (null == infos) { + return; + } + String attrName = (!listAncestor && null != path + && NestedBooleanSearchSupport.isIndexableNestedBoolean(attribute)) + ? path + "_" + attribute.getName() + : attribute.getName(); + this.addAttributeSearchInfoRecords(id, attrName, infos, stat); + } + + private void descendComplexAttributeSearchRecords(String id, AttributeInterface attribute, String path, + boolean listAncestor, PreparedStatement stat) throws Throwable { + List children = ((AbstractComplexAttribute) attribute).getAttributes(); + if (null == children) { + return; + } + boolean composite = (attribute instanceof CompositeAttribute) && !listAncestor; + String childPath = composite + ? ((null == path) ? attribute.getName() : path + "_" + attribute.getName()) + : null; + boolean childListAncestor = listAncestor || !(attribute instanceof CompositeAttribute); + for (AttributeInterface child : children) { + this.addAttributeSearchRecord(id, child, childPath, childListAncestor, stat); + } + } + + private void addAttributeSearchInfoRecords(String id, String attrName, + List infos, PreparedStatement stat) throws SQLException { + // id and attrname are invariant across the info rows of this attribute; set them once and let + // the per-row columns (3-6) be overwritten each iteration before addBatch(). + stat.setString(1, id); + stat.setString(2, attrName); + for (AttributeSearchInfo searchInfo : infos) { + stat.setString(3, searchInfo.getString()); + if (searchInfo.getDate() != null) { + stat.setTimestamp(4, new java.sql.Timestamp(searchInfo.getDate().getTime())); + } else { + stat.setDate(4, null); + } + stat.setBigDecimal(5, searchInfo.getBigDecimal()); + stat.setString(6, searchInfo.getLangCode()); + stat.addBatch(); + } + } protected void addEntityAttributeRoleRecord(String id, IApsEntity entity, Connection conn) { PreparedStatement stat = null; diff --git a/engine/src/main/java/com/agiletec/aps/system/common/entity/ApsEntityManager.java b/engine/src/main/java/com/agiletec/aps/system/common/entity/ApsEntityManager.java index 970cba53fd..c8d49118dd 100644 --- a/engine/src/main/java/com/agiletec/aps/system/common/entity/ApsEntityManager.java +++ b/engine/src/main/java/com/agiletec/aps/system/common/entity/ApsEntityManager.java @@ -256,6 +256,7 @@ public void addEntityPrototype(IApsEntity entityType) throws EntException { throw new EntException("Invalid entity type to add"); } this.sanitizeEntityTypeLabels(entityType); + NestedBooleanSearchSupport.logCollisionProneNestedBooleans(entityType); Map newEntityTypes = this.getEntityTypes(); newEntityTypes.put(entityType.getTypeCode(), entityType); this.updateEntityPrototypes(newEntityTypes); @@ -274,6 +275,7 @@ public void updateEntityPrototype(IApsEntity entityType) throws EntException { throw new EntException("Invalid entity type to update"); } this.sanitizeEntityTypeLabels(entityType); + NestedBooleanSearchSupport.logCollisionProneNestedBooleans(entityType); Map entityTypes = this.getEntityTypes(); IApsEntity oldEntityType = entityTypes.get(entityType.getTypeCode()); if (null == oldEntityType) { diff --git a/engine/src/main/java/com/agiletec/aps/system/common/entity/NestedBooleanSearchSupport.java b/engine/src/main/java/com/agiletec/aps/system/common/entity/NestedBooleanSearchSupport.java new file mode 100644 index 0000000000..b8424a7086 --- /dev/null +++ b/engine/src/main/java/com/agiletec/aps/system/common/entity/NestedBooleanSearchSupport.java @@ -0,0 +1,273 @@ +/* + * Copyright 2015-Present Entando Inc. (http://www.entando.com) All rights reserved. + * + * This library is free software; you can redistribute it and/or modify it under + * the terms of the GNU Lesser General Public License as published by the Free + * Software Foundation; either version 2.1 of the License, or (at your option) + * any later version. + * + * This library is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more + * details. + */ +package com.agiletec.aps.system.common.entity; + +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.regex.Pattern; + +import org.entando.entando.ent.exception.EntRuntimeException; +import org.entando.entando.ent.util.EntLogging.EntLogFactory; +import org.entando.entando.ent.util.EntLogging.EntLogger; + +import com.agiletec.aps.system.common.entity.model.IApsEntity; +import com.agiletec.aps.system.common.entity.model.attribute.AbstractComplexAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.AttributeInterface; +import com.agiletec.aps.system.common.entity.model.attribute.BooleanAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.CompositeAttribute; + +/** + * Single source of truth for indexing/searching boolean attributes nested inside a Composite + * attribute in the DB search tables ({@code contentsearch} / {@code workcontentsearch}). + * + *

    A boolean nested in a Composite is indexed under the path key {@code _} + * (composite names joined by '_') to avoid name collisions. All boolean-like attributes are eligible - + * {@link BooleanAttribute} and its subclasses {@code CheckBoxAttribute} and {@code ThreeStateAttribute} - + * governed by the {@code searchable} flag inherited from the content type. Only Composite + * ancestry is supported: a boolean reached through a {@code MonoListAttribute}/{@code ListAttribute} is + * not path-indexed.

    + * + *

    The write side ({@code AbstractEntityDAO.addEntitySearchRecord}) and the filter-key resolution + * ({@code EntitySearchFilter.getInstance}) share this class so that the key produced by the writer is + * exactly the key the reader resolves. It also gates {@code CompositeAttribute}'s decision to preserve + * the {@code searchable} flag on a composite child.

    + * + * @author Entando + */ +public final class NestedBooleanSearchSupport { + + private static final EntLogger logger = EntLogFactory.getSanitizedLogger(NestedBooleanSearchSupport.class); + + /** Separator used to render a nested attribute's hierarchy for humans (never occurs in a name). */ + public static final String LABEL_SEPARATOR = " > "; + + private NestedBooleanSearchSupport() { + // utility class + } + + /** + * Visitor invoked once per attribute a search form should offer. {@code keyPath} is the machine + * key (segment names joined by '_' - the DB {@code attrname} / Solr field / form field key); + * {@code labelPath} is the human hierarchy (segment names joined by {@link #LABEL_SEPARATOR}) built + * from the real tree boundaries, so it is correct even when a name itself contains '_'. + */ + @FunctionalInterface + private interface SearchableVisitor { + void visit(AttributeInterface attribute, String keyPath, String labelPath, boolean topLevel); + } + + /** + * Whether the given attribute is a boolean-like attribute eligible for nested (path-based) DB + * indexing, i.e. a {@link BooleanAttribute} or one of its subclasses (CheckBox, ThreeState). + * @param attribute the attribute to test. + * @return true if the attribute is boolean-like. + */ + public static boolean isIndexableNestedBoolean(AttributeInterface attribute) { + return attribute instanceof BooleanAttribute; + } + + /** + * Resolve a Composite-nested boolean attribute from its path key {@code _}. + * The traversal descends only through Composite children (never lists) and builds the same + * path the writer uses, so resolution matches indexing exactly. + * @param entity the entity (or type prototype) to inspect. + * @param key the underscore path key. + * @return the matching nested plain boolean attribute, or null if none matches. + */ + public static AttributeInterface resolveNestedBooleanByKey(IApsEntity entity, String key) { + if (null == entity || null == key) { + return null; + } + return resolve(entity.getAttributeList(), null, key); + } + + /** + * Collect the attributes that a search form should offer as filter criteria: every searchable + * top-level attribute (unchanged legacy behaviour, any type) plus every boolean-like attribute + * nested inside a Composite whose inherited {@code searchable} flag is set. Nested booleans + * are returned as lightweight same-class views renamed to their path key {@code _} + * (composite names joined by '_'), so the key a form field carries is exactly the key the DB search + * records were written under. The view keeps its concrete class, so callers relying on {@code + * instanceof BooleanAttribute} keep working. Lists ({@code MonoList}/{@code List}) are never descended, + * matching the write side. + * @param entity the entity (or type prototype) to inspect. + * @return the ordered list of searchable attributes; never null. + */ + public static List collectSearchable(IApsEntity entity) { + List result = new ArrayList<>(); + if (null == entity) { + return result; + } + walkSearchable(entity.getAttributeList(), null, null, + (attribute, keyPath, labelPath, topLevel) -> + result.add(topLevel ? attribute : nestedBooleanView(attribute, keyPath))); + return result; + } + + /** + * Build the human-readable label for every attribute {@link #collectSearchable} offers, keyed by + * the same machine key. The label is the attribute's hierarchy joined by {@link #LABEL_SEPARATOR} + * (e.g. {@code "compo > cmp_bool"}), reconstructed from the real tree boundaries - so it is + * correct even when a composite or a boolean name itself contains a '_'. Callers (the search-form + * JSPs) render this verbatim instead of splitting the flattened key, which would mis-segment such + * names. Top-level attributes map to their own name (unchanged rendering). Insertion order matches + * {@link #collectSearchable}. + * @param entity the entity (or type prototype) to inspect. + * @return a map from machine key to display label; never null. + */ + public static Map buildSearchLabels(IApsEntity entity) { + Map labels = new LinkedHashMap<>(); + if (null == entity) { + return labels; + } + walkSearchable(entity.getAttributeList(), null, null, + (attribute, keyPath, labelPath, topLevel) -> labels.put(keyPath, labelPath)); + return labels; + } + + /** + * Log a {@code WARN} for every Composite-nested searchable boolean whose path has a segment name + * containing the path delimiter '_'. Such a name makes the flattened key ambiguous - e.g. a boolean + * {@code cmp_bool} in composite {@code compo} yields {@code compo_cmp_bool}, indistinguishable from a + * boolean {@code bool} in composite {@code compo_cmp} - so it can collide with a differently + * structured attribute (same DB {@code attrname} / Solr field). Called at content-type persist time + * so authors are alerted before a colliding sibling is added. Detection only; nothing is rejected. + * @param entity the entity type being persisted. + */ + public static void logCollisionProneNestedBooleans(IApsEntity entity) { + Map collisionProne = findCollisionProneNestedBooleans(entity); + for (Map.Entry entry : collisionProne.entrySet()) { + logger.warn("Nested boolean search key '{}' (attribute path '{}') has a segment name " + + "containing '_', the path delimiter; the flattened key can collide with a " + + "differently-structured attribute. Avoid '_' in composite/attribute names used " + + "for nested boolean search.", entry.getKey(), entry.getValue()); + } + } + + /** + * Pure detection behind {@link #logCollisionProneNestedBooleans}: the Composite-nested searchable + * booleans whose path has a segment name containing '_' (the path delimiter), mapped {@code key -> + * label}. Package-private for unit testing. + */ + static Map findCollisionProneNestedBooleans(IApsEntity entity) { + Map found = new LinkedHashMap<>(); + if (null == entity) { + return found; + } + walkSearchable(entity.getAttributeList(), null, null, (attribute, keyPath, labelPath, topLevel) -> { + if (topLevel) { + return; + } + for (String segment : labelPath.split(Pattern.quote(LABEL_SEPARATOR))) { + if (segment.contains("_")) { + found.put(keyPath, labelPath); + return; + } + } + }); + return found; + } + + /** + * Single traversal shared by {@link #collectSearchable}, {@link #buildSearchLabels} and + * {@link #logCollisionProneNestedBooleans}, so machine key and display label are always built from + * the same segments and can never drift apart. Top level offers any active, searchable attribute + * (legacy behaviour); below a Composite only searchable boolean-like leaves are offered. Lists + * ({@code MonoList}/{@code List}) and other complex types are never descended. + */ + private static void walkSearchable(List attributes, String keyPath, + String labelPath, SearchableVisitor visitor) { + if (null == attributes) { + return; + } + for (AttributeInterface attribute : attributes) { + if (null == keyPath) { + visitTopLevel(attribute, visitor); + } else { + visitNested(attribute, keyPath, labelPath, visitor); + } + } + } + + private static void visitTopLevel(AttributeInterface attribute, SearchableVisitor visitor) { + if (attribute.isActive() && attribute.isSearchable()) { + visitor.visit(attribute, attribute.getName(), attribute.getName(), true); + } + if (attribute instanceof CompositeAttribute) { + walkSearchable(((AbstractComplexAttribute) attribute).getAttributes(), + attribute.getName(), attribute.getName(), visitor); + } + } + + private static void visitNested(AttributeInterface attribute, String keyPath, String labelPath, + SearchableVisitor visitor) { + if (attribute.isSimple() && isIndexableNestedBoolean(attribute) && attribute.isSearchable()) { + visitor.visit(attribute, keyPath + "_" + attribute.getName(), + labelPath + LABEL_SEPARATOR + attribute.getName(), false); + } else if (attribute instanceof CompositeAttribute) { + walkSearchable(((AbstractComplexAttribute) attribute).getAttributes(), + keyPath + "_" + attribute.getName(), + labelPath + LABEL_SEPARATOR + attribute.getName(), visitor); + } + } + + /** + * Build a lightweight, same-class stand-in for a Composite-nested boolean, renamed to its path + * key. A search form only needs the name (which becomes the form field / filter key), the type + * (which drives the widget dispatch) and the {@code searchable} flag; it never touches the value, + * handler or validation rules of this stand-in - so, unlike a full {@code getAttributePrototype()} + * clone, this neither depends on the attribute having a handler nor drags along unused state. + * @param source the real nested boolean-like attribute. + * @param pathKey the full path key {@code _} to expose as its name. + * @return a new attribute of the same concrete class, so {@code instanceof BooleanAttribute} holds. + */ + private static AttributeInterface nestedBooleanView(AttributeInterface source, String pathKey) { + try { + AttributeInterface view = source.getClass().getDeclaredConstructor().newInstance(); + view.setName(pathKey); + view.setType(source.getType()); + view.setSearchable(source.isSearchable()); + return view; + } catch (ReflectiveOperationException e) { + throw new EntRuntimeException("Error creating nested boolean search view for '" + pathKey + "'", e); + } + } + + private static AttributeInterface resolve(List attributes, String path, String key) { + if (null == attributes) { + return null; + } + for (int i = 0; i < attributes.size(); i++) { + AttributeInterface attribute = attributes.get(i); + if (attribute.isSimple()) { + if (null != path && isIndexableNestedBoolean(attribute) + && key.equals(path + "_" + attribute.getName())) { + return attribute; + } + } else if (attribute instanceof CompositeAttribute) { + String childPath = (null == path) ? attribute.getName() : path + "_" + attribute.getName(); + AttributeInterface found = resolve(((AbstractComplexAttribute) attribute).getAttributes(), childPath, key); + if (null != found) { + return found; + } + } + // Lists (MonoList/List) and any other complex type are intentionally not descended: + // list-reached booleans are out of scope for path indexing. + } + return null; + } + +} diff --git a/engine/src/main/java/com/agiletec/aps/system/common/entity/model/EntitySearchFilter.java b/engine/src/main/java/com/agiletec/aps/system/common/entity/model/EntitySearchFilter.java index 4686db3c9e..eeb902abd8 100644 --- a/engine/src/main/java/com/agiletec/aps/system/common/entity/model/EntitySearchFilter.java +++ b/engine/src/main/java/com/agiletec/aps/system/common/entity/model/EntitySearchFilter.java @@ -28,6 +28,7 @@ import org.entando.entando.ent.util.EntLogging.EntLogFactory; import com.agiletec.aps.system.common.FieldSearchFilter; +import com.agiletec.aps.system.common.entity.NestedBooleanSearchSupport; import com.agiletec.aps.system.common.entity.model.attribute.AttributeInterface; import com.agiletec.aps.system.common.entity.model.attribute.BooleanAttribute; import com.agiletec.aps.system.common.entity.model.attribute.DateAttribute; @@ -40,6 +41,10 @@ * This class implements a filter to search among entities. * @author E.Santoboni */ +// NOTE: java:S2143 ("use the java.time API") is intentionally suppressed. This filter model uses legacy +// java.util.Date/SimpleDateFormat for its date value/serialization contract; migrating it to java.time +// is out of scope for ESB-1133 (boolean search) and tracked separately. +@SuppressWarnings("java:S2143") public class EntitySearchFilter extends FieldSearchFilter implements Serializable { private static final EntLogger _logger = EntLogFactory.getSanitizedLogger(EntitySearchFilter.class); @@ -371,6 +376,11 @@ public static EntitySearchFilter getInstance(IApsEntity prototype, Properties pr AttributeInterface attr = null; if (null != key) { attr = (AttributeInterface) prototype.getAttribute(key); + if (null == attr) { + // fall back to a Composite-nested boolean referenced by its path key + // '_' (top-level attributes always take precedence) + attr = NestedBooleanSearchSupport.resolveNestedBooleanByKey(prototype, key); + } filter.setKey(key); } else { attr = (AttributeInterface) prototype.getAttributeByRole(roleName); diff --git a/engine/src/main/java/com/agiletec/aps/system/common/entity/model/attribute/CompositeAttribute.java b/engine/src/main/java/com/agiletec/aps/system/common/entity/model/attribute/CompositeAttribute.java index 79522df83c..9779a3ff98 100644 --- a/engine/src/main/java/com/agiletec/aps/system/common/entity/model/attribute/CompositeAttribute.java +++ b/engine/src/main/java/com/agiletec/aps/system/common/entity/model/attribute/CompositeAttribute.java @@ -13,6 +13,7 @@ */ package com.agiletec.aps.system.common.entity.model.attribute; +import com.agiletec.aps.system.common.entity.NestedBooleanSearchSupport; import com.agiletec.aps.system.common.entity.model.AttributeFieldError; import com.agiletec.aps.system.common.entity.model.AttributeTracer; import com.agiletec.aps.system.common.entity.model.IApsEntity; @@ -183,7 +184,12 @@ private void extractAttributeCompositeElement(Map at } compositeAttrElem = (AttributeInterface) compositeAttrElem.getAttributePrototype(); compositeAttrElem.setAttributeConfig(currentAttrJdomElem); - compositeAttrElem.setSearchable(false); + // Composite children are non-searchable by design (they would collide in the DB search tables + // under their unqualified name), EXCEPT plain boolean children, which are indexed under the + // path key "_" and so may keep their configured searchable flag. + if (!NestedBooleanSearchSupport.isIndexableNestedBoolean(compositeAttrElem)) { + compositeAttrElem.setSearchable(false); + } compositeAttrElem.setDefaultLangCode(this.getDefaultLangCode()); this.addAttribute(compositeAttrElem); } diff --git a/engine/src/test/java/com/agiletec/aps/system/common/entity/AbstractEntityDAONestedBooleanTest.java b/engine/src/test/java/com/agiletec/aps/system/common/entity/AbstractEntityDAONestedBooleanTest.java new file mode 100644 index 0000000000..d75aed78e6 --- /dev/null +++ b/engine/src/test/java/com/agiletec/aps/system/common/entity/AbstractEntityDAONestedBooleanTest.java @@ -0,0 +1,302 @@ +/* + * Copyright 2015-Present Entando Inc. (http://www.entando.com) All rights reserved. + * + * This library is free software; you can redistribute it and/or modify it under + * the terms of the GNU Lesser General Public License as published by the Free + * Software Foundation; either version 2.1 of the License, or (at your option) + * any later version. + * + * This library is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more + * details. + */ +package com.agiletec.aps.system.common.entity; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.atLeast; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.agiletec.aps.system.common.entity.model.ApsEntity; +import com.agiletec.aps.system.common.entity.model.ApsEntityRecord; +import com.agiletec.aps.system.common.entity.model.IApsEntity; +import com.agiletec.aps.system.common.entity.model.attribute.AttributeInterface; +import com.agiletec.aps.system.common.entity.model.attribute.BooleanAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.CheckBoxAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.CompositeAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.DateAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.MonoListAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.TextAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.ThreeStateAttribute; +import com.agiletec.aps.system.services.lang.ILangManager; +import com.agiletec.aps.system.services.lang.Lang; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.Timestamp; +import java.util.Collections; +import java.util.Date; +import java.util.List; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; + +/** + * Verifies the {@code attrname} values that {@link AbstractEntityDAO#addEntitySearchRecord} writes to + * the DB search tables, focusing on the new behaviour: a plain boolean nested in a Composite is stored + * under the path key {@code _}, while everything else keeps its historical name. + */ +class AbstractEntityDAONestedBooleanTest { + + private TestEntityDAO dao; + private PreparedStatement stat; + + @BeforeEach + void setUp() { + this.dao = new TestEntityDAO(); + ILangManager langManager = mock(ILangManager.class); + Lang en = new Lang(); + en.setCode("en"); + en.setDescr("English"); + when(langManager.getLangs()).thenReturn(Collections.singletonList(en)); + this.dao.setLangManager(langManager); + this.stat = mock(PreparedStatement.class); + } + + @Test + void topLevelSearchableBooleanKeepsPlainName() throws Throwable { + assertEquals(List.of("flag"), + writtenAttrNames(entity(booleanAttr("flag", true, Boolean.TRUE)))); + } + + @Test + void compositeSearchableBooleanUsesPathName() throws Throwable { + assertEquals(List.of("address_certified"), + writtenAttrNames(entity(composite("address", booleanAttr("certified", true, Boolean.TRUE))))); + } + + @Test + void compositeNonSearchableBooleanIsNotIndexed() throws Throwable { + assertEquals(List.of(), + writtenAttrNames(entity(composite("address", booleanAttr("certified", false, Boolean.TRUE))))); + } + + @Test + void deepCompositeBooleanUsesFullPathName() throws Throwable { + assertEquals(List.of("a_b_c"), + writtenAttrNames(entity(composite("a", composite("b", booleanAttr("c", true, Boolean.TRUE)))))); + } + + @Test + void nestedCheckBoxIsPathIndexed() throws Throwable { + // CheckBox is a boolean-like -> path-qualified like Boolean (value null -> "false") + assertEquals(List.of("address_verified"), + writtenAttrNames(entity(composite("address", checkBox("verified", true))))); + } + + @Test + void nestedThreeStateIsPathIndexed() throws Throwable { + ThreeStateAttribute maybe = threeState("maybe", true); + maybe.setBooleanValue(Boolean.TRUE); // non-null so ThreeState produces a search row + assertEquals(List.of("address_maybe"), + writtenAttrNames(entity(composite("address", maybe)))); + } + + @Test + void listReachedBooleanKeepsPlainName() throws Throwable { + assertEquals(List.of("flag"), + writtenAttrNames(entity(monolist("tags", booleanAttr("flag", true, Boolean.TRUE))))); + } + + @Test + void listOfCompositeBooleanKeepsPlainName() throws Throwable { + assertEquals(List.of("active"), + writtenAttrNames(entity(monolist("rows", composite("row", booleanAttr("active", true, Boolean.TRUE)))))); + } + + @Test + void mixedEntityWritesEachAttributeUnderItsExpectedName() throws Throwable { + ApsEntity entity = entity( + booleanAttr("published", true, Boolean.TRUE), + composite("address", booleanAttr("certified", true, Boolean.TRUE))); + assertEquals(List.of("published", "address_certified"), writtenAttrNames(entity)); + } + + @Test + void nestedNonBooleanSimpleAttributeKeepsPlainName() throws Throwable { + // a non boolean-like attribute nested in a Composite is never path-qualified, even if searchable + assertEquals(List.of("note"), + writtenAttrNames(entity(composite("address", textAttr("note", true, "hello"))))); + } + + @Test + void searchableAttributeWithNoSearchInfosIsNotIndexed() throws Throwable { + // DateAttribute.getSearchInfos() returns null when no date is set + assertEquals(List.of(), writtenAttrNames(entity(dateAttr("published", true, null)))); + } + + @Test + void searchableDateAttributeWritesTimestamp() throws Throwable { + Date date = new Date(); + assertEquals(List.of("published"), writtenAttrNames(entity(dateAttr("published", true, date)))); + verify(this.stat, atLeast(1)).setTimestamp(eq(4), any(Timestamp.class)); + } + + @Test + void nullChildrenListIsSkippedWithoutError() throws Throwable { + CompositeAttribute compositeWithNullChildren = spy(composite("address", booleanAttr("certified", true, Boolean.TRUE))); + when(compositeWithNullChildren.getAttributes()).thenReturn(null); + assertEquals(List.of(), writtenAttrNames(entity(compositeWithNullChildren))); + verify(this.stat, never()).setString(eq(2), any()); + } + + private List writtenAttrNames(IApsEntity entity) throws Throwable { + ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); + this.dao.addSearchRecords("ENTITY1", entity, this.stat); + verify(this.stat, atLeast(0)).setString(eq(2), captor.capture()); + return captor.getAllValues(); + } + + // --- fixtures ---------------------------------------------------------- + + private BooleanAttribute booleanAttr(String name, boolean searchable, Boolean value) { + BooleanAttribute a = new BooleanAttribute(); + a.setName(name); + a.setSearchable(searchable); + a.setBooleanValue(value); + return a; + } + + private CheckBoxAttribute checkBox(String name, boolean searchable) { + CheckBoxAttribute a = new CheckBoxAttribute(); + a.setName(name); + a.setSearchable(searchable); + return a; + } + + private ThreeStateAttribute threeState(String name, boolean searchable) { + ThreeStateAttribute a = new ThreeStateAttribute(); + a.setName(name); + a.setSearchable(searchable); + return a; + } + + private TextAttribute textAttr(String name, boolean searchable, String text) { + TextAttribute a = new TextAttribute(); + a.setName(name); + a.setSearchable(searchable); + a.setText(text, "en"); + return a; + } + + private DateAttribute dateAttr(String name, boolean searchable, Date date) { + DateAttribute a = new DateAttribute(); + a.setName(name); + a.setSearchable(searchable); + a.setDate(date); + return a; + } + + private CompositeAttribute composite(String name, AttributeInterface... children) { + CompositeAttribute c = new CompositeAttribute(); + c.setName(name); + for (AttributeInterface child : children) { + c.getAttributes().add(child); + } + return c; + } + + private MonoListAttribute monolist(String name, AttributeInterface... elements) { + MonoListAttribute list = new MonoListAttribute(); + list.setName(name); + for (AttributeInterface element : elements) { + list.getAttributes().add(element); + } + return list; + } + + private ApsEntity entity(AttributeInterface... attributes) { + ApsEntity entity = new ApsEntity(); + entity.setTypeCode("TST"); + for (AttributeInterface attribute : attributes) { + entity.addAttribute(attribute); + } + return entity; + } + + /** + * Minimal concrete {@link AbstractEntityDAO} exposing the protected search-record writer. + */ + private static class TestEntityDAO extends AbstractEntityDAO { + + public void addSearchRecords(String id, IApsEntity entity, PreparedStatement stat) throws Throwable { + this.addEntitySearchRecord(id, entity, stat); + } + + @Override + protected String getAddEntityRecordQuery() { + return null; + } + + @Override + protected void buildAddEntityStatement(IApsEntity entity, PreparedStatement stat) throws Throwable { + // no-op + } + + @Override + protected String getDeleteEntityRecordQuery() { + return null; + } + + @Override + protected String getUpdateEntityRecordQuery() { + return null; + } + + @Override + protected void buildUpdateEntityStatement(IApsEntity entity, PreparedStatement stat) throws Throwable { + // no-op + } + + @Override + protected String getLoadEntityRecordQuery() { + return null; + } + + @Override + protected ApsEntityRecord createEntityRecord(ResultSet res) throws Throwable { + return null; + } + + @Override + protected String getAddingSearchRecordQuery() { + return null; + } + + @Override + protected String getAddingAttributeRoleRecordQuery() { + return null; + } + + @Override + protected String getRemovingSearchRecordQuery() { + return null; + } + + @Override + protected String getRemovingAttributeRoleRecordQuery() { + return null; + } + + @Override + protected String getExtractingAllEntityIdQuery() { + return null; + } + } + +} diff --git a/engine/src/test/java/com/agiletec/aps/system/common/entity/NestedBooleanSearchSupportTest.java b/engine/src/test/java/com/agiletec/aps/system/common/entity/NestedBooleanSearchSupportTest.java new file mode 100644 index 0000000000..8bcba305f3 --- /dev/null +++ b/engine/src/test/java/com/agiletec/aps/system/common/entity/NestedBooleanSearchSupportTest.java @@ -0,0 +1,344 @@ +/* + * Copyright 2015-Present Entando Inc. (http://www.entando.com) All rights reserved. + * + * This library is free software; you can redistribute it and/or modify it under + * the terms of the GNU Lesser General Public License as published by the Free + * Software Foundation; either version 2.1 of the License, or (at your option) + * any later version. + * + * This library is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more + * details. + */ +package com.agiletec.aps.system.common.entity; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; + +import java.util.List; +import java.util.Map; + +import com.agiletec.aps.system.common.entity.model.ApsEntity; +import com.agiletec.aps.system.common.entity.model.attribute.AttributeInterface; +import com.agiletec.aps.system.common.entity.model.attribute.BooleanAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.CheckBoxAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.CompositeAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.MonoListAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.MonoTextAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.ThreeStateAttribute; +import org.junit.jupiter.api.Test; + +/** + * Unit tests for {@link NestedBooleanSearchSupport} - the shared logic used to index/resolve + * boolean attributes nested inside a Composite in the DB search tables (Solr-disabled path). + */ +class NestedBooleanSearchSupportTest { + + @Test + void isIndexableNestedBoolean_shouldAcceptAllBooleanLikes() { + assertTrue(NestedBooleanSearchSupport.isIndexableNestedBoolean(booleanAttr("b", true, Boolean.TRUE))); + assertTrue(NestedBooleanSearchSupport.isIndexableNestedBoolean(checkBox("c", true))); + assertTrue(NestedBooleanSearchSupport.isIndexableNestedBoolean(threeState("t", true))); + assertFalse(NestedBooleanSearchSupport.isIndexableNestedBoolean(new MonoTextAttribute())); + assertFalse(NestedBooleanSearchSupport.isIndexableNestedBoolean(null)); + } + + @Test + void shouldResolveCompositeNestedBoolean() { + BooleanAttribute certified = booleanAttr("certified", true, Boolean.TRUE); + ApsEntity entity = entity(composite("address", certified)); + assertSame(certified, NestedBooleanSearchSupport.resolveNestedBooleanByKey(entity, "address_certified")); + } + + @Test + void shouldResolveDeepCompositePath() { + BooleanAttribute leaf = booleanAttr("c", true, Boolean.TRUE); + ApsEntity entity = entity(composite("a", composite("b", leaf))); + assertSame(leaf, NestedBooleanSearchSupport.resolveNestedBooleanByKey(entity, "a_b_c")); + } + + @Test + void shouldReturnNullForUnknownKey() { + ApsEntity entity = entity(composite("address", booleanAttr("certified", true, Boolean.TRUE))); + assertNull(NestedBooleanSearchSupport.resolveNestedBooleanByKey(entity, "address_missing")); + assertNull(NestedBooleanSearchSupport.resolveNestedBooleanByKey(entity, "nope")); + assertNull(NestedBooleanSearchSupport.resolveNestedBooleanByKey(null, "address_certified")); + assertNull(NestedBooleanSearchSupport.resolveNestedBooleanByKey(entity, null)); + } + + @Test + void shouldResolveNestedCheckBoxAndThreeState() { + CheckBoxAttribute verified = checkBox("verified", true); + ThreeStateAttribute maybe = threeState("maybe", true); + ApsEntity entity = entity(composite("address", verified, maybe)); + assertSame(verified, NestedBooleanSearchSupport.resolveNestedBooleanByKey(entity, "address_verified")); + assertSame(maybe, NestedBooleanSearchSupport.resolveNestedBooleanByKey(entity, "address_maybe")); + } + + @Test + void shouldNotResolveListReachedBoolean() { + ApsEntity listOfBoolean = entity(monolist("tags", booleanAttr("flag", true, Boolean.TRUE))); + assertNull(NestedBooleanSearchSupport.resolveNestedBooleanByKey(listOfBoolean, "tags_flag")); + + ApsEntity listOfComposite = entity(monolist("rows", composite("row", booleanAttr("active", true, Boolean.TRUE)))); + assertNull(NestedBooleanSearchSupport.resolveNestedBooleanByKey(listOfComposite, "rows_row_active")); + } + + @Test + void shouldNotResolveTopLevelAttribute() { + // the resolver only matches nested booleans; top-level precedence is handled by the caller + ApsEntity entity = entity(booleanAttr("flag", true, Boolean.TRUE)); + assertNull(NestedBooleanSearchSupport.resolveNestedBooleanByKey(entity, "flag")); + } + + @Test + void shouldSkipCompositeWithNullChildrenList() { + CompositeAttribute composite = spy(composite("address", booleanAttr("certified", true, Boolean.TRUE))); + when(composite.getAttributes()).thenReturn(null); + ApsEntity entity = entity(composite); + assertNull(NestedBooleanSearchSupport.resolveNestedBooleanByKey(entity, "address_certified")); + } + + @Test + void shouldNotResolveNestedNonBooleanAttribute() { + // a non boolean-like simple attribute nested in a Composite is never eligible, whatever the key + MonoTextAttribute note = new MonoTextAttribute(); + note.setName("note"); + ApsEntity entity = entity(composite("address", note)); + assertNull(NestedBooleanSearchSupport.resolveNestedBooleanByKey(entity, "address_note")); + } + + // --- collectSearchable (search-form enumeration) ----------------------- + + @Test + void collectSearchable_shouldReturnEmptyForNullEntity() { + assertTrue(NestedBooleanSearchSupport.collectSearchable(null).isEmpty()); + } + + @Test + void collectSearchable_shouldKeepSearchableTopLevelAttributesUnchanged() { + // any searchable top-level attribute is offered (legacy behaviour), non-searchable ones are not + BooleanAttribute topFlag = booleanAttr("flag", true, Boolean.TRUE); + MonoTextAttribute title = monoText("title", true); + MonoTextAttribute hidden = monoText("hidden", false); + ApsEntity entity = entity(topFlag, title, hidden); + List result = NestedBooleanSearchSupport.collectSearchable(entity); + assertEquals(2, result.size()); + // top-level entries are the very same instances, never copied + assertSame(topFlag, result.get(0)); + assertSame(title, result.get(1)); + } + + @Test + void collectSearchable_shouldExposeCompositeNestedBooleanUnderPathKey() { + BooleanAttribute certified = booleanAttr("certified", true, Boolean.TRUE); + certified.setType("Boolean"); + ApsEntity entity = entity(composite("address", certified)); + List result = NestedBooleanSearchSupport.collectSearchable(entity); + assertEquals(1, result.size()); + AttributeInterface view = result.get(0); + assertEquals("address_certified", view.getName()); + assertEquals("Boolean", view.getType()); + assertTrue(view.isSearchable()); + // same concrete class, so instanceof-based dispatch (JSP/EntityActionHelper) keeps working + assertInstanceOf(BooleanAttribute.class, view); + // it is a stand-in and the original attribute is left untouched + assertNotSame(certified, view); + assertEquals("certified", certified.getName()); + } + + @Test + void collectSearchable_shouldExposeAllBooleanLikesNested() { + ApsEntity entity = entity(composite("address", + booleanAttr("b", true, Boolean.TRUE), checkBox("c", true), threeState("t", true))); + List result = NestedBooleanSearchSupport.collectSearchable(entity); + assertEquals(3, result.size()); + assertEquals("address_b", result.get(0).getName()); + assertEquals("address_c", result.get(1).getName()); + assertEquals("address_t", result.get(2).getName()); + } + + @Test + void collectSearchable_shouldExcludeNonSearchableNestedBoolean() { + ApsEntity entity = entity(composite("address", booleanAttr("certified", false, Boolean.TRUE))); + assertTrue(NestedBooleanSearchSupport.collectSearchable(entity).isEmpty()); + } + + @Test + void collectSearchable_shouldExcludeNonBooleanNestedAttribute() { + ApsEntity entity = entity(composite("address", monoText("note", true))); + assertTrue(NestedBooleanSearchSupport.collectSearchable(entity).isEmpty()); + } + + @Test + void collectSearchable_shouldResolveDeepCompositePath() { + ApsEntity entity = entity(composite("a", composite("b", booleanAttr("c", true, Boolean.TRUE)))); + List result = NestedBooleanSearchSupport.collectSearchable(entity); + assertEquals(1, result.size()); + assertEquals("a_b_c", result.get(0).getName()); + } + + @Test + void collectSearchable_shouldNotDescendLists() { + ApsEntity listOfBoolean = entity(monolist("tags", booleanAttr("flag", true, Boolean.TRUE))); + assertTrue(NestedBooleanSearchSupport.collectSearchable(listOfBoolean).isEmpty()); + + ApsEntity listOfComposite = entity(monolist("rows", + composite("row", booleanAttr("active", true, Boolean.TRUE)))); + assertTrue(NestedBooleanSearchSupport.collectSearchable(listOfComposite).isEmpty()); + } + + @Test + void collectSearchable_shouldKeepBothTopLevelAndNested() { + BooleanAttribute topFlag = booleanAttr("flag", true, Boolean.TRUE); + ApsEntity entity = entity(topFlag, composite("address", booleanAttr("certified", true, Boolean.TRUE))); + List result = NestedBooleanSearchSupport.collectSearchable(entity); + assertEquals(2, result.size()); + assertSame(topFlag, result.get(0)); + assertEquals("address_certified", result.get(1).getName()); + } + + // --- buildSearchLabels (hierarchical display labels) ------------------- + + @Test + void buildSearchLabels_shouldKeepSegmentBoundariesWhenNameContainsUnderscore() { + // the reported defect: composite "compo" + boolean "cmp_bool" must read "compo > cmp_bool", + // NOT "compo > cmp > bool" - the label is built from the real tree, not by splitting the key + BooleanAttribute cmpBool = booleanAttr("cmp_bool", true, Boolean.TRUE); + ApsEntity entity = entity(composite("compo", cmpBool)); + Map labels = NestedBooleanSearchSupport.buildSearchLabels(entity); + assertEquals(1, labels.size()); + assertEquals("compo > cmp_bool", labels.get("compo_cmp_bool")); + } + + @Test + void buildSearchLabels_shouldRenderDeepHierarchy() { + ApsEntity entity = entity(composite("a", composite("b", booleanAttr("c", true, Boolean.TRUE)))); + assertEquals("a > b > c", NestedBooleanSearchSupport.buildSearchLabels(entity).get("a_b_c")); + } + + @Test + void buildSearchLabels_shouldLabelTopLevelByItsOwnName() { + ApsEntity entity = entity(booleanAttr("flag", true, Boolean.TRUE), monoText("title", true)); + Map labels = NestedBooleanSearchSupport.buildSearchLabels(entity); + assertEquals("flag", labels.get("flag")); + assertEquals("title", labels.get("title")); + } + + @Test + void buildSearchLabels_keyAndLabelShareTheSameSegments() { + // alignment invariant: substituting the label separator back to '_' reproduces the key exactly, + // for every entry - guarantees label and machine key never drift + ApsEntity entity = entity( + booleanAttr("flag", true, Boolean.TRUE), + composite("compo", booleanAttr("cmp_bool", true, Boolean.TRUE)), + composite("a", composite("b", booleanAttr("c", true, Boolean.TRUE)))); + Map labels = NestedBooleanSearchSupport.buildSearchLabels(entity); + for (Map.Entry e : labels.entrySet()) { + assertEquals(e.getKey(), e.getValue().replace(NestedBooleanSearchSupport.LABEL_SEPARATOR, "_")); + } + } + + @Test + void buildSearchLabels_shouldNotDescendLists() { + ApsEntity entity = entity(monolist("rows", composite("row", booleanAttr("active", true, Boolean.TRUE)))); + assertTrue(NestedBooleanSearchSupport.buildSearchLabels(entity).isEmpty()); + } + + // --- findCollisionProneNestedBooleans (B1 detection) ------------------- + + @Test + void findCollisionProne_shouldFlagUnderscoreInLeafName() { + ApsEntity entity = entity(composite("compo", booleanAttr("cmp_bool", true, Boolean.TRUE))); + Map flagged = NestedBooleanSearchSupport.findCollisionProneNestedBooleans(entity); + assertEquals(1, flagged.size()); + assertEquals("compo > cmp_bool", flagged.get("compo_cmp_bool")); + } + + @Test + void findCollisionProne_shouldFlagUnderscoreInCompositeName() { + ApsEntity entity = entity(composite("compo_cmp", booleanAttr("bool", true, Boolean.TRUE))); + assertTrue(NestedBooleanSearchSupport.findCollisionProneNestedBooleans(entity).containsKey("compo_cmp_bool")); + } + + @Test + void findCollisionProne_shouldBeEmptyForCleanNames() { + ApsEntity entity = entity(composite("compo", booleanAttr("flag", true, Boolean.TRUE))); + assertTrue(NestedBooleanSearchSupport.findCollisionProneNestedBooleans(entity).isEmpty()); + } + + @Test + void findCollisionProne_shouldIgnoreTopLevelUnderscoreName() { + // a top-level attribute is keyed by its own name; only nested-path composition can collide + ApsEntity entity = entity(booleanAttr("top_flag", true, Boolean.TRUE)); + assertTrue(NestedBooleanSearchSupport.findCollisionProneNestedBooleans(entity).isEmpty()); + } + + // --- helpers ----------------------------------------------------------- + + private MonoTextAttribute monoText(String name, boolean searchable) { + MonoTextAttribute a = new MonoTextAttribute(); + a.setName(name); + a.setSearchable(searchable); + return a; + } + + + private BooleanAttribute booleanAttr(String name, boolean searchable, Boolean value) { + BooleanAttribute a = new BooleanAttribute(); + a.setName(name); + a.setSearchable(searchable); + a.setBooleanValue(value); + return a; + } + + private CheckBoxAttribute checkBox(String name, boolean searchable) { + CheckBoxAttribute a = new CheckBoxAttribute(); + a.setName(name); + a.setSearchable(searchable); + return a; + } + + private ThreeStateAttribute threeState(String name, boolean searchable) { + ThreeStateAttribute a = new ThreeStateAttribute(); + a.setName(name); + a.setSearchable(searchable); + return a; + } + + private CompositeAttribute composite(String name, AttributeInterface... children) { + CompositeAttribute c = new CompositeAttribute(); + c.setName(name); + for (AttributeInterface child : children) { + c.getAttributes().add(child); + } + return c; + } + + private MonoListAttribute monolist(String name, AttributeInterface... elements) { + MonoListAttribute list = new MonoListAttribute(); + list.setName(name); + for (AttributeInterface element : elements) { + list.getAttributes().add(element); + } + return list; + } + + private ApsEntity entity(AttributeInterface... attributes) { + ApsEntity entity = new ApsEntity(); + entity.setTypeCode("TST"); + for (AttributeInterface attribute : attributes) { + entity.addAttribute(attribute); + } + return entity; + } + +} diff --git a/engine/src/test/java/com/agiletec/aps/system/common/entity/model/EntitySearchFilterNestedBooleanTest.java b/engine/src/test/java/com/agiletec/aps/system/common/entity/model/EntitySearchFilterNestedBooleanTest.java new file mode 100644 index 0000000000..d0b8dd63e0 --- /dev/null +++ b/engine/src/test/java/com/agiletec/aps/system/common/entity/model/EntitySearchFilterNestedBooleanTest.java @@ -0,0 +1,114 @@ +/* + * Copyright 2015-Present Entando Inc. (http://www.entando.com) All rights reserved. + * + * This library is free software; you can redistribute it and/or modify it under + * the terms of the GNU Lesser General Public License as published by the Free + * Software Foundation; either version 2.1 of the License, or (at your option) + * any later version. + * + * This library is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more + * details. + */ +package com.agiletec.aps.system.common.entity.model; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.agiletec.aps.system.common.entity.model.attribute.AttributeInterface; +import com.agiletec.aps.system.common.entity.model.attribute.BooleanAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.CompositeAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.MonoListAttribute; +import java.util.Properties; +import org.junit.jupiter.api.Test; + +/** + * Verifies that {@link EntitySearchFilter#getInstance(IApsEntity, Properties)} accepts a + * Composite-nested boolean referenced by its path key {@code _} - the read-side + * half of the Solr-disabled nested-boolean feature. + */ +class EntitySearchFilterNestedBooleanTest { + + @Test + void shouldResolveNestedBooleanPathKey() { + ApsEntity prototype = entity(composite("address", booleanAttr("certified", true, Boolean.TRUE))); + EntitySearchFilter filter = EntitySearchFilter.getInstance(prototype, + attributeFilterProps("address_certified", "true")); + assertNotNull(filter); + assertTrue(filter.isAttributeFilter()); + assertEquals("address_certified", filter.getKey()); + assertEquals("true", filter.getValue()); + } + + @Test + void topLevelAttributeTakesPrecedence() { + ApsEntity prototype = entity(booleanAttr("flag", true, Boolean.TRUE)); + EntitySearchFilter filter = EntitySearchFilter.getInstance(prototype, + attributeFilterProps("flag", "true")); + assertNotNull(filter); + assertEquals("flag", filter.getKey()); + } + + @Test + void shouldRejectUnknownKey() { + ApsEntity prototype = entity(composite("address", booleanAttr("certified", true, Boolean.TRUE))); + Properties props = attributeFilterProps("address_missing", "true"); + assertThrows(RuntimeException.class, () -> EntitySearchFilter.getInstance(prototype, props)); + } + + @Test + void shouldNotResolveListReachedBoolean() { + ApsEntity prototype = entity(monolist("tags", booleanAttr("flag", true, Boolean.TRUE))); + Properties props = attributeFilterProps("tags_flag", "true"); + assertThrows(RuntimeException.class, () -> EntitySearchFilter.getInstance(prototype, props)); + } + + // --- helpers ----------------------------------------------------------- + + private Properties attributeFilterProps(String key, String value) { + Properties props = new Properties(); + props.setProperty(EntitySearchFilter.KEY_PARAM, key); + props.setProperty(EntitySearchFilter.FILTER_TYPE_PARAM, Boolean.TRUE.toString()); + props.setProperty(EntitySearchFilter.VALUE_PARAM, value); + return props; + } + + private BooleanAttribute booleanAttr(String name, boolean searchable, Boolean value) { + BooleanAttribute a = new BooleanAttribute(); + a.setName(name); + a.setSearchable(searchable); + a.setBooleanValue(value); + return a; + } + + private CompositeAttribute composite(String name, AttributeInterface... children) { + CompositeAttribute c = new CompositeAttribute(); + c.setName(name); + for (AttributeInterface child : children) { + c.getAttributes().add(child); + } + return c; + } + + private MonoListAttribute monolist(String name, AttributeInterface... elements) { + MonoListAttribute list = new MonoListAttribute(); + list.setName(name); + for (AttributeInterface element : elements) { + list.getAttributes().add(element); + } + return list; + } + + private ApsEntity entity(AttributeInterface... attributes) { + ApsEntity entity = new ApsEntity(); + entity.setTypeCode("TST"); + for (AttributeInterface attribute : attributes) { + entity.addAttribute(attribute); + } + return entity; + } + +} diff --git a/engine/src/test/java/com/agiletec/aps/system/common/entity/model/attribute/CompositeAttributeXmlConfigTest.java b/engine/src/test/java/com/agiletec/aps/system/common/entity/model/attribute/CompositeAttributeXmlConfigTest.java new file mode 100644 index 0000000000..08e27cf294 --- /dev/null +++ b/engine/src/test/java/com/agiletec/aps/system/common/entity/model/attribute/CompositeAttributeXmlConfigTest.java @@ -0,0 +1,104 @@ +/* + * Copyright 2015-Present Entando Inc. (http://www.entando.com) All rights reserved. + * + * This library is free software; you can redistribute it and/or modify it under + * the terms of the GNU Lesser General Public License as published by the Free + * Software Foundation; either version 2.1 of the License, or (at your option) + * any later version. + * + * This library is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more + * details. + */ +package com.agiletec.aps.system.common.entity.model.attribute; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.agiletec.aps.system.common.entity.parse.attribute.BooleanAttributeHandler; +import com.agiletec.aps.system.common.entity.parse.attribute.TextAttributeHandler; +import com.agiletec.aps.system.services.lang.ILangManager; +import com.agiletec.aps.system.services.lang.Lang; +import java.util.Collections; +import java.util.Map; +import org.jdom2.Element; +import org.junit.jupiter.api.Test; + +/** + * Verifies {@link CompositeAttribute#setComplexAttributeConfig} - in particular the new rule that a + * plain boolean composite child may keep its configured {@code searchable} flag, while every other + * type is forced non-searchable regardless of what the XML config says. + */ +class CompositeAttributeXmlConfigTest { + + @Test + void booleanChildKeepsConfiguredSearchableFlag() throws Exception { + CompositeAttribute composite = parseComposite( + childElement("Boolean", "certified", true), Map.of("Boolean", booleanPrototype())); + + AttributeInterface child = composite.getAttributeMap().get("certified"); + assertTrue(child.isSearchable()); + } + + @Test + void nonBooleanChildIsForcedNonSearchable() throws Exception { + CompositeAttribute composite = parseComposite( + childElement("Text", "note", true), Map.of("Text", textPrototype())); + + AttributeInterface child = composite.getAttributeMap().get("note"); + assertFalse(child.isSearchable()); + } + + // --- fixtures ------------------------------------------------------ + + private CompositeAttribute parseComposite(Element childElement, Map attrTypes) + throws Exception { + Element attributesWrapper = new Element("attributes"); + attributesWrapper.addContent(childElement); + Element compositeElement = new Element("composite"); + compositeElement.addContent(attributesWrapper); + + CompositeAttribute composite = new CompositeAttribute(); + composite.setName("address"); + composite.setComplexAttributeConfig(compositeElement, attrTypes); + return composite; + } + + private Element childElement(String attributeType, String name, boolean searchable) { + Element element = new Element(attributeType.toLowerCase()); + element.setAttribute("attributetype", attributeType); + element.setAttribute("name", name); + element.setAttribute("searchable", String.valueOf(searchable)); + return element; + } + + private BooleanAttribute booleanPrototype() { + BooleanAttribute attribute = new BooleanAttribute(); + attribute.setType("Boolean"); + attribute.setHandler(new BooleanAttributeHandler()); + attribute.setLangManager(langManager()); + return attribute; + } + + private TextAttribute textPrototype() { + TextAttribute attribute = new TextAttribute(); + attribute.setType("Text"); + attribute.setHandler(new TextAttributeHandler()); + attribute.setLangManager(langManager()); + return attribute; + } + + private ILangManager langManager() { + ILangManager langManager = mock(ILangManager.class); + Lang en = new Lang(); + en.setCode("en"); + en.setDescr("English"); + when(langManager.getDefaultLang()).thenReturn(en); + when(langManager.getLangs()).thenReturn(Collections.singletonList(en)); + return langManager; + } + +} diff --git a/run-reactor-tests.sh b/run-reactor-tests.sh new file mode 100755 index 0000000000..ce7e78ba2b --- /dev/null +++ b/run-reactor-tests.sh @@ -0,0 +1,256 @@ +#!/usr/bin/env bash +# +# run-reactor-tests.sh +# --------------------------------------------------------------------------- +# Runs the unit/integration tests of the reactor's inner modules with Maven. +# +# 1. reports the Maven version and the JVM in use; +# 2. lets you pick the module(s) to test from an interactive list +# (UP/DOWN arrows to move, SPACE to select/deselect, ENTER to confirm); +# 3. runs the tests and collects a per-module PASS/FAIL summary. +# +# Usage: +# ./run-reactor-tests.sh # interactive module picker +# ./run-reactor-tests.sh engine cms-plugin # non-interactive: given module(s) +# +# Environment overrides: +# PROFILE=pre-deployment-verification # Maven profile enabling the tests +# MVN_OPTS="-o" # extra Maven options (e.g. offline) +# ASSUME_YES=1 # skip the picker, test every module +# DRY_RUN=1 # print the mvn command, do not run it +# --------------------------------------------------------------------------- + +set -u -o pipefail + +PROJECT_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +cd "$PROJECT_ROOT" || { echo "ERROR: cannot cd to $PROJECT_ROOT"; exit 1; } + +PROFILE="${PROFILE:-pre-deployment-verification}" +MVN_OPTS="${MVN_OPTS:-}" +MVN_BIN="$(command -v mvn || true)" + +TS="$(date +%Y%m%d-%H%M%S)" +RESULTS_DIR="$PROJECT_ROOT/test-results" +LOG_FILE="$RESULTS_DIR/reactor-tests-$TS.log" + +c_bold=$'\033[1m'; c_green=$'\033[0;32m'; c_red=$'\033[0;31m' +c_yellow=$'\033[0;33m'; c_cyan=$'\033[0;36m'; c_off=$'\033[0m' +hr() { printf '%s\n' "----------------------------------------------------------------------"; } +say() { printf '%s\n' "$*"; } + +# --- preconditions --------------------------------------------------------- +if [[ -z "$MVN_BIN" ]]; then + say "${c_red}ERROR:${c_off} 'mvn' was not found on the PATH."; exit 127 +fi +if [[ ! -f "$PROJECT_ROOT/pom.xml" ]]; then + say "${c_red}ERROR:${c_off} no pom.xml found in $PROJECT_ROOT (not a reactor root)."; exit 1 +fi + +# --- reactor modules (in pom.xml order) ------------------------------------ +mapfile -t MODULES < <(grep -oE "[^<]+" pom.xml | sed 's/<[^>]*>//g') +if [[ "${#MODULES[@]}" -eq 0 ]]; then + say "${c_red}ERROR:${c_off} no entries found in pom.xml."; exit 1 +fi + +# --- report Maven & JVM ---------------------------------------------------- +hr +say "${c_bold}Entando App Engine — reactor test runner${c_off}" +hr +say "${c_bold}Maven / JVM in use:${c_off}" +# 'mvn -version' prints the Maven version, the Java version/vendor and the JVM home. +"$MVN_BIN" -version +say "" +say "JAVA_HOME : ${JAVA_HOME:-}" +say "Project root : $PROJECT_ROOT" +say "Test profile : -P$PROFILE" +say "Extra opts : ${MVN_OPTS:-}" +hr + +# --------------------------------------------------------------------------- +# Interactive multi-select checklist. +# Fills the global array SELECTED with the chosen module names. +# --------------------------------------------------------------------------- +SELECTED=() +select_modules() { + local -a items=("$@") + local n=${#items[@]} + local -a checked + local i cursor=0 + for ((i = 0; i < n; i++)); do checked[i]=0; done + + printf '%s\n' "${c_bold}Select the module(s) to test:${c_off}" + printf '%s\n' " ${c_cyan}UP/DOWN${c_off} move ${c_cyan}SPACE${c_off} select/deselect ${c_cyan}a${c_off} all ${c_cyan}n${c_off} none ${c_cyan}ENTER${c_off} confirm ${c_cyan}q${c_off} quit" + printf '\033[?25l' # hide cursor + # shellcheck disable=SC2064 + trap 'printf "\033[?25h"' RETURN # restore cursor when function returns + + local first=1 key rest mark pointer line + while true; do + [[ $first -eq 0 ]] && printf '\033[%dA' "$n" # move up to redraw + first=0 + for ((i = 0; i < n; i++)); do + mark=" "; [[ ${checked[i]} -eq 1 ]] && mark="x" + if [[ $i -eq $cursor ]]; then + pointer="${c_yellow}>${c_off}" + line=$(printf '%s [%s] %s%s%s' "$pointer" "$mark" "$c_bold" "${items[i]}" "$c_off") + else + pointer=" " + line=$(printf '%s [%s] %s' "$pointer" "$mark" "${items[i]}") + fi + printf '\r\033[2K%s\n' "$line" # clear line, then print + done + + IFS= read -rsn1 key + if [[ $key == $'\033' ]]; then # escape sequence (arrow keys) + IFS= read -rsn2 -t 0.05 rest || rest="" + key+="$rest" + fi + case "$key" in + $'\033[A' | k) ((cursor = (cursor - 1 + n) % n)) ;; # up + $'\033[B' | j) ((cursor = (cursor + 1) % n)) ;; # down + ' ') checked[cursor]=$((1 - checked[cursor])) ;; + a | A) for ((i = 0; i < n; i++)); do checked[i]=1; done ;; + n | N) for ((i = 0; i < n; i++)); do checked[i]=0; done ;; + '' | $'\n' | $'\r') break ;; # ENTER -> confirm + q | Q | $'\033') printf '\033[?25h'; return 1 ;; # quit / bare ESC + esac + done + + SELECTED=() + for ((i = 0; i < n; i++)); do + [[ ${checked[i]} -eq 1 ]] && SELECTED+=("${items[i]}") + done + return 0 +} + +# --- decide the selection -------------------------------------------------- +if [[ "$#" -gt 0 ]]; then + # explicit module names on the command line -> non-interactive + SELECTED=("$@") +elif [[ "${ASSUME_YES:-0}" == "1" || ! -t 0 || ! -t 1 ]]; then + # no TTY (CI/pipe) or ASSUME_YES -> test everything, no prompt + SELECTED=("${MODULES[@]}") + say "Non-interactive run: testing all ${#MODULES[@]} modules." +else + if ! select_modules "${MODULES[@]}"; then + say ""; say "Aborted by user. No tests were run."; exit 0 + fi +fi + +if [[ "${#SELECTED[@]}" -eq 0 ]]; then + say ""; say "No module selected. No tests were run."; exit 0 +fi + +# --- validate selected names ----------------------------------------------- +for m in "${SELECTED[@]}"; do + found=0 + for known in "${MODULES[@]}"; do [[ "$m" == "$known" ]] && found=1 && break; done + if [[ $found -eq 0 ]]; then + say "${c_red}ERROR:${c_off} '$m' is not a reactor module. Known: ${MODULES[*]}"; exit 1 + fi +done + +# --- selection mode: whole reactor vs a subset ----------------------------- +# NOTE: for a subset we deliberately DO NOT test with '-am'. '-am' ("also make") +# would run the *test* phase of every upstream dependency module too, i.e. test +# far more than the user picked. Instead we build the deps without tests first +# (below) and then test only the selected modules. +SUBSET=0 +MODULE_CSV="" +if [[ "${#SELECTED[@]}" -lt "${#MODULES[@]}" ]]; then + SUBSET=1 + MODULE_CSV="$(IFS=,; echo "${SELECTED[*]}")" +fi +DEPS_LOG="$RESULTS_DIR/reactor-deps-$TS.log" + +hr +say "${c_bold}About to test:${c_off} ${SELECTED[*]}" +say "Log file: $LOG_FILE" +hr + +mkdir -p "$RESULTS_DIR" + +# --- run the tests --------------------------------------------------------- +if [[ "${DRY_RUN:-0}" == "1" ]]; then + say "${c_yellow}DRY_RUN:${c_off} would execute:" + if [[ "$SUBSET" -eq 1 ]]; then + say " [1/2 build deps, no tests] $MVN_BIN $MVN_OPTS -pl $MODULE_CSV -am install -DskipTests" + say " [2/2 test selected only ] $MVN_BIN $MVN_OPTS -P$PROFILE -fae -pl $MODULE_CSV test" + else + say " $MVN_BIN $MVN_OPTS -P$PROFILE -fae test" + fi + exit 0 +fi + +START_EPOCH="$(date +%s)" +set -f +if [[ "$SUBSET" -eq 1 ]]; then + # Phase 1: compile+install the selected modules AND their upstream deps, WITHOUT + # running tests (-DskipTests still builds the test-jars downstream tests need), + # so phase 2 can resolve every dependency from the local repo. + say "${c_bold}[1/2] Building selected modules + upstream dependencies (no tests)...${c_off}" + say " deps log: $DEPS_LOG" + hr + # shellcheck disable=SC2086 + "$MVN_BIN" $MVN_OPTS -pl "$MODULE_CSV" -am install -DskipTests 2>&1 | tee "$DEPS_LOG" + DEPS_STATUS="${PIPESTATUS[0]}" + if [[ "$DEPS_STATUS" -ne 0 ]]; then + set +f + hr + say "${c_red}${c_bold}Dependency build failed — cannot run the selected tests.${c_off}" + say " See $DEPS_LOG" + exit "$DEPS_STATUS" + fi + # Phase 2: run tests for the SELECTED modules only (no -am, so deps are NOT re-tested). + say "" + say "${c_bold}[2/2] Running tests for the selected module(s) only: ${SELECTED[*]}${c_off}" + hr + # shellcheck disable=SC2086 + "$MVN_BIN" $MVN_OPTS -P"$PROFILE" -fae -pl "$MODULE_CSV" test 2>&1 | tee "$LOG_FILE" + MVN_STATUS="${PIPESTATUS[0]}" +else + say "${c_bold}Running tests for all reactor modules...${c_off}" + hr + # shellcheck disable=SC2086 + "$MVN_BIN" $MVN_OPTS -P"$PROFILE" -fae test 2>&1 | tee "$LOG_FILE" + MVN_STATUS="${PIPESTATUS[0]}" +fi +set +f +ELAPSED=$(( $(date +%s) - START_EPOCH )) + +# --- collect results ------------------------------------------------------- +hr +say "${c_bold}Result summary${c_off} (elapsed: $((ELAPSED/60))m $((ELAPSED%60))s)" +hr +say "${c_bold}Per-module (reactor summary):${c_off}" +if grep -q "Reactor Summary" "$LOG_FILE"; then + sed 's/\x1b\[[0-9;]*m//g' "$LOG_FILE" \ + | sed -n '/Reactor Summary/,/^\[INFO\] -\{20,\}$/p' \ + | grep -E "SUCCESS|FAILURE|SKIPPED" | sed -E 's/^\[INFO\] / /' +else + say " (no multi-module reactor summary — single module selected, or the build stopped early;" + say " see the test totals and the BUILD status below)" +fi + +say "" +say "${c_bold}Test totals (surefire, per module):${c_off}" +sed 's/\x1b\[[0-9;]*m//g' "$LOG_FILE" \ + | grep -E "Tests run: [0-9]+, Failures: [0-9]+, Errors: [0-9]+, Skipped: [0-9]+" \ + | grep -vE " -- in | <<< " | sed 's/^\[[A-Z]*\] / /' | sed 's/^/ /' || true + +FAILED_LINES="$(sed 's/\x1b\[[0-9;]*m//g' "$LOG_FILE" | grep -E "<<< (FAILURE|ERROR)!" || true)" +if [[ -n "$FAILED_LINES" ]]; then + say ""; say "${c_red}${c_bold}Failing/errored test classes:${c_off}" + printf '%s\n' "$FAILED_LINES" | sed 's/^/ /' +fi + +hr +if [[ "$MVN_STATUS" -eq 0 ]]; then + say "${c_green}${c_bold}BUILD SUCCESS — all selected module tests passed.${c_off}" +else + say "${c_red}${c_bold}BUILD FAILURE — see failures above and the full log:${c_off}" + say " $LOG_FILE" + say " Surefire reports: /target/surefire-reports/" +fi +hr +exit "$MVN_STATUS" diff --git a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManager.java b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManager.java index 1644d79623..4beef54cce 100644 --- a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManager.java +++ b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManager.java @@ -34,6 +34,8 @@ import org.entando.entando.ent.util.EntLogging.EntLogger; import org.entando.entando.web.common.exceptions.ValidationConflictException; import org.entando.entando.web.common.model.Filter; +import org.entando.entando.web.common.model.FilterOperator; +import org.entando.entando.web.common.model.FilterType; import org.entando.entando.plugins.jpsolr.aps.system.solr.ISolrSearchEngineManager; import org.entando.entando.plugins.jpsolr.aps.system.solr.model.SolrFacetedContentsResult; import org.entando.entando.plugins.jpsolr.aps.system.solr.model.SolrSearchEngineFilter; @@ -56,6 +58,9 @@ public class AdvContentFacetManager implements IAdvContentFacetManager { private static final Pattern SAFE_IDENTIFIER = Pattern.compile("[a-zA-Z0-9_.:,-]+"); private static final Pattern INJECTION_PATTERN = Pattern.compile("\\$\\{|%24%7B", Pattern.CASE_INSENSITIVE); + private static final String INVALID_PARAMETER_CODE = "INVALID_PARAMETER"; + private static final String INVALID_PARAMETER_MESSAGE_KEY = "parameter.invalid"; + private final ICategoryManager categoryManager; private final ICmsSearchEngineManager searchEngineManager; private final IAuthorizationManager authorizationManager; @@ -197,6 +202,45 @@ private void validateFilters(Filter[] filters, String fieldPrefix, rejectIfUnsafeIdentifier(solrFilter.getSearchOption(), fieldPrefix + ".searchOption", bindingResult); } + rejectIfInvalidBooleanFilter(filter, fieldPrefix, bindingResult); + } + } + + /** + * Booleans have no meaningful range and only two valid values. Without this check a range + * operator silently builds a nonsense query (SearcherDAO's string-range fallback), and + * {@code Boolean.parseBoolean} silently coerces any non-"true" string (including a + * three-state "none") to {@code false} instead of failing. + */ + private void rejectIfInvalidBooleanFilter(Filter filter, String fieldPrefix, + BeanPropertyBindingResult bindingResult) { + if (!FilterType.BOOLEAN.getValue().equalsIgnoreCase(filter.getType())) { + return; + } + String operator = filter.getOperator(); + if (FilterOperator.GREATER.getValue().equalsIgnoreCase(operator) + || FilterOperator.LOWER.getValue().equalsIgnoreCase(operator)) { + logger.warn("Rejected range operator '{}' on boolean filter in field '{}'", operator, fieldPrefix); + bindingResult.rejectValue(null, INVALID_PARAMETER_CODE, + new Object[]{fieldPrefix + ".operator"}, INVALID_PARAMETER_MESSAGE_KEY); + } + rejectIfNotStrictBoolean(filter.getValue(), fieldPrefix + ".value", bindingResult); + if (null != filter.getAllowedValues()) { + for (String av : filter.getAllowedValues()) { + rejectIfNotStrictBoolean(av, fieldPrefix + ".allowedValues", bindingResult); + } + } + } + + private static void rejectIfNotStrictBoolean(String value, String field, + BeanPropertyBindingResult bindingResult) { + if (StringUtils.isBlank(value)) { + return; + } + if (!"true".equalsIgnoreCase(value) && !"false".equalsIgnoreCase(value)) { + logger.warn("Rejected non-boolean value in field '{}': '{}'", field, value); + bindingResult.rejectValue(null, INVALID_PARAMETER_CODE, + new Object[]{field}, INVALID_PARAMETER_MESSAGE_KEY); } } @@ -207,8 +251,8 @@ private static void rejectIfUnsafeIdentifier(String value, String field, } if (!SAFE_IDENTIFIER.matcher(value).matches()) { logger.warn("Rejected unsafe identifier in field '{}': '{}'", field, value); - bindingResult.rejectValue(null, "INVALID_PARAMETER", - new Object[]{field}, "parameter.invalid"); + bindingResult.rejectValue(null, INVALID_PARAMETER_CODE, + new Object[]{field}, INVALID_PARAMETER_MESSAGE_KEY); } } @@ -219,8 +263,8 @@ private static void rejectIfInjection(String value, String field, } if (INJECTION_PATTERN.matcher(value).find()) { logger.warn("Rejected injection pattern in field '{}': '{}'", field, value); - bindingResult.rejectValue(null, "INVALID_PARAMETER", - new Object[]{field}, "parameter.invalid"); + bindingResult.rejectValue(null, INVALID_PARAMETER_CODE, + new Object[]{field}, INVALID_PARAMETER_MESSAGE_KEY); } } diff --git a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/IndexerDAO.java b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/IndexerDAO.java index 3f7849c78e..387076bb4f 100644 --- a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/IndexerDAO.java +++ b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/IndexerDAO.java @@ -16,8 +16,11 @@ import com.agiletec.aps.system.common.entity.model.IApsEntity; import com.agiletec.aps.system.common.entity.model.attribute.AbstractComplexAttribute; import com.agiletec.aps.system.common.entity.model.attribute.AttributeInterface; +import com.agiletec.aps.system.common.entity.model.attribute.BooleanAttribute; import com.agiletec.aps.system.common.entity.model.attribute.DateAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.ListAttributeInterface; import com.agiletec.aps.system.common.entity.model.attribute.NumberAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.ThreeStateAttribute; import com.agiletec.aps.system.common.searchengine.IndexableAttributeInterface; import com.agiletec.aps.system.common.tree.ITreeNode; import com.agiletec.aps.system.common.tree.ITreeNodeManager; @@ -46,6 +49,10 @@ /** * Data Access Object dedita alla indicizzazione di documenti. */ +// NOTE: java:S2143 ("use the java.time API") is intentionally suppressed. This class uses legacy +// java.util.Date to interoperate with the content date model (getCreated/getLastModified); migrating +// the date stack to java.time is out of scope for ESB-1133 (boolean search) and tracked separately. +@SuppressWarnings("java:S2143") public class IndexerDAO implements ISolrIndexerDAO { private static final Logger logger = LoggerFactory.getLogger(IndexerDAO.class); @@ -115,31 +122,43 @@ protected SolrInputDocument createDocument(IApsEntity entity) { for (String groupName : entity.getGroups()) { document.addField(SolrFields.SOLR_CONTENT_GROUP_FIELD_NAME, groupName); } - if (entity instanceof Content) { - if (null != entity.getDescription()) { - document.addField(SolrFields.SOLR_CONTENT_DESCRIPTION_FIELD_NAME, entity.getDescription()); - } - Date creation = ((Content) entity).getCreated(); - Date lastModify = - (null != ((Content) entity).getLastModified()) ? ((Content) entity).getLastModified() : creation; - if (null != creation) { - document.addField(SolrFields.SOLR_CONTENT_CREATION_FIELD_NAME, creation); - } - if (null != lastModify) { - document.addField(SolrFields.SOLR_CONTENT_LAST_MODIFY_FIELD_NAME, lastModify); - } + this.addContentMetadata(entity, document); + this.indexAttributes(entity, document); + this.indexCategories(entity, document); + return document; + } + + private void addContentMetadata(IApsEntity entity, SolrInputDocument document) { + if (!(entity instanceof Content)) { + return; } + Content content = (Content) entity; + if (null != entity.getDescription()) { + document.addField(SolrFields.SOLR_CONTENT_DESCRIPTION_FIELD_NAME, entity.getDescription()); + } + Date creation = content.getCreated(); + Date lastModify = (null != content.getLastModified()) ? content.getLastModified() : creation; + if (null != creation) { + document.addField(SolrFields.SOLR_CONTENT_CREATION_FIELD_NAME, creation); + } + if (null != lastModify) { + document.addField(SolrFields.SOLR_CONTENT_LAST_MODIFY_FIELD_NAME, lastModify); + } + } + + private void indexAttributes(IApsEntity entity, SolrInputDocument document) { for (AttributeInterface currentAttribute : entity.getAttributeList()) { Object value = currentAttribute.getValue(); - if (null == value) { + // An uninitialized ThreeStateAttribute must still reach indexAttribute so it is + // indexed as the literal "none": its getValue() returns null on purpose (the third, + // unset state), unlike a plain BooleanAttribute which coerces null to false. + if (null == value && !(currentAttribute instanceof ThreeStateAttribute)) { continue; } for (Lang lang : this.getLangManager().getLangs()) { this.indexAttribute(document, currentAttribute, lang); } } - this.indexCategories(entity, document); - return document; } protected void indexCategories(IApsEntity entity, SolrInputDocument document) { @@ -149,7 +168,7 @@ protected void indexCategories(IApsEntity entity, SolrInputDocument document) { for (ITreeNode category : categories) { this.extractCategoryCodes(category, codes); } - codes.stream().forEach(c -> document.addField(SolrFields.SOLR_CONTENT_CATEGORY_FIELD_NAME, c)); + codes.forEach(c -> document.addField(SolrFields.SOLR_CONTENT_CATEGORY_FIELD_NAME, c)); } } @@ -165,12 +184,13 @@ protected void extractCategoryCodes(ITreeNode category, Set codes) { protected void indexAttribute(SolrInputDocument document, AttributeInterface attribute, Lang lang) { attribute.setRenderingLang(lang.getCode()); if (!attribute.isSimple()) { - this.indexComplexAttribute(document, (AbstractComplexAttribute) attribute, lang); + this.indexComplexAttribute(document, (AbstractComplexAttribute) attribute, lang, + attribute.getName(), false); return; } if (attribute instanceof IndexableAttributeInterface - || ((attribute instanceof DateAttribute || attribute instanceof NumberAttribute) - && attribute.isSearchable())) { + || ((attribute instanceof DateAttribute || attribute instanceof NumberAttribute + || attribute instanceof BooleanAttribute) && attribute.isSearchable())) { Object valueToIndex = null; if (attribute instanceof DateAttribute) { valueToIndex = ((DateAttribute) attribute).getDate(); @@ -179,6 +199,11 @@ protected void indexAttribute(SolrInputDocument document, AttributeInterface att if (null != valueToIndex) { valueToIndex = ((BigDecimal) valueToIndex).intValue(); } + } else if (attribute instanceof ThreeStateAttribute) { + // Must be tested before BooleanAttribute (ThreeStateAttribute is a subclass). + valueToIndex = SolrComplexAttributes.solrValue(attribute); + } else if (attribute instanceof BooleanAttribute) { + valueToIndex = SolrComplexAttributes.solrValue(attribute); } else { valueToIndex = ((IndexableAttributeInterface) attribute).getIndexeableFieldValue(); } @@ -204,14 +229,27 @@ protected void indexAttribute(SolrInputDocument document, AttributeInterface att } private void indexComplexAttribute(SolrInputDocument document, AbstractComplexAttribute complexAttribute, - Lang lang) { + Lang lang, String namePrefix, boolean withinList) { + // Text children are still routed to the full-text "" field wherever they occur + // (including inside lists). Per-attribute boolean fields are created for Composite children + // only: once the traversal has entered a List/Monolist, booleans are skipped so the indexer + // never writes a field the schema (SolrFieldsChecker) does not create. + boolean insideList = withinList || (complexAttribute instanceof ListAttributeInterface); for (AttributeInterface attribute : complexAttribute.getAttributes()) { attribute.setRenderingLang(lang.getCode()); + String path = SolrComplexAttributes.appendPath(namePrefix, attribute.getName()); if (!attribute.isSimple()) { - this.indexComplexAttribute(document, (AbstractComplexAttribute) attribute, lang); + this.indexComplexAttribute(document, (AbstractComplexAttribute) attribute, lang, path, insideList); } else if (attribute instanceof IndexableAttributeInterface){ String valueToIndex = ((IndexableAttributeInterface) attribute).getIndexeableFieldValue(); this.addFieldForFullTextSearch(document, attribute, lang, valueToIndex); + } else if (!insideList + && SolrComplexAttributes.isIndexableCompositeBoolean(attribute)) { + // solrValue() never returns null: an unset ThreeStateAttribute becomes the + // literal "none", an unset Boolean/CheckBoxAttribute coerces to false. + Object valueToIndex = SolrComplexAttributes.solrValue(attribute); + String fieldName = lang.getCode().toLowerCase() + "_" + path; + this.indexValue(document, fieldName, valueToIndex); } } } @@ -232,6 +270,7 @@ protected void addFieldForFullTextSearch(SolrInputDocument document, AttributeIn private void indexValue(SolrInputDocument document, String fieldName, Object valueToIndex) { fieldName = fieldName.replace(":", "_"); + logger.debug("Indexing attribute field '{}' with value '{}'", fieldName, valueToIndex); document.addField(fieldName, valueToIndex); } diff --git a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAO.java b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAO.java index db41072738..5c92284dc9 100644 --- a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAO.java +++ b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAO.java @@ -167,7 +167,11 @@ protected SolrFacetedContentsResult executeQuery(Query query, SearchEngineFilter SolrDocumentList documents = response.getResults(); result.setTotalSize(Math.toIntExact(documents.getNumFound())); for (SolrDocument doc : documents) { - String id = doc.get(SolrFields.SOLR_CONTENT_ID_FIELD_NAME).toString(); + // False positive (javabugs:S2259): the query explicitly requests the id field + // (solrQuery.addField(SOLR_CONTENT_ID_FIELD_NAME) above) and 'id' is the mandatory, + // always-populated unique key of every indexed content document, so doc.get(id) can + // never be null for a document returned by this query. The .toString() is safe. + String id = doc.get(SolrFields.SOLR_CONTENT_ID_FIELD_NAME).toString(); // NOSONAR - id field always present (see note above) contentsId.add(id); } if (faceted) { @@ -375,7 +379,7 @@ private Query createMultipleValuesQuery(SearchEngineFilter filter, String key } //To be improved to manage different type for (Object singleValue : allowedValues) { - if (filter instanceof NumericSearchEngineFilter) { + if (filter instanceof NumericSearchEngineFilter || singleValue instanceof Boolean) { TermQuery term = new TermQuery(new Term(key, singleValue + relevance)); fieldQuery.add(term, BooleanClause.Occur.SHOULD); } else { @@ -457,6 +461,9 @@ private Query createSingleValueQuery(SearchEngineFilter filter, String key, S } else if (value instanceof Number) { TermQuery term = new TermQuery(new Term(key, value + relevance)); fieldQuery.add(term, BooleanClause.Occur.MUST); + } else if (value instanceof Boolean) { + TermQuery term = new TermQuery(new Term(key, value.toString() + relevance)); + fieldQuery.add(term, BooleanClause.Occur.MUST); } return fieldQuery.build(); } diff --git a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrComplexAttributes.java b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrComplexAttributes.java new file mode 100644 index 0000000000..a5a1ad9f2d --- /dev/null +++ b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrComplexAttributes.java @@ -0,0 +1,87 @@ +/* + * Copyright 2021-Present Entando Inc. (http://www.entando.com) All rights reserved. + * + * This library is free software; you can redistribute it and/or modify it under + * the terms of the GNU Lesser General Public License as published by the Free + * Software Foundation; either version 2.1 of the License, or (at your option) + * any later version. + * + * This library is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more + * details. + */ +package org.entando.entando.plugins.jpsolr.aps.system.solr; + +import com.agiletec.aps.system.common.entity.model.attribute.AttributeInterface; +import com.agiletec.aps.system.common.entity.model.attribute.BooleanAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.ThreeStateAttribute; +import org.entando.entando.plugins.jpsolr.aps.system.solr.model.SolrFields; + +/** + * Shared rules for indexing boolean-like attributes (Boolean, CheckBox, ThreeState) nested inside + * Composite attributes. + * + *

    Kept in one place so the schema side ({@code SolrFieldsChecker}), the document side + * ({@code IndexerDAO}) and the content-type settings report ({@code SolrSearchEngineManager}) build + * identical field names and apply the same "is this boolean indexable" decision. Any divergence + * would create fields the indexer never populates, or make the validity check loop forever.

    + * + *

    List and Monolist attributes are intentionally out of scope: booleans reached through a list + * are never indexed as per-attribute fields.

    + * + *

    Public (rather than package-private) because {@code ContentTypeSettings}, which needs the + * same type/value dispatch, lives in the {@code .model} sub-package.

    + */ +public final class SolrComplexAttributes { + + private SolrComplexAttributes() { + } + + /** + * A boolean-like attribute ({@code BooleanAttribute} or a subclass — {@code CheckBoxAttribute}, + * {@code ThreeStateAttribute}) nested in a Composite is indexed when its {@code searchable} flag, + * inherited from the content type, is set. This mirrors how top-level boolean-like attributes are + * gated, so nested and top-level behave uniformly. + */ + static boolean isIndexableCompositeBoolean(AttributeInterface attribute) { + return attribute instanceof BooleanAttribute && attribute.isSearchable(); + } + + /** + * The Solr field type for a boolean-like attribute. {@code ThreeStateAttribute} must be tested + * before {@code BooleanAttribute} (it is a subclass): its third "uninitialized" state cannot be + * represented in Solr's two-valued {@code BoolField}, so it is indexed as a non-analyzed {@code + * string} field with the literal values {@code true}/{@code false}/{@code none}; plain {@code + * BooleanAttribute}/{@code CheckBoxAttribute} are indexed as Solr {@code boolean}. + */ + public static String solrType(AttributeInterface attribute) { + if (attribute instanceof ThreeStateAttribute) { + return SolrFields.TYPE_STRING; + } + return SolrFields.TYPE_BOOLEAN; + } + + /** + * The value to write for a boolean-like attribute. {@code ThreeStateAttribute}: {@code null} + * (uninitialized) becomes the literal {@code "none"}, otherwise the lowercase string literal + * {@code "true"}/{@code "false"}. Plain {@code BooleanAttribute}/{@code CheckBoxAttribute}: + * {@code getValue()} already coerces {@code null} to {@code false}. + */ + public static Object solrValue(AttributeInterface attribute) { + if (attribute instanceof ThreeStateAttribute threeStateAttribute) { + Boolean value = threeStateAttribute.getValue(); + return (null == value) ? "none" : value.toString(); + } + return ((BooleanAttribute) attribute).getValue(); + } + + /** + * Extends a composite name path with a child segment. The resulting field name is + * "<lang>_<path>" (built by the callers), e.g. "en_complexAttrName_boolAttrName" for the boolean + * "boolAttrName" nested in the composite "complexAttrName". + */ + static String appendPath(String namePrefix, String name) { + return namePrefix + "_" + name; + } +} diff --git a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrFieldsChecker.java b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrFieldsChecker.java index 74a81f268b..9ddd9a5677 100644 --- a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrFieldsChecker.java +++ b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrFieldsChecker.java @@ -4,9 +4,11 @@ import static org.entando.entando.plugins.jpsolr.aps.system.solr.model.SolrFields.SOLR_FIELD_NAME; import static org.entando.entando.plugins.jpsolr.aps.system.solr.model.SolrFields.SOLR_FIELD_TYPE; +import com.agiletec.aps.system.common.entity.model.attribute.AbstractComplexAttribute; import com.agiletec.aps.system.common.entity.model.attribute.AttributeInterface; import com.agiletec.aps.system.common.entity.model.attribute.BooleanAttribute; import com.agiletec.aps.system.common.entity.model.attribute.DateAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.ListAttributeInterface; import com.agiletec.aps.system.common.entity.model.attribute.NumberAttribute; import com.agiletec.aps.system.common.searchengine.IndexableAttributeInterface; import com.agiletec.aps.system.services.lang.Lang; @@ -95,33 +97,74 @@ private void checkLangFields() { private void checkAttribute(AttributeInterface attribute, Lang lang) { attribute.setRenderingLang(lang.getCode()); - if (attribute instanceof IndexableAttributeInterface - || ((attribute instanceof DateAttribute || attribute instanceof NumberAttribute) - && attribute.isSearchable())) { - String type; - if (attribute instanceof DateAttribute) { - type = SolrFields.TYPE_PDATES; - } else if (attribute instanceof NumberAttribute) { - type = SolrFields.TYPE_PLONGS; - } else if (attribute instanceof BooleanAttribute) { - type = SolrFields.TYPE_BOOLEAN; - } else { - type = SolrFields.TYPE_TEXT_GEN_SORT; - } - String fieldName = lang.getCode().toLowerCase() + "_" + attribute.getName(); - fieldName = fieldName.replace(":", "_"); + if (!attribute.isSimple()) { + this.checkComplexAttributeChildren(attribute, lang, attribute.getName()); + return; + } + if (this.isIndexableAttributeField(attribute)) { + String type = this.getFieldType(attribute); + String fieldName = this.buildFieldName(lang, attribute.getName()); this.checkField(fieldName, type); if (null == attribute.getRoles()) { return; } for (String role : attribute.getRoles()) { - String roleFieldName = lang.getCode().toLowerCase() + "_" + role; - roleFieldName = roleFieldName.replace(":", "_"); + String roleFieldName = this.buildFieldName(lang, role); this.checkField(roleFieldName, type); } } } + private boolean isIndexableAttributeField(AttributeInterface attribute) { + return attribute instanceof IndexableAttributeInterface + || ((attribute instanceof DateAttribute || attribute instanceof NumberAttribute + || attribute instanceof BooleanAttribute) && attribute.isSearchable()); + } + + private String getFieldType(AttributeInterface attribute) { + if (attribute instanceof DateAttribute) { + return SolrFields.TYPE_PDATES; + } else if (attribute instanceof NumberAttribute) { + return SolrFields.TYPE_PLONGS; + } else if (attribute instanceof BooleanAttribute) { + // Covers ThreeStateAttribute too (subclass): SolrComplexAttributes.solrType tests it + // first internally and returns TYPE_STRING for it, TYPE_BOOLEAN otherwise. + return SolrComplexAttributes.solrType(attribute); + } else { + return SolrFields.TYPE_TEXT_GEN_SORT; + } + } + + private String buildFieldName(Lang lang, String name) { + return (lang.getCode().toLowerCase() + "_" + name).replace(":", "_"); + } + + // Nested booleans only, Composite attributes only: List/Monolist attributes are excluded, and + // Date/Number/Text children remain full-text-only (unchanged, matching the baseline Lucene + // engine). The field name carries the full composite path, e.g. "en_complexAttrName_boolAttrName". + private void checkComplexAttributeChildren(AttributeInterface attribute, Lang lang, String namePrefix) { + if (attribute instanceof AbstractComplexAttribute complexAttribute && !(attribute instanceof ListAttributeInterface)) { + for (AttributeInterface child : complexAttribute.getAttributes()) { + this.checkNestedAttribute(child, lang, namePrefix); + } + } + } + + private void checkNestedAttribute(AttributeInterface attribute, Lang lang, String namePrefix) { + attribute.setRenderingLang(lang.getCode()); + String path = SolrComplexAttributes.appendPath(namePrefix, attribute.getName()); + if (!attribute.isSimple()) { + this.checkComplexAttributeChildren(attribute, lang, path); + return; + } + if (SolrComplexAttributes.isIndexableCompositeBoolean(attribute)) { + String fieldName = this.buildFieldName(lang, path); + // Single-valued: a Composite occurs at most once per document per lang (Monolist and + // Monolist-of-Composite are excluded above), so the nested field is never repeated. + this.checkField(fieldName, SolrComplexAttributes.solrType(attribute), false); + } + } + private void checkField(String fieldName, String type) { this.checkField(fieldName, type, false); } diff --git a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrSearchEngineManager.java b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrSearchEngineManager.java index 1cc685429c..1afad65234 100644 --- a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrSearchEngineManager.java +++ b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrSearchEngineManager.java @@ -19,7 +19,9 @@ import com.agiletec.aps.system.common.entity.event.EntityTypesChangingObserver; import com.agiletec.aps.system.common.entity.model.IApsEntity; import com.agiletec.aps.system.common.entity.model.SmallEntityType; +import com.agiletec.aps.system.common.entity.model.attribute.AbstractComplexAttribute; import com.agiletec.aps.system.common.entity.model.attribute.AttributeInterface; +import com.agiletec.aps.system.common.entity.model.attribute.ListAttributeInterface; import com.agiletec.aps.system.services.lang.ILangManager; import com.agiletec.aps.system.services.lang.Lang; import com.agiletec.aps.util.ApsTenantApplicationUtils; @@ -58,6 +60,10 @@ * @author E.Santoboni */ @Slf4j +// NOTE: java:S2143 ("use the java.time API") is intentionally suppressed. Legacy java.util.Date is +// used only to stamp a reload timestamp (new Date()); java.time migration is out of scope for +// ESB-1133 (boolean search) and tracked separately. +@SuppressWarnings("java:S2143") public class SolrSearchEngineManager extends SearchEngineManager implements ISolrSearchEngineManager, PublicContentChangedObserver, EntityTypesChangingObserver, InitializingBean { @@ -173,17 +179,15 @@ private List getContentTypesSettings(ISolrSchemaDAO schemaD entityType.getDescription()); list.add(typeSettings); Content prototype = this.getContentManager().createContentType(entityType.getCode()); + List languages = this.langManager.getLangs(); for (AttributeInterface attribute : prototype.getAttributeList()) { - Map> currentConfig = new HashMap<>(); - List languages = this.langManager.getLangs(); - for (Lang lang : languages) { - String fieldName = lang.getCode().toLowerCase() + "_" + attribute.getName(); - fields.stream() - .filter(f -> f.get(SOLR_FIELD_NAME).equals(fieldName)) - .findFirst().ifPresent(currentField -> - currentConfig.put(fieldName, (Map) currentField)); - } + Map> currentConfig = + this.buildCurrentFieldConfig(attribute.getName(), languages, fields); typeSettings.addAttribute(attribute, currentConfig, languages); + if (!attribute.isSimple()) { + this.collectNestedBooleanAttributes(attribute, attribute.getName(), fields, languages, + typeSettings); + } } } } catch (Exception e) { @@ -192,6 +196,42 @@ private List getContentTypesSettings(ISolrSchemaDAO schemaD return list; } + private Map> buildCurrentFieldConfig(String attributeName, + List languages, List> fields) { + Map> currentConfig = new HashMap<>(); + for (Lang lang : languages) { + String fieldName = lang.getCode().toLowerCase() + "_" + attributeName; + fields.stream() + .filter(f -> f.get(SOLR_FIELD_NAME).equals(fieldName)) + .findFirst().ifPresent(currentField -> + currentConfig.put(fieldName, (Map) currentField)); + } + return currentConfig; + } + + // Composite attributes only (List/Monolist excluded), mirroring + // SolrFieldsChecker.checkComplexAttributeChildren: reports boolean children under their full + // composite path (e.g. "en_complexAttrName_boolAttrName") so the admin "content types settings" endpoint and + // the lazy schema-refresh validity check (isValid()) stay in sync with the schema/index. + private void collectNestedBooleanAttributes(AttributeInterface attribute, String namePrefix, + List> fields, List languages, ContentTypeSettings typeSettings) { + if (!(attribute instanceof AbstractComplexAttribute) || attribute instanceof ListAttributeInterface) { + return; + } + for (AttributeInterface child : ((AbstractComplexAttribute) attribute).getAttributes()) { + String childPath = SolrComplexAttributes.appendPath(namePrefix, child.getName()); + if (child.isSimple()) { + if (SolrComplexAttributes.isIndexableCompositeBoolean(child)) { + Map> currentConfig = + this.buildCurrentFieldConfig(childPath, languages, fields); + typeSettings.addNestedBooleanAttribute(child, currentConfig, languages); + } + } else { + this.collectNestedBooleanAttributes(child, childPath, fields, languages, typeSettings); + } + } + } + @Override public void refreshCmsFields() throws EntException { refreshCmsFields(solrProxy.getSolrTenantResources()); diff --git a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/model/ContentTypeSettings.java b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/model/ContentTypeSettings.java index 7ea42c2633..8a4410a7e3 100644 --- a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/model/ContentTypeSettings.java +++ b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/model/ContentTypeSettings.java @@ -28,6 +28,7 @@ import java.util.List; import java.util.Map; import java.util.stream.Collectors; +import org.entando.entando.plugins.jpsolr.aps.system.solr.SolrComplexAttributes; /** * @author E.Santoboni @@ -72,15 +73,16 @@ public void addAttribute(AttributeInterface attribute, Map> currentField, List languages) { + AttributeSettings settings = new AttributeSettings(attribute, languages); + this.getAttributeSettings().add(settings); + settings.setCurrentConfig(currentField); + Map newField = new HashMap<>(); + newField.put(SOLR_FIELD_TYPE, SolrComplexAttributes.solrType(attribute)); + newField.put(SOLR_FIELD_MULTIVALUED, false); + settings.setExpectedConfig(newField); + } + public boolean isValid() { return this.getAttributeSettings().stream().allMatch(AttributeSettings::isValid); } diff --git a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManagerTest.java b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManagerTest.java index b73301e9d1..86babffd49 100644 --- a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManagerTest.java +++ b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManagerTest.java @@ -210,6 +210,98 @@ void shouldRejectInjectionInSearchOption() { () -> facetManager.getFacetedContents(request, null)); } + @ParameterizedTest + @ValueSource(strings = {"gt", "lt"}) + void shouldRejectRangeOperatorOnBooleanFilter(String operator) { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + SolrFilter filter = new SolrFilter("myFlag", "true", operator); + filter.setType("boolean"); + request.setFilters(new Filter[]{filter}); + Assertions.assertThrows(ValidationConflictException.class, + () -> facetManager.getFacetedContents(request, null)); + } + + @Test + void shouldRejectNonStrictBooleanValue() { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + // A three-state "none" must not be sent as a boolean value: FilterType.BOOLEAN would + // silently coerce it to false via Boolean.parseBoolean. + SolrFilter filter = new SolrFilter("myFlag", "none", "eq"); + filter.setType("boolean"); + request.setFilters(new Filter[]{filter}); + Assertions.assertThrows(ValidationConflictException.class, + () -> facetManager.getFacetedContents(request, null)); + } + + @Test + void shouldRejectNonStrictBooleanAllowedValue() { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + SolrFilter filter = new SolrFilter(); + filter.setEntityAttr("myFlag"); + filter.setType("boolean"); + filter.setOperator("eq"); + filter.setAllowedValues(new String[]{"true", "maybe"}); + request.setFilters(new Filter[]{filter}); + Assertions.assertThrows(ValidationConflictException.class, + () -> facetManager.getFacetedContents(request, null)); + } + + @Test + void shouldAcceptStrictBooleanValue() throws Exception { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + SolrFilter filter = new SolrFilter("myFlag", "true", "eq"); + filter.setType("boolean"); + filter.setEntityAttr("myFlag"); + request.setFilters(new Filter[]{filter}); + when(langManager.getDefaultLang()).thenReturn(createLang("en")); + when(((ISolrSearchEngineManager) searchEngineManager) + .searchFacetedEntities( + any(SolrSearchEngineFilter[][].class), + any(SolrSearchEngineFilter[].class), + any(List.class))) + .thenReturn(new SolrFacetedContentsResult()); + Assertions.assertDoesNotThrow(() -> facetManager.getFacetedContents(request, null)); + } + + @Test + void shouldAcceptStrictBooleanFalseValue() throws Exception { + // Mirrors shouldAcceptStrictBooleanValue with "false": closes the remaining branch of + // rejectIfNotStrictBoolean's "!true && !false" check (the value == "false" combination), + // never exercised by the "true"/"maybe"/"none" cases above. + AdvRestContentListRequest request = new AdvRestContentListRequest(); + SolrFilter filter = new SolrFilter("myFlag", "false", "eq"); + filter.setType("boolean"); + filter.setEntityAttr("myFlag"); + request.setFilters(new Filter[]{filter}); + when(langManager.getDefaultLang()).thenReturn(createLang("en")); + when(((ISolrSearchEngineManager) searchEngineManager) + .searchFacetedEntities( + any(SolrSearchEngineFilter[][].class), + any(SolrSearchEngineFilter[].class), + any(List.class))) + .thenReturn(new SolrFacetedContentsResult()); + Assertions.assertDoesNotThrow(() -> facetManager.getFacetedContents(request, null)); + } + + @Test + void shouldAcceptNoValueNotEqualBooleanFilterAsExistenceQuery() throws Exception { + // The only valid recipe for querying a ThreeState "none": no value + not_equal. + AdvRestContentListRequest request = new AdvRestContentListRequest(); + SolrFilter filter = new SolrFilter(); + filter.setEntityAttr("myFlag"); + filter.setType("boolean"); + filter.setOperator("not"); + request.setFilters(new Filter[]{filter}); + when(langManager.getDefaultLang()).thenReturn(createLang("en")); + when(((ISolrSearchEngineManager) searchEngineManager) + .searchFacetedEntities( + any(SolrSearchEngineFilter[][].class), + any(SolrSearchEngineFilter[].class), + any(List.class))) + .thenReturn(new SolrFacetedContentsResult()); + Assertions.assertDoesNotThrow(() -> facetManager.getFacetedContents(request, null)); + } + private static Lang createLang(String code) { Lang lang = new Lang(); lang.setCode(code); diff --git a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/IndexerDAOTest.java b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/IndexerDAOTest.java new file mode 100644 index 0000000000..fcff8bfee2 --- /dev/null +++ b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/IndexerDAOTest.java @@ -0,0 +1,320 @@ +/* + * Copyright 2021-Present Entando Inc. (http://www.entando.com) All rights reserved. + * + * This library is free software; you can redistribute it and/or modify it under + * the terms of the GNU Lesser General Public License as published by the Free + * Software Foundation; either version 2.1 of the License, or (at your option) + * any later version. + * + * This library is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more + * details. + */ +package org.entando.entando.plugins.jpsolr.aps.system.solr; + +import com.agiletec.aps.system.common.entity.model.attribute.AttributeInterface; +import com.agiletec.aps.system.common.entity.model.attribute.BooleanAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.CheckBoxAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.CompositeAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.MonoListAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.MonoTextAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.ThreeStateAttribute; +import com.agiletec.aps.system.common.searchengine.IndexableAttributeInterface; +import com.agiletec.aps.system.services.lang.ILangManager; +import com.agiletec.aps.system.services.lang.Lang; +import com.agiletec.plugins.jacms.aps.system.services.content.model.Content; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import org.apache.solr.client.solrj.SolrClient; +import org.apache.solr.common.SolrInputDocument; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +class IndexerDAOTest { + + private IndexerDAO newIndexerDAO() { + IndexerDAO indexerDAO = new IndexerDAO(mock(SolrClient.class), "core"); + ILangManager langManager = mock(ILangManager.class); + when(langManager.getLangs()).thenReturn(List.of(lang("en"))); + indexerDAO.setLangManager(langManager); + return indexerDAO; + } + + private Lang lang(String code) { + Lang lang = new Lang(); + lang.setCode(code); + lang.setDescr(code); + return lang; + } + + private Content newContent() { + Content content = new Content(); + content.setId("1"); + content.setTypeCode("TST"); + content.setMainGroup("free"); + return content; + } + + private BooleanAttribute booleanAttribute(String name, boolean searchable, Boolean value) { + BooleanAttribute attribute = new BooleanAttribute(); + attribute.setName(name); + attribute.setType("Boolean"); + attribute.setSearchable(searchable); + attribute.setBooleanValue(value); + return attribute; + } + + private ThreeStateAttribute threeStateAttribute(String name, boolean searchable, Boolean value) { + ThreeStateAttribute attribute = new ThreeStateAttribute(); + attribute.setName(name); + attribute.setType("ThreeState"); + attribute.setSearchable(searchable); + attribute.setBooleanValue(value); + return attribute; + } + + private CheckBoxAttribute checkBoxAttribute(String name, boolean searchable, Boolean value) { + CheckBoxAttribute attribute = new CheckBoxAttribute(); + attribute.setName(name); + attribute.setType("CheckBox"); + attribute.setSearchable(searchable); + attribute.setBooleanValue(value); + return attribute; + } + + private MonoTextAttribute fullTextMonoText(String name, String text) { + MonoTextAttribute attribute = new MonoTextAttribute(); + attribute.setName(name); + attribute.setType("Monotext"); + attribute.setIndexingType(IndexableAttributeInterface.INDEXING_TYPE_TEXT); + attribute.setText(text); + return attribute; + } + + /** Exposes the protected {@code addAttribute} so tests can build composite fixtures. */ + private static class TestComposite extends CompositeAttribute { + + void addChild(AttributeInterface attribute) { + this.addAttribute(attribute); + } + } + + private TestComposite composite(String name, AttributeInterface... children) { + TestComposite composite = new TestComposite(); + composite.setName(name); + composite.setType("Composite"); + for (AttributeInterface child : children) { + composite.addChild(child); + } + return composite; + } + + // ---- top-level booleans: unchanged, still gated by the "searchable" flag ---- + + @Test + void shouldIndexSearchableTopLevelBooleanAttribute() { + Content content = newContent(); + content.addAttribute(booleanAttribute("myFlag", true, Boolean.TRUE)); + + SolrInputDocument document = newIndexerDAO().createDocument(content); + + Assertions.assertEquals(Boolean.TRUE, document.getFieldValue("en_myFlag")); + Assertions.assertNull(document.getField("en"), "boolean values must not pollute the full-text field"); + } + + @Test + void shouldIndexFalseTopLevelBooleanAttribute() { + Content content = newContent(); + content.addAttribute(booleanAttribute("myFlag", true, Boolean.FALSE)); + + SolrInputDocument document = newIndexerDAO().createDocument(content); + + Assertions.assertEquals(Boolean.FALSE, document.getFieldValue("en_myFlag")); + } + + @Test + void shouldNotIndexNonSearchableTopLevelBoolean() { + Content content = newContent(); + content.addAttribute(booleanAttribute("myFlag", false, Boolean.TRUE)); + + SolrInputDocument document = newIndexerDAO().createDocument(content); + + Assertions.assertNull(document.getField("en_myFlag")); + } + + @Test + void shouldIndexUnsetThreeStateAttributeAsNoneLiteral() { + // Unlike a plain Boolean/CheckBox (which coerces null to false), an unset ThreeState must + // reach the index as the literal "none" string so it stays distinguishable and queryable. + Content content = newContent(); + content.addAttribute(threeStateAttribute("flag3", true, null)); + + SolrInputDocument document = newIndexerDAO().createDocument(content); + + Assertions.assertEquals("none", document.getFieldValue("en_flag3")); + } + + @Test + void shouldIndexTrueTopLevelThreeStateAttributeAsStringLiteral() { + Content content = newContent(); + content.addAttribute(threeStateAttribute("flag3", true, Boolean.TRUE)); + + SolrInputDocument document = newIndexerDAO().createDocument(content); + + Assertions.assertEquals("true", document.getFieldValue("en_flag3")); + } + + @Test + void shouldIndexSearchableTopLevelCheckBoxAttribute() { + Content content = newContent(); + content.addAttribute(checkBoxAttribute("myCheck", true, Boolean.TRUE)); + + SolrInputDocument document = newIndexerDAO().createDocument(content); + + Assertions.assertEquals(Boolean.TRUE, document.getFieldValue("en_myCheck")); + } + + @Test + void shouldIndexSearchableTopLevelThreeStateAttributeWithFalseValue() { + Content content = newContent(); + content.addAttribute(threeStateAttribute("flag3", true, Boolean.FALSE)); + + SolrInputDocument document = newIndexerDAO().createDocument(content); + + Assertions.assertEquals("false", document.getFieldValue("en_flag3")); + } + + @Test + void shouldNotIndexTopLevelAttributeOfUnsupportedType() { + // Closes the final "instanceof BooleanAttribute" false outcome of indexAttribute's + // top-level dispatch condition: an attribute that is neither IndexableAttributeInterface, + // Date, Number nor Boolean must be skipped entirely rather than throw. + Content content = newContent(); + AttributeInterface unsupported = mock(AttributeInterface.class); + when(unsupported.isSimple()).thenReturn(true); + when(unsupported.getName()).thenReturn("unsupported"); + when(unsupported.getValue()).thenReturn("nonNullValue"); + content.addAttribute(unsupported); + + SolrInputDocument document = newIndexerDAO().createDocument(content); + + Assertions.assertNull(document.getField("en_unsupported")); + } + + // ---- composite children: qualified path, gated by the inherited searchable flag ---- + + @Test + void shouldIndexSearchableBooleanChildOfCompositeUnderQualifiedName() { + Content content = newContent(); + content.addAttribute(composite("myComposite", booleanAttribute("featured", true, Boolean.TRUE))); + + SolrInputDocument document = newIndexerDAO().createDocument(content); + + Assertions.assertEquals(Boolean.TRUE, document.getFieldValue("en_myComposite_featured")); + } + + @Test + void shouldNotIndexNonSearchableCompositeChild() { + Content content = newContent(); + content.addAttribute(composite("myComposite", booleanAttribute("featured", false, Boolean.TRUE))); + + SolrInputDocument document = newIndexerDAO().createDocument(content); + + Assertions.assertNull(document.getField("en_myComposite_featured")); + } + + @Test + void shouldIndexCheckBoxChildOfCompositeUnderQualifiedName() { + Content content = newContent(); + content.addAttribute(composite("myComposite", checkBoxAttribute("featuredCheck", true, Boolean.TRUE))); + + SolrInputDocument document = newIndexerDAO().createDocument(content); + + Assertions.assertEquals(Boolean.TRUE, document.getFieldValue("en_myComposite_featuredCheck")); + } + + @Test + void shouldIndexValuedThreeStateChildOfCompositeUnderQualifiedName() { + Content content = newContent(); + content.addAttribute(composite("myComposite", threeStateAttribute("featured3", true, Boolean.FALSE))); + + SolrInputDocument document = newIndexerDAO().createDocument(content); + + Assertions.assertEquals("false", document.getFieldValue("en_myComposite_featured3")); + } + + @Test + void shouldIndexUnsetThreeStateChildOfCompositeAsNoneLiteral() { + Content content = newContent(); + content.addAttribute(composite("myComposite", threeStateAttribute("featured3", true, null))); + + SolrInputDocument document = newIndexerDAO().createDocument(content); + + Assertions.assertEquals("none", document.getFieldValue("en_myComposite_featured3")); + } + + @Test + void shouldIndexBooleanInNestedCompositeUnderFullPath() { + Content content = newContent(); + TestComposite inner = composite("inner", booleanAttribute("boolAttrName", true, Boolean.TRUE)); + content.addAttribute(composite("outer", inner)); + + SolrInputDocument document = newIndexerDAO().createDocument(content); + + Assertions.assertEquals(Boolean.TRUE, document.getFieldValue("en_outer_inner_boolAttrName")); + } + + // ---- List / Monolist: excluded from boolean indexing ---- + + @Test + void shouldNotIndexBooleanElementsOfMonolist() { + Content content = newContent(); + MonoListAttribute list = new MonoListAttribute(); + list.setName("myList"); + list.setType("Monolist"); + list.getAttributes().add(booleanAttribute("myList", true, Boolean.TRUE)); + list.getAttributes().add(booleanAttribute("myList", true, Boolean.FALSE)); + content.addAttribute(list); + + SolrInputDocument document = newIndexerDAO().createDocument(content); + + Assertions.assertNull(document.getField("en_myList")); + } + + @Test + void shouldNotIndexBooleanInCompositeNestedInMonolist() { + Content content = newContent(); + MonoListAttribute list = new MonoListAttribute(); + list.setName("comboList"); + list.setType("Monolist"); + list.getAttributes().add(composite("comboList", booleanAttribute("subFlag", true, Boolean.TRUE))); + content.addAttribute(list); + + SolrInputDocument document = newIndexerDAO().createDocument(content); + + Assertions.assertNull(document.getField("en_comboList_subFlag")); + Assertions.assertNull(document.getField("en_subFlag")); + } + + @Test + void shouldStillFullTextIndexTextChildrenInsideMonolist() { + // Regression guard: the list exclusion gates only the per-attribute boolean branch; text + // children inside a list must still feed the full-text "" field. + Content content = newContent(); + MonoListAttribute list = new MonoListAttribute(); + list.setName("notes"); + list.setType("Monolist"); + list.getAttributes().add(fullTextMonoText("notes", "hello")); + content.addAttribute(list); + + SolrInputDocument document = newIndexerDAO().createDocument(content); + + Collection fullText = document.getFieldValues("en"); + Assertions.assertNotNull(fullText, "text children in a list must still feed the full-text field"); + Assertions.assertTrue(new ArrayList<>(fullText).contains("hello"), String.valueOf(fullText)); + } +} diff --git a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAOTest.java b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAOTest.java index 90ac5337b3..5cd2ba6420 100644 --- a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAOTest.java +++ b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAOTest.java @@ -1,5 +1,7 @@ package org.entando.entando.plugins.jpsolr.aps.system.solr; +import com.agiletec.aps.system.common.entity.model.attribute.CheckBoxAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.ThreeStateAttribute; import com.agiletec.aps.system.common.tree.ITreeNode; import com.agiletec.aps.system.common.tree.ITreeNodeManager; import com.agiletec.aps.system.services.group.Group; @@ -22,10 +24,12 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; -import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; @ExtendWith(MockitoExtension.class) @@ -310,6 +314,151 @@ void shouldFilterOnRangeOfNumbers() throws Exception { "+(+entity_key:[10 TO 20]) +(entity_group:free)"); } + @Test + void shouldFilterByBooleanSingleValueTrue() throws Exception { + Boolean value = Boolean.TRUE; + SolrSearchEngineFilter filter = new SolrSearchEngineFilter<>("key", value); + + SearchEngineFilter[] filters = new SearchEngineFilter[]{filter}; + SearchEngineFilter[] categories = new SearchEngineFilter[]{}; + List allowedGroups = new ArrayList<>(); + + testSearchFacetedContents(filters, categories, allowedGroups, + "+(+entity_key:true) +(entity_group:free)"); + } + + @Test + void shouldFilterByBooleanSingleValueFalse() throws Exception { + Boolean value = Boolean.FALSE; + SolrSearchEngineFilter filter = new SolrSearchEngineFilter<>("key", value); + + SearchEngineFilter[] filters = new SearchEngineFilter[]{filter}; + SearchEngineFilter[] categories = new SearchEngineFilter[]{}; + List allowedGroups = new ArrayList<>(); + + testSearchFacetedContents(filters, categories, allowedGroups, + "+(+entity_key:false) +(entity_group:free)"); + } + + @Test + void shouldResolveBooleanAttributeFilterToLangQualifiedField() throws Exception { + // Mirrors GET .../contents?filters[0].entityAttr=&operator=eq&value=true. + // An entityAttr filter is an attribute filter, so the key resolves to "_". + // The indexer writes a boolean under that same "_" field whether the boolean + // is top-level, a Composite child (its own name) or a Monolist element (the list's name), + // so addressing a nested boolean by its (leaf) name is fully supported. + SolrSearchEngineFilter filter = new SolrSearchEngineFilter<>("featured", true, Boolean.TRUE); + filter.setLangCode("en"); + + SearchEngineFilter[] filters = new SearchEngineFilter[]{filter}; + SearchEngineFilter[] categories = new SearchEngineFilter[]{}; + List allowedGroups = new ArrayList<>(); + + testSearchFacetedContents(filters, categories, allowedGroups, + "+(+en_featured:true) +(entity_group:free)"); + } + + @Test + void shouldResolveNestedBooleanCompositePathFilter() throws Exception { + // A composite-nested boolean is addressed by its full composite path, e.g. "complexAttrName_boolAttrName", + // which resolves to the "_complexAttrName_boolAttrName" field the indexer/schema build. + SolrSearchEngineFilter filter = new SolrSearchEngineFilter<>("complexAttrName_boolAttrName", true, Boolean.TRUE); + filter.setLangCode("en"); + + SearchEngineFilter[] filters = new SearchEngineFilter[]{filter}; + SearchEngineFilter[] categories = new SearchEngineFilter[]{}; + List allowedGroups = new ArrayList<>(); + + testSearchFacetedContents(filters, categories, allowedGroups, + "+(+en_complexAttrName_boolAttrName:true) +(entity_group:free)"); + } + + @Test + void shouldFilterUsingValueSourcedFromCheckBoxAttribute() throws Exception { + // The query layer never sees the source AttributeInterface, only the Boolean value a + // filter carries - proving CheckBoxAttribute's getValue() flows through identically to + // a plain BooleanAttribute's. + CheckBoxAttribute checkBoxAttribute = new CheckBoxAttribute(); + checkBoxAttribute.setBooleanValue(Boolean.TRUE); + SolrSearchEngineFilter filter = new SolrSearchEngineFilter<>("key", checkBoxAttribute.getValue()); + + SearchEngineFilter[] filters = new SearchEngineFilter[]{filter}; + SearchEngineFilter[] categories = new SearchEngineFilter[]{}; + List allowedGroups = new ArrayList<>(); + + testSearchFacetedContents(filters, categories, allowedGroups, + "+(+entity_key:true) +(entity_group:free)"); + } + + @Test + void shouldFilterUsingValueSourcedFromThreeStateAttribute() throws Exception { + ThreeStateAttribute threeStateAttribute = new ThreeStateAttribute(); + threeStateAttribute.setBooleanValue(Boolean.FALSE); + SolrSearchEngineFilter filter = new SolrSearchEngineFilter<>("key", threeStateAttribute.getValue()); + + SearchEngineFilter[] filters = new SearchEngineFilter[]{filter}; + SearchEngineFilter[] categories = new SearchEngineFilter[]{}; + List allowedGroups = new ArrayList<>(); + + testSearchFacetedContents(filters, categories, allowedGroups, + "+(+entity_key:false) +(entity_group:free)"); + } + + @Test + void shouldHandleBooleanFilterAllowedValues() throws Exception { + mockDefaultLang(); + SearchEngineFilter filter = SearchEngineFilter.createAllowedValuesFilter("key", true, + List.of(Boolean.TRUE, Boolean.FALSE), null); + + SearchEngineFilter[] filters = new SearchEngineFilter[]{filter}; + SearchEngineFilter[] categories = new SearchEngineFilter[]{}; + List allowedGroups = new ArrayList<>(); + + testSearchFacetedContents(filters, categories, allowedGroups, + "+(en_key:true en_key:false) +(entity_group:free)"); + } + + @Test + void shouldNegateBooleanFilter() throws Exception { + Boolean value = Boolean.TRUE; + SolrSearchEngineFilter filter = new SolrSearchEngineFilter<>("key", value); + filter.setNotOption(true); + + SearchEngineFilter[] filters = new SearchEngineFilter[]{filter}; + SearchEngineFilter[] categories = new SearchEngineFilter[]{}; + List allowedGroups = new ArrayList<>(); + + testSearchFacetedContents(filters, categories, allowedGroups, + "-(+entity_key:true) +(entity_group:free)"); + } + + @Test + void shouldFilterOnNullValueBooleanFilterAsExistenceQuery() throws Exception { + SolrSearchEngineFilter filter = new SolrSearchEngineFilter<>("key", true); + filter.setLangCode("en"); + + SearchEngineFilter[] filters = new SearchEngineFilter[]{filter}; + SearchEngineFilter[] categories = new SearchEngineFilter[]{}; + List allowedGroups = new ArrayList<>(); + + testSearchFacetedContents(filters, categories, allowedGroups, "+(+en_key:*) +(entity_group:free)"); + } + + @Test + void shouldIgnoreUnsupportedSingleValueType() throws Exception { + // Closes the "else if (value instanceof Boolean)" false branch of createSingleValueQuery: + // a value that is neither String, Date, Number nor Boolean falls through every branch, + // producing a no-op (empty) sub-query for that field instead of throwing. + SolrSearchEngineFilter filter = new SolrSearchEngineFilter<>("key", new Object()); + + SearchEngineFilter[] filters = new SearchEngineFilter[]{filter}; + SearchEngineFilter[] categories = new SearchEngineFilter[]{}; + List allowedGroups = new ArrayList<>(); + + testSearchFacetedContents(filters, categories, allowedGroups, + "+() +(entity_group:free)"); + } + @Test void shouldFilterOnRangeOfStrings() throws Exception { SolrSearchEngineFilter filter = new SolrSearchEngineFilter("key", "A", "E"); @@ -328,7 +477,7 @@ void shouldHandleArrayOfArraysFilters() throws Exception { ArgumentCaptor queryCaptor = ArgumentCaptor.forClass(SolrQuery.class); QueryResponse queryResponse = mockQueryResponse(); - Mockito.when(solrClient.query(Mockito.any(), queryCaptor.capture())).thenReturn(queryResponse); + when(solrClient.query(any(), queryCaptor.capture())).thenReturn(queryResponse); SolrSearchEngineFilter filterSort = new SolrSearchEngineFilter("created", false); SolrSearchEngineFilter filterValue = new SolrSearchEngineFilter("key", true, "value"); @@ -358,7 +507,7 @@ private void testSearchFacetedContents(SearchEngineFilter[] filters, SearchEngin List allowedGroups, String expectedQuery) throws Exception { ArgumentCaptor queryCaptor = ArgumentCaptor.forClass(SolrQuery.class); QueryResponse queryResponse = mockQueryResponse(); - Mockito.when(solrClient.query(Mockito.any(), queryCaptor.capture())).thenReturn(queryResponse); + when(solrClient.query(any(), queryCaptor.capture())).thenReturn(queryResponse); searcherDAO.searchFacetedContents(filters, categories, allowedGroups); @@ -369,11 +518,11 @@ private void testSearchFacetedContents(SearchEngineFilter[] filters, SearchEngin private void mockDefaultLang() { Lang lang = new Lang(); lang.setCode("en"); - Mockito.when(langManager.getDefaultLang()).thenReturn(lang); + when(langManager.getDefaultLang()).thenReturn(lang); } private void mockCategory(String categoryCode) { - Mockito.when(treeNodeManager.getNode(categoryCode)).thenReturn(Mockito.mock(ITreeNode.class)); + when(treeNodeManager.getNode(categoryCode)).thenReturn(mock(ITreeNode.class)); } private Date getDate(String date) throws Exception { @@ -383,9 +532,9 @@ private Date getDate(String date) throws Exception { } private QueryResponse mockQueryResponse() { - QueryResponse queryResponse = Mockito.mock(QueryResponse.class); + QueryResponse queryResponse = mock(QueryResponse.class); SolrDocumentList documents = new SolrDocumentList(); - Mockito.when(queryResponse.getResults()).thenReturn(documents); + when(queryResponse.getResults()).thenReturn(documents); return queryResponse; } } diff --git a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrFieldsCheckerTest.java b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrFieldsCheckerTest.java index 2d3b722d7a..85b01233d4 100644 --- a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrFieldsCheckerTest.java +++ b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrFieldsCheckerTest.java @@ -13,9 +13,15 @@ */ package org.entando.entando.plugins.jpsolr.aps.system.solr; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import com.agiletec.aps.system.common.entity.model.attribute.AttributeInterface; +import com.agiletec.aps.system.common.entity.model.attribute.BooleanAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.CheckBoxAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.CompositeAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.MonoListAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.ThreeStateAttribute; import com.agiletec.aps.system.services.lang.Lang; import java.util.Collections; import java.util.List; @@ -24,6 +30,8 @@ import org.entando.entando.plugins.jpsolr.aps.system.solr.model.SolrFields; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; class SolrFieldsCheckerTest { @@ -34,16 +42,18 @@ private Lang lang(String code) { return lang; } + private SolrFieldsChecker checker(List attributes, Lang... langs) { + return new SolrFieldsChecker(Collections.emptyList(), attributes, List.of(langs)); + } + @Test void shouldCreateMainAndAttachmentFieldPerLanguage() { // No pre-existing fields, no content-type attributes: only the base + per-language // fields are generated. Each language must yield BOTH the main "" field and the // "_attachment" field, so a full-text search with includeAttachments=true never // queries a field the schema doesn't know about. - SolrFieldsChecker checker = new SolrFieldsChecker( - Collections.emptyList(), - Collections.emptyList(), - List.of(lang("en"), lang("it"))); + SolrFieldsChecker checker = this.checker( + Collections.emptyList(), lang("en"), lang("it")); List createdFieldNames = checker.checkFields().getFieldsToAdd().stream() .map(field -> (String) field.get(SolrFields.SOLR_FIELD_NAME)) @@ -59,10 +69,8 @@ void shouldCreateMainAndAttachmentFieldPerLanguage() { @Test void attachmentFieldMatchesMainLanguageFieldTypeAndMultiplicity() { - SolrFieldsChecker checker = new SolrFieldsChecker( - Collections.emptyList(), - Collections.emptyList(), - List.of(lang("en"))); + SolrFieldsChecker checker = this.checker( + Collections.emptyList(), lang("en")); List> added = checker.checkFields().getFieldsToAdd(); Map main = added.stream() @@ -76,4 +84,212 @@ void attachmentFieldMatchesMainLanguageFieldTypeAndMultiplicity() { Assertions.assertEquals(main.get(SolrFields.SOLR_FIELD_MULTIVALUED), attachment.get(SolrFields.SOLR_FIELD_MULTIVALUED)); } + + /** Exposes the protected {@code addAttribute} so tests can build composite fixtures. */ + private static class TestComposite extends CompositeAttribute { + + void addChild(AttributeInterface attribute) { + this.addAttribute(attribute); + } + } + + private BooleanAttribute booleanAttribute(String name, boolean searchable) { + BooleanAttribute attribute = new BooleanAttribute(); + attribute.setName(name); + attribute.setType("Boolean"); + attribute.setSearchable(searchable); + return attribute; + } + + private CheckBoxAttribute checkBoxAttribute(String name, boolean searchable) { + CheckBoxAttribute attribute = new CheckBoxAttribute(); + attribute.setName(name); + attribute.setType("CheckBox"); + attribute.setSearchable(searchable); + return attribute; + } + + private ThreeStateAttribute threeStateAttribute(String name, boolean searchable) { + ThreeStateAttribute attribute = new ThreeStateAttribute(); + attribute.setName(name); + attribute.setType("ThreeState"); + attribute.setSearchable(searchable); + return attribute; + } + + + private TestComposite composite(String name, AttributeInterface... children) { + TestComposite composite = new TestComposite(); + composite.setName(name); + composite.setType("Composite"); + for (AttributeInterface child : children) { + composite.addChild(child); + } + return composite; + } + + // ---- top-level booleans: unchanged, still gated by the "searchable" flag ---- + + @Test + void shouldCreateSingleValuedBooleanFieldForSearchableTopLevelAttribute() { + SolrFieldsChecker checker = this.checker(List.of(booleanAttribute("myFlag", true)), lang("en")); + + Map field = fieldsToAdd(checker, "en_myFlag"); + Assertions.assertEquals(SolrFields.TYPE_BOOLEAN, field.get(SolrFields.SOLR_FIELD_TYPE)); + Assertions.assertEquals(false, field.get(SolrFields.SOLR_FIELD_MULTIVALUED)); + } + + @Test + void shouldNotCreateFieldForNonSearchableTopLevelBoolean() { + SolrFieldsChecker checker = this.checker(List.of(booleanAttribute("myFlag", false)), lang("en")); + + assertFalse(fieldNames(checker).contains("en_myFlag"), fieldNames(checker).toString()); + } + + @Test + void shouldCreateBooleanFieldForSearchableTopLevelCheckBoxAttribute() { + SolrFieldsChecker checker = this.checker(List.of(checkBoxAttribute("myCheck", true)), lang("en")); + + Map field = fieldsToAdd(checker, "en_myCheck"); + Assertions.assertEquals(SolrFields.TYPE_BOOLEAN, field.get(SolrFields.SOLR_FIELD_TYPE)); + Assertions.assertEquals(false, field.get(SolrFields.SOLR_FIELD_MULTIVALUED)); + } + + @Test + void shouldCreateStringFieldForSearchableTopLevelThreeStateAttribute() { + // ThreeState is indexed as a Solr "string" (not "boolean"): its third, uninitialized state + // cannot be represented in a two-valued BoolField. Pins the ThreeState-before-Boolean + // dispatch order. + SolrFieldsChecker checker = this.checker(List.of(threeStateAttribute("myFlag3", true)), lang("en")); + + Map field = fieldsToAdd(checker, "en_myFlag3"); + Assertions.assertEquals(SolrFields.TYPE_STRING, field.get(SolrFields.SOLR_FIELD_TYPE)); + Assertions.assertEquals(false, field.get(SolrFields.SOLR_FIELD_MULTIVALUED)); + } + + @Test + void shouldNotCreateFieldForTopLevelAttributeOfUnsupportedType() { + // Closes the final "instanceof BooleanAttribute" false outcome of + // isIndexableAttributeField: an attribute that is neither IndexableAttributeInterface, + // Date, Number nor Boolean must not produce a field. + AttributeInterface unsupported = mock(AttributeInterface.class); + when(unsupported.isSimple()).thenReturn(true); + when(unsupported.getName()).thenReturn("unsupported"); + + SolrFieldsChecker checker = this.checker(List.of(unsupported), lang("en")); + + List names = fieldNames(checker); + assertFalse(names.stream().anyMatch(n -> n.contains("unsupported")), names.toString()); + } + + @Test + void shouldSkipNonComplexAttributeChildrenEvenWhenNotSimple() { + // Defensive branch of checkComplexAttributeChildren: an attribute that reports + // isSimple()==false but is not an AbstractComplexAttribute (unlike every real + // Composite/List attribute) must be skipped rather than throw a ClassCastException. + AttributeInterface fakeComplexAttribute = mock(AttributeInterface.class); + when(fakeComplexAttribute.isSimple()).thenReturn(false); + when(fakeComplexAttribute.getName()).thenReturn("fake"); + + SolrFieldsChecker checker = this.checker(List.of(fakeComplexAttribute), lang("en")); + + List names = fieldNames(checker); + assertFalse(names.stream().anyMatch(n -> n.contains("fake")), names.toString()); + } + + // ---- composite children: qualified path, single-valued, gated by the inherited searchable flag ---- + // Single-valued because a Composite occurs at most once per document per lang: Monolist and + // Monolist-of-Composite ancestry are excluded below, so the field can never repeat. + + @Test + void shouldCreateSingleValuedQualifiedFieldForSearchableCompositeChild() { + TestComposite composite = composite("myComposite", booleanAttribute("featured", true)); + SolrFieldsChecker checker = this.checker(List.of(composite), lang("en")); + + Map field = fieldsToAdd(checker, "en_myComposite_featured"); + Assertions.assertEquals(SolrFields.TYPE_BOOLEAN, field.get(SolrFields.SOLR_FIELD_TYPE)); + Assertions.assertEquals(false, field.get(SolrFields.SOLR_FIELD_MULTIVALUED)); + } + + @Test + void shouldNotCreateFieldForNonSearchableCompositeChild() { + TestComposite composite = composite("myComposite", booleanAttribute("featured", false)); + SolrFieldsChecker checker = this.checker(List.of(composite), lang("en")); + + assertFalse(fieldNames(checker).contains("en_myComposite_featured"), fieldNames(checker).toString()); + } + + @Test + void shouldCreateQualifiedFieldForCompositeChildCheckBoxAttribute() { + TestComposite composite = composite("myComposite", checkBoxAttribute("featuredCheck", true)); + SolrFieldsChecker checker = this.checker(List.of(composite), lang("en")); + + Map field = fieldsToAdd(checker, "en_myComposite_featuredCheck"); + Assertions.assertEquals(SolrFields.TYPE_BOOLEAN, field.get(SolrFields.SOLR_FIELD_TYPE)); + Assertions.assertEquals(false, field.get(SolrFields.SOLR_FIELD_MULTIVALUED)); + } + + @Test + void shouldCreateStringFieldForCompositeChildThreeStateAttribute() { + TestComposite composite = composite("myComposite", threeStateAttribute("featured3", true)); + SolrFieldsChecker checker = this.checker(List.of(composite), lang("en")); + + Map field = fieldsToAdd(checker, "en_myComposite_featured3"); + Assertions.assertEquals(SolrFields.TYPE_STRING, field.get(SolrFields.SOLR_FIELD_TYPE)); + Assertions.assertEquals(false, field.get(SolrFields.SOLR_FIELD_MULTIVALUED)); + } + + @Test + void shouldCreateFullPathFieldForBooleanInNestedComposite() { + TestComposite inner = composite("inner", booleanAttribute("boolAttrName", true)); + TestComposite outer = composite("outer", inner); + SolrFieldsChecker checker = this.checker(List.of(outer), lang("en")); + + Map field = fieldsToAdd(checker, "en_outer_inner_boolAttrName"); + Assertions.assertEquals(SolrFields.TYPE_BOOLEAN, field.get(SolrFields.SOLR_FIELD_TYPE)); + Assertions.assertEquals(false, field.get(SolrFields.SOLR_FIELD_MULTIVALUED)); + } + + // ---- List / Monolist: excluded, no boolean field regardless of the flag ---- + + @Test + void shouldNotCreateFieldForBooleanElementOfMonolist() { + BooleanAttribute nestedType = booleanAttribute("myList", true); + MonoListAttribute list = new MonoListAttribute(); + list.setName("myList"); + list.setType("Monolist"); + list.setNestedAttributeType(nestedType); + + SolrFieldsChecker checker = this.checker(List.of(list), lang("en")); + + assertFalse(fieldNames(checker).contains("en_myList"), fieldNames(checker).toString()); + } + + @Test + void shouldNotCreateFieldForBooleanInsideCompositeNestedInMonolist() { + TestComposite compositePrototype = composite("comboList", booleanAttribute("subFlag", true)); + MonoListAttribute listOfComposite = new MonoListAttribute(); + listOfComposite.setName("comboList"); + listOfComposite.setType("Monolist"); + listOfComposite.setNestedAttributeType(compositePrototype); + + SolrFieldsChecker checker = this.checker(List.of(listOfComposite), lang("en")); + + List names = fieldNames(checker); + assertFalse(names.contains("en_comboList_subFlag"), names.toString()); + assertFalse(names.contains("en_subFlag"), names.toString()); + } + + private List fieldNames(SolrFieldsChecker checker) { + return checker.checkFields().getFieldsToAdd().stream() + .map(field -> (String) field.get(SolrFields.SOLR_FIELD_NAME)) + .toList(); + } + + private Map fieldsToAdd(SolrFieldsChecker checker, String fieldName) { + return checker.checkFields().getFieldsToAdd().stream() + .filter(f -> fieldName.equals(f.get(SolrFields.SOLR_FIELD_NAME))) + .findFirst() + .orElseThrow(() -> new AssertionError("Field not created: " + fieldName)); + } } diff --git a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrSearchEngineManagerTest.java b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrSearchEngineManagerTest.java index 6b4d38d9d7..67edfce737 100644 --- a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrSearchEngineManagerTest.java +++ b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrSearchEngineManagerTest.java @@ -5,18 +5,36 @@ import static com.agiletec.plugins.jacms.aps.system.services.searchengine.ICmsSearchEngineManager.STATUS_RELOADING_INDEXES_IN_PROGRESS; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockConstruction; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import com.agiletec.aps.system.common.entity.model.SmallEntityType; +import com.agiletec.aps.system.common.entity.model.attribute.AttributeInterface; +import com.agiletec.aps.system.common.entity.model.attribute.BooleanAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.CompositeAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.MonoListAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.TextAttribute; import com.agiletec.aps.system.services.category.ICategoryManager; import com.agiletec.aps.system.services.lang.ILangManager; +import com.agiletec.aps.system.services.lang.Lang; import com.agiletec.plugins.jacms.aps.system.services.content.IContentManager; +import com.agiletec.plugins.jacms.aps.system.services.content.model.Content; import java.util.List; import org.apache.http.impl.client.HttpClientBuilder; import org.apache.solr.client.solrj.impl.HttpSolrClient; import org.apache.solr.client.solrj.request.schema.SchemaRequest; import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.SimpleOrderedMap; import org.entando.entando.aps.system.services.cache.ICacheInfoManager; import org.entando.entando.aps.system.services.tenants.ITenantManager; import org.entando.entando.ent.exception.EntException; +import org.entando.entando.plugins.jpsolr.aps.system.solr.model.ContentTypeSettings; +import org.entando.entando.plugins.jpsolr.aps.system.solr.model.ContentTypeSettings.AttributeSettings; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; @@ -24,7 +42,6 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.MockedConstruction; -import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; @ExtendWith(MockitoExtension.class) @@ -50,12 +67,16 @@ class SolrSearchEngineManagerTest { private HttpSolrClient solrClient; @BeforeEach + // NOTE: java:S9024 ("use @InjectMocks") intentionally suppressed - false positive here: the object + // under test is assembled from a mockConstruction of HttpSolrClient.Builder plus explicit setter + // wiring (afterPropertiesSet), which @InjectMocks cannot express. + @SuppressWarnings("java:S9024") void setUp() throws Exception { - mockedConstructionSolrClientBuilder = Mockito.mockConstruction(HttpSolrClient.Builder.class, + mockedConstructionSolrClientBuilder = mockConstruction(HttpSolrClient.Builder.class, (builder, context) -> { - solrClient = Mockito.mock(HttpSolrClient.class); - Mockito.when(builder.withHttpClient(any())).thenReturn(builder); - Mockito.when(builder.build()).thenReturn(solrClient); + solrClient = mock(HttpSolrClient.class); + when(builder.withHttpClient(any())).thenReturn(builder); + when(builder.build()).thenReturn(solrClient); }); solrProxy = new SolrProxyTenantAware( langManager, categoryManager, tenantManager, solrHttpClientBuilder @@ -76,8 +97,8 @@ void tearDown() { @Test void shouldRollbackIndexStatusIfReloadThreadDoesNotStart() throws Exception { solrProxy.getIndexStatus().setValue(STATUS_NEED_TO_RELOAD_INDEXES); - try (MockedConstruction c = Mockito.mockConstruction(SolrIndexLoaderThread.class, - (thread, context) -> Mockito.doThrow(RuntimeException.class).when(thread).start())) { + try (MockedConstruction c = mockConstruction(SolrIndexLoaderThread.class, + (thread, context) -> doThrow(RuntimeException.class).when(thread).start())) { Assertions.assertThrows(EntException.class, () -> solrSearchEngineManager.startReloadContentsReferences()); } Assertions.assertEquals(STATUS_NEED_TO_RELOAD_INDEXES, solrProxy.getIndexStatus().getValue()); @@ -86,8 +107,8 @@ void shouldRollbackIndexStatusIfReloadThreadDoesNotStart() throws Exception { @Test void shouldSetStatusInProgressIfReloadThreadIsStartedStart() throws Exception { solrProxy.getIndexStatus().setValue(STATUS_READY); - try (MockedConstruction c = Mockito.mockConstruction(SolrIndexLoaderThread.class, - (thread, context) -> Mockito.doNothing().when(thread).start())) { + try (MockedConstruction c = mockConstruction(SolrIndexLoaderThread.class, + (thread, context) -> doNothing().when(thread).start())) { Assertions.assertNotNull(solrSearchEngineManager.startReloadContentsReferences()); } Assertions.assertEquals(STATUS_RELOADING_INDEXES_IN_PROGRESS, solrProxy.getIndexStatus().getValue()); @@ -103,8 +124,8 @@ void shouldSkipReloadThreadIfReloadIsAlreadyInProgress() throws Exception { @Test void shouldReloadByType() throws Exception { solrProxy.getIndexStatus().setValue(STATUS_READY); - try (MockedConstruction c = Mockito.mockConstruction(SolrIndexLoaderThread.class, - (thread, context) -> Mockito.doNothing().when(thread).start())) { + try (MockedConstruction c = mockConstruction(SolrIndexLoaderThread.class, + (thread, context) -> doNothing().when(thread).start())) { solrSearchEngineManager.startReloadContentsReferencesByType("ART"); } Assertions.assertEquals(STATUS_RELOADING_INDEXES_IN_PROGRESS, solrProxy.getIndexStatus().getValue()); @@ -115,13 +136,147 @@ void shouldRefreshCmsFields() throws Exception { solrProxy.getIndexStatus().setValue(STATUS_READY); NamedList solrClientResponse = new NamedList<>(); solrClientResponse.add("fields", List.of()); - Mockito.when(solrClient.request(Mockito.any(SchemaRequest.Fields.class), eq("entando"))) + when(solrClient.request(any(SchemaRequest.Fields.class), eq("entando"))) .thenReturn(solrClientResponse); - try (MockedConstruction c = Mockito.mockConstruction(SolrIndexLoaderThread.class, - (thread, context) -> Mockito.doNothing().when(thread).start())) { + try (MockedConstruction c = mockConstruction(SolrIndexLoaderThread.class, + (thread, context) -> doNothing().when(thread).start())) { solrSearchEngineManager.refreshCmsFields(); } - Mockito.verify(solrClient, Mockito.times(1)) - .request(Mockito.any(SchemaRequest.MultiUpdate.class), eq("entando")); + verify(solrClient, times(1)) + .request(any(SchemaRequest.MultiUpdate.class), eq("entando")); + } + + /** Exposes the protected {@code addAttribute} so tests can build composite fixtures. */ + private static class TestComposite extends CompositeAttribute { + + void addChild(AttributeInterface attribute) { + this.addAttribute(attribute); + } + } + + private Lang lang(String code) { + Lang lang = new Lang(); + lang.setCode(code); + lang.setDescr(code); + return lang; + } + + @Test + void shouldBuildContentTypesSettingsWithNestedBooleanAttributes() throws Exception { + // getContentTypesSettings()/collectNestedBooleanAttributes() had no dedicated unit test at + // all: this closes buildCurrentFieldConfig's "existing field found" branch, and + // collectNestedBooleanAttributes' simple/complex child dispatch, boolean/non-boolean leaf + // dispatch, recursion into a nested Composite, and the top-level Monolist exclusion. + when(langManager.getLangs()).thenReturn(List.of(lang("en"))); + + SimpleOrderedMap existingField = new SimpleOrderedMap<>(); + existingField.add("name", "en_myComposite_featured"); + existingField.add("type", "boolean"); + existingField.add("multiValued", false); + NamedList solrClientResponse = new NamedList<>(); + solrClientResponse.add("fields", List.of(existingField)); + when(solrClient.request(any(SchemaRequest.Fields.class), eq("entando"))) + .thenReturn(solrClientResponse); + + when(contentManager.getSmallEntityTypes()) + .thenReturn(List.of(new SmallEntityType("TST", "Test type"))); + + BooleanAttribute featured = new BooleanAttribute(); + featured.setName("featured"); + featured.setType("Boolean"); + featured.setSearchable(true); + + TextAttribute note = new TextAttribute(); + note.setName("note"); + note.setType("Text"); + + BooleanAttribute innerFlag = new BooleanAttribute(); + innerFlag.setName("innerFlag"); + innerFlag.setType("Boolean"); + innerFlag.setSearchable(true); + TestComposite inner = new TestComposite(); + inner.setName("inner"); + inner.setType("Composite"); + inner.addChild(innerFlag); + + TestComposite composite = new TestComposite(); + composite.setName("myComposite"); + composite.setType("Composite"); + composite.addChild(featured); + composite.addChild(note); + composite.addChild(inner); + + BooleanAttribute listItemType = new BooleanAttribute(); + listItemType.setName("myListItem"); + listItemType.setType("Boolean"); + listItemType.setSearchable(true); + MonoListAttribute list = new MonoListAttribute(); + list.setName("myList"); + list.setType("Monolist"); + list.setNestedAttributeType(listItemType); + + // A plain top-level simple attribute (isSimple()==true) must skip nested-boolean collection + // entirely rather than attempt it: covers the "false" outcome of "!attribute.isSimple()". + TextAttribute title = new TextAttribute(); + title.setName("title"); + title.setType("Text"); + + Content prototype = new Content(); + prototype.setTypeCode("TST"); + prototype.addAttribute(title); + prototype.addAttribute(composite); + prototype.addAttribute(list); + when(contentManager.createContentType("TST")).thenReturn(prototype); + + List settings = solrSearchEngineManager.getContentTypesSettings(); + + Assertions.assertEquals(1, settings.size()); + List attributeSettings = settings.get(0).getAttributeSettings(); + List codes = attributeSettings.stream().map(AttributeSettings::getCode).toList(); + // The nested boolean "featured" (found in the schema fields) and "inner.innerFlag" (recursion, + // missing from the schema fields) are both registered; the plain-text "note" child and the + // boolean nested inside the top-level Monolist are not (List/Monolist ancestry is excluded). + Assertions.assertTrue(codes.contains("featured"), codes.toString()); + Assertions.assertTrue(codes.contains("innerFlag"), codes.toString()); + Assertions.assertFalse(codes.contains("myListItem"), codes.toString()); + Assertions.assertFalse(codes.contains("note"), codes.toString()); + + AttributeSettings featuredSettings = attributeSettings.stream() + .filter(s -> "featured".equals(s.getCode())).findFirst().orElseThrow(); + Assertions.assertTrue(featuredSettings.isValid()); + + AttributeSettings innerFlagSettings = attributeSettings.stream() + .filter(s -> "innerFlag".equals(s.getCode())).findFirst().orElseThrow(); + Assertions.assertFalse(innerFlagSettings.isValid(), "no schema field exists yet for the nested innerFlag"); + } + + @Test + void shouldSkipNonComplexAttributeInterfaceInstanceEvenWhenNotSimple() throws Exception { + // Defensive branch of collectNestedBooleanAttributes: an attribute that reports + // isSimple()==false but is not an AbstractComplexAttribute (unlike every real + // Composite/List attribute) must be skipped rather than throw a ClassCastException. + when(langManager.getLangs()).thenReturn(List.of(lang("en"))); + NamedList solrClientResponse = new NamedList<>(); + solrClientResponse.add("fields", List.of()); + when(solrClient.request(any(SchemaRequest.Fields.class), eq("entando"))) + .thenReturn(solrClientResponse); + + when(contentManager.getSmallEntityTypes()) + .thenReturn(List.of(new SmallEntityType("TST", "Test type"))); + + AttributeInterface fakeComplexAttribute = mock(AttributeInterface.class); + when(fakeComplexAttribute.isSimple()).thenReturn(false); + when(fakeComplexAttribute.getName()).thenReturn("fake"); + when(fakeComplexAttribute.getType()).thenReturn("Fake"); + + Content prototype = new Content(); + prototype.setTypeCode("TST"); + prototype.addAttribute(fakeComplexAttribute); + when(contentManager.createContentType("TST")).thenReturn(prototype); + + List settings = solrSearchEngineManager.getContentTypesSettings(); + + Assertions.assertEquals(1, settings.size()); + Assertions.assertEquals(1, settings.get(0).getAttributeSettings().size()); } } diff --git a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/model/ContentTypeSettingsTest.java b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/model/ContentTypeSettingsTest.java index 3857aa6295..c4f66234a1 100644 --- a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/model/ContentTypeSettingsTest.java +++ b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/model/ContentTypeSettingsTest.java @@ -1,6 +1,10 @@ package org.entando.entando.plugins.jpsolr.aps.system.solr.model; +import com.agiletec.aps.system.common.entity.model.attribute.AttributeInterface; +import com.agiletec.aps.system.common.entity.model.attribute.BooleanAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.CheckBoxAttribute; import com.agiletec.aps.system.common.entity.model.attribute.TextAttribute; +import com.agiletec.aps.system.common.entity.model.attribute.ThreeStateAttribute; import com.agiletec.aps.system.services.lang.Lang; import java.io.Serializable; import java.util.Arrays; @@ -10,6 +14,8 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import org.mockito.junit.jupiter.MockitoExtension; @ExtendWith(MockitoExtension.class) @@ -75,6 +81,227 @@ void shouldDetectValidLangField() { Assertions.assertTrue(contentTypeSettings.isValid()); } + @Test + void shouldExpectBooleanTypeForSearchableBooleanAttribute() { + + ContentTypeSettings contentTypeSettings = new ContentTypeSettings("NWS", "News"); + + BooleanAttribute flagAttribute = new BooleanAttribute(); + flagAttribute.setName("flag"); + flagAttribute.setType("Boolean"); + flagAttribute.setSearchable(true); + + Map> currentField = Map.of("en_flag", Map.of( + "name", "en_flag", + "type", "boolean", + "multiValued", false + )); + + contentTypeSettings.addAttribute(flagAttribute, currentField, getLanguages("en")); + + Assertions.assertTrue(contentTypeSettings.isValid()); + } + + @Test + void shouldDetectFieldTypeMismatchForBooleanAttribute() { + + ContentTypeSettings contentTypeSettings = new ContentTypeSettings("NWS", "News"); + + BooleanAttribute flagAttribute = new BooleanAttribute(); + flagAttribute.setName("flag"); + flagAttribute.setType("Boolean"); + flagAttribute.setSearchable(true); + + Map> currentField = Map.of("en_flag", Map.of( + "name", "en_flag", + "type", "text_gen_sort", + "multiValued", false + )); + + contentTypeSettings.addAttribute(flagAttribute, currentField, getLanguages("en")); + + Assertions.assertFalse(contentTypeSettings.isValid()); + } + + @Test + void shouldNotExpectAnyFieldForNonSearchableBooleanAttribute() { + + ContentTypeSettings contentTypeSettings = new ContentTypeSettings("NWS", "News"); + + BooleanAttribute flagAttribute = new BooleanAttribute(); + flagAttribute.setName("flag"); + flagAttribute.setType("Boolean"); + flagAttribute.setSearchable(false); + + contentTypeSettings.addAttribute(flagAttribute, Map.of(), getLanguages("en")); + + Assertions.assertTrue(contentTypeSettings.isValid()); + } + + @Test + void shouldExpectBooleanTypeForSearchableCheckBoxAttribute() { + + ContentTypeSettings contentTypeSettings = new ContentTypeSettings("NWS", "News"); + + CheckBoxAttribute checkAttribute = new CheckBoxAttribute(); + checkAttribute.setName("check"); + checkAttribute.setType("CheckBox"); + checkAttribute.setSearchable(true); + + Map> currentField = Map.of("en_check", Map.of( + "name", "en_check", + "type", "boolean", + "multiValued", false + )); + + contentTypeSettings.addAttribute(checkAttribute, currentField, getLanguages("en")); + + Assertions.assertTrue(contentTypeSettings.isValid()); + } + + @Test + void shouldExpectStringTypeForSearchableThreeStateAttribute() { + // ThreeState's third, uninitialized state cannot be represented in a two-valued Solr + // BoolField, so it is expected as "string" (true|false|none), not "boolean". + + ContentTypeSettings contentTypeSettings = new ContentTypeSettings("NWS", "News"); + + ThreeStateAttribute flag3Attribute = new ThreeStateAttribute(); + flag3Attribute.setName("flag3"); + flag3Attribute.setType("ThreeState"); + flag3Attribute.setSearchable(true); + + Map> currentField = Map.of("en_flag3", Map.of( + "name", "en_flag3", + "type", "string", + "multiValued", false + )); + + contentTypeSettings.addAttribute(flag3Attribute, currentField, getLanguages("en")); + + Assertions.assertTrue(contentTypeSettings.isValid()); + } + + @Test + void shouldNotExpectFieldForUnsupportedAttributeType() { + // Closes the final "instanceof BooleanAttribute" false outcome of addAttribute's dispatch + // condition: an attribute that is neither IndexableAttributeInterface, Date, Number nor + // Boolean must not get an expected field configuration (isValid() trivially true). + + ContentTypeSettings contentTypeSettings = new ContentTypeSettings("NWS", "News"); + + AttributeInterface unsupported = mock(AttributeInterface.class); + when(unsupported.getName()).thenReturn("unsupported"); + when(unsupported.getType()).thenReturn("Unsupported"); + + contentTypeSettings.addAttribute(unsupported, Map.of(), getLanguages("en")); + + Assertions.assertTrue(contentTypeSettings.isValid()); + } + + @Test + void shouldExpectSingleValuedBooleanTypeForNestedCheckBoxAttribute() { + // Single-valued: a Composite occurs at most once per document per lang (the caller + // excludes List/Monolist ancestry), so the nested field can never repeat. + + ContentTypeSettings contentTypeSettings = new ContentTypeSettings("NWS", "News"); + + CheckBoxAttribute nestedCheck = new CheckBoxAttribute(); + nestedCheck.setName("featuredCheck"); + nestedCheck.setType("CheckBox"); + nestedCheck.setSearchable(true); + + Map> currentField = Map.of("en_featuredCheck", Map.of( + "name", "en_featuredCheck", + "type", "boolean", + "multiValued", false + )); + + contentTypeSettings.addNestedBooleanAttribute(nestedCheck, currentField, getLanguages("en")); + + Assertions.assertTrue(contentTypeSettings.isValid()); + } + + @Test + void shouldExpectSingleValuedStringTypeForNestedThreeStateAttribute() { + + ContentTypeSettings contentTypeSettings = new ContentTypeSettings("NWS", "News"); + + ThreeStateAttribute nestedFlag3 = new ThreeStateAttribute(); + nestedFlag3.setName("featured3"); + nestedFlag3.setType("ThreeState"); + nestedFlag3.setSearchable(true); + + Map> currentField = Map.of("en_featured3", Map.of( + "name", "en_featured3", + "type", "string", + "multiValued", false + )); + + contentTypeSettings.addNestedBooleanAttribute(nestedFlag3, currentField, getLanguages("en")); + + Assertions.assertTrue(contentTypeSettings.isValid()); + } + + @Test + void shouldExpectSingleValuedBooleanTypeForNestedBooleanAttribute() { + + ContentTypeSettings contentTypeSettings = new ContentTypeSettings("NWS", "News"); + + BooleanAttribute nestedFlag = new BooleanAttribute(); + nestedFlag.setName("featured"); + nestedFlag.setType("Boolean"); + nestedFlag.setSearchable(true); + + Map> currentField = Map.of("en_featured", Map.of( + "name", "en_featured", + "type", "boolean", + "multiValued", false + )); + + contentTypeSettings.addNestedBooleanAttribute(nestedFlag, currentField, getLanguages("en")); + + Assertions.assertTrue(contentTypeSettings.isValid()); + } + + @Test + void shouldDetectMissingNestedBooleanField() { + + ContentTypeSettings contentTypeSettings = new ContentTypeSettings("NWS", "News"); + + BooleanAttribute nestedFlag = new BooleanAttribute(); + nestedFlag.setName("featured"); + nestedFlag.setType("Boolean"); + nestedFlag.setSearchable(true); + + contentTypeSettings.addNestedBooleanAttribute(nestedFlag, Map.of(), getLanguages("en")); + + Assertions.assertFalse(contentTypeSettings.isValid()); + } + + @Test + void shouldDetectMultiValuedNestedBooleanFieldAsInvalid() { + + ContentTypeSettings contentTypeSettings = new ContentTypeSettings("NWS", "News"); + + BooleanAttribute nestedFlag = new BooleanAttribute(); + nestedFlag.setName("featured"); + nestedFlag.setType("Boolean"); + nestedFlag.setSearchable(true); + + // multiValued=true on a nested boolean field is stale (nested fields are always + // single-valued); the schema must be refreshed to a single-valued field. + Map> currentField = Map.of("en_featured", Map.of( + "name", "en_featured", + "type", "boolean", + "multiValued", true + )); + + contentTypeSettings.addNestedBooleanAttribute(nestedFlag, currentField, getLanguages("en")); + + Assertions.assertFalse(contentTypeSettings.isValid()); + } + private List getLanguages(String... codes) { return Arrays.stream(codes).map(c -> { Lang lang = new Lang(); diff --git a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/web/content/AdvContentSearchControllerTest.java b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/web/content/AdvContentSearchControllerTest.java index 9f76ba5587..f2f71731d6 100644 --- a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/web/content/AdvContentSearchControllerTest.java +++ b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/web/content/AdvContentSearchControllerTest.java @@ -92,7 +92,7 @@ void testGetContents() throws Exception { String bodyResult = result.andReturn().getResponse().getContentAsString(); result.andExpect(status().isOk()); int totalPayloadSize = JsonPath.read(bodyResult, "$.payload.size()"); - Assertions.assertEquals(24, totalPayloadSize); + Assertions.assertEquals(28, totalPayloadSize); result = mockMvc .perform(get("/plugins/advcontentsearch/contents") @@ -346,7 +346,7 @@ void testGetContentsByGuestUser_2() throws Exception { result.andExpect(status().isOk()); System.out.println(bodyResult); int payloadSize = JsonPath.read(bodyResult, "$.payload.size()"); - Assertions.assertEquals(15, payloadSize); + Assertions.assertEquals(19, payloadSize); ResultActions evnResult = mockMvc .perform(get("/plugins/advcontentsearch/contents")