From dc3c2b9ca1ecb7aea4f817ff2915c692ccf7e1a3 Mon Sep 17 00:00:00 2001 From: elijahpetty <128415452+elijahpetty@users.noreply.github.com> Date: Fri, 6 Sep 2024 14:52:28 -0500 Subject: [PATCH 1/4] fix: Correct misspelling in TableDiff Javadoc (#6027) --- .../table/src/main/java/io/deephaven/engine/util/TableDiff.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/util/TableDiff.java b/engine/table/src/main/java/io/deephaven/engine/util/TableDiff.java index 179a3b9fc32..156b8219c58 100644 --- a/engine/table/src/main/java/io/deephaven/engine/util/TableDiff.java +++ b/engine/table/src/main/java/io/deephaven/engine/util/TableDiff.java @@ -182,7 +182,7 @@ public enum DiffItems { */ DoublesExact, /** - * Columns that exist in both tables, but in diferent orders are not treated as differences. + * Columns that exist in both tables, but in different orders are not treated as differences. */ ColumnsOrder, /** From 1bb5f093307006a414fa8c4027a235f914bb4f41 Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Fri, 6 Sep 2024 14:27:53 -0700 Subject: [PATCH 2/4] feat: add generic Iceberg catalog adapter creation to Java / Python (#5754) ### Java, connecting to a RESTCatalog using MinIO ``` import io.deephaven.iceberg.util.*; properties = new HashMap<>(); properties.put("type", "rest"); properties.put("uri", "http://rest:8181"); properties.put("client.region", "us-east-1"); properties.put("s3.access-key-id", "admin"); properties.put("s3.secret-access-key", "password"); properties.put("s3.endpoint", "http://minio:9000"); adapter = IcebergTools.createAdapter("generic-adapter", properties); ``` ### Python, connecting to a RESTCatalog using MinIO ``` from deephaven.experimental import iceberg adapter = iceberg.adapter(name="generic-adapter", properties={ "type" : "rest", "uri" : "http://rest:8181", "client.region" : "us-east-1", "s3.access-key-id" : "admin", "s3.secret-access-key" : "password", "s3.endpoint" : "http://minio:9000" }); ``` ### Java, connecting to AWS Glue NOTE: credentials set in local environment ``` import io.deephaven.iceberg.util.*; properties = new HashMap<>(); properties.put("type", "glue"); properties.put("uri", "s3://lab-warehouse/sales"); adapter = IcebergTools.createAdapter("generic-adapter", properties); ``` ### Python, connecting to AWS Glue NOTE: credentials set in local environment ``` from deephaven.experimental import iceberg adapter = iceberg.adapter(name="generic-adapter", properties={ "type" : "glue", "uri" : "s3://lab-warehouse/sales", "warehouse" : "s3://lab-warehouse/sales", }); ``` --- extensions/iceberg/s3/build.gradle | 4 +- .../iceberg/util/IcebergToolsS3.java | 10 +- .../util/S3InstructionsProviderPlugin.java | 54 ++++++++ .../iceberg/util/IcebergLocalStackTest.java | 7 + .../iceberg/util/IcebergMinIOTest.java | 8 ++ .../iceberg/util/IcebergToolsTest.java | 99 ++++++--------- .../DataInstructionsProviderLoader.java | 88 +++++++++++++ .../DataInstructionsProviderPlugin.java | 20 +++ .../iceberg/layout/IcebergBaseLayout.java | 22 +++- .../iceberg/layout/IcebergFlatLayout.java | 6 +- .../IcebergKeyValuePartitionedLayout.java | 8 +- .../iceberg/util/IcebergCatalogAdapter.java | 82 +++++++++--- .../deephaven/iceberg/util/IcebergTools.java | 120 +++++++++++++++++- .../TestCatalog/IcebergTestCatalog.java | 10 +- .../TestCatalog/IcebergTestFileIO.java | 49 ------- .../iceberg/TestCatalog/IcebergTestTable.java | 22 +++- .../s3/testlib/SingletonContainers.java | 17 +++ py/server/deephaven/experimental/iceberg.py | 74 +++++++++++ 18 files changed, 543 insertions(+), 157 deletions(-) create mode 100644 extensions/iceberg/s3/src/main/java/io/deephaven/iceberg/util/S3InstructionsProviderPlugin.java create mode 100644 extensions/iceberg/src/main/java/io/deephaven/iceberg/internal/DataInstructionsProviderLoader.java create mode 100644 extensions/iceberg/src/main/java/io/deephaven/iceberg/internal/DataInstructionsProviderPlugin.java delete mode 100644 extensions/iceberg/src/test/java/io/deephaven/iceberg/TestCatalog/IcebergTestFileIO.java diff --git a/extensions/iceberg/s3/build.gradle b/extensions/iceberg/s3/build.gradle index c580989fcb2..dfb53c52388 100644 --- a/extensions/iceberg/s3/build.gradle +++ b/extensions/iceberg/s3/build.gradle @@ -26,8 +26,10 @@ dependencies { runtimeOnly libs.awssdk.sts runtimeOnly libs.awssdk.glue - testImplementation libs.junit4 + compileOnly libs.autoservice + annotationProcessor libs.autoservice.compiler + testImplementation libs.junit4 testImplementation project(':engine-test-utils') testImplementation libs.testcontainers diff --git a/extensions/iceberg/s3/src/main/java/io/deephaven/iceberg/util/IcebergToolsS3.java b/extensions/iceberg/s3/src/main/java/io/deephaven/iceberg/util/IcebergToolsS3.java index 166b47e5d28..c545ab68540 100644 --- a/extensions/iceberg/s3/src/main/java/io/deephaven/iceberg/util/IcebergToolsS3.java +++ b/extensions/iceberg/s3/src/main/java/io/deephaven/iceberg/util/IcebergToolsS3.java @@ -5,11 +5,9 @@ import com.google.common.base.Strings; import org.apache.iceberg.CatalogProperties; -import org.apache.iceberg.CatalogUtil; import org.apache.iceberg.aws.AwsClientProperties; import org.apache.iceberg.aws.glue.GlueCatalog; import org.apache.iceberg.aws.s3.S3FileIOProperties; -import org.apache.iceberg.io.FileIO; import org.apache.iceberg.rest.RESTCatalog; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -68,12 +66,10 @@ public static IcebergCatalogAdapter createS3Rest( properties.put(S3FileIOProperties.ENDPOINT, endpointOverride); } - final FileIO fileIO = CatalogUtil.loadFileIO(S3_FILE_IO_CLASS, properties, null); - final String catalogName = name != null ? name : "IcebergCatalog-" + catalogURI; catalog.initialize(catalogName, properties); - return new IcebergCatalogAdapter(catalog, fileIO); + return new IcebergCatalogAdapter(catalog, properties); } /** @@ -101,11 +97,9 @@ public static IcebergCatalogAdapter createGlue( properties.put(CatalogProperties.URI, catalogURI); properties.put(CatalogProperties.WAREHOUSE_LOCATION, warehouseLocation); - final FileIO fileIO = CatalogUtil.loadFileIO(S3_FILE_IO_CLASS, properties, null); - final String catalogName = name != null ? name : "IcebergCatalog-" + catalogURI; catalog.initialize(catalogName, properties); - return new IcebergCatalogAdapter(catalog, fileIO); + return new IcebergCatalogAdapter(catalog, properties); } } diff --git a/extensions/iceberg/s3/src/main/java/io/deephaven/iceberg/util/S3InstructionsProviderPlugin.java b/extensions/iceberg/s3/src/main/java/io/deephaven/iceberg/util/S3InstructionsProviderPlugin.java new file mode 100644 index 00000000000..6302ede7ff9 --- /dev/null +++ b/extensions/iceberg/s3/src/main/java/io/deephaven/iceberg/util/S3InstructionsProviderPlugin.java @@ -0,0 +1,54 @@ +// +// Copyright (c) 2016-2024 Deephaven Data Labs and Patent Pending +// +package io.deephaven.iceberg.util; + +import com.google.auto.service.AutoService; +import io.deephaven.extensions.s3.Credentials; +import io.deephaven.extensions.s3.S3Instructions; +import io.deephaven.iceberg.internal.DataInstructionsProviderPlugin; +import org.apache.iceberg.aws.AwsClientProperties; +import org.apache.iceberg.aws.s3.S3FileIOProperties; +import org.jetbrains.annotations.NotNull; + +import java.net.URI; +import java.util.Map; + +/** + * {@link io.deephaven.iceberg.internal.DataInstructionsProviderPlugin} implementation used for reading files from S3. + */ +@AutoService(io.deephaven.iceberg.internal.DataInstructionsProviderPlugin.class) +@SuppressWarnings("unused") +public final class S3InstructionsProviderPlugin implements DataInstructionsProviderPlugin { + @Override + public Object createInstructions(@NotNull final URI uri, @NotNull final Map properties) { + // If the URI scheme is "s3","s3a","s3n" or if the properties contain one of these specific keys, we can + // create a useful S3Instructions object. + if (uri.getScheme().equals("s3") + || uri.getScheme().equals("s3a") + || uri.getScheme().equals("s3n") + || properties.containsKey(AwsClientProperties.CLIENT_REGION) + || properties.containsKey(S3FileIOProperties.ACCESS_KEY_ID) + || properties.containsKey(S3FileIOProperties.SECRET_ACCESS_KEY) + || properties.containsKey(S3FileIOProperties.ENDPOINT)) { + + final S3Instructions.Builder builder = S3Instructions.builder(); + if (properties.containsKey(AwsClientProperties.CLIENT_REGION)) { + builder.regionName(properties.get(AwsClientProperties.CLIENT_REGION)); + } + if (properties.containsKey(S3FileIOProperties.ENDPOINT)) { + builder.endpointOverride(properties.get(S3FileIOProperties.ENDPOINT)); + } + if (properties.containsKey(S3FileIOProperties.ACCESS_KEY_ID) + && properties.containsKey(S3FileIOProperties.SECRET_ACCESS_KEY)) { + builder.credentials( + Credentials.basic(properties.get(S3FileIOProperties.ACCESS_KEY_ID), + properties.get(S3FileIOProperties.SECRET_ACCESS_KEY))); + } + return builder.build(); + } + + // We have no useful properties for creating an S3Instructions object. + return null; + } +} diff --git a/extensions/iceberg/s3/src/test/java/io/deephaven/iceberg/util/IcebergLocalStackTest.java b/extensions/iceberg/s3/src/test/java/io/deephaven/iceberg/util/IcebergLocalStackTest.java index de15eceaa04..6683bd42db1 100644 --- a/extensions/iceberg/s3/src/test/java/io/deephaven/iceberg/util/IcebergLocalStackTest.java +++ b/extensions/iceberg/s3/src/test/java/io/deephaven/iceberg/util/IcebergLocalStackTest.java @@ -8,6 +8,8 @@ import org.junit.BeforeClass; import software.amazon.awssdk.services.s3.S3AsyncClient; +import java.util.Map; + public class IcebergLocalStackTest extends IcebergToolsTest { @BeforeClass @@ -25,4 +27,9 @@ public Builder s3Instructions(final Builder builder) { public S3AsyncClient s3AsyncClient() { return SingletonContainers.LocalStack.s3AsyncClient(); } + + @Override + public Map s3Properties() { + return SingletonContainers.LocalStack.s3Properties(); + } } diff --git a/extensions/iceberg/s3/src/test/java/io/deephaven/iceberg/util/IcebergMinIOTest.java b/extensions/iceberg/s3/src/test/java/io/deephaven/iceberg/util/IcebergMinIOTest.java index 0e789a64df2..946f3eca90d 100644 --- a/extensions/iceberg/s3/src/test/java/io/deephaven/iceberg/util/IcebergMinIOTest.java +++ b/extensions/iceberg/s3/src/test/java/io/deephaven/iceberg/util/IcebergMinIOTest.java @@ -10,6 +10,8 @@ import org.junit.BeforeClass; import software.amazon.awssdk.services.s3.S3AsyncClient; +import java.util.Map; + public class IcebergMinIOTest extends IcebergToolsTest { @BeforeClass @@ -29,4 +31,10 @@ public Builder s3Instructions(final Builder builder) { public S3AsyncClient s3AsyncClient() { return SingletonContainers.MinIO.s3AsyncClient(); } + + @Override + public Map s3Properties() { + return SingletonContainers.MinIO.s3Properties(); + } + } diff --git a/extensions/iceberg/s3/src/test/java/io/deephaven/iceberg/util/IcebergToolsTest.java b/extensions/iceberg/s3/src/test/java/io/deephaven/iceberg/util/IcebergToolsTest.java index 2218e9e3556..eb1640f07c2 100644 --- a/extensions/iceberg/s3/src/test/java/io/deephaven/iceberg/util/IcebergToolsTest.java +++ b/extensions/iceberg/s3/src/test/java/io/deephaven/iceberg/util/IcebergToolsTest.java @@ -12,18 +12,16 @@ import io.deephaven.engine.testutil.junit4.EngineCleanup; import io.deephaven.extensions.s3.S3Instructions; import io.deephaven.iceberg.TestCatalog.IcebergTestCatalog; -import io.deephaven.iceberg.TestCatalog.IcebergTestFileIO; import io.deephaven.test.types.OutOfBandTest; import org.apache.iceberg.Snapshot; import org.apache.iceberg.catalog.Catalog; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; -import org.apache.iceberg.io.FileIO; import org.junit.After; import org.junit.Before; import org.junit.Rule; -import org.junit.Test; import org.junit.experimental.categories.Category; +import org.junit.Test; import software.amazon.awssdk.core.async.AsyncRequestBody; import software.amazon.awssdk.services.s3.S3AsyncClient; import software.amazon.awssdk.services.s3.model.CreateBucketRequest; @@ -37,10 +35,7 @@ import java.time.LocalDate; import java.time.LocalDateTime; import java.time.LocalTime; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.List; +import java.util.*; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; @@ -106,6 +101,8 @@ public abstract class IcebergToolsTest { public abstract S3Instructions.Builder s3Instructions(S3Instructions.Builder builder); + public abstract Map s3Properties(); + private S3AsyncClient asyncClient; private String bucket; @@ -113,7 +110,6 @@ public abstract class IcebergToolsTest { private String warehousePath; private Catalog resourceCatalog; - private FileIO resourceFileIO; @Rule public final EngineCleanup framework = new EngineCleanup(); @@ -125,10 +121,9 @@ public void setUp() throws ExecutionException, InterruptedException { asyncClient.createBucket(CreateBucketRequest.builder().bucket(bucket).build()).get(); warehousePath = IcebergToolsTest.class.getResource("/warehouse").getPath(); - resourceFileIO = new IcebergTestFileIO("s3://warehouse", warehousePath); // Create the test catalog for the tests - resourceCatalog = IcebergTestCatalog.create(warehousePath, resourceFileIO); + resourceCatalog = IcebergTestCatalog.create(warehousePath, s3Properties()); final S3Instructions s3Instructions = s3Instructions(S3Instructions.builder()).build(); @@ -147,12 +142,12 @@ public void tearDown() throws ExecutionException, InterruptedException { asyncClient.close(); } - private void uploadParquetFiles(final File root, final String prefixToRemove) + private void uploadFiles(final File root, final String prefixToRemove) throws ExecutionException, InterruptedException, TimeoutException { for (final File file : root.listFiles()) { if (file.isDirectory()) { - uploadParquetFiles(file, prefixToRemove); - } else if (file.getName().endsWith(".parquet")) { + uploadFiles(file, prefixToRemove); + } else { final String key = file.getPath().substring(prefixToRemove.length() + 1); keys.add(key); @@ -169,33 +164,33 @@ private void uploadParquetFiles(final File root, final String prefixToRemove) } private void uploadSalesPartitioned() throws ExecutionException, InterruptedException, TimeoutException { - uploadParquetFiles(new File(IcebergToolsTest.class.getResource("/warehouse/sales/sales_partitioned").getPath()), + uploadFiles(new File(IcebergToolsTest.class.getResource("/warehouse/sales/sales_partitioned").getPath()), warehousePath); } private void uploadAllTypes() throws ExecutionException, InterruptedException, TimeoutException { - uploadParquetFiles(new File(IcebergToolsTest.class.getResource("/warehouse/sample/all_types").getPath()), + uploadFiles(new File(IcebergToolsTest.class.getResource("/warehouse/sample/all_types").getPath()), warehousePath); } private void uploadSalesSingle() throws ExecutionException, InterruptedException, TimeoutException { - uploadParquetFiles(new File(IcebergToolsTest.class.getResource("/warehouse/sales/sales_single").getPath()), + uploadFiles(new File(IcebergToolsTest.class.getResource("/warehouse/sales/sales_single").getPath()), warehousePath); } private void uploadSalesMulti() throws ExecutionException, InterruptedException, TimeoutException { - uploadParquetFiles(new File(IcebergToolsTest.class.getResource("/warehouse/sales/sales_multi").getPath()), + uploadFiles(new File(IcebergToolsTest.class.getResource("/warehouse/sales/sales_multi").getPath()), warehousePath); } private void uploadSalesRenamed() throws ExecutionException, InterruptedException, TimeoutException { - uploadParquetFiles(new File(IcebergToolsTest.class.getResource("/warehouse/sales/sales_renamed").getPath()), + uploadFiles(new File(IcebergToolsTest.class.getResource("/warehouse/sales/sales_renamed").getPath()), warehousePath); } @Test public void testListNamespaces() { - final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog, resourceFileIO); + final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog); final Collection namespaces = adapter.listNamespaces(); final Collection namespaceNames = @@ -212,7 +207,7 @@ public void testListNamespaces() { @Test public void testListTables() { - final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog, resourceFileIO); + final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog); final Namespace ns = Namespace.of("sales"); @@ -236,7 +231,7 @@ public void testListTables() { @Test public void testListSnapshots() { - final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog, resourceFileIO); + final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog); final TLongArrayList snapshotIds = new TLongArrayList(); final TableIdentifier tableIdentifier = TableIdentifier.of("sales", "sales_multi"); @@ -264,8 +259,7 @@ public void testListSnapshots() { public void testOpenTableA() throws ExecutionException, InterruptedException, TimeoutException { uploadSalesPartitioned(); - final IcebergCatalogAdapter adapter = - IcebergTools.createAdapter(resourceCatalog, resourceFileIO); + final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog); final Namespace ns = Namespace.of("sales"); final TableIdentifier tableId = TableIdentifier.of(ns, "sales_partitioned"); @@ -287,8 +281,7 @@ public void testOpenTableA() throws ExecutionException, InterruptedException, Ti public void testOpenTableB() throws ExecutionException, InterruptedException, TimeoutException { uploadSalesMulti(); - final IcebergCatalogAdapter adapter = - IcebergTools.createAdapter(resourceCatalog, resourceFileIO); + final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog); final Namespace ns = Namespace.of("sales"); final TableIdentifier tableId = TableIdentifier.of(ns, "sales_multi"); @@ -309,8 +302,7 @@ public void testOpenTableB() throws ExecutionException, InterruptedException, Ti public void testOpenTableC() throws ExecutionException, InterruptedException, TimeoutException { uploadSalesSingle(); - final IcebergCatalogAdapter adapter = - IcebergTools.createAdapter(resourceCatalog, resourceFileIO); + final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog); final Namespace ns = Namespace.of("sales"); final TableIdentifier tableId = TableIdentifier.of(ns, "sales_single"); @@ -332,7 +324,7 @@ public void testOpenTableC() throws ExecutionException, InterruptedException, Ti public void testOpenTableS3Only() throws ExecutionException, InterruptedException, TimeoutException { uploadSalesPartitioned(); - final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog, resourceFileIO); + final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog); final Namespace ns = Namespace.of("sales"); final TableIdentifier tableId = TableIdentifier.of(ns, "sales_partitioned"); @@ -352,8 +344,7 @@ public void testOpenTableDefinition() throws ExecutionException, InterruptedExce .dataInstructions(instructions.dataInstructions().get()) .build(); - final IcebergCatalogAdapter adapter = - IcebergTools.createAdapter(resourceCatalog, resourceFileIO); + final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog); final Namespace ns = Namespace.of("sales"); final TableIdentifier tableId = TableIdentifier.of(ns, "sales_partitioned"); @@ -380,8 +371,7 @@ public void testOpenTablePartitionTypeException() { .dataInstructions(instructions.dataInstructions().get()) .build(); - final IcebergCatalogAdapter adapter = - IcebergTools.createAdapter(resourceCatalog, resourceFileIO); + final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog); final Namespace ns = Namespace.of("sales"); final TableIdentifier tableId = TableIdentifier.of(ns, "sales_partitioned"); @@ -424,8 +414,7 @@ public void testOpenTableDefinitionRename() throws ExecutionException, Interrupt .putColumnRenames("month", "__month") .build(); - final IcebergCatalogAdapter adapter = - IcebergTools.createAdapter(resourceCatalog, resourceFileIO); + final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog); final Namespace ns = Namespace.of("sales"); final TableIdentifier tableId = TableIdentifier.of(ns, "sales_partitioned"); @@ -454,8 +443,7 @@ public void testSkippedPartitioningColumn() throws ExecutionException, Interrupt .dataInstructions(instructions.dataInstructions().get()) .build(); - final IcebergCatalogAdapter adapter = - IcebergTools.createAdapter(resourceCatalog, resourceFileIO); + final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog); final Namespace ns = Namespace.of("sales"); final TableIdentifier tableId = TableIdentifier.of(ns, "sales_partitioned"); @@ -484,8 +472,7 @@ public void testReorderedPartitioningColumn() throws ExecutionException, Interru .dataInstructions(instructions.dataInstructions().get()) .build(); - final IcebergCatalogAdapter adapter = - IcebergTools.createAdapter(resourceCatalog, resourceFileIO); + final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog); final Namespace ns = Namespace.of("sales"); final TableIdentifier tableId = TableIdentifier.of(ns, "sales_partitioned"); @@ -505,8 +492,7 @@ public void testZeroPartitioningColumns() throws ExecutionException, Interrupted .dataInstructions(instructions.dataInstructions().get()) .build(); - final IcebergCatalogAdapter adapter = - IcebergTools.createAdapter(resourceCatalog, resourceFileIO); + final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog); final Namespace ns = Namespace.of("sales"); final TableIdentifier tableId = TableIdentifier.of(ns, "sales_partitioned"); @@ -533,8 +519,7 @@ public void testIncorrectPartitioningColumns() throws ExecutionException, Interr .dataInstructions(instructions.dataInstructions().get()) .build(); - final IcebergCatalogAdapter adapter = - IcebergTools.createAdapter(resourceCatalog, resourceFileIO); + final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog); final Namespace ns = Namespace.of("sales"); final TableIdentifier tableId = TableIdentifier.of(ns, "sales_partitioned"); @@ -569,8 +554,7 @@ public void testMissingPartitioningColumns() { .dataInstructions(instructions.dataInstructions().get()) .build(); - final IcebergCatalogAdapter adapter = - IcebergTools.createAdapter(resourceCatalog, resourceFileIO); + final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog); final Namespace ns = Namespace.of("sales"); final TableIdentifier tableId = TableIdentifier.of(ns, "sales_partitioned"); @@ -598,8 +582,7 @@ public void testOpenTableColumnRename() throws ExecutionException, InterruptedEx .putColumnRenames("Item_Type", "ItemType") .build(); - final IcebergCatalogAdapter adapter = - IcebergTools.createAdapter(resourceCatalog, resourceFileIO); + final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog); final Namespace ns = Namespace.of("sales"); final TableIdentifier tableId = TableIdentifier.of(ns, "sales_partitioned"); @@ -617,8 +600,7 @@ public void testOpenTableColumnLegalization() throws ExecutionException, Interru .dataInstructions(instructions.dataInstructions().get()) .build(); - final IcebergCatalogAdapter adapter = - IcebergTools.createAdapter(resourceCatalog, resourceFileIO); + final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog); final Namespace ns = Namespace.of("sales"); final TableIdentifier tableId = TableIdentifier.of(ns, "sales_renamed"); @@ -640,8 +622,7 @@ public void testOpenTableColumnLegalizationRename() .putColumnRenames("Units/Sold", "Units_Sold") .build(); - final IcebergCatalogAdapter adapter = - IcebergTools.createAdapter(resourceCatalog, resourceFileIO); + final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog); final Namespace ns = Namespace.of("sales"); final TableIdentifier tableId = TableIdentifier.of(ns, "sales_renamed"); @@ -672,8 +653,7 @@ public void testOpenTableColumnLegalizationPartitionException() { .dataInstructions(instructions.dataInstructions().get()) .build(); - final IcebergCatalogAdapter adapter = - IcebergTools.createAdapter(resourceCatalog, resourceFileIO); + final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog); final Namespace ns = Namespace.of("sales"); final TableIdentifier tableId = TableIdentifier.of(ns, "sales_partitioned"); @@ -703,8 +683,7 @@ public void testOpenTableColumnRenamePartitioningColumns() .putColumnRenames("year", "__year") .build(); - final IcebergCatalogAdapter adapter = - IcebergTools.createAdapter(resourceCatalog, resourceFileIO); + final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog); final Namespace ns = Namespace.of("sales"); final TableIdentifier tableId = TableIdentifier.of(ns, "sales_partitioned"); @@ -728,7 +707,7 @@ public void testOpenTableColumnRenamePartitioningColumns() public void testOpenTableSnapshot() throws ExecutionException, InterruptedException, TimeoutException { uploadSalesMulti(); - final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog, resourceFileIO); + final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog); final Namespace ns = Namespace.of("sales"); final TableIdentifier tableId = TableIdentifier.of(ns, "sales_multi"); @@ -760,7 +739,7 @@ public void testOpenTableSnapshot() throws ExecutionException, InterruptedExcept public void testOpenTableSnapshotByID() throws ExecutionException, InterruptedException, TimeoutException { uploadSalesMulti(); - final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog, resourceFileIO); + final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog); final Namespace ns = Namespace.of("sales"); final TableIdentifier tableId = TableIdentifier.of(ns, "sales_multi"); @@ -807,7 +786,7 @@ public void testOpenTableSnapshotByID() throws ExecutionException, InterruptedEx public void testOpenAllTypesTable() throws ExecutionException, InterruptedException, TimeoutException { uploadAllTypes(); - final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog, resourceFileIO); + final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog); final Namespace ns = Namespace.of("sample"); final TableIdentifier tableId = TableIdentifier.of(ns, "all_types"); @@ -820,7 +799,7 @@ public void testOpenAllTypesTable() throws ExecutionException, InterruptedExcept @Test public void testTableDefinition() { - final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog, resourceFileIO); + final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog); final Namespace ns = Namespace.of("sales"); final TableIdentifier tableId = TableIdentifier.of(ns, "sales_multi"); @@ -845,7 +824,7 @@ public void testTableDefinition() { @Test public void testTableDefinitionTable() { - final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog, resourceFileIO); + final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog); final Namespace ns = Namespace.of("sales"); final TableIdentifier tableId = TableIdentifier.of(ns, "sales_multi"); @@ -878,7 +857,7 @@ public void testTableDefinitionTable() { @Test public void testTableDefinitionWithInstructions() { - final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog, resourceFileIO); + final IcebergCatalogAdapter adapter = IcebergTools.createAdapter(resourceCatalog); IcebergInstructions localInstructions = IcebergInstructions.builder() .dataInstructions(instructions.dataInstructions().get()) diff --git a/extensions/iceberg/src/main/java/io/deephaven/iceberg/internal/DataInstructionsProviderLoader.java b/extensions/iceberg/src/main/java/io/deephaven/iceberg/internal/DataInstructionsProviderLoader.java new file mode 100644 index 00000000000..4ae28e0e044 --- /dev/null +++ b/extensions/iceberg/src/main/java/io/deephaven/iceberg/internal/DataInstructionsProviderLoader.java @@ -0,0 +1,88 @@ +// +// Copyright (c) 2016-2024 Deephaven Data Labs and Patent Pending +// +package io.deephaven.iceberg.internal; + +import org.jetbrains.annotations.NotNull; + +import java.net.URI; +import java.util.*; + +/** + * A service loader class for loading {@link DataInstructionsProviderPlugin} implementations at runtime which provide + * {@link DataInstructionsProviderLoader} implementations for different URI paths. + */ +public final class DataInstructionsProviderLoader { + /** + * The list of plugins loaded by the {@link ServiceLoader}. + */ + private static volatile List cachedProviders; + + /** + * Ensure that the {@link DataInstructionsProviderPlugin plugins} are loaded exactly once. + */ + private static void ensureProviders() { + if (cachedProviders == null) { + synchronized (DataInstructionsProviderLoader.class) { + if (cachedProviders == null) { + cachedProviders = new ArrayList<>(); + // Load the plugins + for (final DataInstructionsProviderPlugin plugin : ServiceLoader + .load(DataInstructionsProviderPlugin.class)) { + cachedProviders.add(plugin); + } + } + } + } + } + + /** + * Get a {@link DataInstructionsProviderLoader} instance for the given property collection. + * + * @param properties The property collection. + * @return A {@link DataInstructionsProviderLoader} instance. + */ + public static DataInstructionsProviderLoader create(final Map properties) { + ensureProviders(); + return new DataInstructionsProviderLoader(properties); + } + + /** + * The properties collection for this instance. + */ + private final Map properties; + + /** + * The local list of plugins loaded by the {@link ServiceLoader}. + */ + private final List providers; + + /** + * Create a new {@link DataInstructionsProviderLoader} instance for the given property collection. + * + * @param properties The property collection. + */ + private DataInstructionsProviderLoader(final Map properties) { + this.properties = properties; + providers = cachedProviders; + } + + /** + * Create a new data instructions object compatible with reading from and writing to the given URI, using the + * plugins loaded by the {@link ServiceLoader}. For example, for a "S3" URI, we will create an + * {@code S3Instructions} object which can read files from S3. + * + * @param uri The URI + * @return A data instructions object for the given URI or null if one cannot be found + */ + public Object fromServiceLoader(@NotNull final URI uri) { + for (final DataInstructionsProviderPlugin plugin : providers) { + final Object pluginInstructions = plugin.createInstructions(uri, properties); + if (pluginInstructions != null) { + return pluginInstructions; + } + } + // No plugin found for this URI and property collection. + return null; + } +} diff --git a/extensions/iceberg/src/main/java/io/deephaven/iceberg/internal/DataInstructionsProviderPlugin.java b/extensions/iceberg/src/main/java/io/deephaven/iceberg/internal/DataInstructionsProviderPlugin.java new file mode 100644 index 00000000000..b0e48cc9166 --- /dev/null +++ b/extensions/iceberg/src/main/java/io/deephaven/iceberg/internal/DataInstructionsProviderPlugin.java @@ -0,0 +1,20 @@ +// +// Copyright (c) 2016-2024 Deephaven Data Labs and Patent Pending +// +package io.deephaven.iceberg.internal; + +import org.jetbrains.annotations.NotNull; + +import java.net.URI; +import java.util.Map; + +/** + * A plugin interface for providing {@link DataInstructionsProviderPlugin} implementations for different property + * collections and URI values. Check out {@link DataInstructionsProviderLoader} for more details. + */ +public interface DataInstructionsProviderPlugin { + /** + * Create a data instructions object for the given URI. + */ + Object createInstructions(@NotNull URI uri, @NotNull final Map properties); +} diff --git a/extensions/iceberg/src/main/java/io/deephaven/iceberg/layout/IcebergBaseLayout.java b/extensions/iceberg/src/main/java/io/deephaven/iceberg/layout/IcebergBaseLayout.java index f5334cf866c..7bf0f5222a2 100644 --- a/extensions/iceberg/src/main/java/io/deephaven/iceberg/layout/IcebergBaseLayout.java +++ b/extensions/iceberg/src/main/java/io/deephaven/iceberg/layout/IcebergBaseLayout.java @@ -11,6 +11,7 @@ import io.deephaven.iceberg.location.IcebergTableParquetLocationKey; import io.deephaven.iceberg.util.IcebergInstructions; import io.deephaven.parquet.table.ParquetInstructions; +import io.deephaven.iceberg.internal.DataInstructionsProviderLoader; import org.apache.iceberg.*; import org.apache.iceberg.io.FileIO; import org.jetbrains.annotations.NotNull; @@ -53,6 +54,11 @@ public abstract class IcebergBaseLayout implements TableLocationKeyFinder cache; + /** + * The data instructions provider for creating instructions from URI and user-supplied properties. + */ + final DataInstructionsProviderLoader dataInstructionsProvider; + /** * The {@link ParquetInstructions} object that will be used to read any Parquet data files in this table. Only * accessed while synchronized on {@code this}. @@ -79,8 +85,16 @@ protected IcebergTableLocationKey locationKey( } } - // Add the data instructions. - instructions.dataInstructions().ifPresent(builder::setSpecialInstructions); + // Add the data instructions if provided as part of the IcebergInstructions. + if (instructions.dataInstructions().isPresent()) { + builder.setSpecialInstructions(instructions.dataInstructions().get()); + } else { + // Attempt to create data instructions from the properties collection and URI. + final Object dataInstructions = dataInstructionsProvider.fromServiceLoader(fileUri); + if (dataInstructions != null) { + builder.setSpecialInstructions(dataInstructions); + } + } parquetInstructions = builder.build(); } @@ -102,12 +116,14 @@ public IcebergBaseLayout( @NotNull final Table table, @NotNull final Snapshot tableSnapshot, @NotNull final FileIO fileIO, - @NotNull final IcebergInstructions instructions) { + @NotNull final IcebergInstructions instructions, + @NotNull final DataInstructionsProviderLoader dataInstructionsProvider) { this.tableDef = tableDef; this.table = table; this.snapshot = tableSnapshot; this.fileIO = fileIO; this.instructions = instructions; + this.dataInstructionsProvider = dataInstructionsProvider; this.cache = new HashMap<>(); } diff --git a/extensions/iceberg/src/main/java/io/deephaven/iceberg/layout/IcebergFlatLayout.java b/extensions/iceberg/src/main/java/io/deephaven/iceberg/layout/IcebergFlatLayout.java index ac4c19283f9..fd407d7702e 100644 --- a/extensions/iceberg/src/main/java/io/deephaven/iceberg/layout/IcebergFlatLayout.java +++ b/extensions/iceberg/src/main/java/io/deephaven/iceberg/layout/IcebergFlatLayout.java @@ -7,6 +7,7 @@ import io.deephaven.engine.table.impl.locations.impl.TableLocationKeyFinder; import io.deephaven.iceberg.location.IcebergTableLocationKey; import io.deephaven.iceberg.util.IcebergInstructions; +import io.deephaven.iceberg.internal.DataInstructionsProviderLoader; import org.apache.iceberg.*; import org.apache.iceberg.io.FileIO; import org.jetbrains.annotations.NotNull; @@ -30,8 +31,9 @@ public IcebergFlatLayout( @NotNull final Table table, @NotNull final Snapshot tableSnapshot, @NotNull final FileIO fileIO, - @NotNull final IcebergInstructions instructions) { - super(tableDef, table, tableSnapshot, fileIO, instructions); + @NotNull final IcebergInstructions instructions, + @NotNull final DataInstructionsProviderLoader dataInstructionsProvider) { + super(tableDef, table, tableSnapshot, fileIO, instructions, dataInstructionsProvider); } @Override diff --git a/extensions/iceberg/src/main/java/io/deephaven/iceberg/layout/IcebergKeyValuePartitionedLayout.java b/extensions/iceberg/src/main/java/io/deephaven/iceberg/layout/IcebergKeyValuePartitionedLayout.java index 47ec05dfd74..f1a3cc9a5ea 100644 --- a/extensions/iceberg/src/main/java/io/deephaven/iceberg/layout/IcebergKeyValuePartitionedLayout.java +++ b/extensions/iceberg/src/main/java/io/deephaven/iceberg/layout/IcebergKeyValuePartitionedLayout.java @@ -9,6 +9,7 @@ import io.deephaven.engine.table.impl.locations.impl.TableLocationKeyFinder; import io.deephaven.iceberg.location.IcebergTableLocationKey; import io.deephaven.iceberg.util.IcebergInstructions; +import io.deephaven.iceberg.internal.DataInstructionsProviderLoader; import io.deephaven.util.type.TypeUtils; import org.apache.commons.lang3.mutable.MutableInt; import org.apache.iceberg.*; @@ -24,7 +25,7 @@ * a {@link Snapshot} */ public final class IcebergKeyValuePartitionedLayout extends IcebergBaseLayout { - private class ColumnData { + private static class ColumnData { final String name; final Class type; final int index; @@ -52,8 +53,9 @@ public IcebergKeyValuePartitionedLayout( @NotNull final org.apache.iceberg.Snapshot tableSnapshot, @NotNull final FileIO fileIO, @NotNull final PartitionSpec partitionSpec, - @NotNull final IcebergInstructions instructions) { - super(tableDef, table, tableSnapshot, fileIO, instructions); + @NotNull final IcebergInstructions instructions, + @NotNull final DataInstructionsProviderLoader dataInstructionsProvider) { + super(tableDef, table, tableSnapshot, fileIO, instructions, dataInstructionsProvider); // We can assume due to upstream validation that there are no duplicate names (after renaming) that are included // in the output definition, so we can ignore duplicates. diff --git a/extensions/iceberg/src/main/java/io/deephaven/iceberg/util/IcebergCatalogAdapter.java b/extensions/iceberg/src/main/java/io/deephaven/iceberg/util/IcebergCatalogAdapter.java index 4955ca9223b..b76a750602d 100644 --- a/extensions/iceberg/src/main/java/io/deephaven/iceberg/util/IcebergCatalogAdapter.java +++ b/extensions/iceberg/src/main/java/io/deephaven/iceberg/util/IcebergCatalogAdapter.java @@ -18,6 +18,7 @@ import io.deephaven.engine.table.impl.sources.regioned.RegionedTableComponentFactoryImpl; import io.deephaven.engine.updategraph.UpdateSourceRegistrar; import io.deephaven.engine.util.TableTools; +import io.deephaven.iceberg.internal.DataInstructionsProviderLoader; import io.deephaven.iceberg.layout.IcebergFlatLayout; import io.deephaven.iceberg.layout.IcebergKeyValuePartitionedLayout; import io.deephaven.iceberg.location.IcebergTableLocationFactory; @@ -32,7 +33,6 @@ import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.SupportsNamespaces; import org.apache.iceberg.catalog.TableIdentifier; -import org.apache.iceberg.io.FileIO; import org.apache.iceberg.types.Type; import org.apache.iceberg.types.Types; import org.jetbrains.annotations.NotNull; @@ -65,16 +65,25 @@ public class IcebergCatalogAdapter { ColumnDefinition.fromGenericType("SnapshotObject", Snapshot.class)); private final Catalog catalog; - private final FileIO fileIO; + + private final DataInstructionsProviderLoader dataInstructionsProvider; + + /** + * Construct an IcebergCatalogAdapter from a catalog. + */ + IcebergCatalogAdapter(@NotNull final Catalog catalog) { + this(catalog, Map.of()); + } /** - * Construct an IcebergCatalogAdapter from a catalog and file IO. + * Construct an IcebergCatalogAdapter from a catalog and property collection. */ IcebergCatalogAdapter( @NotNull final Catalog catalog, - @NotNull final FileIO fileIO) { + @NotNull final Map properties) { this.catalog = catalog; - this.fileIO = fileIO; + + dataInstructionsProvider = DataInstructionsProviderLoader.create(Map.copyOf(properties)); } /** @@ -648,6 +657,54 @@ public Table readTable( return readTable(TableIdentifier.parse(tableIdentifier), instructions); } + /** + * Retrieve a snapshot of an Iceberg table from the Iceberg catalog. + * + * @param tableIdentifier The table identifier to load + * @param tableSnapshotId The snapshot id to load + * @return The loaded table + * @throws IllegalArgumentException if the snapshot with the given id is not found + */ + private Snapshot getTableSnapshot(@NotNull TableIdentifier tableIdentifier, long tableSnapshotId) { + return listSnapshots(tableIdentifier).stream() + .filter(snapshot -> snapshot.snapshotId() == tableSnapshotId) + .findFirst() + .orElseThrow(() -> new IllegalArgumentException( + "Snapshot with id " + tableSnapshotId + " for table " + tableIdentifier + " not found")); + } + + /** + * Read a static snapshot of an Iceberg table from the Iceberg catalog. + * + * @param tableIdentifier The table identifier to load + * @param tableSnapshotId The snapshot id to load + * @return The loaded table + */ + @SuppressWarnings("unused") + public Table readTable(@NotNull final TableIdentifier tableIdentifier, final long tableSnapshotId) { + // Find the snapshot with the given snapshot id + final Snapshot tableSnapshot = getTableSnapshot(tableIdentifier, tableSnapshotId); + + return readTableInternal(tableIdentifier, tableSnapshot, null); + } + + + /** + * Read a static snapshot of an Iceberg table from the Iceberg catalog. + * + * @param tableIdentifier The table identifier to load + * @param tableSnapshotId The snapshot id to load + * @return The loaded table + */ + @SuppressWarnings("unused") + public Table readTable(@NotNull final String tableIdentifier, final long tableSnapshotId) { + final TableIdentifier tableId = TableIdentifier.parse(tableIdentifier); + // Find the snapshot with the given snapshot id + final Snapshot tableSnapshot = getTableSnapshot(tableId, tableSnapshotId); + + return readTableInternal(tableId, tableSnapshot, null); + } + /** * Read a static snapshot of an Iceberg table from the Iceberg catalog. * @@ -738,11 +795,12 @@ private Table readTableInternal( if (partitionSpec.isUnpartitioned()) { // Create the flat layout location key finder - keyFinder = new IcebergFlatLayout(tableDef, table, snapshot, fileIO, userInstructions); + keyFinder = new IcebergFlatLayout(tableDef, table, snapshot, table.io(), userInstructions, + dataInstructionsProvider); } else { // Create the partitioning column location key finder - keyFinder = new IcebergKeyValuePartitionedLayout(tableDef, table, snapshot, fileIO, partitionSpec, - userInstructions); + keyFinder = new IcebergKeyValuePartitionedLayout(tableDef, table, snapshot, table.io(), partitionSpec, + userInstructions, dataInstructionsProvider); } refreshService = null; @@ -772,12 +830,4 @@ private Table readTableInternal( public Catalog catalog() { return catalog; } - - /** - * Returns the underlying Iceberg {@link FileIO} used by this adapter. - */ - @SuppressWarnings("unused") - public FileIO fileIO() { - return fileIO; - } } diff --git a/extensions/iceberg/src/main/java/io/deephaven/iceberg/util/IcebergTools.java b/extensions/iceberg/src/main/java/io/deephaven/iceberg/util/IcebergTools.java index bcdda326dca..5dd20f699a9 100644 --- a/extensions/iceberg/src/main/java/io/deephaven/iceberg/util/IcebergTools.java +++ b/extensions/iceberg/src/main/java/io/deephaven/iceberg/util/IcebergTools.java @@ -3,8 +3,14 @@ // package io.deephaven.iceberg.util; +import org.apache.hadoop.conf.Configuration; +import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.CatalogUtil; import org.apache.iceberg.catalog.Catalog; -import org.apache.iceberg.io.FileIO; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.Map; /** * Tools for accessing tables in the Iceberg table format. @@ -12,8 +18,114 @@ public abstract class IcebergTools { @SuppressWarnings("unused") public static IcebergCatalogAdapter createAdapter( - final Catalog catalog, - final FileIO fileIO) { - return new IcebergCatalogAdapter(catalog, fileIO); + final Catalog catalog) { + return new IcebergCatalogAdapter(catalog); + } + + /** + *

+ * Create an Iceberg catalog adapter for an Iceberg catalog created from configuration properties. These properties + * map to the Iceberg catalog Java API properties and are used to create the catalog and file IO implementations. + *

+ *

+ * The minimal set of properties required to create an Iceberg catalog are: + *

    + *
  • {@code "catalog-impl"} or {@code "type"} - the Java catalog implementation to use. When providing + * {@code "catalog-impl"}, the implementing Java class should be provided (e.g. + * {@code "org.apache.iceberg.rest.RESTCatalog"} or {@code "org.apache.iceberg.aws.glue.GlueCatalog")}. Choices for + * {@code "type"} include {@code "hive"}, {@code "hadoop"}, {@code "rest"}, {@code "glue"}, {@code "nessie"}, + * {@code "jdbc"}.
    • + *
  • {@code "uri"} - the URI of the catalog.
    • + *
+ *

+ * Other common properties include: + *

+ *
    + *
  • {@code "warehouse"} - the location of the data warehouse.
    • + *
  • {@code "client.region"} - the region of the AWS client.
    • + *
  • {@code "s3.access-key-id"} - the S3 access key for reading files.
    • + *
  • {@code "s3.secret-access-key"} - the S3 secret access key for reading files.
    • + *
  • {@code "s3.endpoint"} - the S3 endpoint to connect to.
    • + *
+ *

+ * Additional properties for the specific catalog should also be included, such as as S3-specific properties for + * authentication or endpoint overriding. + *

+ * + * @param name the name of the catalog; if omitted, the catalog URI will be used to generate a name + * @param properties a map containing the Iceberg catalog properties to use + * @return the Iceberg catalog adapter + */ + @SuppressWarnings("unused") + public static IcebergCatalogAdapter createAdapter( + @Nullable final String name, + @NotNull final Map properties) { + return createAdapter(name, properties, Map.of()); + } + + /** + *

+ * Create an Iceberg catalog adapter for an Iceberg catalog created from configuration properties. These properties + * map to the Iceberg catalog Java API properties and are used to create the catalog and file IO implementations. + *

+ *

+ * The minimal set of properties required to create an Iceberg catalog are: + *

    + *
  • {@code "catalog-impl"} or {@code "type"} - the Java catalog implementation to use. When providing + * {@code "catalog-impl"}, the implementing Java class should be provided (e.g. + * {@code "org.apache.iceberg.rest.RESTCatalog"} or {@code "org.apache.iceberg.aws.glue.GlueCatalog")}. Choices for + * {@code "type"} include {@code "hive"}, {@code "hadoop"}, {@code "rest"}, {@code "glue"}, {@code "nessie"}, + * {@code "jdbc"}.
    • + *
  • {@code "uri"} - the URI of the catalog.
    • + *
+ *

+ * Other common properties include: + *

+ *
    + *
  • {@code "warehouse"} - the location of the data warehouse.
    • + *
  • {@code "client.region"} - the region of the AWS client.
    • + *
  • {@code "s3.access-key-id"} - the S3 access key for reading files.
    • + *
  • {@code "s3.secret-access-key"} - the S3 secret access key for reading files.
    • + *
  • {@code "s3.endpoint"} - the S3 endpoint to connect to.
    • + *
+ *

+ * Additional properties for the specific catalog should also be included, such as as S3-specific properties for + * authentication or endpoint overriding. + *

+ * + * @param name the name of the catalog; if omitted, the catalog URI will be used to generate a name + * @param properties a map containing the Iceberg catalog properties to use + * @param hadoopConfig a map containing Hadoop configuration properties to use + * @return the Iceberg catalog adapter + */ + @SuppressWarnings("unused") + public static IcebergCatalogAdapter createAdapter( + @Nullable final String name, + @NotNull final Map properties, + @NotNull final Map hadoopConfig) { + // Validate the minimum required properties are set + if (!properties.containsKey(CatalogProperties.CATALOG_IMPL) + && !properties.containsKey(CatalogUtil.ICEBERG_CATALOG_TYPE)) { + throw new IllegalArgumentException( + String.format("Catalog type '%s' or implementation class '%s' is required", + CatalogUtil.ICEBERG_CATALOG_TYPE, CatalogProperties.CATALOG_IMPL)); + } + if (!properties.containsKey(CatalogProperties.URI)) { + throw new IllegalArgumentException(String.format("Catalog URI property '%s' is required", + CatalogProperties.URI)); + } + + final String catalogUri = properties.get(CatalogProperties.URI); + final String catalogName = name != null ? name : "IcebergCatalog-" + catalogUri; + + // Load the Hadoop configuration with the provided properties + final Configuration hadoopConf = new Configuration(); + hadoopConfig.forEach(hadoopConf::set); + + // Create the Iceberg catalog from the properties + final Catalog catalog = CatalogUtil.buildIcebergCatalog(catalogName, properties, hadoopConf); + + return new IcebergCatalogAdapter(catalog, properties); } + } diff --git a/extensions/iceberg/src/test/java/io/deephaven/iceberg/TestCatalog/IcebergTestCatalog.java b/extensions/iceberg/src/test/java/io/deephaven/iceberg/TestCatalog/IcebergTestCatalog.java index e62fbd282e0..3d95032e1f8 100644 --- a/extensions/iceberg/src/test/java/io/deephaven/iceberg/TestCatalog/IcebergTestCatalog.java +++ b/extensions/iceberg/src/test/java/io/deephaven/iceberg/TestCatalog/IcebergTestCatalog.java @@ -10,7 +10,7 @@ import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.NamespaceNotEmptyException; import org.apache.iceberg.exceptions.NoSuchNamespaceException; -import org.apache.iceberg.io.FileIO; +import org.jetbrains.annotations.NotNull; import java.io.File; import java.util.*; @@ -19,7 +19,7 @@ public class IcebergTestCatalog implements Catalog, SupportsNamespaces { private final Map> namespaceTableMap; private final Map tableMap; - private IcebergTestCatalog(final String path, final FileIO fileIO) { + private IcebergTestCatalog(final String path, @NotNull final Map properties) { namespaceTableMap = new HashMap<>(); tableMap = new HashMap<>(); @@ -33,7 +33,7 @@ private IcebergTestCatalog(final String path, final FileIO fileIO) { if (tableFile.isDirectory()) { // Second level is table name. final TableIdentifier tableId = TableIdentifier.of(namespace, tableFile.getName()); - final Table table = IcebergTestTable.loadFromMetadata(tableFile.getAbsolutePath(), fileIO); + final Table table = IcebergTestTable.loadFromMetadata(tableFile.getAbsolutePath(), properties); // Add it to the maps. namespaceTableMap.get(namespace).put(tableId, table); @@ -44,8 +44,8 @@ private IcebergTestCatalog(final String path, final FileIO fileIO) { } } - public static IcebergTestCatalog create(final String path, final FileIO fileIO) { - return new IcebergTestCatalog(path, fileIO); + public static IcebergTestCatalog create(final String path, @NotNull final Map properties) { + return new IcebergTestCatalog(path, properties); } @Override diff --git a/extensions/iceberg/src/test/java/io/deephaven/iceberg/TestCatalog/IcebergTestFileIO.java b/extensions/iceberg/src/test/java/io/deephaven/iceberg/TestCatalog/IcebergTestFileIO.java deleted file mode 100644 index 03be6ca1b5e..00000000000 --- a/extensions/iceberg/src/test/java/io/deephaven/iceberg/TestCatalog/IcebergTestFileIO.java +++ /dev/null @@ -1,49 +0,0 @@ -// -// Copyright (c) 2016-2024 Deephaven Data Labs and Patent Pending -// -package io.deephaven.iceberg.TestCatalog; - -import org.apache.iceberg.inmemory.InMemoryFileIO; -import org.apache.iceberg.io.InputFile; -import org.apache.iceberg.io.OutputFile; - -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.HashSet; -import java.util.Set; - -public class IcebergTestFileIO extends InMemoryFileIO { - private final Set inputFiles; - private final String matchPrefix; - private final String replacePrefix; - - public IcebergTestFileIO(final String matchPrefix, final String replacePrefix) { - this.matchPrefix = matchPrefix; - this.replacePrefix = replacePrefix; - inputFiles = new HashSet<>(); - } - - @Override - public InputFile newInputFile(String s) { - if (!inputFiles.contains(s)) { - try { - final String replaced = s.replace(matchPrefix, replacePrefix); - final byte[] data = Files.readAllBytes(Path.of(replaced)); - addFile(s, data); - inputFiles.add(s); - } catch (IOException e) { - throw new RuntimeException(e); - } - } - return super.newInputFile(s); - } - - @Override - public OutputFile newOutputFile(String s) { - return null; - } - - @Override - public void deleteFile(String s) {} -} diff --git a/extensions/iceberg/src/test/java/io/deephaven/iceberg/TestCatalog/IcebergTestTable.java b/extensions/iceberg/src/test/java/io/deephaven/iceberg/TestCatalog/IcebergTestTable.java index d1cf5c2ee0e..bcac783def0 100644 --- a/extensions/iceberg/src/test/java/io/deephaven/iceberg/TestCatalog/IcebergTestTable.java +++ b/extensions/iceberg/src/test/java/io/deephaven/iceberg/TestCatalog/IcebergTestTable.java @@ -3,10 +3,12 @@ // package io.deephaven.iceberg.TestCatalog; +import org.apache.hadoop.conf.Configuration; import org.apache.iceberg.*; import org.apache.iceberg.encryption.EncryptionManager; import org.apache.iceberg.io.FileIO; import org.apache.iceberg.io.LocationProvider; +import org.apache.iceberg.io.ResolvingFileIO; import org.jetbrains.annotations.NotNull; import org.testcontainers.shaded.org.apache.commons.lang3.NotImplementedException; @@ -18,11 +20,14 @@ public class IcebergTestTable implements Table { private final TableMetadata metadata; - private final FileIO fileIO; + private final Map properties; + private final Configuration hadoopConf; + + private IcebergTestTable(@NotNull final String path, @NotNull final Map properties) { + this.properties = properties; + hadoopConf = new Configuration(); - private IcebergTestTable(@NotNull final String path, @NotNull final FileIO fileIO) { final File metadataRoot = new File(path, "metadata"); - this.fileIO = fileIO; final List metadataFiles = new ArrayList<>(); @@ -44,8 +49,10 @@ private IcebergTestTable(@NotNull final String path, @NotNull final FileIO fileI } } - public static IcebergTestTable loadFromMetadata(@NotNull final String path, @NotNull final FileIO fileIO) { - return new IcebergTestTable(path, fileIO); + public static IcebergTestTable loadFromMetadata( + @NotNull final String path, + @NotNull final Map properties) { + return new IcebergTestTable(path, properties); } @Override @@ -214,7 +221,10 @@ public Transaction newTransaction() { @Override public FileIO io() { - return fileIO; + final ResolvingFileIO io = new ResolvingFileIO(); + io.setConf(hadoopConf); + io.initialize(properties); + return io; } @Override diff --git a/extensions/s3/src/test/java/io/deephaven/extensions/s3/testlib/SingletonContainers.java b/extensions/s3/src/test/java/io/deephaven/extensions/s3/testlib/SingletonContainers.java index befd758f980..5d5b550089d 100644 --- a/extensions/s3/src/test/java/io/deephaven/extensions/s3/testlib/SingletonContainers.java +++ b/extensions/s3/src/test/java/io/deephaven/extensions/s3/testlib/SingletonContainers.java @@ -16,6 +16,7 @@ import software.amazon.awssdk.services.s3.S3AsyncClient; import java.net.URI; +import java.util.Map; public final class SingletonContainers { @@ -53,6 +54,14 @@ public static S3AsyncClient s3AsyncClient() { AwsBasicCredentials.create(LOCALSTACK_S3.getAccessKey(), LOCALSTACK_S3.getSecretKey()))) .build(); } + + public static Map s3Properties() { + return Map.of( + "s3.endpoint", LOCALSTACK_S3.getEndpoint().toString(), + "client.region", LOCALSTACK_S3.getRegion(), + "s3.access-key-id", LOCALSTACK_S3.getAccessKey(), + "s3.secret-access-key", LOCALSTACK_S3.getSecretKey()); + } } public static final class MinIO { @@ -87,5 +96,13 @@ public static S3AsyncClient s3AsyncClient() { AwsBasicCredentials.create(MINIO.getUserName(), MINIO.getPassword()))) .build(); } + + public static Map s3Properties() { + return Map.of( + "s3.endpoint", MINIO.getS3URL(), + "client.region", Region.AWS_GLOBAL.toString(), + "s3.access-key-id", MINIO.getUserName(), + "s3.secret-access-key", MINIO.getPassword()); + } } } diff --git a/py/server/deephaven/experimental/iceberg.py b/py/server/deephaven/experimental/iceberg.py index 0a99d3f1880..16fcd08f37d 100644 --- a/py/server/deephaven/experimental/iceberg.py +++ b/py/server/deephaven/experimental/iceberg.py @@ -11,8 +11,11 @@ from deephaven.experimental import s3 from deephaven.table import Table, TableDefinition, TableDefinitionLike +from deephaven.jcompat import j_hashmap + _JIcebergInstructions = jpy.get_type("io.deephaven.iceberg.util.IcebergInstructions") _JIcebergCatalogAdapter = jpy.get_type("io.deephaven.iceberg.util.IcebergCatalogAdapter") +_JIcebergTools = jpy.get_type("io.deephaven.iceberg.util.IcebergTools") # IcebergToolsS3 is an optional library try: @@ -245,3 +248,74 @@ def adapter_aws_glue( except Exception as e: raise DHError(e, "Failed to build Iceberg Catalog Adapter") from e + +def adapter( + name: Optional[str] = None, + properties: Optional[Dict[str, str]] = None, + hadoopConfig: Optional[Dict[str, str]] = None +) -> IcebergCatalogAdapter: + """ + Create an Iceberg catalog adapter from configuration properties. These properties map to the Iceberg catalog Java + API properties and are used to select the catalog and file IO implementations. + + The minimal set of properties required to create an Iceberg catalog are the following: + - `catalog-impl` or `type` - the Java catalog implementation to use. When providing `catalog-impl`, the + implementing Java class should be provided (e.g. `org.apache.iceberg.rest.RESTCatalog` or + `org.apache.iceberg.aws.glue.GlueCatalog`). Choices for `type` include `hive`, `hadoop`, `rest`, `glue`, + `nessie`, `jdbc`. + - `uri` - the URI of the catalog + + Other common properties include: + - `warehouse` - the root path of the data warehouse. + - `client.region` - the region of the AWS client. + - `s3.access-key-id` - the S3 access key for reading files. + - `s3.secret-access-key` - the S3 secret access key for reading files. + - `s3.endpoint` - the S3 endpoint to connect to. + + Example usage #1 - REST catalog with an S3 backend (using MinIO): + ``` + from deephaven.experimental import iceberg + + adapter = iceberg.adapter_generic(name="generic-adapter", properties={ + "type" : "rest", + "uri" : "http://rest:8181", + "client.region" : "us-east-1", + "s3.access-key-id" : "admin", + "s3.secret-access-key" : "password", + "s3.endpoint" : "http://minio:9000" + }) + ``` + + Example usage #2 - AWS Glue catalog: + ``` + from deephaven.experimental import iceberg + + ## Note: region and credential information are loaded by the catalog from the environment + adapter = iceberg.adapter_generic(name="generic-adapter", properties={ + "type" : "glue", + "uri" : "s3://lab-warehouse/sales", + }); + ``` + + Args: + name (Optional[str]): a descriptive name of the catalog; if omitted the catalog name is inferred from the + catalog URI property. + properties (Optional[Dict[str, str]]): the properties of the catalog to load + hadoopConfig (Optional[Dict[str, str]]): hadoop configuration properties for the catalog to load + + Returns: + IcebergCatalogAdapter: the catalog adapter created from the provided properties + + Raises: + DHError: If unable to build the catalog adapter + """ + + try: + return IcebergCatalogAdapter( + _JIcebergTools.createAdapter( + name, + j_hashmap(properties if properties is not None else {}), + j_hashmap(hadoopConfig if hadoopConfig is not None else {}))) + except Exception as e: + raise DHError(e, "Failed to build Iceberg Catalog Adapter") from e + From 29184e8e3b45d3df5cce667280803a4098c8e857 Mon Sep 17 00:00:00 2001 From: Corey Kosak Date: Sat, 7 Sep 2024 17:48:48 -0400 Subject: [PATCH 3/4] feat(csharp/ExcelAddIn): ExcelAddIn demo v6: Respond to demo feedback (#6030) Respond to demo feedback, make a variety of changes. --- csharp/ExcelAddIn/StateManager.cs | 4 + .../ConnectionManagerDialogFactory.cs | 122 ++++----- .../factories/CredentialsDialogFactory.cs | 3 +- .../ConnectionManagerDialogManager.cs | 66 +++++ .../ConnectionManagerDialogRowManager.cs | 140 +++++++++++ csharp/ExcelAddIn/models/Session.cs | 16 +- csharp/ExcelAddIn/models/SimpleModels.cs | 4 + .../ExcelAddIn/providers/SessionProvider.cs | 23 +- .../ExcelAddIn/providers/SessionProviders.cs | 35 ++- csharp/ExcelAddIn/util/ObservableConverter.cs | 44 ++++ .../viewmodels/ConnectionManagerDialogRow.cs | 79 ++---- .../viewmodels/CredentialsDialogViewModel.cs | 20 +- .../views/ConnectionManagerDialog.Designer.cs | 68 ++++- .../views/ConnectionManagerDialog.cs | 97 ++++---- .../views/ConnectionManagerDialog.resx | 4 +- .../views/CredentialsDialog.Designer.cs | 233 +++++++++--------- csharp/ExcelAddIn/views/CredentialsDialog.cs | 19 +- 17 files changed, 666 insertions(+), 311 deletions(-) create mode 100644 csharp/ExcelAddIn/managers/ConnectionManagerDialogManager.cs create mode 100644 csharp/ExcelAddIn/managers/ConnectionManagerDialogRowManager.cs create mode 100644 csharp/ExcelAddIn/util/ObservableConverter.cs diff --git a/csharp/ExcelAddIn/StateManager.cs b/csharp/ExcelAddIn/StateManager.cs index a4bb4e44e4e..fafcd158648 100644 --- a/csharp/ExcelAddIn/StateManager.cs +++ b/csharp/ExcelAddIn/StateManager.cs @@ -80,4 +80,8 @@ public void SetDefaultCredentials(CredentialsBase credentials) { public void Reconnect(EndpointId id) { _sessionProviders.Reconnect(id); } + + public void SwitchOnEmpty(EndpointId id, Action onEmpty, Action onNotEmpty) { + _sessionProviders.SwitchOnEmpty(id, onEmpty, onNotEmpty); + } } diff --git a/csharp/ExcelAddIn/factories/ConnectionManagerDialogFactory.cs b/csharp/ExcelAddIn/factories/ConnectionManagerDialogFactory.cs index cdf6053f327..3df7ee89504 100644 --- a/csharp/ExcelAddIn/factories/ConnectionManagerDialogFactory.cs +++ b/csharp/ExcelAddIn/factories/ConnectionManagerDialogFactory.cs @@ -1,14 +1,15 @@ -using Deephaven.ExcelAddIn.Viewmodels; +using System.Collections.Concurrent; +using Deephaven.ExcelAddIn.Managers; +using Deephaven.ExcelAddIn.Viewmodels; using Deephaven.ExcelAddIn.ViewModels; using Deephaven.ExcelAddIn.Views; -using System.Diagnostics; -using Deephaven.ExcelAddIn.Models; -using Deephaven.ExcelAddIn.Util; namespace Deephaven.ExcelAddIn.Factories; internal static class ConnectionManagerDialogFactory { public static void CreateAndShow(StateManager sm) { + var rowToManager = new ConcurrentDictionary(); + // The "new" button creates a "New/Edit Credentials" dialog void OnNewButtonClicked() { var cvm = CredentialsDialogViewModel.OfEmpty(); @@ -16,87 +17,58 @@ void OnNewButtonClicked() { dialog.Show(); } - var cmDialog = new ConnectionManagerDialog(OnNewButtonClicked); - cmDialog.Show(); - var cmso = new ConnectionManagerSessionObserver(sm, cmDialog); - var disposer = sm.SubscribeToSessions(cmso); - - cmDialog.Closed += (_, _) => { - disposer.Dispose(); - cmso.Dispose(); - }; - } -} - -internal class ConnectionManagerSessionObserver( - StateManager stateManager, - ConnectionManagerDialog cmDialog) : IObserver>, IDisposable { - private readonly List _disposables = new(); - - public void OnNext(AddOrRemove aor) { - if (!aor.IsAdd) { - // TODO(kosak) - Debug.WriteLine("Remove is not handled"); - return; + void OnDeleteButtonClicked(ConnectionManagerDialogRow[] rows) { + foreach (var row in rows) { + if (!rowToManager.TryGetValue(row, out var manager)) { + continue; + } + manager.DoDelete(); + } } - var endpointId = aor.Value; + void OnReconnectButtonClicked(ConnectionManagerDialogRow[] rows) { + foreach (var row in rows) { + if (!rowToManager.TryGetValue(row, out var manager)) { + continue; + } + manager.DoReconnect(); + } + } - var statusRow = new ConnectionManagerDialogRow(endpointId.Id, stateManager); - // We watch for session and credential state changes in our ID - var sessDisposable = stateManager.SubscribeToSession(endpointId, statusRow); - var credDisposable = stateManager.SubscribeToCredentials(endpointId, statusRow); + void OnMakeDefaultButtonClicked(ConnectionManagerDialogRow[] rows) { + // Make the last selected row the default + if (rows.Length == 0) { + return; + } - // And we also watch for credentials changes in the default session (just to keep - // track of whether we are still the default) - var dct = new DefaultCredentialsTracker(statusRow); - var defaultCredDisposable = stateManager.SubscribeToDefaultCredentials(dct); + var row = rows[^1]; + if (!rowToManager.TryGetValue(row, out var manager)) { + return; + } - // We'll do our AddRow on the GUI thread, and, while we're on the GUI thread, we'll add - // our disposables to our saved disposables. - cmDialog.Invoke(() => { - _disposables.Add(sessDisposable); - _disposables.Add(credDisposable); - _disposables.Add(defaultCredDisposable); - cmDialog.AddRow(statusRow); - }); - } + manager.DoSetAsDefault(); + } - public void Dispose() { - // Since the GUI thread is where we added these disposables, the GUI thread is where we will - // access and dispose them. - cmDialog.Invoke(() => { - var temp = _disposables.ToArray(); - _disposables.Clear(); - foreach (var disposable in temp) { - disposable.Dispose(); + void OnEditButtonClicked(ConnectionManagerDialogRow[] rows) { + foreach (var row in rows) { + if (!rowToManager.TryGetValue(row, out var manager)) { + continue; + } + manager.DoEdit(); } - }); - } + } - public void OnCompleted() { - // TODO(kosak) - throw new NotImplementedException(); - } + var cmDialog = new ConnectionManagerDialog(OnNewButtonClicked, OnDeleteButtonClicked, + OnReconnectButtonClicked, OnMakeDefaultButtonClicked, OnEditButtonClicked); + cmDialog.Show(); + var dm = new ConnectionManagerDialogManager(cmDialog, rowToManager, sm); + var disposer = sm.SubscribeToSessions(dm); - public void OnError(Exception error) { - // TODO(kosak) - throw new NotImplementedException(); + cmDialog.Closed += (_, _) => { + disposer.Dispose(); + dm.Dispose(); + }; } } -internal class DefaultCredentialsTracker(ConnectionManagerDialogRow statusRow) : IObserver> { - public void OnNext(StatusOr value) { - statusRow.SetDefaultCredentials(value); - } - public void OnCompleted() { - // TODO(kosak) - throw new NotImplementedException(); - } - - public void OnError(Exception error) { - // TODO(kosak) - throw new NotImplementedException(); - } -} \ No newline at end of file diff --git a/csharp/ExcelAddIn/factories/CredentialsDialogFactory.cs b/csharp/ExcelAddIn/factories/CredentialsDialogFactory.cs index c748847e852..4d179569bff 100644 --- a/csharp/ExcelAddIn/factories/CredentialsDialogFactory.cs +++ b/csharp/ExcelAddIn/factories/CredentialsDialogFactory.cs @@ -1,4 +1,5 @@ -using Deephaven.ExcelAddIn.Models; +using System.Diagnostics; +using Deephaven.ExcelAddIn.Models; using Deephaven.ExcelAddIn.Util; using Deephaven.ExcelAddIn.ViewModels; using ExcelAddIn.views; diff --git a/csharp/ExcelAddIn/managers/ConnectionManagerDialogManager.cs b/csharp/ExcelAddIn/managers/ConnectionManagerDialogManager.cs new file mode 100644 index 00000000000..8cf9ec5435d --- /dev/null +++ b/csharp/ExcelAddIn/managers/ConnectionManagerDialogManager.cs @@ -0,0 +1,66 @@ +using System.Collections.Concurrent; +using Deephaven.ExcelAddIn.Models; +using Deephaven.ExcelAddIn.Viewmodels; +using Deephaven.ExcelAddIn.Views; +using System.Diagnostics; +using Deephaven.ExcelAddIn.Util; + +namespace Deephaven.ExcelAddIn.Managers; + +internal class ConnectionManagerDialogManager( + ConnectionManagerDialog cmDialog, + ConcurrentDictionary rowToManager, + StateManager stateManager) : IObserver>, IDisposable { + private readonly WorkerThread _workerThread = stateManager.WorkerThread; + private readonly Dictionary _idToRow = new(); + private readonly List _disposables = new(); + + public void OnNext(AddOrRemove aor) { + if (_workerThread.InvokeIfRequired(() => OnNext(aor))) { + return; + } + + if (aor.IsAdd) { + var endpointId = aor.Value; + var row = new ConnectionManagerDialogRow(endpointId.Id); + var statusRowManager = ConnectionManagerDialogRowManager.Create(row, endpointId, stateManager); + _ = rowToManager.TryAdd(row, statusRowManager); + _idToRow.Add(endpointId, row); + _disposables.Add(statusRowManager); + + cmDialog.AddRow(row); + return; + } + + // Remove! + if (!_idToRow.Remove(aor.Value, out var rowToDelete) || + !rowToManager.TryRemove(rowToDelete, out var rowManager)) { + return; + } + + cmDialog.RemoveRow(rowToDelete); + rowManager.Dispose(); + } + + public void Dispose() { + // Since the GUI thread is where we added these disposables, the GUI thread is where we will + // access and dispose them. + cmDialog.Invoke(() => { + var temp = _disposables.ToArray(); + _disposables.Clear(); + foreach (var disposable in temp) { + disposable.Dispose(); + } + }); + } + + public void OnCompleted() { + // TODO(kosak) + throw new NotImplementedException(); + } + + public void OnError(Exception error) { + // TODO(kosak) + throw new NotImplementedException(); + } +} diff --git a/csharp/ExcelAddIn/managers/ConnectionManagerDialogRowManager.cs b/csharp/ExcelAddIn/managers/ConnectionManagerDialogRowManager.cs new file mode 100644 index 00000000000..366c0c596ab --- /dev/null +++ b/csharp/ExcelAddIn/managers/ConnectionManagerDialogRowManager.cs @@ -0,0 +1,140 @@ +using Deephaven.ExcelAddIn.Factories; +using Deephaven.ExcelAddIn.Models; +using Deephaven.ExcelAddIn.Util; +using Deephaven.ExcelAddIn.Viewmodels; +using Deephaven.ExcelAddIn.ViewModels; + +namespace Deephaven.ExcelAddIn.Managers; + +public sealed class ConnectionManagerDialogRowManager : IObserver>, + IObserver>, IObserver, IDisposable { + + public static ConnectionManagerDialogRowManager Create(ConnectionManagerDialogRow row, + EndpointId endpointId, StateManager stateManager) { + var result = new ConnectionManagerDialogRowManager(row, endpointId, stateManager); + result.Resubscribe(); + return result; + } + + private readonly ConnectionManagerDialogRow _row; + private readonly EndpointId _endpointId; + private readonly StateManager _stateManager; + private readonly WorkerThread _workerThread; + private readonly List _disposables = new(); + + private ConnectionManagerDialogRowManager(ConnectionManagerDialogRow row, EndpointId endpointId, + StateManager stateManager) { + _row = row; + _endpointId = endpointId; + _stateManager = stateManager; + _workerThread = stateManager.WorkerThread; + } + + public void Dispose() { + Unsubcribe(); + } + + private void Resubscribe() { + if (_workerThread.InvokeIfRequired(Resubscribe)) { + return; + } + + if (_disposables.Count != 0) { + throw new Exception("State error: already subscribed"); + } + // We watch for session and credential state changes in our ID + var d1 = _stateManager.SubscribeToSession(_endpointId, this); + var d2 = _stateManager.SubscribeToCredentials(_endpointId, this); + // Now we have a problem. We would also like to watch for credential + // state changes in the default session. But the default session + // has the same observable type (IObservable>) + // as the specific session we are watching. To work around this, + // we create an Observer that translates StatusOr to + // MyWrappedSOSB and then we subscribe to that. + var converter = ObservableConverter.Create( + (StatusOr socb) => new MyWrappedSocb(socb), _workerThread); + var d3 = _stateManager.SubscribeToDefaultCredentials(converter); + var d4 = converter.Subscribe(this); + + _disposables.AddRange(new[] { d1, d2, d3, d4 }); + } + + private void Unsubcribe() { + if (_workerThread.InvokeIfRequired(Unsubcribe)) { + return; + } + var temp = _disposables.ToArray(); + _disposables.Clear(); + + foreach (var disposable in temp) { + disposable.Dispose(); + } + } + + public void OnNext(StatusOr value) { + _row.SetCredentialsSynced(value); + } + + public void OnNext(StatusOr value) { + _row.SetSessionSynced(value); + } + + public void OnNext(MyWrappedSocb value) { + _row.SetDefaultCredentialsSynced(value.Value); + } + + public void DoEdit() { + var creds = _row.GetCredentialsSynced(); + // If we have valid credentials, then make a populated viewmodel. + // If we don't, then make an empty viewmodel with only Id populated. + var cvm = creds.AcceptVisitor( + crs => CredentialsDialogViewModel.OfIdAndCredentials(_endpointId.Id, crs), + _ => CredentialsDialogViewModel.OfIdButOtherwiseEmpty(_endpointId.Id)); + var cd = CredentialsDialogFactory.Create(_stateManager, cvm); + cd.Show(); + } + + public void DoDelete() { + // Strategy: + // 1. Unsubscribe to everything + // 2. If it turns out that we were the last subscriber to the session, then great, the + // delete can proceed. + // 3. Otherwise (there is some other subscriber to the session), then the delete operation + // should be denied. In that case we restore our state by resubscribing to everything. + Unsubcribe(); + + _stateManager.SwitchOnEmpty(_endpointId, () => { }, Resubscribe); + } + + public void DoReconnect() { + _stateManager.Reconnect(_endpointId); + } + + public void DoSetAsDefault() { + // If the connection is already the default, do nothing. + if (_row.IsDefault) { + return; + } + + // If we don't have credentials, then we can't make them the default. + var credentials = _row.GetCredentialsSynced(); + if (!credentials.GetValueOrStatus(out var creds, out _)) { + return; + } + + _stateManager.SetDefaultCredentials(creds); + } + + public void OnCompleted() { + // TODO(kosak) + throw new NotImplementedException(); + } + + public void OnError(Exception error) { + // TODO(kosak) + throw new NotImplementedException(); + } + + public record MyWrappedSocb(StatusOr Value) { + } +} diff --git a/csharp/ExcelAddIn/models/Session.cs b/csharp/ExcelAddIn/models/Session.cs index b3851ef7a77..3934ff74226 100644 --- a/csharp/ExcelAddIn/models/Session.cs +++ b/csharp/ExcelAddIn/models/Session.cs @@ -26,7 +26,13 @@ public override T Visit(Func onCore, Func } public override void Dispose() { - Utility.Exchange(ref _client, null)?.Dispose(); + var temp = Utility.Exchange(ref _client, null); + if (temp == null) { + return; + } + + // Do the actual dispose work on a helper thread. + Utility.RunInBackground(temp.Dispose); } public Client Client { @@ -52,7 +58,13 @@ public override void Dispose() { return; } - Utility.Exchange(ref _sessionManager, null)?.Dispose(); + var temp = Utility.Exchange(ref _sessionManager, null); + if (temp == null) { + return; + } + + // Do the actual dispose work on a helper thread. + Utility.RunInBackground(temp.Dispose); } public SessionManager SessionManager { diff --git a/csharp/ExcelAddIn/models/SimpleModels.cs b/csharp/ExcelAddIn/models/SimpleModels.cs index 02a89b370b7..4593e58fef9 100644 --- a/csharp/ExcelAddIn/models/SimpleModels.cs +++ b/csharp/ExcelAddIn/models/SimpleModels.cs @@ -4,6 +4,10 @@ public record AddOrRemove(bool IsAdd, T Value) { public static AddOrRemove OfAdd(T value) { return new AddOrRemove(true, value); } + + public static AddOrRemove OfRemove(T value) { + return new AddOrRemove(false, value); + } } public record EndpointId(string Id) { diff --git a/csharp/ExcelAddIn/providers/SessionProvider.cs b/csharp/ExcelAddIn/providers/SessionProvider.cs index 11882df8144..16a60e935a1 100644 --- a/csharp/ExcelAddIn/providers/SessionProvider.cs +++ b/csharp/ExcelAddIn/providers/SessionProvider.cs @@ -1,8 +1,8 @@ -using Deephaven.DeephavenClient.ExcelAddIn.Util; +using System.Diagnostics; +using Deephaven.DeephavenClient.ExcelAddIn.Util; using Deephaven.ExcelAddIn.Factories; using Deephaven.ExcelAddIn.Models; using Deephaven.ExcelAddIn.Util; -using System.Net; namespace Deephaven.ExcelAddIn.Providers; @@ -12,7 +12,8 @@ internal class SessionProvider(WorkerThread workerThread) : IObservable> _credentialsObservers = new(); private readonly ObserverContainer> _sessionObservers = new(); /// - /// This is used to ignore the results from multiple invocations of "SetCredentials". + /// This is used to track the results from multiple invocations of "SetCredentials" and + /// to keep only the latest. /// private readonly SimpleAtomicReference _sharedSetCredentialsCookie = new(new object()); @@ -66,7 +67,8 @@ public IDisposable Subscribe(IObserver> observer) { return ActionAsDisposable.Create(() => { workerThread.Invoke(() => { - _sessionObservers.Remove(observer, out _); + _sessionObservers.Remove(observer, out var isLast); + Debug.WriteLine(isLast); }); }); } @@ -90,6 +92,19 @@ public void SetCredentials(CredentialsBase credentials) { Utility.RunInBackground(() => CreateSessionBaseInSeparateThread(credentials)); } + public void SwitchOnEmpty(Action callerOnEmpty, Action callerOnNotEmpty) { + if (workerThread.InvokeIfRequired(() => SwitchOnEmpty(callerOnEmpty, callerOnNotEmpty))) { + return; + } + + if (_credentialsObservers.Count != 0 || _sessionObservers.Count != 0) { + callerOnNotEmpty(); + return; + } + + callerOnEmpty(); + } + void CreateSessionBaseInSeparateThread(CredentialsBase credentials) { // Make a unique sentinel object to indicate that this thread should be // the one privileged to provide the system with the Session corresponding diff --git a/csharp/ExcelAddIn/providers/SessionProviders.cs b/csharp/ExcelAddIn/providers/SessionProviders.cs index 5e5db2366e3..042d0a3d1a7 100644 --- a/csharp/ExcelAddIn/providers/SessionProviders.cs +++ b/csharp/ExcelAddIn/providers/SessionProviders.cs @@ -1,4 +1,5 @@ -using Deephaven.DeephavenClient.ExcelAddIn.Util; +using System.Diagnostics; +using Deephaven.DeephavenClient.ExcelAddIn.Util; using Deephaven.ExcelAddIn.Models; using Deephaven.ExcelAddIn.Util; @@ -10,7 +11,6 @@ internal class SessionProviders(WorkerThread workerThread) : IObservable> _endpointsObservers = new(); public IDisposable Subscribe(IObserver> observer) { - IDisposable? disposable = null; // We need to run this on our worker thread because we want to protect // access to our dictionary. workerThread.Invoke(() => { @@ -25,7 +25,7 @@ public IDisposable Subscribe(IObserver> observer) { return ActionAsDisposable.Create(() => { workerThread.Invoke(() => { - Utility.Exchange(ref disposable, null)?.Dispose(); + _endpointsObservers.Remove(observer, out _); }); }); } @@ -92,6 +92,35 @@ public void Reconnect(EndpointId id) { ApplyTo(id, sp => sp.Reconnect()); } + public void SwitchOnEmpty(EndpointId id, Action callerOnEmpty, Action callerOnNotEmpty) { + if (workerThread.InvokeIfRequired(() => SwitchOnEmpty(id, callerOnEmpty, callerOnNotEmpty))) { + return; + } + + Debug.WriteLine("It's SwitchOnEmpty time"); + if (!_providerMap.TryGetValue(id, out var sp)) { + // No provider. That's weird. callerOnEmpty I guess + callerOnEmpty(); + return; + } + + // Make a wrapped onEmpty that removes stuff from my dictionary and invokes + // the observer, then calls the caller's onEmpty + + Action? myOnEmpty = null; + myOnEmpty = () => { + if (workerThread.InvokeIfRequired(myOnEmpty!)) { + return; + } + _providerMap.Remove(id); + _endpointsObservers.OnNext(AddOrRemove.OfRemove(id)); + callerOnEmpty(); + }; + + sp.SwitchOnEmpty(myOnEmpty, callerOnNotEmpty); + } + + private void ApplyTo(EndpointId id, Action action) { if (workerThread.InvokeIfRequired(() => ApplyTo(id, action))) { return; diff --git a/csharp/ExcelAddIn/util/ObservableConverter.cs b/csharp/ExcelAddIn/util/ObservableConverter.cs new file mode 100644 index 00000000000..530f0785a63 --- /dev/null +++ b/csharp/ExcelAddIn/util/ObservableConverter.cs @@ -0,0 +1,44 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using System.Windows.Forms; +using Deephaven.DeephavenClient.ExcelAddIn.Util; + +namespace Deephaven.ExcelAddIn.Util; + +internal static class ObservableConverter { + public static ObservableConverter Create( + Func converter, WorkerThread workerThread) { + return new ObservableConverter(converter, workerThread); + } +} + +internal class ObservableConverter(Func converter, WorkerThread workerThread) : + IObserver, IObservable { + private readonly ObserverContainer _observers = new(); + + public void OnNext(TFrom value) { + var converted = converter(value); + _observers.OnNext(converted); + } + + public void OnCompleted() { + _observers.OnCompleted(); + } + + public void OnError(Exception error) { + _observers.OnError(error); + } + + public IDisposable Subscribe(IObserver observer) { + workerThread.Invoke(() => _observers.Add(observer, out _)); + + return ActionAsDisposable.Create(() => { + workerThread.Invoke(() => { + _observers.Remove(observer, out _); + }); + }); + } +} diff --git a/csharp/ExcelAddIn/viewmodels/ConnectionManagerDialogRow.cs b/csharp/ExcelAddIn/viewmodels/ConnectionManagerDialogRow.cs index 0e544131282..8692d3836ac 100644 --- a/csharp/ExcelAddIn/viewmodels/ConnectionManagerDialogRow.cs +++ b/csharp/ExcelAddIn/viewmodels/ConnectionManagerDialogRow.cs @@ -1,14 +1,11 @@ -using Deephaven.ExcelAddIn.Factories; -using Deephaven.ExcelAddIn.ViewModels; -using System.ComponentModel; +using System.ComponentModel; using Deephaven.ExcelAddIn.Models; using Deephaven.ExcelAddIn.Util; namespace Deephaven.ExcelAddIn.Viewmodels; -public sealed class ConnectionManagerDialogRow(string id, StateManager stateManager) : - IObserver>, IObserver>, - INotifyPropertyChanged { +public sealed class ConnectionManagerDialogRow(string id) : INotifyPropertyChanged { + public event PropertyChangedEventHandler? PropertyChanged; private readonly object _sync = new(); @@ -41,40 +38,23 @@ public string ServerType { } } - public bool IsDefault => - _credentials.GetValueOrStatus(out var creds1, out _) && - _defaultCredentials.GetValueOrStatus(out var creds2, out _) && - creds1.Id == creds2.Id; - - public void SettingsClicked() { - var creds = GetCredentialsSynced(); - // If we have valid credentials, - var cvm = creds.AcceptVisitor( - crs => CredentialsDialogViewModel.OfIdAndCredentials(Id, crs), - _ => CredentialsDialogViewModel.OfIdButOtherwiseEmpty(Id)); - var cd = CredentialsDialogFactory.Create(stateManager, cvm); - cd.Show(); - } - - public void ReconnectClicked() { - stateManager.Reconnect(new EndpointId(Id)); - } - - public void IsDefaultClicked() { - // If the box is already checked, do nothing. - if (IsDefault) { - return; + public bool IsDefault { + get { + var creds = GetCredentialsSynced(); + var defaultCreds = GetDefaultCredentialsSynced(); + return creds.GetValueOrStatus(out var creds1, out _) && + defaultCreds.GetValueOrStatus(out var creds2, out _) && + creds1.Id == creds2.Id; } + } - // If we don't have credentials, then we can't make them the default. - if (!_credentials.GetValueOrStatus(out var creds, out _)) { - return; + public StatusOr GetCredentialsSynced() { + lock (_sync) { + return _credentials; } - - stateManager.SetDefaultCredentials(creds); } - public void OnNext(StatusOr value) { + public void SetCredentialsSynced(StatusOr value) { lock (_sync) { _credentials = value; } @@ -83,41 +63,30 @@ public void OnNext(StatusOr value) { OnPropertyChanged(nameof(IsDefault)); } - public void OnNext(StatusOr value) { + public StatusOr GetDefaultCredentialsSynced() { lock (_sync) { - _session = value; + return _defaultCredentials; } - - OnPropertyChanged(nameof(Status)); } - public void SetDefaultCredentials(StatusOr creds) { + public void SetDefaultCredentialsSynced(StatusOr value) { lock (_sync) { - _defaultCredentials = creds; + _defaultCredentials = value; } OnPropertyChanged(nameof(IsDefault)); } - public void OnCompleted() { - // TODO(kosak) - throw new NotImplementedException(); - } - - public void OnError(Exception error) { - // TODO(kosak) - throw new NotImplementedException(); - } - - private StatusOr GetCredentialsSynced() { + public StatusOr GetSessionSynced() { lock (_sync) { - return _credentials; + return _session; } } - private StatusOr GetSessionSynced() { + public void SetSessionSynced(StatusOr value) { lock (_sync) { - return _session; + _session = value; } + OnPropertyChanged(nameof(Status)); } private void OnPropertyChanged(string name) { diff --git a/csharp/ExcelAddIn/viewmodels/CredentialsDialogViewModel.cs b/csharp/ExcelAddIn/viewmodels/CredentialsDialogViewModel.cs index 6634640031f..2b3e613c7d3 100644 --- a/csharp/ExcelAddIn/viewmodels/CredentialsDialogViewModel.cs +++ b/csharp/ExcelAddIn/viewmodels/CredentialsDialogViewModel.cs @@ -31,7 +31,7 @@ public static CredentialsDialogViewModel OfIdAndCredentials(string id, Credentia result.JsonUrl = corePlus.JsonUrl; result.UserId = corePlus.User; result.Password = corePlus.Password; - result.OperateAs = corePlus.OperateAs; + result.OperateAs = corePlus.OperateAs.Equals(corePlus.User) ? "" : corePlus.OperateAs; result.ValidateCertificate = corePlus.ValidateCertificate; return Unit.Instance; }); @@ -68,15 +68,15 @@ void CheckMissing(string field, string name) { } } - CheckMissing(_id, "Connection Id"); + CheckMissing(Id, "Connection Id"); if (!_isCorePlus) { - CheckMissing(_connectionString, "Connection String"); + CheckMissing(ConnectionString, "Connection String"); } else { - CheckMissing(_jsonUrl, "JSON URL"); - CheckMissing(_userId, "User Id"); - CheckMissing(_password, "Password"); - CheckMissing(_operateAs, "Operate As"); + CheckMissing(JsonUrl, "JSON URL"); + CheckMissing(UserId, "User Id"); + CheckMissing(Password, "Password"); + CheckMissing(OperateAsToUse, "Operate As"); } if (missingFields.Count > 0) { @@ -86,8 +86,8 @@ void CheckMissing(string field, string name) { var epId = new EndpointId(_id); result = _isCorePlus - ? CredentialsBase.OfCorePlus(epId, _jsonUrl, _userId, _password, _operateAs, _validateCertificate) - : CredentialsBase.OfCore(epId, _connectionString, _sessionTypeIsPython); + ? CredentialsBase.OfCorePlus(epId, JsonUrl, UserId, Password, OperateAsToUse, ValidateCertificate) + : CredentialsBase.OfCore(epId, ConnectionString, SessionTypeIsPython); return true; } @@ -207,6 +207,8 @@ public string OperateAs { } } + public string OperateAsToUse => _operateAs.Length != 0 ? _operateAs : UserId; + public bool ValidateCertificate { get => _validateCertificate; set { diff --git a/csharp/ExcelAddIn/views/ConnectionManagerDialog.Designer.cs b/csharp/ExcelAddIn/views/ConnectionManagerDialog.Designer.cs index fee2096228f..604c97b2d12 100644 --- a/csharp/ExcelAddIn/views/ConnectionManagerDialog.Designer.cs +++ b/csharp/ExcelAddIn/views/ConnectionManagerDialog.Designer.cs @@ -27,6 +27,10 @@ private void InitializeComponent() { dataGridView1 = new DataGridView(); newButton = new Button(); connectionsLabel = new Label(); + editButton = new Button(); + deleteButton = new Button(); + reconnectButton = new Button(); + makeDefaultButton = new Button(); ((System.ComponentModel.ISupportInitialize)dataGridView1).BeginInit(); SuspendLayout(); // @@ -34,21 +38,25 @@ private void InitializeComponent() { // dataGridView1.AllowUserToAddRows = false; dataGridView1.AllowUserToDeleteRows = false; + dataGridView1.Anchor = AnchorStyles.Top | AnchorStyles.Bottom | AnchorStyles.Left | AnchorStyles.Right; + dataGridView1.AutoSizeColumnsMode = DataGridViewAutoSizeColumnsMode.Fill; dataGridView1.ColumnHeadersHeightSizeMode = DataGridViewColumnHeadersHeightSizeMode.AutoSize; dataGridView1.Location = new Point(68, 83); dataGridView1.Name = "dataGridView1"; dataGridView1.ReadOnly = true; dataGridView1.RowHeadersWidth = 62; + dataGridView1.SelectionMode = DataGridViewSelectionMode.FullRowSelect; dataGridView1.Size = new Size(979, 454); dataGridView1.TabIndex = 0; // // newButton // - newButton.Location = new Point(869, 560); + newButton.Anchor = AnchorStyles.Bottom | AnchorStyles.Right; + newButton.Location = new Point(919, 560); newButton.Name = "newButton"; - newButton.Size = new Size(178, 34); - newButton.TabIndex = 1; - newButton.Text = "New Connection"; + newButton.Size = new Size(128, 34); + newButton.TabIndex = 5; + newButton.Text = "New..."; newButton.UseVisualStyleBackColor = true; newButton.Click += newButton_Click; // @@ -62,11 +70,59 @@ private void InitializeComponent() { connectionsLabel.TabIndex = 2; connectionsLabel.Text = "Connections"; // + // editButton + // + editButton.Anchor = AnchorStyles.Bottom | AnchorStyles.Right; + editButton.Location = new Point(776, 560); + editButton.Name = "editButton"; + editButton.Size = new Size(112, 34); + editButton.TabIndex = 4; + editButton.Text = "Edit..."; + editButton.UseVisualStyleBackColor = true; + editButton.Click += editButton_Click; + // + // deleteButton + // + deleteButton.Anchor = AnchorStyles.Bottom | AnchorStyles.Right; + deleteButton.Location = new Point(339, 560); + deleteButton.Name = "deleteButton"; + deleteButton.Size = new Size(112, 34); + deleteButton.TabIndex = 1; + deleteButton.Text = "Delete"; + deleteButton.UseVisualStyleBackColor = true; + deleteButton.Click += deleteButton_Click; + // + // reconnectButton + // + reconnectButton.Anchor = AnchorStyles.Bottom | AnchorStyles.Right; + reconnectButton.Location = new Point(636, 560); + reconnectButton.Name = "reconnectButton"; + reconnectButton.Size = new Size(112, 34); + reconnectButton.TabIndex = 3; + reconnectButton.Text = "Reconnect"; + reconnectButton.UseVisualStyleBackColor = true; + reconnectButton.Click += reconnectButton_Click; + // + // makeDefaultButton + // + makeDefaultButton.Anchor = AnchorStyles.Bottom | AnchorStyles.Right; + makeDefaultButton.Location = new Point(473, 560); + makeDefaultButton.Name = "makeDefaultButton"; + makeDefaultButton.Size = new Size(139, 34); + makeDefaultButton.TabIndex = 2; + makeDefaultButton.Text = "Make Default"; + makeDefaultButton.UseVisualStyleBackColor = true; + makeDefaultButton.Click += makeDefaultButton_Click; + // // ConnectionManagerDialog // AutoScaleDimensions = new SizeF(10F, 25F); AutoScaleMode = AutoScaleMode.Font; ClientSize = new Size(1115, 615); + Controls.Add(makeDefaultButton); + Controls.Add(reconnectButton); + Controls.Add(deleteButton); + Controls.Add(editButton); Controls.Add(connectionsLabel); Controls.Add(newButton); Controls.Add(dataGridView1); @@ -83,5 +139,9 @@ private void InitializeComponent() { private DataGridView dataGridView1; private Button newButton; private Label connectionsLabel; + private Button editButton; + private Button deleteButton; + private Button reconnectButton; + private Button makeDefaultButton; } } \ No newline at end of file diff --git a/csharp/ExcelAddIn/views/ConnectionManagerDialog.cs b/csharp/ExcelAddIn/views/ConnectionManagerDialog.cs index 750fd39e0de..a3177133202 100644 --- a/csharp/ExcelAddIn/views/ConnectionManagerDialog.cs +++ b/csharp/ExcelAddIn/views/ConnectionManagerDialog.cs @@ -2,74 +2,83 @@ namespace Deephaven.ExcelAddIn.Views; +using SelectedRowsAction = Action; + public partial class ConnectionManagerDialog : Form { private const string IsDefaultColumnName = "IsDefault"; - private const string SettingsButtonColumnName = "settings_button_column"; - private const string ReconnectButtonColumnName = "reconnect_button_column"; private readonly Action _onNewButtonClicked; + private readonly SelectedRowsAction _onDeleteButtonClicked; + private readonly SelectedRowsAction _onReconnectButtonClicked; + private readonly SelectedRowsAction _onMakeDefaultButtonClicked; + private readonly SelectedRowsAction _onEditButtonClicked; private readonly BindingSource _bindingSource = new(); - public ConnectionManagerDialog(Action onNewButtonClicked) { + public ConnectionManagerDialog(Action onNewButtonClicked, + SelectedRowsAction onDeleteButtonClicked, + SelectedRowsAction onReconnectButtonClicked, + SelectedRowsAction onMakeDefaultButtonClicked, + SelectedRowsAction onEditButtonClicked) { _onNewButtonClicked = onNewButtonClicked; + _onDeleteButtonClicked = onDeleteButtonClicked; + _onReconnectButtonClicked = onReconnectButtonClicked; + _onMakeDefaultButtonClicked = onMakeDefaultButtonClicked; + _onEditButtonClicked = onEditButtonClicked; InitializeComponent(); _bindingSource.DataSource = typeof(ConnectionManagerDialogRow); dataGridView1.DataSource = _bindingSource; - - var settingsButtonColumn = new DataGridViewButtonColumn { - Name = SettingsButtonColumnName, - HeaderText = "Credentials", - Text = "Edit", - UseColumnTextForButtonValue = true - }; - - var reconnectButtonColumn = new DataGridViewButtonColumn { - Name = ReconnectButtonColumnName, - HeaderText = "Reconnect", - Text = "Reconnect", - UseColumnTextForButtonValue = true - }; - - dataGridView1.Columns.Add(settingsButtonColumn); - dataGridView1.Columns.Add(reconnectButtonColumn); - - dataGridView1.CellClick += DataGridView1_CellClick; } public void AddRow(ConnectionManagerDialogRow row) { + if (InvokeRequired) { + Invoke(() => AddRow(row)); + return; + } _bindingSource.Add(row); + dataGridView1.ClearSelection(); } - private void DataGridView1_CellClick(object? sender, DataGridViewCellEventArgs e) { - if (e.RowIndex < 0) { + public void RemoveRow(ConnectionManagerDialogRow row) { + if (InvokeRequired) { + Invoke(() => RemoveRow(row)); return; } + _bindingSource.Remove(row); + } - if (_bindingSource[e.RowIndex] is not ConnectionManagerDialogRow row) { - return; - } - var name = dataGridView1.Columns[e.ColumnIndex].Name; + private void newButton_Click(object sender, EventArgs e) { + _onNewButtonClicked(); + } - switch (name) { - case SettingsButtonColumnName: { - row.SettingsClicked(); - break; - } + private void reconnectButton_Click(object sender, EventArgs e) { + var selections = GetSelectedRows(); + _onReconnectButtonClicked(selections); + } - case ReconnectButtonColumnName: { - row.ReconnectClicked(); - break; - } + private void editButton_Click(object sender, EventArgs e) { + var selections = GetSelectedRows(); + _onEditButtonClicked(selections); + } - case IsDefaultColumnName: { - row.IsDefaultClicked(); - break; - } - } + private void deleteButton_Click(object sender, EventArgs e) { + var selections = GetSelectedRows(); + _onDeleteButtonClicked(selections); } - private void newButton_Click(object sender, EventArgs e) { - _onNewButtonClicked(); + private void makeDefaultButton_Click(object sender, EventArgs e) { + var selections = GetSelectedRows(); + _onMakeDefaultButtonClicked(selections); + } + + private ConnectionManagerDialogRow[] GetSelectedRows() { + var result = new List(); + var sr = dataGridView1.SelectedRows; + var count = sr.Count; + for (var i = 0; i != count; ++i) { + result.Add((ConnectionManagerDialogRow)sr[i].DataBoundItem); + } + + return result.ToArray(); } } diff --git a/csharp/ExcelAddIn/views/ConnectionManagerDialog.resx b/csharp/ExcelAddIn/views/ConnectionManagerDialog.resx index b3e33e7e100..7f2cf2b8014 100644 --- a/csharp/ExcelAddIn/views/ConnectionManagerDialog.resx +++ b/csharp/ExcelAddIn/views/ConnectionManagerDialog.resx @@ -1,7 +1,7 @@  + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + text/microsoft-resx + + + 2.0 + + + System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + \ No newline at end of file