jamesfredley commented on code in PR #16337: URL: https://github.com/apache/grails-core/pull/16337#discussion_r4031999806
########## grails-forge/grails-forge-core/src/main/java/org/grails/forge/ForgeContexts.java: ########## @@ -0,0 +1,48 @@ +/* + * 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; + +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.MapPropertySource; +import org.springframework.core.env.StandardEnvironment; + +import java.util.Map; + +public final class ForgeContexts { + + private ForgeContexts() { + } + + public static AnnotationConfigApplicationContext create() { + return create(Map.of()); + } + + public static AnnotationConfigApplicationContext create(Map<String, Object> configuration) { + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); + if (configuration != null && !configuration.isEmpty()) { + ConfigurableEnvironment environment = new StandardEnvironment(); + environment.getPropertySources().addFirst(new MapPropertySource("forge-test", configuration)); + context.setEnvironment(environment); + } Review Comment: Resolved in 932c1cbd81: dropped the unused ForgeContexts.create(Map) overload. GrailsForgeConfiguration binding is covered in the hosted Grails app via ForgeApiIntegrationSpec (HTML redirect to the bound URL). ########## grails-forge/grails-forge-core/src/main/java/org/grails/forge/ForgeContexts.java: ########## @@ -0,0 +1,48 @@ +/* + * 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; + +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.MapPropertySource; +import org.springframework.core.env.StandardEnvironment; + +import java.util.Map; + +public final class ForgeContexts { + + private ForgeContexts() { + } + + public static AnnotationConfigApplicationContext create() { + return create(Map.of()); + } + + public static AnnotationConfigApplicationContext create(Map<String, Object> configuration) { Review Comment: Resolved in 932c1cbd81: removed the Map overload and the empty configuration hook from ApplicationContextSpec / CommandSpec. ########## grails-forge/grails-forge-cli/src/main/java/org/grails/forge/cli/Application.java: ########## @@ -91,31 +93,36 @@ public static void main(String[] args) { static CommandLine createCommandLine() { boolean noOpConsole = Application.interactiveShell; - try (BeanContext beanContext = ApplicationContext.builder().deduceEnvironment(false).start()) { + try (AnnotationConfigApplicationContext beanContext = ForgeContexts.create()) { return createCommandLine(beanContext, noOpConsole); } } static int execute(String[] args) { boolean noOpConsole = args.length > 0 && args[0].startsWith("update-cli-config"); - try (BeanContext beanContext = ApplicationContext.builder().deduceEnvironment(false).start()) { + try (AnnotationConfigApplicationContext beanContext = ForgeContexts.create()) { Review Comment: Resolved in 932c1cbd81: the interactive shell now creates one AnnotationConfigApplicationContext for the session and reuses it for each line. One-shot execute still opens and closes its own context. Covered by ApplicationContextReuseSpec. ########## grails-forge/grails-forge-cli/src/main/java/org/grails/forge/cli/Application.java: ########## @@ -46,28 +50,28 @@ optionListHeading = "%n@|bold,underline Options:|@%n", commandListHeading = "%n@|bold,underline Commands:|@%n", subcommands = { - // Creation commands CreateAppCommand.class, CreateWebappCommand.class, CreatePluginCommand.class, CreateWebPluginCommand.class, CreateRestApiCommand.class }) -@Prototype -@TypeHint({ - Application.class, - GormImplCandidates.class, - GormImplConverter.class, - ServletImplCandidates.class, - ServletImplConverter.class, - CommonOptionsMixin.class, - DevelopmentReloadingCandidates.class, - DevelopmentReloadingConverter.class -}) +@Component +@Scope("prototype") public class Application extends BaseCommand implements Callable<Integer> { private static Boolean interactiveShell = false; + private static final List<Class<? extends CodeGenCommand>> CODE_GEN_COMMANDS = List.of( Review Comment: Resolved in 932c1cbd81: added a comment on CODE_GEN_COMMANDS stating it is the registration point because these commands take CodeGenConfig in their constructor and are not Spring beans. ########## gradle/forge-test-config.gradle: ########## @@ -20,23 +20,37 @@ // Add JUnit Platform launcher dependency required by Gradle 9 for running tests // Also add ByteBuddy for Spock mocking on Java 17+ (CGLIB doesn't support Java 17 class files) dependencies { - testRuntimeOnly 'org.junit.platform:junit-platform-launcher:1.12.2' + testRuntimeOnly 'org.junit.platform:junit-platform-launcher' testImplementation "net.bytebuddy:byte-buddy" } -tasks.withType(Test).configureEach { +// Skipped Test tasks still execute their dependsOn. Only wire the local-Maven +// publish chain when Forge tests will actually run. +boolean forgeTestsEnabled = !['skipTests', 'skipForgeTests'].any { project.hasProperty(it) } && + (project.hasProperty('onlyForgeTests') || + !['onlyCoreTests', + 'onlyFunctionalTests', + 'onlyHibernate5Tests', + 'onlyHibernate7Tests', + 'onlyMongodbTests', + 'onlyNeo4jTests', + 'onlyRedisTests', + 'onlySpringSecurityTests'].any { project.hasProperty(it) }) + +tasks.withType(Test).configureEach { Test testTask -> + if (forgeTestsEnabled) { + testTask.dependsOn( Review Comment: Resolved in 932c1cbd81: local-Maven publish and GRAILS_REPO_URL are now gated on ext.forgeTestsNeedLocalMaven, set only by grails-forge-cli and grails-forge-test-core. ########## grails-forge/grails-forge-web/src/test/groovy/org/grails/forge/web/GrailsForgeConfigurationSpec.groovy: ########## @@ -17,25 +17,32 @@ * under the License. */ -package org.grails.forge.api +package org.grails.forge.web - -import io.micronaut.test.extensions.spock.annotation.MicronautTest -import jakarta.inject.Inject +import org.grails.forge.api.GrailsForgeConfiguration import spock.lang.Specification -@MicronautTest(startApplication = false) -class FeatureOperationsSpec extends Specification { +class GrailsForgeConfigurationSpec extends Specification { + + void "redirectUrl is exposed as a redirect URI without a redirectUri setter"() { + given: + GrailsForgeConfiguration config = new GrailsForgeConfiguration() + + when: + config.redirectUrl = 'https://start.grails.org/' - @Inject - FeatureOperations featureOperations + then: + config.redirectUrl == 'https://start.grails.org/' + config.redirectUri().get().toString() == 'https://start.grails.org/' + } - void "only visible features are exposed"() { + void "cors origins remain overridable by environment variables"() { Review Comment: Resolved in 932c1cbd81: dropped the application.yml text assertion. ForgeApiIntegrationSpec now covers GET / with Accept: text/html (301 to the bound UI URL) and asserts CORS allowedOrigins have no unresolved placeholders. ########## grails-forge/grails-forge-core/build.gradle: ########## @@ -42,48 +47,38 @@ sourceSets { } dependencies { - annotationProcessor platform("io.micronaut.platform:micronaut-platform:$micronautVersion") - implementation platform("io.micronaut.platform:micronaut-platform:$micronautVersion") - annotationProcessor 'io.micronaut:micronaut-inject-java' - testAnnotationProcessor platform("io.micronaut.platform:micronaut-platform:$micronautVersion") - testAnnotationProcessor 'io.micronaut:micronaut-inject-java' + implementation platform(project(':grails-bom')) - api platform("io.micronaut.platform:micronaut-platform:$micronautVersion") // TODO: Should this be api? + api 'org.springframework:spring-context' + api 'jakarta.annotation:jakarta.annotation-api' + api 'jakarta.inject:jakarta.inject-api' api 'com.fasterxml.jackson.core:jackson-databind' api 'jakarta.validation:jakarta.validation-api' api "com.fizzed:rocker-runtime:$rockerVersion" api "io.github.java-diff-utils:java-diff-utils:$javaDiffUtils" - api 'io.micronaut:micronaut-http' - api 'io.micronaut:micronaut-http-client' - api 'io.micronaut:micronaut-inject' implementation "com.typesafe:config:$typesafeConfigVersion" implementation "org.apache.commons:commons-compress:$commonsCompressVersion" implementation "org.yaml:snakeyaml" - - compileOnly 'com.google.code.findbugs:jsr305' + implementation 'org.slf4j:slf4j-api' testImplementation 'org.apache.groovy:groovy-yaml' - - testCompileOnly 'io.micronaut:micronaut-inject-groovy' - testImplementation 'org.apache.groovy:groovy' testImplementation "org.spockframework:spock-core", { exclude group: 'org.apache.groovy', module: 'groovy-all' } testImplementation "ch.qos.logback:logback-classic" - testRuntimeOnly "org.objenesis:objenesis:$objenesisVersion" + testRuntimeOnly 'org.objenesis:objenesis' testImplementation 'org.apache.groovy:groovy-test' } def grailsVersionsPath = layout.buildDirectory.dir('version-info') def grailsVersionInfoTask = tasks.register('grailsVersionInfo', WriteGrailsVersionInfoTask) Review Comment: Resolved in 932c1cbd81: WriteGrailsVersionInfoTask now throws the GradleException on parse failure. WriteGrailsVersionInfoTaskSpec covers that path. ########## grails-forge/README.md: ########## @@ -43,15 +43,13 @@ The user interface is [written in React](https://github.com/apache/grails-forge- ## API -API documentation for the production instance can be found at: +API usage for the production instance is available at: -* [Swagger / OpenAPI Doc](https://latest.grails.org/swagger-ui/index.html) -* [RAPI Doc](https://latest.grails.org/rapidoc/index.html) +* [Production API](https://latest.grails.org/) Review Comment: Resolved in 932c1cbd81: restored OpenAPI 3 for the hosted generator from grails-forge-web (not Micronaut OpenAPI) at /v3/api-docs, with Swagger UI and RapiDoc. README now points at those URLs. A general grails-openapi module is not required for these endpoints. ########## grails-forge/README.md: ########## @@ -43,15 +43,13 @@ The user interface is [written in React](https://github.com/apache/grails-forge- ## API -API documentation for the production instance can be found at: Review Comment: Preserved in 932c1cbd81 with a checked-in OpenAPI document served by ForgeOpenApiController. A later auto-generated grails-openapi module can replace this Forge-specific description without blocking this PR. -- 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]
