More external reader refinements (#766)

* org.pkl.core.Readers -> org.pkl.core.Closeables
* Remove coupling between pkl.core.module/resource and pkl.core.messaging

pkl-cli/src/main/kotlin/org/pkl/cli/CliEvaluator.kt

@@ -28,10 +28,10 @@ import org.pkl.commons.cli.CliException
 import org.pkl.commons.createParentDirectories
 import org.pkl.commons.currentWorkingDir
 import org.pkl.commons.writeString
+import org.pkl.core.Closeables
 import org.pkl.core.EvaluatorBuilder
 import org.pkl.core.ModuleSource
 import org.pkl.core.PklException
-import org.pkl.core.Readers
 import org.pkl.core.module.ModulePathResolver
 import org.pkl.core.runtime.ModuleResolver
 import org.pkl.core.runtime.VmException
@@ -100,8 +100,8 @@ constructor(
         writeOutput(builder)
       }
     } finally {
-      Readers.closeQuietly(builder.moduleKeyFactories)
+      Closeables.closeQuietly(builder.moduleKeyFactories)
-      Readers.closeQuietly(builder.resourceReaders)
+      Closeables.closeQuietly(builder.resourceReaders)
     }
   }
 

pkl-cli/src/main/kotlin/org/pkl/cli/CliImportAnalyzer.kt

@@ -19,8 +19,8 @@ import java.io.Writer
 import org.pkl.commons.cli.CliCommand
 import org.pkl.commons.createParentDirectories
 import org.pkl.commons.writeString
+import org.pkl.core.Closeables
 import org.pkl.core.ModuleSource
-import org.pkl.core.Readers
 
 class CliImportAnalyzer
 @JvmOverloads
@@ -73,8 +73,8 @@ constructor(
         .build()
         .use { it.evaluateOutputText(sourceModule) }
     } finally {
-      Readers.closeQuietly(builder.moduleKeyFactories)
+      Closeables.closeQuietly(builder.moduleKeyFactories)
-      Readers.closeQuietly(builder.resourceReaders)
+      Closeables.closeQuietly(builder.resourceReaders)
     }
   }
 }

pkl-cli/src/main/kotlin/org/pkl/cli/CliTestRunner.kt

@@ -17,9 +17,9 @@ package org.pkl.cli
 
 import java.io.Writer
 import org.pkl.commons.cli.*
+import org.pkl.core.Closeables
 import org.pkl.core.EvaluatorBuilder
 import org.pkl.core.ModuleSource.uri
-import org.pkl.core.Readers
 import org.pkl.core.stdlib.test.report.JUnitReport
 import org.pkl.core.stdlib.test.report.SimpleReport
 import org.pkl.core.util.ErrorMessages
@@ -38,8 +38,8 @@ constructor(
     try {
       evalTest(builder)
     } finally {
-      Readers.closeQuietly(builder.moduleKeyFactories)
+      Closeables.closeQuietly(builder.moduleKeyFactories)
-      Readers.closeQuietly(builder.resourceReaders)
+      Closeables.closeQuietly(builder.resourceReaders)
     }
   }
 

pkl-codegen-java/src/main/kotlin/org/pkl/codegen/java/CliJavaCodeGenerator.kt

@@ -20,8 +20,8 @@ import org.pkl.commons.cli.CliCommand
 import org.pkl.commons.cli.CliException
 import org.pkl.commons.createParentDirectories
 import org.pkl.commons.writeString
+import org.pkl.core.Closeables
 import org.pkl.core.ModuleSource
-import org.pkl.core.Readers
 
 /** API for the Java code generator CLI. */
 class CliJavaCodeGenerator(private val options: CliJavaCodeGeneratorOptions) :
@@ -49,8 +49,8 @@ class CliJavaCodeGenerator(private val options: CliJavaCodeGeneratorOptions) :
         }
       }
     } finally {
-      Readers.closeQuietly(builder.moduleKeyFactories)
+      Closeables.closeQuietly(builder.moduleKeyFactories)
-      Readers.closeQuietly(builder.resourceReaders)
+      Closeables.closeQuietly(builder.resourceReaders)
     }
   }
 }

pkl-codegen-kotlin/src/main/kotlin/org/pkl/codegen/kotlin/CliKotlinCodeGenerator.kt

@@ -20,8 +20,8 @@ import org.pkl.commons.cli.CliCommand
 import org.pkl.commons.cli.CliException
 import org.pkl.commons.createParentDirectories
 import org.pkl.commons.writeString
+import org.pkl.core.Closeables
 import org.pkl.core.ModuleSource
-import org.pkl.core.Readers
 
 /** API for the Kotlin code generator CLI. */
 class CliKotlinCodeGenerator(private val options: CliKotlinCodeGeneratorOptions) :
@@ -50,8 +50,8 @@ class CliKotlinCodeGenerator(private val options: CliKotlinCodeGeneratorOptions)
         }
       }
     } finally {
-      Readers.closeQuietly(builder.moduleKeyFactories)
+      Closeables.closeQuietly(builder.moduleKeyFactories)
-      Readers.closeQuietly(builder.resourceReaders)
+      Closeables.closeQuietly(builder.resourceReaders)
     }
   }
 }

pkl-core/src/main/java/org/pkl/core/Closeables.java

@@ -15,8 +15,8 @@
  */
 package org.pkl.core;
 
-public final class Readers {
+public final class Closeables {
-  private Readers() {}
+  private Closeables() {}
 
   /** Closes the given readers, ignoring any exceptions. */
   public static void closeQuietly(Iterable<? extends AutoCloseable> readers) {

pkl-core/src/main/java/org/pkl/core/externalreader/ExternalReaderProcess.java

@@ -17,8 +17,6 @@ package org.pkl.core.externalreader;
 
 import java.io.IOException;
 import org.pkl.core.evaluatorSettings.PklEvaluatorSettings.ExternalReader;
-import org.pkl.core.messaging.Messages.ModuleReaderSpec;
-import org.pkl.core.messaging.Messages.ResourceReaderSpec;
 import org.pkl.core.module.ExternalModuleResolver;
 import org.pkl.core.resource.ExternalResourceResolver;
 import org.pkl.core.util.Nullable;
@@ -58,8 +56,7 @@ public interface ExternalReaderProcess extends AutoCloseable {
    * @throws IllegalStateException if this process has already been {@linkplain #close closed}
    * @throws IOException if an I/O error occurs
    */
-  @Nullable
+  ExternalModuleResolver.@Nullable Spec getModuleReaderSpec(String scheme) throws IOException;
-  ModuleReaderSpec getModuleReaderSpec(String scheme) throws IOException;
 
   /**
    * Returns the spec, if available, of this process's resource reader with the given scheme.
@@ -67,8 +64,7 @@ public interface ExternalReaderProcess extends AutoCloseable {
    * @throws IllegalStateException if this process has already been {@linkplain #close closed}
    * @throws IOException if an I/O error occurs
    */
-  @Nullable
+  ExternalResourceResolver.@Nullable Spec getResourceReaderSpec(String scheme) throws IOException;
-  ResourceReaderSpec getResourceReaderSpec(String scheme) throws IOException;
 
   /**
    * Closes this process, releasing any associated resources.

pkl-core/src/main/java/org/pkl/core/externalreader/ExternalReaderProcessImpl.java

@@ -32,8 +32,6 @@ import org.pkl.core.evaluatorSettings.PklEvaluatorSettings.ExternalReader;
 import org.pkl.core.externalreader.ExternalReaderMessages.*;
 import org.pkl.core.messaging.MessageTransport;
 import org.pkl.core.messaging.MessageTransports;
-import org.pkl.core.messaging.Messages.ModuleReaderSpec;
-import org.pkl.core.messaging.Messages.ResourceReaderSpec;
 import org.pkl.core.messaging.ProtocolException;
 import org.pkl.core.module.ExternalModuleResolver;
 import org.pkl.core.resource.ExternalResourceResolver;
@@ -47,9 +45,9 @@ final class ExternalReaderProcessImpl implements ExternalReaderProcess {
 
   private final ExternalReader spec;
   private final @Nullable String logPrefix;
-  private final Map<String, Future<@Nullable ModuleReaderSpec>> initializeModuleReaderResponses =
+  private final Map<String, Future<ExternalModuleResolver.@Nullable Spec>>
-      new ConcurrentHashMap<>();
+      initializeModuleReaderResponses = new ConcurrentHashMap<>();
-  private final Map<String, Future<@Nullable ResourceReaderSpec>>
+  private final Map<String, Future<ExternalResourceResolver.@Nullable Spec>>
       initializeResourceReaderResponses = new ConcurrentHashMap<>();
   private final Random requestIdGenerator = new Random();
 
@@ -191,12 +189,13 @@ final class ExternalReaderProcessImpl implements ExternalReaderProcess {
   }
 
   @Override
-  public @Nullable ModuleReaderSpec getModuleReaderSpec(String uriScheme) throws IOException {
+  public ExternalModuleResolver.@Nullable Spec getModuleReaderSpec(String uriScheme)
+      throws IOException {
     return MessageTransports.resolveFuture(
         initializeModuleReaderResponses.computeIfAbsent(
             uriScheme,
             (scheme) -> {
-              var future = new CompletableFuture<@Nullable ModuleReaderSpec>();
+              var future = new CompletableFuture<ExternalModuleResolver.@Nullable Spec>();
               var request =
                   new InitializeModuleReaderRequest(requestIdGenerator.nextLong(), scheme);
               try {
@@ -219,12 +218,13 @@ final class ExternalReaderProcessImpl implements ExternalReaderProcess {
   }
 
   @Override
-  public @Nullable ResourceReaderSpec getResourceReaderSpec(String uriScheme) throws IOException {
+  public ExternalResourceResolver.@Nullable Spec getResourceReaderSpec(String uriScheme)
+      throws IOException {
     return MessageTransports.resolveFuture(
         initializeResourceReaderResponses.computeIfAbsent(
             uriScheme,
             (scheme) -> {
-              var future = new CompletableFuture<@Nullable ResourceReaderSpec>();
+              var future = new CompletableFuture<ExternalResourceResolver.@Nullable Spec>();
               var request =
                   new InitializeResourceReaderRequest(requestIdGenerator.nextLong(), scheme);
               try {

pkl-core/src/main/java/org/pkl/core/messaging/Messages.java

@@ -20,17 +20,20 @@ import java.util.Arrays;
 import java.util.List;
 import java.util.Objects;
 import org.pkl.core.messaging.Message.*;
+import org.pkl.core.module.ExternalModuleResolver;
 import org.pkl.core.module.PathElement;
+import org.pkl.core.resource.ExternalResourceResolver;
 import org.pkl.core.util.Nullable;
 
 public final class Messages {
   private Messages() {}
 
   public record ModuleReaderSpec(
-      String scheme, boolean hasHierarchicalUris, boolean isLocal, boolean isGlobbable) {}
+      String scheme, boolean hasHierarchicalUris, boolean isLocal, boolean isGlobbable)
+      implements ExternalModuleResolver.Spec {}
 
-  public record ResourceReaderSpec(
+  public record ResourceReaderSpec(String scheme, boolean hasHierarchicalUris, boolean isGlobbable)
-      String scheme, boolean hasHierarchicalUris, boolean isGlobbable) {}
+      implements ExternalResourceResolver.Spec {}
 
   public record ListResourcesRequest(long requestId, long evaluatorId, URI uri)
       implements Server.Request {

pkl-core/src/main/java/org/pkl/core/module/ExternalModuleResolver.java

@@ -35,6 +35,17 @@ import org.pkl.core.messaging.Messages.ReadModuleResponse;
 import org.pkl.core.messaging.ProtocolException;
 
 public class ExternalModuleResolver {
+
+  public interface Spec {
+    boolean hasHierarchicalUris();
+
+    boolean isGlobbable();
+
+    boolean isLocal();
+
+    String scheme();
+  }
+
   private final MessageTransport transport;
   private final long evaluatorId;
   private final Map<URI, Future<String>> readResponses = new ConcurrentHashMap<>();

pkl-core/src/main/java/org/pkl/core/module/ModuleKeyFactories.java

@@ -27,7 +27,7 @@ import java.util.List;
 import java.util.Optional;
 import java.util.ServiceLoader;
 import javax.annotation.concurrent.GuardedBy;
-import org.pkl.core.Readers;
+import org.pkl.core.Closeables;
 import org.pkl.core.externalreader.ExternalReaderProcess;
 import org.pkl.core.externalreader.ExternalReaderProcessException;
 import org.pkl.core.util.ErrorMessages;
@@ -102,7 +102,7 @@ public final class ModuleKeyFactories {
   /**
    * Closes the given factories, ignoring any exceptions.
    *
-   * @deprecated Replaced by {@link Readers#closeQuietly}.
+   * @deprecated Replaced by {@link Closeables#closeQuietly}.
    */
   @Deprecated(since = "0.27.0", forRemoval = true)
   public static void closeQuietly(Iterable<ModuleKeyFactory> factories) {
@@ -267,7 +267,7 @@ public final class ModuleKeyFactories {
     @GuardedBy("this")
     private ExternalModuleResolver resolver;
 
-    public ExternalProcess(String scheme, ExternalReaderProcess process, long evaluatorId) {
+    ExternalProcess(String scheme, ExternalReaderProcess process, long evaluatorId) {
       this.scheme = scheme;
       this.process = process;
       this.evaluatorId = evaluatorId;
@@ -283,8 +283,7 @@ public final class ModuleKeyFactories {
       return resolver;
     }
 
-    public Optional<ModuleKey> create(URI uri)
+    public Optional<ModuleKey> create(URI uri) throws ExternalReaderProcessException, IOException {
-        throws URISyntaxException, ExternalReaderProcessException, IOException {
       if (!scheme.equalsIgnoreCase(uri.getScheme())) return Optional.empty();
 
       var spec = process.getModuleReaderSpec(scheme);

pkl-core/src/main/java/org/pkl/core/module/ModuleKeys.java

@@ -30,7 +30,6 @@ import java.util.Map;
 import org.pkl.core.SecurityManager;
 import org.pkl.core.SecurityManagerException;
 import org.pkl.core.externalreader.ExternalReaderProcessException;
-import org.pkl.core.messaging.Messages.ModuleReaderSpec;
 import org.pkl.core.packages.Dependency;
 import org.pkl.core.packages.Dependency.LocalDependency;
 import org.pkl.core.packages.PackageAssetUri;
@@ -131,7 +130,7 @@ public final class ModuleKeys {
 
   /** Creates a module key for an externally read module. */
   public static ModuleKey externalResolver(
-      URI uri, ModuleReaderSpec spec, ExternalModuleResolver resolver) {
+      URI uri, ExternalModuleResolver.Spec spec, ExternalModuleResolver resolver) {
     return new ExternalResolver(uri, spec, resolver);
   }
 
@@ -779,10 +778,10 @@ public final class ModuleKeys {
   public static class ExternalResolver implements ModuleKey {
 
     private final URI uri;
-    private final ModuleReaderSpec spec;
+    private final ExternalModuleResolver.Spec spec;
     private final ExternalModuleResolver resolver;
 
-    public ExternalResolver(URI uri, ModuleReaderSpec spec, ExternalModuleResolver resolver) {
+    ExternalResolver(URI uri, ExternalModuleResolver.Spec spec, ExternalModuleResolver resolver) {
       this.uri = uri;
       this.spec = spec;
       this.resolver = resolver;

pkl-core/src/main/java/org/pkl/core/resource/ExternalResourceResolver.java

@@ -34,6 +34,15 @@ import org.pkl.core.messaging.ProtocolException;
 import org.pkl.core.module.PathElement;
 
 public class ExternalResourceResolver {
+
+  public interface Spec {
+    boolean hasHierarchicalUris();
+
+    boolean isGlobbable();
+
+    String scheme();
+  }
+
   private final MessageTransport transport;
   private final long evaluatorId;
   private final Map<URI, Future<byte[]>> readResponses = new ConcurrentHashMap<>();

pkl-core/src/main/java/org/pkl/core/resource/ResourceReaders.java

@@ -31,7 +31,6 @@ import org.pkl.core.SecurityManager;
 import org.pkl.core.SecurityManagerException;
 import org.pkl.core.externalreader.ExternalReaderProcess;
 import org.pkl.core.externalreader.ExternalReaderProcessException;
-import org.pkl.core.messaging.Messages.ResourceReaderSpec;
 import org.pkl.core.module.FileResolver;
 import org.pkl.core.module.ModulePathResolver;
 import org.pkl.core.module.PathElement;
@@ -163,7 +162,7 @@ public final class ResourceReaders {
 
   /** Returns a reader for external and client reader resources. */
   public static ResourceReader externalResolver(
-      ResourceReaderSpec spec, ExternalResourceResolver resolver) {
+      ExternalResourceResolver.Spec spec, ExternalResourceResolver resolver) {
     return new ExternalResolver(spec, resolver);
   }
 
@@ -692,10 +691,11 @@ public final class ResourceReaders {
   }
 
   private static final class ExternalResolver implements ResourceReader {
-    private final ResourceReaderSpec readerSpec;
+    private final ExternalResourceResolver.Spec readerSpec;
     private final ExternalResourceResolver resolver;
 
-    public ExternalResolver(ResourceReaderSpec readerSpec, ExternalResourceResolver resolver) {
+    public ExternalResolver(
+        ExternalResourceResolver.Spec readerSpec, ExternalResourceResolver resolver) {
       this.readerSpec = readerSpec;
       this.resolver = resolver;
     }
@@ -721,7 +721,7 @@ public final class ResourceReaders {
     }
 
     @Override
-    public boolean hasElement(org.pkl.core.SecurityManager securityManager, URI elementUri)
+    public boolean hasElement(SecurityManager securityManager, URI elementUri)
         throws SecurityManagerException {
       return resolver.hasElement(securityManager, elementUri);
     }

pkl-core/src/main/java/org/pkl/core/service/ExecutorSpiImpl.java

@@ -28,7 +28,6 @@ import java.util.Set;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import org.pkl.core.*;
-import org.pkl.core.Readers;
 import org.pkl.core.http.HttpClient;
 import org.pkl.core.module.ModuleKeyFactories;
 import org.pkl.core.module.ModulePathResolver;
@@ -135,8 +134,8 @@ public final class ExecutorSpiImpl implements ExecutorSpi {
     } catch (PklException e) {
       throw new ExecutorSpiException(e.getMessage(), e.getCause());
     } finally {
-      Readers.closeQuietly(builder.getModuleKeyFactories());
+      Closeables.closeQuietly(builder.getModuleKeyFactories());
-      Readers.closeQuietly(builder.getResourceReaders());
+      Closeables.closeQuietly(builder.getResourceReaders());
     }
   }
 

pkl-doc/src/main/kotlin/org/pkl/doc/CliDocGenerator.kt

@@ -24,7 +24,6 @@ import org.pkl.commons.cli.CliCommand
 import org.pkl.commons.cli.CliException
 import org.pkl.commons.toPath
 import org.pkl.core.*
-import org.pkl.core.Readers
 import org.pkl.core.packages.*
 
 /**
@@ -250,8 +249,8 @@ class CliDocGenerator(private val options: CliDocGeneratorOptions) : CliCommand(
         importedModules[pklBaseUri] = evaluator.evaluateSchema(ModuleSource.uri(pklBaseUri))
       }
     } finally {
-      Readers.closeQuietly(builder.moduleKeyFactories)
+      Closeables.closeQuietly(builder.moduleKeyFactories)
-      Readers.closeQuietly(builder.resourceReaders)
+      Closeables.closeQuietly(builder.resourceReaders)
     }
 
     val versions = mutableMapOf<String, Version>()