Repository: jakartaee/data
Pull request #966 (2025-02-07 19:02:14)

address review comment to format Restriction toString more like JDQL
Address this review comment from #962

              > `ALL (price GREATER_THAN_EQUAL 50, price LESS_THAN 100)`

Why not align the `toString()`s with the equivalent JDQL, so:

```hql
(price >= 50 AND price < 100)
```

_Originally posted by @gavinking in https://github.com/jakartaee/data/issues/962#issuecomment-2640902362_


njr-11: > Just a suggestion: in modern Java I would use a `switch` expression instead of adding a constructor to the enum. I find that to be cleaner.

I had it that way at first but switched to the constructor to be more performant - to return a fixed known value instead of re-computing it every time.  But I suppose this is only for a toString operation that is most likely to be used when dumping information for debug and performance doesn't matter at all then.  I can change it if you like.
gavinking: Switching on an enum should be very very fast, I believe. 
njr-11: > Switching on an enum should be very very fast, I believe.

It's updated now with your suggestions, plus I added a commit to apply the same pattern to the other enum where I had added constructor parameters.
gavinking: Awesome, thanks 🙏 
otaviojava: @njr-11 could you review this PR?
gavinking: @otaviojava I fixed the conflict.

Patch:
diff --git a/api/src/main/java/jakarta/data/metamodel/restrict/BasicRestrictionRecord.java b/api/src/main/java/jakarta/data/metamodel/restrict/BasicRestrictionRecord.java
index 59ed4eda..cab38a0b 100644
--- a/api/src/main/java/jakarta/data/metamodel/restrict/BasicRestrictionRecord.java
+++ b/api/src/main/java/jakarta/data/metamodel/restrict/BasicRestrictionRecord.java
@@ -44,20 +44,21 @@ record BasicRestrictionRecord<T>(
     /**
      * Textual representation of a basic restriction.
      * For example,
-     * <pre>price LESS_THAN 50.0</pre>
+     * <pre>price < 50.0</pre>
      *
      * @return textual representation of a basic restriction.
      */
     @Override
     public String toString() {
+        String comparisonString = comparison.asQueryLanguage();
         String valueString = value.toString();
         StringBuilder builder = new StringBuilder(
                 attribute.length() +
-                comparison.name().length() +
+                comparisonString.length() +
                 valueString.length() +
                 4); // number of additional characters that might be appended
         builder.append(attribute).append(' ')
-         .append(comparison.name()).append(' ');
+               .append(comparisonString).append(' ');
         if (value instanceof CharSequence) {
             builder.append('"').append(valueString).append('"');
         } else {
diff --git a/api/src/main/java/jakarta/data/metamodel/restrict/CompositeRestriction.java b/api/src/main/java/jakarta/data/metamodel/restrict/CompositeRestriction.java
index df9f5abb..238a4f78 100644
--- a/api/src/main/java/jakarta/data/metamodel/restrict/CompositeRestriction.java
+++ b/api/src/main/java/jakarta/data/metamodel/restrict/CompositeRestriction.java
@@ -31,6 +31,20 @@ public interface CompositeRestriction<T> extends Restriction<T> {
 
     enum Type {
         ALL,
-        ANY
+        ANY;
+
+        /**
+         * Representation of the composite restriction type as a query language
+         * logical operator.
+         * For example, {@link #ALL} is represented as {@code AND} in query language.
+         *
+         * @return the representation as a logical operator in query language.
+         */
+        String asQueryLanguage() {
+            return switch (this) {
+                case ALL -> "AND";
+                case ANY -> "OR";
+            };
+        }
     }
 }
diff --git a/api/src/main/java/jakarta/data/metamodel/restrict/CompositeRestrictionRecord.java b/api/src/main/java/jakarta/data/metamodel/restrict/CompositeRestrictionRecord.java
index 9d19d986..515fd112 100644
--- a/api/src/main/java/jakarta/data/metamodel/restrict/CompositeRestrictionRecord.java
+++ b/api/src/main/java/jakarta/data/metamodel/restrict/CompositeRestrictionRecord.java
@@ -52,29 +52,33 @@ record CompositeRestrictionRecord<T>(
     /**
      * Textual representation of a composite restriction.
      * For example,
-     * <pre>ALL (price LESS_THAN 50.0, name LIKE "%Jakarta EE%")</pre>
+     * <pre>(price < 50.0) AND (name LIKE "%Jakarta EE%")</pre>
      *
      * @return textual representation of a composite restriction.
      */
     @Override
     public String toString() {
+        String logicalOperator = type.asQueryLanguage();
         StringBuilder builder = new StringBuilder(
                 restrictions.size() * SINGLE_RESTRICTION_LENGTH_ESTIMATE +
-                7); // number of additional characters that might be appended
+                6); // number of additional characters that might be appended
         if (isNegated) {
-            builder.append("NOT ");
+            builder.append("NOT (");
         }
-        builder.append(type.name()).append(" (");
+
         boolean first = true;
         for (Restriction<T> restriction : restrictions) {
             if (first) {
                 first = false;
             } else {
-                builder.append(", ");
+                builder.append(' ').append(logicalOperator).append(' ');
             }
-            builder.append(restriction);
+            builder.append('(').append(restriction).append(')');
+        }
+
+        if (isNegated) {
+            builder.append(')');
         }
-        builder.append(')');
         return builder.toString();
     }
 }
\ No newline at end of file
diff --git a/api/src/main/java/jakarta/data/metamodel/restrict/Operator.java b/api/src/main/java/jakarta/data/metamodel/restrict/Operator.java
index 8662bd56..8cb178c1 100644
--- a/api/src/main/java/jakarta/data/metamodel/restrict/Operator.java
+++ b/api/src/main/java/jakarta/data/metamodel/restrict/Operator.java
@@ -29,6 +29,28 @@ public enum Operator {
     NOT_IN,
     NOT_LIKE;
 
+    /**
+     * Representation of the operator as it appears in query language.
+     * For example, {@link #GREATER_THAN} is represented as {@code >}
+     * in query langugae.
+     *
+     * @return the representation of the operator in query language.
+     */
+    String asQueryLanguage() {
+        return switch (this) {
+            case EQUAL -> "=";
+            case GREATER_THAN -> ">";
+            case GREATER_THAN_EQUAL -> ">=";
+            case IN -> "IN";
+            case LESS_THAN -> "<";
+            case LESS_THAN_EQUAL -> "<=";
+            case LIKE -> "LIKE";
+            case NOT_EQUAL -> "<>";
+            case NOT_IN -> "NOT IN";
+            case NOT_LIKE -> "NOT LIKE";
+        };
+    }
+
     /**
      * Returns the operator that is the negation of this operator.
      *
diff --git a/api/src/main/java/jakarta/data/metamodel/restrict/TextRestrictionRecord.java b/api/src/main/java/jakarta/data/metamodel/restrict/TextRestrictionRecord.java
index 9c219fd1..4e3ea398 100644
--- a/api/src/main/java/jakarta/data/metamodel/restrict/TextRestrictionRecord.java
+++ b/api/src/main/java/jakarta/data/metamodel/restrict/TextRestrictionRecord.java
@@ -61,24 +61,21 @@ record TextRestrictionRecord<T>(
     /**
      * Textual representation of a text restriction.
      * For example,
-     * <pre>name LIKE_IGNORE_CASE "Jakarta EE %"</pre>
+     * <pre>name LIKE "Jakarta EE %" IGNORE_CASE</pre>
      *
      * @return textual representation of a text restriction.
      */
     @Override
     public String toString() {
+        String comparisonString = comparison.asQueryLanguage();
         String valueString = value == null ? "null" : value;
         StringBuilder builder = new StringBuilder(
                 attribute.length() +
-                comparison.name().length() +
+                comparisonString.length() +
                 valueString.length() +
                 24); // number of additional characters that might be appended
         builder.append(attribute).append(' ')
-               .append(comparison.name());
-        if (!isCaseSensitive) {
-            builder.append("_IGNORE_CASE");
-        }
-        builder.append(' ');
+               .append(comparisonString).append(' ');
         if (value == null) {
             builder.append(valueString);
         } else {
@@ -87,6 +84,9 @@ record TextRestrictionRecord<T>(
         if (isEscaped) {
             builder.append(" ESCAPED");
         }
+        if (!isCaseSensitive) {
+            builder.append(" IGNORE_CASE");
+        }
         return builder.toString();
     }
 }
diff --git a/api/src/main/resources/rules.xml b/api/src/main/resources/rules.xml
index c044db08..389cac37 100644
--- a/api/src/main/resources/rules.xml
+++ b/api/src/main/resources/rules.xml
@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <!--
-  ~ Copyright (c) 2024 Contributors to the Eclipse Foundation
+  ~ Copyright (c) 2024,2025 Contributors to the Eclipse Foundation
   ~
   ~ Licensed under the Apache License, Version 2.0 (the "License");
   ~ you may not use this file except in compliance with the License.
@@ -16,7 +16,7 @@
   ~
   ~ SPDX-License-Identifier: Apache-2.0
   -->
-<ruleset name="Jakrata Data"
+<ruleset name="Jakarta Data"
     xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 https://pmd.sourceforge.io/ruleset_2_0_0.xsd">
@@ -27,6 +27,12 @@
         <exclude name="AbstractClassWithoutAbstractMethod"/>
     </rule>
     <rule ref="category/java/security.xml"/>
-    <rule ref="category/java/performance.xml"/>
+    <rule ref="category/java/performance.xml">
+        <!-- Sure, a switch with only 2 options might be inefficient, but that
+             isn't the point. I want switch instead of if/else to enforce that if
+             someone ever adds another enum constant, the switch will cause a
+             compile error, forcing them to implement the needed handling of it. -->
+        <exclude name="TooFewBranchesForSwitch"/>
+    </rule>
 
 </ruleset>
\ No newline at end of file


Test patch:
diff --git a/api/src/test/java/jakarta/data/metamodel/restrict/BasicRestrictionRecordTest.java b/api/src/test/java/jakarta/data/metamodel/restrict/BasicRestrictionRecordTest.java
index dac4734c..69f168f1 100644
--- a/api/src/test/java/jakarta/data/metamodel/restrict/BasicRestrictionRecordTest.java
+++ b/api/src/test/java/jakarta/data/metamodel/restrict/BasicRestrictionRecordTest.java
@@ -102,7 +102,7 @@ class BasicRestrictionRecordTest {
 
         SoftAssertions.assertSoftly(soft -> {
             soft.assertThat(restriction.toString())
-                .isEqualTo("numPages GREATER_THAN 100");
+                .isEqualTo("numPages > 100");
         });
     }
 
diff --git a/api/src/test/java/jakarta/data/metamodel/restrict/CompositeRestrictionRecordTest.java b/api/src/test/java/jakarta/data/metamodel/restrict/CompositeRestrictionRecordTest.java
index c8547414..103f7f05 100644
--- a/api/src/test/java/jakarta/data/metamodel/restrict/CompositeRestrictionRecordTest.java
+++ b/api/src/test/java/jakarta/data/metamodel/restrict/CompositeRestrictionRecordTest.java
@@ -126,10 +126,10 @@ class CompositeRestrictionRecordTest {
 
         SoftAssertions.assertSoftly(soft -> {
             soft.assertThat(namedJackKarta.toString()).isEqualTo("""
-                    ALL (firstName EQUAL "Jack", lastName EQUAL "Karta")\
+                    (firstName = "Jack") AND (lastName = "Karta")\
                     """);
             soft.assertThat(notMinorOrMissingName.toString()).isEqualTo("""
-                    NOT ANY (age LESS_THAN 18, name EQUAL null)\
+                    NOT ((age < 18) OR (name = null))\
                     """);
         });
     }
diff --git a/api/src/test/java/jakarta/data/metamodel/restrict/TextRestrictionRecordTest.java b/api/src/test/java/jakarta/data/metamodel/restrict/TextRestrictionRecordTest.java
index 8895efa6..020a49c6 100644
--- a/api/src/test/java/jakarta/data/metamodel/restrict/TextRestrictionRecordTest.java
+++ b/api/src/test/java/jakarta/data/metamodel/restrict/TextRestrictionRecordTest.java
@@ -152,7 +152,7 @@ class TextRestrictionRecordTest {
                     title LIKE "%Jakarta Data%" ESCAPED\
                     """);
             soft.assertThat(authorRestriction.toString()).isEqualTo("""
-                    author NOT_EQUAL_IGNORE_CASE "Myself"\
+                    author <> "Myself" IGNORE_CASE\
                     """);
         });
     }