Pico example pre req changes (#6801)

pico/api/pom.xml

@@ -60,7 +60,6 @@
         <dependency>
             <groupId>io.helidon.builder</groupId>
             <artifactId>helidon-builder</artifactId>
-            <scope>provided</scope>
         </dependency>
         <dependency>
             <groupId>io.helidon.builder</groupId>

pico/api/src/main/java/module-info.java

@@ -22,7 +22,7 @@
     requires io.helidon.common.types;
     requires io.helidon.common;
     requires io.helidon.common.config;
-    requires static io.helidon.builder;
+    requires transitive io.helidon.builder;
     requires static io.helidon.config.metadata;
     requires static jakarta.annotation;
 

pico/configdriven/runtime/pom.xml

@@ -84,6 +84,11 @@
             <artifactId>helidon-common-testing-junit5</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-core</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>

pico/configdriven/runtime/src/main/java/io/helidon/pico/configdriven/runtime/AbstractConfiguredServiceProvider.java

@@ -116,13 +116,13 @@ static Comparator<String> configBeanComparator() {
     static BindableConfigBeanRegistry resolveConfigBeanRegistry() {
         HelidonConfigBeanRegistry cbr = ConfigBeanRegistryHolder.configBeanRegistry().orElse(null);
         if (cbr == null) {
-            LOGGER.log(System.Logger.Level.INFO, "Config-Driven Services disabled (config bean registry not found)");
+            LOGGER.log(System.Logger.Level.INFO, "Config-Driven Services is disabled (config bean registry not found)");
             return null;
         }
 
         if (!(cbr instanceof BindableConfigBeanRegistry)) {
             Optional<CallingContext> callingContext = CallingContextFactory.create(false);
-            String desc = "Config-Driven Services disabled (unsupported implementation): " + cbr;
+            String desc = "Config-Driven Services is disabled (unsupported implementation): " + cbr;
             String msg = (callingContext.isEmpty()) ? toErrorMessage(desc) : toErrorMessage(callingContext.get(), desc);
             throw new PicoException(msg);
         }

pico/configdriven/runtime/src/main/java/io/helidon/pico/configdriven/runtime/DefaultPicoConfigBeanRegistry.java

@@ -123,7 +123,8 @@ static void validateUsingBeanAttributes(Supplier<Object> valueSupplier,
 
     @Override
     public boolean reset(boolean deep) {
-        System.Logger.Level level = isInitialized() ? System.Logger.Level.INFO : System.Logger.Level.DEBUG;
+        System.Logger.Level level = (isInitialized() && PicoServices.isDebugEnabled())
+        ? System.Logger.Level.INFO : System.Logger.Level.DEBUG;
         LOGGER.log(level, "Resetting");
         configuredServiceProviderMetaConfigBeanMap.clear();
         configuredServiceProvidersByConfigKey.clear();
@@ -351,28 +352,32 @@ <T, CB extends GeneratedConfigBean> void registerChildConfigBeans(io.helidon.con
         config.asNodeList()
                 .ifPresent(list -> {
                     list.forEach(beanCfg -> {
-                        // use explicit name, otherwise index or name of the config node (for lists, the name is 0 bases index)
-                        String name = beanCfg.get("name").asString().orElseGet(beanCfg::name);
                         CB configBean = toConfigBean(beanCfg, configuredServiceProvider);
-                        registerConfigBean(configBean, name, beanCfg, configuredServiceProvider, metaAttributes);
+                        String instanceId = toInstanceId(beanCfg);
+                        registerConfigBean(configBean, instanceId, beanCfg, configuredServiceProvider, metaAttributes);
                     });
                 });
     }
 
-
-    <T, GCB extends GeneratedConfigBean> GCB toConfigBean(io.helidon.config.Config config,
-                                                          ConfiguredServiceProvider<T, ?> configuredServiceProvider) {
+    static <T, GCB extends GeneratedConfigBean> GCB toConfigBean(io.helidon.config.Config config,
+                                                                 ConfiguredServiceProvider<T, ?> configuredServiceProvider) {
         GCB configBean = (GCB) Objects.requireNonNull(configuredServiceProvider.toConfigBean(config),
-                                               "unable to create default config bean for " + configuredServiceProvider);
+                                               "Unable to create default config bean for " + configuredServiceProvider);
         if (configuredServiceProvider instanceof AbstractConfiguredServiceProvider) {
-            AbstractConfiguredServiceProvider<T, GCB> csp = (AbstractConfiguredServiceProvider<T, GCB>) configuredServiceProvider;
-            csp.configBeanInstanceId(configBean, config.key().toString());
-            assert (config.name().equals(configBean.__name().orElseThrow())) : csp;
+            setConfigBeanInstanceId(config, configBean, (AbstractConfiguredServiceProvider<T, GCB>) configuredServiceProvider);
         }
 
         return configBean;
     }
 
+    @SuppressWarnings("rawtypes")
+    static void setConfigBeanInstanceId(io.helidon.config.Config config,
+                                        GeneratedConfigBean configBean,
+                                        AbstractConfiguredServiceProvider csp) {
+        String instanceId = toInstanceId(config);
+        csp.configBeanInstanceId(configBean, instanceId);
+    }
+
     /**
      * Validates the config bean against the declared policy, coming by way of annotations on the
      * {@code ConfiguredOption}'s.
@@ -391,8 +396,6 @@ <GCB extends GeneratedConfigBean> void validate(GCB configBean,
         Set<String> problems = new LinkedHashSet<>();
         String instanceId = csp.toConfigBeanInstanceId(configBean);
         assert (hasValue(key));
-        assert (config == null || DEFAULT_INSTANCE_ID.equals(key) || (config.key().toString().equals(key)))
-                : key + " and " + config.key().toString();
 
         AttributeVisitor<Object> visitor = new AttributeVisitor<>() {
             @Override
@@ -458,7 +461,7 @@ <GCB extends GeneratedConfigBean> void registerConfigBean(GCB configBean,
         if (instanceId != null) {
             csp.configBeanInstanceId(configBean, instanceId);
         } else {
-            instanceId = configuredServiceProvider.toConfigBeanInstanceId((GCB) configBean);
+            instanceId = configuredServiceProvider.toConfigBeanInstanceId(configBean);
         }
 
         if (DEFAULT_INSTANCE_ID.equals(instanceId)) {
@@ -586,4 +589,10 @@ private void visitAndInitialize(List<io.helidon.config.Config> configs,
         });
     }
 
+    static String toInstanceId(io.helidon.config.Config config) {
+        // use explicit name, otherwise index or fq id of the config node (for lists, the name is 0 bases index)
+        String id = config.get("name").asString().orElseGet(config::name);
+        return hasValue(id) ? id : DEFAULT_INSTANCE_ID;
+    }
+
 }

pico/configdriven/runtime/src/test/java/io/helidon/pico/configdriven/runtime/DefaultPicoConfigBeanRegistryTest.java

@@ -0,0 +1,97 @@
+/*
+ * Copyright (c) 2023 Oracle and/or its affiliates.
+ *
+ * Licensed 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
+ *
+ *     http://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 io.helidon.pico.configdriven.runtime;
+
+import java.util.Map;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
+
+import io.helidon.builder.config.spi.GeneratedConfigBeanBase;
+import io.helidon.config.Config;
+import io.helidon.config.ConfigSources;
+
+import org.junit.jupiter.api.Test;
+
+import static io.helidon.pico.configdriven.runtime.DefaultPicoConfigBeanRegistry.DEFAULT_INSTANCE_ID;
+import static io.helidon.pico.configdriven.runtime.DefaultPicoConfigBeanRegistry.setConfigBeanInstanceId;
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+class DefaultPicoConfigBeanRegistryTest {
+
+    @Test
+    void testsetConfigBeanInstanceId() {
+        // the setup
+        AtomicInteger setCalls = new AtomicInteger();
+        AtomicReference<String> instanceId = new AtomicReference<>(DEFAULT_INSTANCE_ID);
+
+        GeneratedConfigBeanBase cfgBean = mock(GeneratedConfigBeanBase.class);
+        when(cfgBean.__instanceId()).thenAnswer(im -> instanceId.get());
+        cfgBean.__instanceId(anyString());
+        doAnswer(im -> {
+            setCalls.incrementAndGet();
+            instanceId.set(im.getArgument(1).toString());
+            return null;
+        }).when(cfgBean).__instanceId(anyString());
+
+        Config config = Config.builder(
+                ConfigSources.create(
+                        Map.of("other.than.name", "the-name"), "config-root-plus-one-socket"))
+                .disableEnvironmentVariablesSource()
+                .disableSystemPropertiesSource()
+                .build();
+
+        AbstractConfiguredServiceProvider<?, ?> csp = mock(AbstractConfiguredServiceProvider.class);
+        doAnswer(im -> {
+            setCalls.incrementAndGet();
+            instanceId.set(im.getArgument(1).toString());
+            return null;
+        }).when(csp).configBeanInstanceId(any(), anyString());
+        when(csp.configBean()).thenAnswer(im -> Optional.of(cfgBean));
+
+        // the 1st test
+        setConfigBeanInstanceId(config, cfgBean, csp);
+        assertThat(instanceId.get(),
+                   equalTo(DEFAULT_INSTANCE_ID));
+        assertThat(setCalls.get(),
+                   is(1));
+
+        // reset
+        setCalls.set(0);
+        config = Config.builder(
+                        ConfigSources.create(
+                                Map.of("name", "the-name"), "config-root-plus-one-socket"))
+                .disableEnvironmentVariablesSource()
+                .disableSystemPropertiesSource()
+                .build();
+
+        // the next test
+        setConfigBeanInstanceId(config, cfgBean, csp);
+        assertThat(instanceId.get(),
+                   equalTo("the-name"));
+        assertThat(setCalls.get(),
+                   is(1));
+    }
+
+}

pico/configdriven/tests/configuredby/src/main/java/io/helidon/pico/configdriven/configuredby/test/AbstractConfiguredByTest.java

@@ -88,7 +88,7 @@ public MapConfigSource.Builder createBasicTestingConfigSource() {
     public MapConfigSource.Builder createRootDefault8080TestingConfigSource() {
         return ConfigSources.create(
                 Map.of(
-                        FAKE_SERVER_CONFIG + ".name", "root",
+                        FAKE_SERVER_CONFIG + ".name", "fake-server",
                         FAKE_SERVER_CONFIG + ".port", "8080",
                         FAKE_SERVER_CONFIG + ".worker-count", "1"
                 ), "config-root-default-8080");

pico/configdriven/tests/configuredby/src/test/java/io/helidon/pico/configdriven/configuredby/test/ConfiguredByTest.java

@@ -54,7 +54,6 @@ protected MapConfigSource.Builder createNested8080TestingConfigSource() {
     public MapConfigSource.Builder createRootPlusOneSocketTestingConfigSource() {
         return ConfigSources.create(
                 Map.of(
-                        FAKE_SERVER_CONFIG + ".name", "root",
                         FAKE_SERVER_CONFIG + ".port", "8080",
                         FAKE_SERVER_CONFIG + "." + FAKE_SOCKET_CONFIG + ".0.name", "first",
                         FAKE_SERVER_CONFIG + "." + FAKE_SOCKET_CONFIG + ".0.port", "8081"

pico/maven-plugin/src/main/java/io/helidon/pico/maven/plugin/AbstractApplicationCreatorMojo.java

@@ -98,14 +98,14 @@ public abstract class AbstractApplicationCreatorMojo extends AbstractCreatorMojo
 
     /**
      * Sets the named types permitted for providers, assuming use of
-     * {@link PermittedProviderType#NAMED}.
+     * {@link io.helidon.pico.tools.ApplicationCreatorConfigOptions.PermittedProviderType#NAMED}.
      */
     @Parameter(property = PicoServicesConfig.NAME + ".permitted.provider.type.names", readonly = true)
     private List<String> permittedProviderTypeNames;
 
     /**
      * Sets the named qualifier types permitted for providers, assuming use of
-     * {@link PermittedProviderType#NAMED}.
+     * {@link io.helidon.pico.tools.ApplicationCreatorConfigOptions.PermittedProviderType#NAMED}.
      */
     @Parameter(property = PicoServicesConfig.NAME + ".permitted.provider.qualifier.type.names", readonly = true)
     private List<String> permittedProviderQualifierTypeNames;
@@ -134,18 +134,22 @@ String getThisModuleName() {
             // try to recover it from a previous tooling step
             String appPackageName = loadAppPackageName().orElse(null);
             if (appPackageName == null) {
-                // throw noModuleFoundError();
-                getLog().warn(noModuleFoundError().getMessage());
+                getLog().info(noModuleFoundError().getMessage());
             } else {
                 moduleName = appPackageName;
             }
         }
         return moduleName;
     }
 
-    ServiceProvider<ModuleComponent> lookupThisModule(String name,
-                                                      Services services) {
-        return services.lookupFirst(ModuleComponent.class, name, false).orElseThrow(() -> noModuleFoundError(name));
+    Optional<ServiceProvider<ModuleComponent>> lookupThisModule(String name,
+                                                                Services services,
+                                                                boolean expected) {
+        Optional<ServiceProvider<ModuleComponent>> result = services.lookupFirst(ModuleComponent.class, name, false);
+        if (result.isEmpty() && expected) {
+            throw noModuleFoundError(name);
+        }
+        return result;
     }
 
     String getClassPrefixName() {
@@ -250,7 +254,7 @@ protected void innerExecute() {
 
             PicoServices picoServices = picoServices(false);
             if (picoServices.config().usesCompileTimeApplications()) {
-                String desc = "should not be using 'application' bindings";
+                String desc = "Should not be using 'application' bindings";
                 String msg = (callCtx == null) ? toErrorMessage(desc) : toErrorMessage(callCtx, desc);
                 throw new IllegalStateException(msg);
             }
@@ -263,9 +267,13 @@ protected void innerExecute() {
                     .lookupAll(ServiceInfoCriteriaDefault.builder()
                                        .addContractImplemented(ModuleComponent.class.getName())
                                        .build());
-            getLog().info("processing modules: " + toDescriptions(allModules));
+            if (PicoServices.isDebugEnabled()) {
+                getLog().info("processing modules: " + toDescriptions(allModules));
+            } else {
+                getLog().debug("processing modules: " + toDescriptions(allModules));
+            }
             if (allModules.isEmpty()) {
-                warn("no modules to process");
+                warn("No modules to process");
             }
 
             // retrieves all the services in the registry
@@ -288,8 +296,8 @@ protected void innerExecute() {
                     ? moduleInfoPathRef.get().getPath()
                     : null;
             String moduleInfoModuleName = getThisModuleName();
-            ServiceProvider<ModuleComponent> moduleSp = lookupThisModule(moduleInfoModuleName, services);
-            String packageName = determinePackageName(Optional.ofNullable(moduleSp), serviceTypeNames, descriptor, true);
+            Optional<ServiceProvider<ModuleComponent>> moduleSp = lookupThisModule(moduleInfoModuleName, services, false);
+            String packageName = determinePackageName(moduleSp, serviceTypeNames, descriptor, true);
 
             CodeGenPaths codeGenPaths = CodeGenPathsDefault.builder()
                     .generatedSourcesPath(getGeneratedSourceDirectory().getPath())

pico/maven-plugin/src/main/java/io/helidon/pico/maven/plugin/AbstractCreatorMojo.java

@@ -21,7 +21,6 @@
 import java.util.Collection;
 import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Objects;
 import java.util.Optional;
 
 import io.helidon.common.types.TypeName;
@@ -35,6 +34,7 @@
 import io.helidon.pico.tools.ModuleUtils;
 import io.helidon.pico.tools.Options;
 import io.helidon.pico.tools.TemplateHelper;
+import io.helidon.pico.tools.ToolsException;
 
 import org.apache.maven.artifact.Artifact;
 import org.apache.maven.plugin.AbstractMojo;
@@ -66,6 +66,8 @@ public abstract class AbstractCreatorMojo extends AbstractMojo {
      */
     static final String TAG_FAIL_ON_WARNING = PicoServicesConfig.NAME + ".failOnWarning";
 
+    static final String TAG_PACKAGE_NAME = PicoServicesConfig.NAME + ".package.name";
+
     /**
      * The file name written to ./target/pico/ to track the last package name generated for this application.
      * This application package name is what we fall back to for the application name and the module name if not otherwise
@@ -135,7 +137,7 @@ public abstract class AbstractCreatorMojo extends AbstractMojo {
     /**
      * The package name to apply. If not found the package name will be inferred.
      */
-    @Parameter(property = PicoServicesConfig.NAME + ".package.name", readonly = true)
+    @Parameter(property = TAG_PACKAGE_NAME, readonly = true)
     private String packageName;
 
     /**
@@ -284,7 +286,10 @@ protected String determinePackageName(Optional<ServiceProvider<ModuleComponent>>
             }
         }
 
-        Objects.requireNonNull(packageName, "unable to determine package name");
+        if (packageName == null) {
+            throw new ToolsException("Unable to determine the package name. The package name can be set using "
+                                             + TAG_PACKAGE_NAME);
+        }
 
         if (persistIt) {
             // record it to scratch file for later consumption (during test build for example)

pico/maven-plugin/src/main/java/io/helidon/pico/maven/plugin/ExecHandler.java

@@ -86,7 +86,7 @@ <Req, Res> Res apply(Supplier<Req> reqSupplier,
                 }
                 result.set(fn.apply(req));
             } catch (Throwable t) {
-                throw new ToolsException("error in apply", t);
+                throw new ToolsException("An error occurred in apply", t);
             } finally {
                 latch.countDown();
             }

pico/maven-plugin/src/main/java/io/helidon/pico/maven/plugin/ExecutableClassLoader.java

@@ -49,7 +49,7 @@ public static URLClassLoader create(Collection<Path> classPath,
                 urls.add(dependency.toUri().toURL());
             }
         } catch (MalformedURLException e) {
-            throw new ToolsException("unable to build classpath", e);
+            throw new ToolsException("Unable to build the classpath", e);
         }
 
         if (parent == null) {

pico/maven-plugin/src/main/java/io/helidon/pico/maven/plugin/TestApplicationCreatorMojo.java

@@ -55,7 +55,7 @@
 public class TestApplicationCreatorMojo extends AbstractApplicationCreatorMojo {
 
     /**
-     * The classname to use for the Pico {@link io.helidon.pico.Application} test class.
+     * The classname to use for the Pico {@link io.helidon.pico.api.Application} test class.
      * If not found the classname will be inferred.
      */
     @Parameter(property = PicoServicesConfig.FQN + ".application.class.name", readonly = true