Skip to content

Commit e19417a

Browse files
Merge branch 'main' into feat-classpath-extension-loading
# Conflicts: # jjava-kernel/src/main/java/org/dflib/jjava/kernel/JavaKernel.java
2 parents 03d382e + 8ccda13 commit e19417a

39 files changed

Lines changed: 420 additions & 963 deletions

RELEASE-NOTES.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@
1212
- #82 Refactoring: Move JavaKernel init logic from constructor to Builder
1313
- #83 Simplify Kernel metadata loading approach
1414
- #84 Refactoring: reorg Maven modules for reusability
15-
15+
- #86 ineffecient / incorrect classpath handling
16+
- #89 Deprecate redundant magics: %jars, %addMavenDependency
17+
- #90 Deprecate Ivy artifact syntax for %maven magic
1618

1719
## 1.0-a5
1820

jjava-distro/src/main/java/org/dflib/jjava/distro/Env.java

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,12 @@
11
package org.dflib.jjava.distro;
22

3-
import org.dflib.jjava.jupyter.kernel.util.GlobFinder;
3+
import org.dflib.jjava.jupyter.kernel.util.PathsHandler;
44

5-
import java.io.File;
65
import java.io.IOException;
76
import java.nio.file.Files;
87
import java.nio.file.Path;
98
import java.util.ArrayList;
109
import java.util.List;
11-
import java.util.regex.Pattern;
1210

1311
/**
1412
* Defines environment variables supported by the kernel.
@@ -29,8 +27,6 @@ final class Env {
2927
// not used by JJava, but rather by the kernel launcher script
3028
public static final String JJAVA_JVM_OPTS = "JJAVA_JVM_OPTS";
3129

32-
private static final Pattern PATH_SPLITTER = Pattern.compile(File.pathSeparator, Pattern.LITERAL);
33-
3430
public static String timeout() {
3531
return System.getenv(Env.JJAVA_TIMEOUT);
3632
}
@@ -52,9 +48,8 @@ public static List<String> compilerOpts() {
5248
return optsString != null ? Opts.splitOpts(optsString) : java.util.List.of();
5349
}
5450

55-
public static List<String> extraClasspath() {
56-
String cpString = System.getenv(Env.JJAVA_CLASSPATH);
57-
return cpString != null ? java.util.List.of(PATH_SPLITTER.split(cpString)) : java.util.List.of();
51+
public static String extraClasspath() {
52+
return System.getenv(Env.JJAVA_CLASSPATH);
5853
}
5954

6055
public static List<String> startupSnippets() {
@@ -74,22 +69,12 @@ public static List<String> startupSnippets() {
7469
}
7570

7671
private static void appendSnippestFromScriptPaths(List<String> startupScripts, String scriptPaths) {
77-
for (String glob : PATH_SPLITTER.split(scriptPaths)) {
78-
Iterable<Path> globScriptPaths;
79-
80-
try {
81-
globScriptPaths = new GlobFinder(glob).computeMatchingPaths();
82-
} catch (IOException e) {
83-
throw new RuntimeException(java.lang.String.format("Error while computing startup scripts for '%s': %s", glob, e.getMessage()), e);
84-
}
85-
86-
for (Path path : globScriptPaths) {
87-
if (Files.isRegularFile(path) && Files.isReadable(path)) {
88-
try {
89-
startupScripts.add(Files.readString(path));
90-
} catch (IOException e) {
91-
throw new RuntimeException(java.lang.String.format("Error while loading startup script for '%s': %s", path, e.getMessage()), e);
92-
}
72+
for (Path path : PathsHandler.splitAndResolveGlobs(scriptPaths)) {
73+
if (Files.isRegularFile(path) && Files.isReadable(path)) {
74+
try {
75+
startupScripts.add(Files.readString(path));
76+
} catch (IOException e) {
77+
throw new RuntimeException(java.lang.String.format("Error while loading startup script for '%s': %s", path, e.getMessage()), e);
9378
}
9479
}
9580
}

jjava-distro/src/main/java/org/dflib/jjava/distro/JJava.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import org.dflib.jjava.kernel.magics.JarsMagic;
99
import org.dflib.jjava.kernel.magics.LoadCodeMagic;
1010
import org.dfllib.jjava.maven.MavenDependencyResolver;
11+
import org.dfllib.jjava.maven.magics.AddMavenDependencyMagic;
1112
import org.dfllib.jjava.maven.magics.LoadFromPomCellMagic;
1213
import org.dfllib.jjava.maven.magics.LoadFromPomLineMagic;
1314
import org.dfllib.jjava.maven.magics.MavenMagic;
@@ -62,8 +63,11 @@ public static void main(String[] args) throws Exception {
6263
.lineMagic("maven", new MavenMagic(mavenResolver))
6364
.lineMagic("mavenRepo", new MavenRepoMagic(mavenResolver))
6465
.lineMagic("loadFromPOM", new LoadFromPomLineMagic(mavenResolver))
65-
.lineMagic("jars", new JarsMagic()) // TODO: deprecate redundant "jars" alias; "classpath" is a superset of this
66-
.lineMagic("addMavenDependency", new MavenMagic(mavenResolver)) // TODO: deprecate redundant "addMavenDependency" alias
66+
67+
// temporarily support a few deprecated magics
68+
.lineMagic("jars", new JarsMagic())
69+
.lineMagic("addMavenDependency", new AddMavenDependencyMagic(mavenResolver))
70+
6771
.cellMagic("loadFromPOM", new LoadFromPomCellMagic(mavenResolver))
6872

6973
.build();

jjava-distro/src/test/java/org/dflib/jjava/distro/KernelMagicIT.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
public class KernelMagicIT extends ContainerizedKernelCase {
1212

13+
@Deprecated
1314
@Test
1415
public void jars() throws Exception {
1516
String jar = CONTAINER_RESOURCES + "/jakarta.annotation-api-3.0.0.jar";
@@ -45,7 +46,7 @@ public void classpath() throws Exception {
4546
}
4647

4748
@Test
48-
public void addMavenDependency() throws Exception {
49+
public void maven() throws Exception {
4950
String snippet = String.join("\n",
5051
"%maven org.dflib:dflib-jupyter:1.0.0-RC1",
5152
"System.getProperty(\"java.class.path\")"
@@ -56,8 +57,9 @@ public void addMavenDependency() throws Exception {
5657
assertThat(snippetResult.getStdout(), containsString("dflib-jupyter-1.0.0-RC1.jar"));
5758
}
5859

60+
@Deprecated
5961
@Test
60-
public void addIvyDependency() throws Exception {
62+
public void mavenIvySyntax() throws Exception {
6163
String snippet = String.join("\n",
6264
"%maven jakarta.annotation#jakarta.annotation-api;3.0.0",
6365
"System.getProperty(\"java.class.path\")"

jjava-jupyter/src/main/java/org/dflib/jjava/jupyter/ExtensionLoader.java

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package org.dflib.jjava.jupyter;
22

33
import org.dflib.jjava.jupyter.kernel.BaseKernel;
4+
import org.dflib.jjava.jupyter.kernel.util.PathsHandler;
45

56
import java.io.IOException;
67
import java.net.MalformedURLException;
@@ -12,7 +13,6 @@
1213
import java.util.ServiceLoader;
1314
import java.util.Set;
1415
import java.util.stream.Collectors;
15-
import java.util.stream.StreamSupport;
1616

1717
/**
1818
* This class is responsible for discovering and initializing a set of {@link Extension} instances.
@@ -35,24 +35,28 @@ public ExtensionLoader() {
3535
}
3636

3737
/**
38-
* Loads available {@code Extension} implementations and initializes them. Calls should manually invoke
39-
* {@link Extension#install(BaseKernel)} to initialize extension. This method performs discovery of extension
40-
* classes based on a {@link ServiceLoader} API, using the kernel default ClassLoader
38+
* Locates and loads {@code Extension} implementations. Extension classes discovery is performed with
39+
* {@link ServiceLoader}, using the kernel's default ClassLoader. Callers should manually invoke
40+
* {@link Extension#install(BaseKernel)} to initialize each extension.
41+
*
42+
* @return a list of the discovered extensions
4143
*/
42-
public List<Extension> loadFromClasspath() {
44+
public List<Extension> loadFromDefaultClasspath() {
4345
return load(getClass().getClassLoader());
4446
}
4547

4648
/**
47-
* Loads available {@code Extension} implementations and initializes them. Calls should manually invoke
48-
* {@link Extension#install(BaseKernel)} to initialize extension. This method performs discovery of extension
49-
* classes based on a {@link ServiceLoader} API, but only scanning the specified jars, not the entire classpath.
49+
* Locates and loads {@code Extension} implementations. Extension classes discovery is performed with
50+
* {@link ServiceLoader}, only scanning the locations present in the classpath argument, not the entire classpath.
51+
* Callers should invoke {@link Extension#install(BaseKernel)} to initialize each extension.
5052
*
51-
* @param jarPaths paths to jar files to scan for extensions
52-
* @return list of the available extensions
53+
* @param classpath one or more filesystem paths separated by {@link java.io.File#pathSeparator}.
54+
* @return a list of the discovered extensions
5355
*/
54-
public List<Extension> loadFromJars(Iterable<String> jarPaths) {
55-
URL[] urls = StreamSupport.stream(jarPaths.spliterator(), false)
56+
public List<Extension> loadFromClasspath(String classpath) {
57+
58+
URL[] urls = PathsHandler.split(classpath)
59+
.stream()
5660
.map(ExtensionLoader::toURL)
5761
.toArray(URL[]::new);
5862

jjava-jupyter/src/main/java/org/dflib/jjava/jupyter/kernel/BaseKernelBuilder.java

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@
1414

1515
import java.nio.charset.Charset;
1616
import java.nio.charset.StandardCharsets;
17-
import java.util.LinkedHashMap;
18-
import java.util.Map;
1917

2018
/**
2119
* A common builder superclass for BaseKernel subclasses.
@@ -31,12 +29,10 @@ public abstract class BaseKernelBuilder<
3129
protected MagicTranspiler magicTranspiler;
3230
protected HistoryManager historyManager;
3331
protected Boolean extensionsEnabled;
34-
protected final Map<String, LineMagic<?, ?>> lineMagics;
35-
protected final Map<String, CellMagic<?, ?>> cellMagics;
32+
protected final MagicsRegistry magicsRegistry;
3633

3734
protected BaseKernelBuilder() {
38-
this.cellMagics = new LinkedHashMap<>();
39-
this.lineMagics = new LinkedHashMap<>();
35+
this.magicsRegistry = new MagicsRegistry();
4036
}
4137

4238
public B name(String name) {
@@ -60,12 +56,12 @@ public B historyManager(HistoryManager historyManager) {
6056
}
6157

6258
public B lineMagic(String name, LineMagic<?, ?> magic) {
63-
lineMagics.put(name, magic);
59+
magicsRegistry.registerLineMagic(name, magic);
6460
return (B) this;
6561
}
6662

6763
public B cellMagic(String name, CellMagic<?, ?> magic) {
68-
cellMagics.put(name, magic);
64+
magicsRegistry.registerCellMagic(name, magic);
6965
return (B) this;
7066
}
7167

@@ -119,7 +115,7 @@ protected boolean buildExtensionsEnabled() {
119115
}
120116

121117
protected MagicsRegistry buildMagicsRegistry() {
122-
return new MagicsRegistry(lineMagics, cellMagics);
118+
return magicsRegistry;
123119
}
124120

125121
protected MagicTranspiler buildMagicTranspiler() {

jjava-jupyter/src/main/java/org/dflib/jjava/jupyter/kernel/comm/Comm.java

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,7 @@ public abstract class Comm {
1616

1717
private boolean closed = false;
1818

19-
public Comm(CommManager manager, String id, String targetName) {
20-
this(manager, id, targetName, null);
21-
}
22-
23-
public Comm(CommManager manager, String id, String targetName, JsonElement initializationData) {
19+
protected Comm(CommManager manager, String id, String targetName) {
2420
this.manager = manager;
2521
this.id = id;
2622
this.targetName = targetName;
@@ -47,7 +43,7 @@ public void send(JsonObject data, Map<String, Object> metadata, List<byte[]> blo
4743
}
4844

4945
/**
50-
* A callback for when the kernel receives a message who's destination is
46+
* A callback for when the kernel receives a message whose destination is
5147
* this comm. This handler gets access to the entire message so that if desired
5248
* the comms may make use of the low level blob segments or want to make use of
5349
* the parent, identities, etc.

jjava-jupyter/src/main/java/org/dflib/jjava/jupyter/kernel/comm/CommFactory.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,14 @@ public interface CommFactory<T extends Comm> {
88

99
/**
1010
* Create a new {@link Comm} and optionally attach data to the open message before it is sent.
11-
* @param manager the {@link CommManager} that will be responsible for transporting messages
12-
* to and from the created {@link Comm}.
13-
* @param id the id of the new {@link Comm}
14-
* @param target the name of the target on the front-end to communicate with
11+
*
12+
* @param manager the {@link CommManager} that will be responsible for transporting messages
13+
* to and from the created {@link Comm}.
14+
* @param id the id of the new {@link Comm}
15+
* @param target the name of the target on the front-end to communicate with
1516
* @param openMessageToSend the message that will be sent after creating the comm. There are 2 places to attach
1617
* additional data to the send
1718
* @return a new comm. If data must be immediately sent it should be appended to the {@code openMessageToSend}.
1819
*/
19-
public T produce(CommManager manager, String id, String target, Message<CommOpenCommand> openMessageToSend);
20+
T produce(CommManager manager, String id, String target, Message<CommOpenCommand> openMessageToSend);
2021
}

jjava-jupyter/src/main/java/org/dflib/jjava/jupyter/kernel/comm/CommManager.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public Comm getCommByID(String id) {
5959

6060
/**
6161
* Register a new comm that this manager should forward messages to in the event that
62-
* it receives one addressed to a comm with the the {@code comm}'s id.
62+
* it receives one addressed to a comm with the {@code comm}'s id.
6363
*
6464
* @param comm the comm to register with this handler
6565
*/
@@ -195,7 +195,7 @@ public CommTarget getTarget(String targetName) {
195195
}
196196

197197
// Default comm message handlers. These shouldn't need to be overridden but are more like
198-
// lambda targets that capture this comm manager in it's scope.
198+
// lambda targets that capture this comm manager in its scope.
199199

200200
public void handleCommOpenCommand(ReplyEnvironment env, Message<CommOpenCommand> commOpenCommandMessage) {
201201
CommOpenCommand openCommand = commOpenCommandMessage.getContent();

jjava-jupyter/src/main/java/org/dflib/jjava/jupyter/kernel/comm/CommTarget.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,12 @@
55

66
@FunctionalInterface
77
public interface CommTarget {
8+
89
/**
910
* Create a new comm as a result of the frontend making a {@code comm_open} request. This
1011
* is designed to be a constructor reference to a class that extends {@link Comm} overriding
1112
* the {@link Comm#onMessage(Message)}.
12-
*
13+
* <p>
1314
* For example a plain no-op handler may be {@code CommTarget noop = Comm::new;}. Which would create
1415
* comms that do nothing when the receive a message.
1516
*
@@ -20,8 +21,7 @@ public interface CommTarget {
2021
* @param msg the entire message that the manager received commanding it to open the comm. This may
2122
* carry additional data in the messages content. Specifically the {@link
2223
* CommOpenCommand#getData() data field}.
23-
*
2424
* @return the newly created comm
2525
*/
26-
public Comm createComm(CommManager commManager, String id, String targetName, Message<CommOpenCommand> msg);
26+
Comm createComm(CommManager commManager, String id, String targetName, Message<CommOpenCommand> msg);
2727
}

0 commit comments

Comments
 (0)