From 1e23e876b69565805cf3f54820876967ed6a9ef3 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Mon, 5 Feb 2024 13:57:25 +0100 Subject: [PATCH 1/4] Construct cursor without reflection --- .../pl/net/was/trino/git/GitRecordSet.java | 34 ++++++------------- .../was/trino/git/RecordCursorProvider.java | 25 ++++++++++++++ 2 files changed, 35 insertions(+), 24 deletions(-) create mode 100644 src/main/java/pl/net/was/trino/git/RecordCursorProvider.java diff --git a/src/main/java/pl/net/was/trino/git/GitRecordSet.java b/src/main/java/pl/net/was/trino/git/GitRecordSet.java index 177d603..810f8d8 100644 --- a/src/main/java/pl/net/was/trino/git/GitRecordSet.java +++ b/src/main/java/pl/net/was/trino/git/GitRecordSet.java @@ -28,8 +28,6 @@ import java.io.File; import java.io.IOException; import java.io.UncheckedIOException; -import java.lang.reflect.Constructor; -import java.lang.reflect.InvocationTargetException; import java.net.URI; import java.util.List; import java.util.Map; @@ -78,30 +76,18 @@ public List getColumnTypes() @Override public RecordCursor cursor() { - Map> map = Map.of( - "branches", BranchesRecordCursor.class, - "commits", CommitsRecordCursor.class, - "diff_stats", DiffStatsRecordCursor.class, - "objects", ObjectsRecordCursor.class, - "tags", TagsRecordCursor.class, - "trees", TreesRecordCursor.class); - Class clazz = map.get(tableName); - if (clazz == null) { + Map map = Map.of( + "branches", BranchesRecordCursor::new, + "commits", CommitsRecordCursor::new, + "diff_stats", DiffStatsRecordCursor::new, + "objects", ObjectsRecordCursor::new, + "tags", TagsRecordCursor::new, + "trees", TreesRecordCursor::new); + RecordCursorProvider recordCursorProvider = map.get(tableName); + if (recordCursorProvider == null) { return null; } - Constructor ctr; - try { - ctr = clazz.getConstructor(List.class, Git.class, Optional.class); - } - catch (NoSuchMethodException e) { - throw new RuntimeException("Missing cursor constructor", e); - } - try { - return (RecordCursor) ctr.newInstance(columnHandles, repo, commitIds); - } - catch (InstantiationException | IllegalAccessException | InvocationTargetException e) { - throw new RuntimeException("Unknown exception", e); - } + return recordCursorProvider.create(columnHandles, repo, commitIds); } private Git getRepo(URI uri) diff --git a/src/main/java/pl/net/was/trino/git/RecordCursorProvider.java b/src/main/java/pl/net/was/trino/git/RecordCursorProvider.java new file mode 100644 index 0000000..063d34e --- /dev/null +++ b/src/main/java/pl/net/was/trino/git/RecordCursorProvider.java @@ -0,0 +1,25 @@ +/* + * 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 pl.net.was.trino.git; + +import io.trino.spi.connector.RecordCursor; +import org.eclipse.jgit.api.Git; + +import java.util.List; +import java.util.Optional; + +public interface RecordCursorProvider +{ + RecordCursor create(List columnHandles, Git repo, Optional> commitIds); +} From 5a84506e8715061ae93d54b0b9be92e5e4031b69 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Mon, 5 Feb 2024 13:59:56 +0100 Subject: [PATCH 2/4] Remove unused constructor parameters --- .../java/pl/net/was/trino/git/BranchesRecordCursor.java | 3 +-- src/main/java/pl/net/was/trino/git/GitRecordSet.java | 6 +++--- src/main/java/pl/net/was/trino/git/ObjectsRecordCursor.java | 3 +-- src/main/java/pl/net/was/trino/git/TagsRecordCursor.java | 3 +-- 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/main/java/pl/net/was/trino/git/BranchesRecordCursor.java b/src/main/java/pl/net/was/trino/git/BranchesRecordCursor.java index fd21a9d..c009fa4 100644 --- a/src/main/java/pl/net/was/trino/git/BranchesRecordCursor.java +++ b/src/main/java/pl/net/was/trino/git/BranchesRecordCursor.java @@ -27,7 +27,6 @@ import java.io.IOException; import java.util.Iterator; import java.util.List; -import java.util.Optional; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; @@ -46,7 +45,7 @@ public class BranchesRecordCursor private List fields; - public BranchesRecordCursor(List columnHandles, Git repo, Optional> commitIds) + public BranchesRecordCursor(List columnHandles, Git repo) { this.columnHandles = columnHandles; diff --git a/src/main/java/pl/net/was/trino/git/GitRecordSet.java b/src/main/java/pl/net/was/trino/git/GitRecordSet.java index 810f8d8..86c5678 100644 --- a/src/main/java/pl/net/was/trino/git/GitRecordSet.java +++ b/src/main/java/pl/net/was/trino/git/GitRecordSet.java @@ -77,11 +77,11 @@ public List getColumnTypes() public RecordCursor cursor() { Map map = Map.of( - "branches", BranchesRecordCursor::new, + "branches", (columnHandles, repo, commitIds) -> new BranchesRecordCursor(columnHandles, repo), "commits", CommitsRecordCursor::new, "diff_stats", DiffStatsRecordCursor::new, - "objects", ObjectsRecordCursor::new, - "tags", TagsRecordCursor::new, + "objects", (columnHandles, repo, commitIds) -> new ObjectsRecordCursor(columnHandles, repo), + "tags", (columnHandles, repo, commitIds) -> new TagsRecordCursor(columnHandles, repo), "trees", TreesRecordCursor::new); RecordCursorProvider recordCursorProvider = map.get(tableName); if (recordCursorProvider == null) { diff --git a/src/main/java/pl/net/was/trino/git/ObjectsRecordCursor.java b/src/main/java/pl/net/was/trino/git/ObjectsRecordCursor.java index 3ff9e2d..9fcaa65 100644 --- a/src/main/java/pl/net/was/trino/git/ObjectsRecordCursor.java +++ b/src/main/java/pl/net/was/trino/git/ObjectsRecordCursor.java @@ -30,7 +30,6 @@ import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.function.Function; import static com.google.common.base.Preconditions.checkArgument; @@ -49,7 +48,7 @@ public class ObjectsRecordCursor private final Map> strFieldGetters = new HashMap<>(); - public ObjectsRecordCursor(List columnHandles, Git repo, Optional> commitIds) + public ObjectsRecordCursor(List columnHandles, Git repo) { this.columnHandles = columnHandles; diff --git a/src/main/java/pl/net/was/trino/git/TagsRecordCursor.java b/src/main/java/pl/net/was/trino/git/TagsRecordCursor.java index 5fa83da..8666247 100644 --- a/src/main/java/pl/net/was/trino/git/TagsRecordCursor.java +++ b/src/main/java/pl/net/was/trino/git/TagsRecordCursor.java @@ -23,7 +23,6 @@ import java.util.Iterator; import java.util.List; -import java.util.Optional; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; @@ -38,7 +37,7 @@ public class TagsRecordCursor private List fields; - public TagsRecordCursor(List columnHandles, Git repo, Optional> commitIds) + public TagsRecordCursor(List columnHandles, Git repo) { this.columnHandles = columnHandles; From 104264301424d0fc07fb100365094dd3abe82584 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Mon, 5 Feb 2024 14:00:35 +0100 Subject: [PATCH 3/4] Mark final fields --- src/main/java/pl/net/was/trino/git/GitMetadata.java | 2 +- src/main/java/pl/net/was/trino/git/GitRecordSet.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/pl/net/was/trino/git/GitMetadata.java b/src/main/java/pl/net/was/trino/git/GitMetadata.java index edf1586..7065b2b 100644 --- a/src/main/java/pl/net/was/trino/git/GitMetadata.java +++ b/src/main/java/pl/net/was/trino/git/GitMetadata.java @@ -63,7 +63,7 @@ public class GitMetadata implements ConnectorMetadata { private final GitClient gitClient; - private String catalogName; + private final String catalogName; @Inject public GitMetadata(@CatalogName String catalogName, GitClient gitClient) diff --git a/src/main/java/pl/net/was/trino/git/GitRecordSet.java b/src/main/java/pl/net/was/trino/git/GitRecordSet.java index 86c5678..ce5992b 100644 --- a/src/main/java/pl/net/was/trino/git/GitRecordSet.java +++ b/src/main/java/pl/net/was/trino/git/GitRecordSet.java @@ -41,7 +41,7 @@ public class GitRecordSet private final List columnHandles; private final List columnTypes; private final String tableName; - private Git repo; + private final Git repo; private final Optional> commitIds; public GitRecordSet(GitSplit split, GitTableHandle table, List columnHandles) From 861ddf2a97bcfb58464431283df00df5ea307d70 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Mon, 5 Feb 2024 14:28:40 +0100 Subject: [PATCH 4/4] Delete directory recursively with a helper Lean on Guava. --- src/test/java/pl/net/was/trino/git/TestGitClient.java | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/test/java/pl/net/was/trino/git/TestGitClient.java b/src/test/java/pl/net/was/trino/git/TestGitClient.java index 994ae45..b8f4b76 100644 --- a/src/test/java/pl/net/was/trino/git/TestGitClient.java +++ b/src/test/java/pl/net/was/trino/git/TestGitClient.java @@ -24,13 +24,12 @@ import java.io.IOException; import java.io.PrintWriter; import java.net.URI; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.Comparator; import java.util.Date; import java.util.Set; import java.util.TimeZone; +import static com.google.common.io.MoreFiles.deleteRecursively; +import static com.google.common.io.RecursiveDeleteOption.ALLOW_INSECURE; import static org.assertj.core.api.Assertions.assertThat; public class TestGitClient @@ -54,10 +53,7 @@ public static void setupRepo(URI uri) return; } if (localPath.exists()) { - Files.walk(localPath.toPath()) - .sorted(Comparator.reverseOrder()) - .map(Path::toFile) - .forEach(File::delete); + deleteRecursively(localPath.toPath(), ALLOW_INSECURE); } Repository repository = FileRepositoryBuilder.create(new File(localPath, ".git"));