fix: show autoform non-rendered property errors (#2833)

* Shows hidden property errors to the form and scroll to it Fixes: #2217 * Resolve jakarta persistence id and version annotations as nullable by default Fixes: #2809 * fix formatting * Fix type check Fixes: #2217 * Add test for nonnull and nullable hierarchy Fixes: #2833 * Add test for nonnull and nullable hierarchy Fixes: #2833 * fix formatting --------- Co-authored-by: Anton Platonov <platosha@gmail.com>
vaadin · Nov 26, 2024 · e2af537 · e2af537
1 parent d88726b
commit e2af537
Show file tree

Hide file tree

Showing 12 changed files with 326 additions and 8 deletions.
diff --git a/packages/java/parser-jvm-plugin-nonnull/pom.xml b/packages/java/parser-jvm-plugin-nonnull/pom.xml
@@ -54,5 +54,15 @@
       <version>${project.version}</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>jakarta.persistence</groupId>
+      <artifactId>jakarta.persistence-api</artifactId>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.springframework</groupId>
+      <artifactId>spring-core</artifactId>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 </project>
diff --git a/...in-nonnull/src/main/java/com/vaadin/hilla/parser/plugins/nonnull/NonnullPluginConfig.java b/...in-nonnull/src/main/java/com/vaadin/hilla/parser/plugins/nonnull/NonnullPluginConfig.java
@@ -47,6 +47,11 @@ static class Processor extends ConfigList.Processor<AnnotationMatcher> {
                 // Package-level annotations have low score
                 new AnnotationMatcher("org.springframework.lang.NonNullApi",
                         false, 10),
+                // Id and Version annotation usually mark nullable fields for
+                // CRUD operations.
+                // Low score allows other annotations to override them.
+                new AnnotationMatcher("jakarta.persistence.Id", true, 20),
+                new AnnotationMatcher("jakarta.persistence.Version", true, 20),
                 // Nullable-like annotations get a higher score. This should
                 // only matter when they are used in conjunction with
                 // package-level annotations

diff --git a/...ugin-nonnull/src/test/java/com/vaadin/hilla/parser/plugins/nonnull/nullable/Endpoint.java b/...ugin-nonnull/src/test/java/com/vaadin/hilla/parser/plugins/nonnull/nullable/Endpoint.java
@@ -0,0 +1,11 @@
+package com.vaadin.hilla.parser.plugins.nonnull.nullable;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+@Retention(RetentionPolicy.RUNTIME)
+@Target(ElementType.TYPE)
+public @interface Endpoint {
+}
diff --git a/...nnull/src/test/java/com/vaadin/hilla/parser/plugins/nonnull/nullable/EndpointExposed.java b/...nnull/src/test/java/com/vaadin/hilla/parser/plugins/nonnull/nullable/EndpointExposed.java
@@ -0,0 +1,11 @@
+package com.vaadin.hilla.parser.plugins.nonnull.nullable;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+@Retention(RetentionPolicy.RUNTIME)
+@Target(ElementType.TYPE)
+public @interface EndpointExposed {
+}
diff --git a/...null/src/test/java/com/vaadin/hilla/parser/plugins/nonnull/nullable/NullableEndpoint.java b/...null/src/test/java/com/vaadin/hilla/parser/plugins/nonnull/nullable/NullableEndpoint.java
@@ -0,0 +1,20 @@
+package com.vaadin.hilla.parser.plugins.nonnull.nullable;
+
+import jakarta.persistence.Id;
+import jakarta.persistence.Version;
+
+@Endpoint
+public class NullableEndpoint {
+
+    public NullableFieldModel nullableFieldModel(
+            NullableFieldModel nullableFieldModel) {
+        return nullableFieldModel;
+    }
+
+    public static class NullableFieldModel {
+        @Id
+        public String id;
+        @Version
+        public Long version;
+    }
+}
diff --git a/...-nonnull/src/test/java/com/vaadin/hilla/parser/plugins/nonnull/nullable/NullableTest.java b/...-nonnull/src/test/java/com/vaadin/hilla/parser/plugins/nonnull/nullable/NullableTest.java
@@ -0,0 +1,48 @@
+package com.vaadin.hilla.parser.plugins.nonnull.nullable;
+
+import com.vaadin.hilla.parser.core.Parser;
+import com.vaadin.hilla.parser.plugins.backbone.BackbonePlugin;
+import com.vaadin.hilla.parser.plugins.nonnull.AnnotationMatcher;
+import com.vaadin.hilla.parser.plugins.nonnull.NonnullPlugin;
+import com.vaadin.hilla.parser.plugins.nonnull.NonnullPluginConfig;
+import com.vaadin.hilla.parser.plugins.nonnull.nullable.nonNullApi.NullableNonNullEndpoint;
+import com.vaadin.hilla.parser.plugins.nonnull.test.helpers.TestHelper;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import java.io.IOException;
+import java.net.URISyntaxException;
+import java.util.Set;
+
+public class NullableTest {
+    private final TestHelper helper = new TestHelper(getClass());
+
+    @Test
+    public void should_ApplyNullableAnnotation()
+            throws IOException, URISyntaxException {
+        var plugin = new NonnullPlugin();
+        plugin.setConfiguration(new NonnullPluginConfig());
+
+        var openAPI = new Parser().classLoader(getClass().getClassLoader())
+                .classPath(Set.of(helper.getTargetDir().toString()))
+                .endpointAnnotation(Endpoint.class.getName())
+                .endpointExposedAnnotation(EndpointExposed.class.getName())
+                .addPlugin(new BackbonePlugin()).addPlugin(plugin).execute();
+
+        helper.executeParserWithConfig(openAPI);
+    }
+
+    @Test
+    public void annotationMatcher_shouldHaveDefaultConstructorAndSetter() {
+        // to enable maven initialize instances of AnnotationMatcher from
+        // pom.xml configurations, properly, it should have the default
+        // constructor and setter methods:
+        AnnotationMatcher annotationMatcher = new AnnotationMatcher();
+        annotationMatcher.setName("name");
+        annotationMatcher.setScore(100);
+        annotationMatcher.setMakesNullable(true);
+        Assertions.assertEquals("name", annotationMatcher.getName());
+        Assertions.assertEquals(100, annotationMatcher.getScore());
+        Assertions.assertTrue(annotationMatcher.doesMakeNullable());
+    }
+}
diff --git a/.../com/vaadin/hilla/parser/plugins/nonnull/nullable/nonNullApi/NullableNonNullEndpoint.java b/.../com/vaadin/hilla/parser/plugins/nonnull/nullable/nonNullApi/NullableNonNullEndpoint.java
@@ -0,0 +1,26 @@
+package com.vaadin.hilla.parser.plugins.nonnull.nullable.nonNullApi;
+
+import com.vaadin.hilla.parser.plugins.nonnull.nullable.Endpoint;
+import jakarta.annotation.Nonnull;
+import jakarta.persistence.Id;
+import jakarta.persistence.Version;
+
+@Endpoint
+public class NullableNonNullEndpoint {
+
+    public NullableNonNullFieldModel nullableNonNullFieldModel(
+            NullableNonNullFieldModel nullableNonNullFieldModel) {
+        return nullableNonNullFieldModel;
+    }
+
+    public static class NullableNonNullFieldModel {
+        public String required;
+        @Id
+        public String id;
+        @Version
+        public Long version;
+        @Version
+        @Nonnull
+        public Long notNullVersion;
+    }
+}
diff --git a/...c/test/java/com/vaadin/hilla/parser/plugins/nonnull/nullable/nonNullApi/package-info.java b/...c/test/java/com/vaadin/hilla/parser/plugins/nonnull/nullable/nonNullApi/package-info.java
@@ -0,0 +1,4 @@
+@NonNullApi
+package com.vaadin.hilla.parser.plugins.nonnull.nullable.nonNullApi;
+
+import org.springframework.lang.NonNullApi;
diff --git a/...-nonnull/src/test/resources/com/vaadin/hilla/parser/plugins/nonnull/nullable/openapi.json b/...-nonnull/src/test/resources/com/vaadin/hilla/parser/plugins/nonnull/nullable/openapi.json
@@ -0,0 +1,150 @@
+{
+  "openapi" : "3.0.1",
+  "info" : {
+    "title" : "Hilla Application",
+    "version" : "1.0.0"
+  },
+  "servers" : [
+    {
+      "url" : "http://localhost:8080/connect",
+      "description" : "Hilla Backend"
+    }
+  ],
+  "tags" : [
+    {
+      "name" : "NullableEndpoint",
+      "x-class-name" : "com.vaadin.hilla.parser.plugins.nonnull.nullable.NullableEndpoint"
+    },
+    {
+      "name" : "NullableNonNullEndpoint",
+      "x-class-name" : "com.vaadin.hilla.parser.plugins.nonnull.nullable.nonNullApi.NullableNonNullEndpoint"
+    }
+  ],
+  "paths" : {
+    "/NullableEndpoint/nullableFieldModel" : {
+      "post" : {
+        "tags" : [
+          "NullableEndpoint"
+        ],
+        "operationId" : "NullableEndpoint_nullableFieldModel_POST",
+        "requestBody" : {
+          "content" : {
+            "application/json" : {
+              "schema" : {
+                "type" : "object",
+                "properties" : {
+                  "nullableFieldModel" : {
+                    "nullable" : true,
+                    "anyOf" : [
+                      {
+                        "$ref" : "#/components/schemas/com.vaadin.hilla.parser.plugins.nonnull.nullable.NullableEndpoint$NullableFieldModel"
+                      }
+                    ]
+                  }
+                }
+              }
+            }
+          }
+        },
+        "responses" : {
+          "200" : {
+            "description" : "",
+            "content" : {
+              "application/json" : {
+                "schema" : {
+                  "nullable" : true,
+                  "anyOf" : [
+                    {
+                      "$ref" : "#/components/schemas/com.vaadin.hilla.parser.plugins.nonnull.nullable.NullableEndpoint$NullableFieldModel"
+                    }
+                  ]
+                }
+              }
+            }
+          }
+        }
+      }
+    },
+    "/NullableNonNullEndpoint/nullableNonNullFieldModel" : {
+      "post" : {
+        "tags" : [
+          "NullableNonNullEndpoint"
+        ],
+        "operationId" : "NullableNonNullEndpoint_nullableNonNullFieldModel_POST",
+        "requestBody" : {
+          "content" : {
+            "application/json" : {
+              "schema" : {
+                "type" : "object",
+                "properties" : {
+                  "nullableNonNullFieldModel" : {
+                    "anyOf" : [
+                      {
+                        "$ref" : "#/components/schemas/com.vaadin.hilla.parser.plugins.nonnull.nullable.nonNullApi.NullableNonNullEndpoint$NullableNonNullFieldModel"
+                      }
+                    ]
+                  }
+                }
+              }
+            }
+          }
+        },
+        "responses" : {
+          "200" : {
+            "description" : "",
+            "content" : {
+              "application/json" : {
+                "schema" : {
+                  "anyOf" : [
+                    {
+                      "$ref" : "#/components/schemas/com.vaadin.hilla.parser.plugins.nonnull.nullable.nonNullApi.NullableNonNullEndpoint$NullableNonNullFieldModel"
+                    }
+                  ]
+                }
+              }
+            }
+          }
+        }
+      }
+    }
+  },
+  "components" : {
+    "schemas" : {
+      "com.vaadin.hilla.parser.plugins.nonnull.nullable.NullableEndpoint$NullableFieldModel" : {
+        "type" : "object",
+        "properties" : {
+          "id" : {
+            "type" : "string",
+            "nullable" : true
+          },
+          "version" : {
+            "type" : "integer",
+            "format" : "int64",
+            "nullable" : true
+          }
+        }
+      },
+      "com.vaadin.hilla.parser.plugins.nonnull.nullable.nonNullApi.NullableNonNullEndpoint$NullableNonNullFieldModel" : {
+        "type" : "object",
+        "properties" : {
+          "required" : {
+            "type" : "string"
+          },
+          "id" : {
+            "type" : "string",
+            "nullable" : true
+          },
+          "version" : {
+            "type" : "integer",
+            "format" : "int64",
+            "nullable" : true
+          },
+          "notNullVersion" : {
+            "type" : "integer",
+            "format" : "int64"
+          }
+        }
+      }
+    }
+  }
+}
diff --git a/.../spring/react-grid-test/src/main/java/com/vaadin/hilla/test/reactgrid/AbstractEntity.java b/.../spring/react-grid-test/src/main/java/com/vaadin/hilla/test/reactgrid/AbstractEntity.java
@@ -6,17 +6,13 @@
 import jakarta.persistence.MappedSuperclass;
 import jakarta.persistence.Version;
 
-import com.vaadin.hilla.Nullable;
-
 @MappedSuperclass
 public class AbstractEntity {
     @Id
-    @Nullable
     @GeneratedValue(strategy = GenerationType.IDENTITY)
     private Long id;
 
     @Version
-    @Nullable
     private Long version;
 
     public Long getId() {

diff --git a/packages/ts/react-crud/src/autoform.tsx b/packages/ts/react-crud/src/autoform.tsx
@@ -12,6 +12,7 @@ import {
   type ReactElement,
   useEffect,
   useMemo,
+  useRef,
   useState,
 } from 'react';
 import { AutoFormField, type AutoFormFieldProps, type FieldOptions } from './autoform-field.js';
@@ -278,6 +279,7 @@ export function AutoForm<M extends AbstractModel>({
   const form = useForm(model, {
     onSubmit: async (formItem) => service.save(formItem),
   });
+  const formErrorRef = useRef<HTMLDivElement>(null);
   const [formError, setFormError] = useState<JSX.Element | string>('');
   const [showDeleteDialog, setShowDeleteDialog] = useState(false);
   const modelInfo = useMemo(() => new ModelInfo(model, itemIdProperty), [model]);
@@ -294,21 +296,33 @@ export function AutoForm<M extends AbstractModel>({
     }
   }, [item]);
 
+  useEffect(() => {
+    formErrorRef.current?.scrollIntoView({ behavior: 'smooth', block: 'end' });
+  }, [formError]);
+
   function handleSubmitError(error: unknown) {
     if (error instanceof ValidationError) {
       const nonPropertyErrorMessages = error.errors
-        .filter((validationError) => !validationError.property)
-        .map((validationError) => validationError.validatorMessage ?? validationError.message);
+        .filter((validationError) => !validationError.property || typeof validationError.property === 'string')
+        .map((validationError) => {
+          const property =
+            validationError.property && typeof validationError.property === 'string'
+              ? `${validationError.property}: `
+              : '';
+          return `${property}${
+            validationError.validatorMessage ? validationError.validatorMessage : validationError.message
+          }`;
+        });
       if (nonPropertyErrorMessages.length > 0) {
         setFormError(
-          <>
+          <div ref={formErrorRef}>
             Validation errors:
             <ul>
               {nonPropertyErrorMessages.map((message, index) => (
                 <li key={index}>{message}</li>
               ))}
             </ul>
-          </>,
+          </div>,
         );
       }
     } else if (error instanceof EndpointError) {