Copilot commented on code in PR #15944:
URL: https://github.com/apache/grails-core/pull/15944#discussion_r3553906096
##########
grails-forge/grails-forge-cli/src/main/java/org/grails/forge/cli/command/CreateCommand.java:
##########
@@ -50,7 +50,7 @@ public abstract class CreateCommand extends BaseCommand
implements Callable<Inte
DevelopmentReloading reloading;
@ReflectiveAccess
- @CommandLine.Option(names = {"-g", "--gorm"}, paramLabel = "GORM
Implementation", description = "Which GORM Implementation to configure.
Possible values: ${COMPLETION-CANDIDATES}.", completionCandidates =
GormImplCandidates.class, converter = GormImplConverter.class)
+ @CommandLine.Option(names = {"-d", "--data", "-g", "--gorm"}, paramLabel =
"Grails Data Implementation", description = "Which Grails Data implementation
to configure (-g, --gorm are legacy aliases). Possible values:
${COMPLETION-CANDIDATES}.", completionCandidates = GormImplCandidates.class,
converter = GormImplConverter.class)
Review Comment:
The CLI help text advertises only the completion-candidate values, but the
implementation also accepts the legacy value `hibernate` (mapped to Hibernate 5
via `GormImpl.parse`). To avoid confusing users (especially when following
older docs), the option description should explicitly mention that `hibernate`
is still accepted as a legacy alias for `hibernate5`.
##########
grails-doc/src/en/ref/Command Line/create-plugin.adoc:
##########
@@ -106,7 +106,7 @@ This command will create a Grails plugin project named
"minimal-plugin" with the
+
[source,shell]
----
-grails create-plugin mongodb-plugin --gorm=mongodb
+grails create-plugin mongodb-plugin --data=mongodb
----
+
This command will generate a Grails plugin project named "mongodb-plugin"
configured to use MongoDB as the GORM implementation.
Review Comment:
The example was updated to use `--data=mongodb`, but the surrounding text
still says “GORM implementation” / “customizing GORM implementations”. For
consistency with the new terminology elsewhere in this PR, these references
should be updated to “Grails Data implementation”.
##########
grails-doc/src/en/ref/Command Line/help.adoc:
##########
@@ -148,9 +148,9 @@ Creates an application
grails-console, views-markup, asset-pipeline-grails,
views-json,
gorm-neo4j, asciidoctor, grails-web-console,
logbackGroovy, mongo-sync, shade, geb, properties
- -g, --gorm=GORM Implementation
- Which GORM Implementation to configure. Possible
values: hibernate,
- mongodb, neo4j.
+ -d, --data, -g, --gorm=Grails Data Implementation
+ Which Grails Data implementation to configure (-g,
--gorm are legacy
+ aliases). Possible values: hibernate5, hibernate7,
mongodb.
Review Comment:
The `--help` output snippet is internally inconsistent: the `Usage:` line
still shows the old `-g=GORM Implementation` flag, and the features list omits
the newly-added `gorm-hibernate7` feature, even though the option section below
documents `hibernate7`. Since this is presented as example output, it should be
regenerated/updated to match the current CLI behavior.
##########
grails-doc/src/en/ref/Command Line/create-plugin.adoc:
##########
@@ -71,7 +71,7 @@ Options include specifying features, configuring the GORM
implementation, select
Here are the available options for the create-plugin command:
- -f, --features=FEATURE[,FEATURE...]: Specifies the features to include in
the plugin. Available options include h2, gorm-hibernate5,
spring-boot-starter-jetty, micronaut-http-client, cache-ehcache,
hibernate-validator, postgres, mysql, cache, database-migration, grails-gsp,
hamcrest, gorm-mongodb, assertj, mockito, spring-boot-starter-undertow,
github-workflow-java-ci, jrebel, testcontainers, sqlserver, grails-console,
views-markup, views-json, gorm-neo4j, asciidoctor, grails-web-console,
logbackGroovy, mongo-sync, shade, properties.
-- -g, --gorm=GORM Implementation: Specifies the GORM Implementation to
configure for the plugin. Possible values are hibernate, mongodb, neo4j.
+- -d, --data=Grails Data Implementation (-g, --gorm are legacy aliases):
Specifies the Grails Data implementation to configure for the plugin. Possible
values are hibernate5, hibernate7, mongodb.
Review Comment:
This page still uses the old “GORM implementation” wording (line 67) and the
feature list omits the new `gorm-hibernate7` feature (line 73), even though the
options below document `--data=hibernate7`. Updating both keeps the terminology
and feature inventory consistent.
##########
grails-forge/grails-forge-core/src/main/java/org/grails/forge/options/GormImplTypeConverter.java:
##########
@@ -0,0 +1,41 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.grails.forge.options;
+
+import java.util.Optional;
+
+import io.micronaut.core.convert.ConversionContext;
+import io.micronaut.core.convert.TypeConverter;
+import jakarta.inject.Singleton;
+
+/**
+ * Converts user-supplied selection values (including the legacy {@code
hibernate}
+ * value) to {@link GormImpl} for HTTP parameter binding.
+ */
+@Singleton
+public class GormImplTypeConverter implements TypeConverter<CharSequence,
GormImpl> {
+
+ @Override
+ public Optional<GormImpl> convert(CharSequence object, Class<GormImpl>
targetType, ConversionContext context) {
+ if (object == null) {
+ return Optional.empty();
+ }
+ return Optional.ofNullable(GormImpl.parse(object.toString()));
+ }
Review Comment:
`GormImplTypeConverter` returns `Optional.empty()` for unknown values, and
`ApplicationController#getOptions` falls back to `GormImpl.DEFAULT_OPTION` when
the bound value is null. Depending on Micronaut’s binding behavior, this can
turn an invalid `gorm`/`data` HTTP parameter into a silent default rather than
a client-visible error (unlike the CLI converter, which throws). Consider
explicitly rejecting unknown values via the `ConversionContext` so bad inputs
reliably produce a 400 with a clear message.
##########
grails-doc/src/en/ref/Command Line/create-web-plugin.adoc:
##########
@@ -40,7 +40,7 @@ Here are the available options for the `create-web-plugin`
command:
- `-f, --features=FEATURE[,FEATURE...]`: Specifies the features to include in
the plugin. Available options include h2, gorm-hibernate5,
spring-boot-starter-jetty, micronaut-http-client, cache-ehcache,
hibernate-validator, postgres, mysql, cache, database-migration, grails-gsp,
hamcrest, gorm-mongodb, assertj, mockito, spring-boot-starter-undertow,
github-workflow-java-ci, jrebel, testcontainers, sqlserver, grails-console,
views-markup, views-json, gorm-neo4j, asciidoctor, grails-web-console,
logbackGroovy, mongo-sync, shade, properties.
Review Comment:
This command reference page’s feature list still omits the new
`gorm-hibernate7` feature even though the command now supports
`--data=hibernate7`. To keep the docs consistent, include `gorm-hibernate7` in
the enumerated feature list.
##########
grails-doc/src/en/ref/Command Line/create-restapi.adoc:
##########
@@ -42,7 +42,7 @@ The `create-restapi` command accepts the following options:
- `-f, --features=FEATURE[,FEATURE...]`: Specifies the features to include.
Available options include h2, scaffolding, gorm-hibernate5,
spring-boot-starter-jetty, spring-boot-starter-tomcat, micronaut-http-client,
cache-ehcache, hibernate-validator, postgres, mysql, cache, database-migration,
grails-gsp, hamcrest, gorm-mongodb, assertj, mockito,
spring-boot-starter-undertow, github-workflow-java-ci, jrebel, testcontainers,
sqlserver, grails-console, views-markup, asset-pipeline-grails, views-json,
gorm-neo4j, asciidoctor, grails-web-console, logbackGroovy, mongo-sync, shade,
geb, properties.
Review Comment:
This command reference page’s feature list still omits the new
`gorm-hibernate7` feature even though the command now supports
`--data=hibernate7`. To keep the docs consistent, include `gorm-hibernate7` in
the enumerated feature list.
##########
grails-forge/grails-forge-core/src/main/java/org/grails/forge/feature/database/DatabaseDriverConfigurationFeature.java:
##########
@@ -28,7 +28,7 @@
import static
org.grails.forge.feature.config.ConfigurationFeature.PROD_ENVIRONMENT_KEY;
import static
org.grails.forge.feature.config.ConfigurationFeature.ENVIRONMENTS_KEY;
import static
org.grails.forge.feature.config.ConfigurationFeature.PROPERTIES_KEY;
-import static org.grails.forge.feature.database.HibernateGorm.PREFIX;
+import static org.grails.forge.feature.database.GrailsDataHibernate5.PREFIX;
Review Comment:
`DatabaseDriverConfigurationFeature` statically imports `PREFIX` from
`GrailsDataHibernate5`, but the interface is also implemented by
`GrailsDataHibernate7`. This creates an unnecessary coupling to a specific
implementation class for a generic datasource helper. Consider defining a
datasource prefix constant on this interface (or on a small shared utility) and
referencing it directly, instead of importing it from `GrailsDataHibernate5`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
