Skip to content

Commit e8a19da

Browse files
committed
Deprecating redundant magics: %jars, %addMavenDependency #89
.. also removing unused stuff from GlobFinder
1 parent 8e7118d commit e8a19da

8 files changed

Lines changed: 138 additions & 236 deletions

File tree

RELEASE-NOTES.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@
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-
- $86 ineffecient / incorrect classpath handling
15+
- #86 ineffecient / incorrect classpath handling
16+
- #89 Deprecating redundant magics: %jars, %addMavenDependency
1617

1718
## 1.0-a5
1819

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-jupyter/src/main/java/org/dflib/jjava/jupyter/kernel/util/GlobFinder.java renamed to jjava-jupyter/src/main/java/org/dflib/jjava/jupyter/kernel/util/GlobResolver.java

Lines changed: 79 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
import java.io.IOException;
44
import java.nio.file.DirectoryStream;
55
import java.nio.file.FileSystem;
6-
import java.nio.file.FileSystems;
76
import java.nio.file.Files;
87
import java.nio.file.Path;
98
import java.nio.file.attribute.BasicFileAttributes;
@@ -21,78 +20,9 @@
2120
* match a single character. A glob ending in {@code "/"} will match all files in a directories matching
2221
* the glob.
2322
* <p>
24-
* Important note for Windows file systems: Globs should use {@code "/"} to separate the glob despite it not being the
25-
* platform separator.
23+
* On Windows globs should use {@code "/"} to separate the glob despite it not being the platform separator.
2624
*/
27-
public class GlobFinder {
28-
private static class GlobSegment {
29-
public enum FilterRestriction {
30-
ONLY_FILES(true, false),
31-
ONLY_DIRECTORIES(false, true),
32-
ANYTHING(true, true);
33-
34-
private final boolean acceptsFiles;
35-
private final boolean acceptsDirectories;
36-
37-
FilterRestriction(boolean acceptsFiles, boolean acceptsDirectories) {
38-
this.acceptsFiles = acceptsFiles;
39-
this.acceptsDirectories = acceptsDirectories;
40-
}
41-
42-
public boolean acceptsFiles() {
43-
return acceptsFiles;
44-
}
45-
46-
public boolean acceptsDirectories() {
47-
return acceptsDirectories;
48-
}
49-
}
50-
51-
public static final GlobSegment ANY = new GlobSegment(Pattern.compile("^.*$"));
52-
53-
private final String literal;
54-
private final Pattern regex;
55-
56-
public GlobSegment(String literal) {
57-
this.literal = literal;
58-
this.regex = null;
59-
}
60-
61-
public GlobSegment(Pattern regex) {
62-
this.literal = null;
63-
this.regex = regex;
64-
}
65-
66-
public boolean isLiteral() {
67-
return this.literal != null;
68-
}
69-
70-
public DirectoryStream.Filter<Path> filter(FilterRestriction restriction) {
71-
return s -> {
72-
BasicFileAttributes attributes = Files.readAttributes(s, BasicFileAttributes.class);
73-
74-
if ((attributes.isRegularFile() && !restriction.acceptsFiles()) || (attributes.isDirectory() && !restriction.acceptsDirectories())) {
75-
return false;
76-
}
77-
78-
Path pathName = s.getFileName();
79-
80-
if (pathName == null) {
81-
return false;
82-
}
83-
84-
String name = pathName.toString();
85-
return this.literal != null
86-
? this.literal.equals(name)
87-
: this.regex.matcher(name).matches();
88-
};
89-
}
90-
91-
@Override
92-
public String toString() {
93-
return this.isLiteral() ? this.literal : this.regex.pattern();
94-
}
95-
}
25+
class GlobResolver {
9626

9727
private static final Pattern GLOB_SEGMENT_COMPONENT = Pattern.compile(
9828
"" +
@@ -106,12 +36,10 @@ public String toString() {
10636

10737
private final Path base;
10838
private final List<GlobSegment> segments;
109-
private final boolean isExplicitDirectory;
11039

111-
public GlobFinder(FileSystem fs, String glob) {
40+
public GlobResolver(FileSystem fs, String glob) {
11241
// Split with "/" but match with the actual separator
11342
String[] segments = SPLITTER.split(glob);
114-
this.isExplicitDirectory = glob.endsWith("/");
11543

11644
List<GlobSegment> matchers = new ArrayList<>(segments.length);
11745
int lastBaseSegmentIdx = 0;
@@ -169,72 +97,109 @@ public GlobFinder(FileSystem fs, String glob) {
16997
this.segments = matchers.subList(lastBaseSegmentIdx, matchers.size());
17098
}
17199

172-
public GlobFinder(String glob) {
173-
this(FileSystems.getDefault(), glob);
174-
}
175-
176-
public Iterable<Path> computeMatchingPaths() throws IOException {
177-
if (this.segments.isEmpty()) {
178-
if (Files.exists(this.base))
179-
return Collections.singletonList(this.base);
180-
else
181-
return Collections.emptyList();
100+
public List<Path> resolve() throws IOException {
101+
if (segments.isEmpty()) {
102+
return Files.exists(this.base)
103+
? Collections.singletonList(base)
104+
: Collections.emptyList();
182105
}
183106

184107
List<Path> paths = new ArrayList<>();
185108
GlobSegment head = this.segments.get(0);
186109
List<GlobSegment> tail = this.segments.subList(1, this.segments.size());
187110

188-
collectExplicit(GlobSegment.FilterRestriction.ANYTHING, this.base, head, tail, paths);
111+
collectExplicit(Filter.ANYTHING, this.base, head, tail, paths);
189112

190113
return paths;
191114
}
192115

193-
private void collectExplicit(GlobSegment.FilterRestriction finalFilterRestriction, Path dir, GlobSegment segment, List<GlobSegment> segments, Collection<Path> into) throws IOException {
116+
private void collectExplicit(
117+
Filter finalFilter,
118+
Path dir,
119+
GlobSegment segment,
120+
List<GlobSegment> segments,
121+
Collection<Path> into) throws IOException {
122+
194123
boolean isMoreSegments = !segments.isEmpty();
195124
// Should match files if there are more segments in which case this must be a directory so
196-
// we can continue. Otherwise we let the search determine if a file is acceptable.
197-
GlobSegment.FilterRestriction filterRestriction = isMoreSegments ? GlobSegment.FilterRestriction.ONLY_DIRECTORIES : finalFilterRestriction;
125+
// we can continue. Otherwise, we let the search determine if a file is acceptable.
126+
Filter filter = isMoreSegments ? Filter.ONLY_DIRECTORIES : finalFilter;
198127

199-
try (DirectoryStream<Path> files = Files.newDirectoryStream(dir, segment.filter(filterRestriction))) {
128+
try (DirectoryStream<Path> files = Files.newDirectoryStream(dir, segment.filter(filter))) {
200129
GlobSegment head = isMoreSegments ? segments.get(0) : null;
201130
List<GlobSegment> tail = isMoreSegments ? segments.subList(1, segments.size()) : Collections.emptyList();
202131

203132
for (Path p : files) {
204-
if (isMoreSegments)
205-
collectExplicit(finalFilterRestriction, p, head, tail, into);
206-
else
133+
if (isMoreSegments) {
134+
collectExplicit(finalFilter, p, head, tail, into);
135+
} else {
207136
into.add(p);
137+
}
208138
}
209139
}
210140
}
211141

212-
public Iterable<Path> computeMatchingFiles() throws IOException {
213-
if (this.segments.isEmpty()) {
214-
if (Files.isDirectory(this.base) && this.isExplicitDirectory)
215-
return Files.newDirectoryStream(this.base, Files::isRegularFile);
216-
if (Files.isRegularFile(this.base))
217-
return Collections.singleton(this.base);
218-
return Collections.emptyList();
142+
private enum Filter {
143+
ONLY_DIRECTORIES(false, true),
144+
ANYTHING(true, true);
145+
146+
private final boolean acceptsFiles;
147+
private final boolean acceptsDirectories;
148+
149+
Filter(boolean acceptsFiles, boolean acceptsDirectories) {
150+
this.acceptsFiles = acceptsFiles;
151+
this.acceptsDirectories = acceptsDirectories;
219152
}
220153

221-
List<Path> paths = new ArrayList<>();
222-
GlobSegment head = this.segments.get(0);
223-
List<GlobSegment> tail;
224-
225-
// If explicitly ends with a "/" then the pattern means match all files in this directory
226-
// otherwise we assume the last pattern is a file matcher.
227-
if (this.isExplicitDirectory) {
228-
tail = new ArrayList<>(this.segments.size() + 1);
229-
Collections.copy(tail, this.segments.subList(1, this.segments.size()));
230-
tail.add(GlobSegment.ANY);
231-
} else {
232-
tail = this.segments.subList(1, this.segments.size());
154+
public boolean acceptsFiles() {
155+
return acceptsFiles;
233156
}
234157

235-
collectExplicit(GlobSegment.FilterRestriction.ONLY_FILES, this.base, head, tail, paths);
158+
public boolean acceptsDirectories() {
159+
return acceptsDirectories;
160+
}
161+
}
236162

237-
return paths;
163+
private static class GlobSegment {
164+
165+
private final String literal;
166+
private final Pattern regex;
167+
168+
public GlobSegment(String literal) {
169+
this.literal = literal;
170+
this.regex = null;
171+
}
172+
173+
public GlobSegment(Pattern regex) {
174+
this.literal = null;
175+
this.regex = regex;
176+
}
177+
178+
public DirectoryStream.Filter<Path> filter(Filter restriction) {
179+
return s -> {
180+
BasicFileAttributes attributes = Files.readAttributes(s, BasicFileAttributes.class);
181+
182+
if ((attributes.isRegularFile() && !restriction.acceptsFiles()) || (attributes.isDirectory() && !restriction.acceptsDirectories())) {
183+
return false;
184+
}
185+
186+
Path pathName = s.getFileName();
187+
188+
if (pathName == null) {
189+
return false;
190+
}
191+
192+
String name = pathName.toString();
193+
return literal != null
194+
? literal.equals(name)
195+
: regex.matcher(name).matches();
196+
};
197+
}
198+
199+
@Override
200+
public String toString() {
201+
return literal != null ? literal : regex.pattern();
202+
}
238203
}
239204
}
240205

jjava-jupyter/src/main/java/org/dflib/jjava/jupyter/kernel/util/PathsHandler.java

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import java.io.File;
44
import java.io.IOException;
5+
import java.nio.file.FileSystems;
56
import java.nio.file.Path;
67
import java.util.ArrayList;
78
import java.util.Arrays;
@@ -10,7 +11,7 @@
1011
import java.util.stream.Collectors;
1112

1213
/**
13-
* Helper methods to split paths and resolve globs.
14+
* Helper methods to split and join paths and resolve glob patterns.
1415
*/
1516
public class PathsHandler {
1617

@@ -41,12 +42,7 @@ public static List<Path> splitAndResolveGlobs(String paths) {
4142
continue;
4243
}
4344

44-
Iterable<Path> subPaths;
45-
try {
46-
subPaths = new GlobFinder(p).computeMatchingPaths();
47-
} catch (IOException e) {
48-
throw new RuntimeException(String.format("Error computing classpath entries for '%s': %s", p, e.getMessage()), e);
49-
}
45+
List<Path> subPaths = resolveGlobs(p);
5046

5147
for (Path entry : subPaths) {
5248
result.add(entry);
@@ -55,4 +51,12 @@ public static List<Path> splitAndResolveGlobs(String paths) {
5551

5652
return result;
5753
}
54+
55+
public static List<Path> resolveGlobs(String path) {
56+
try {
57+
return new GlobResolver(FileSystems.getDefault(), path).resolve();
58+
} catch (IOException e) {
59+
throw new RuntimeException(String.format("Error resolving glob '%s': %s", path, e.getMessage()), e);
60+
}
61+
}
5862
}

0 commit comments

Comments
 (0)