Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow replacing the DocumentDBSchemaService #477

Open
wants to merge 21 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
543b65a
[AD-1014] Developer Guide. (#451)
Dec 12, 2022
c1281a5
Bump JDBC version from 1.4.1 to 1.4.2 (#457)
affonsov Dec 12, 2022
94c65d9
Add Mongo Driver Information (#433)
affonsov Dec 14, 2022
ad1d4e5
[AD-1017] Implement single-instance SSH tunnel. (#453)
Dec 20, 2022
a341734
[AD-1039] Updated release version and dependencies. (#467)
Dec 20, 2022
7958a6d
[AD-1043] [JDBC] Bug: Build not working in manual workflow action. (#…
Dec 21, 2022
196ab46
Refactored to allow setting a schema metadata service
Dec 12, 2022
6aeab4a
Exposed two methods for subclasses
Jan 4, 2023
87c4190
Merge remote-tracking branch 'aws/develop' into override-schema-service
Jan 5, 2023
8db4f2b
Attempting fix on branch retrieval.
Jan 5, 2023
78eac43
Merge remote-tracking branch 'aws/develop' into override-schema-service
birschick-bq Jan 30, 2023
b7e4346
Changed setGetTableFunction() to be a noop if tables is already set.
Jan 30, 2023
30bf479
Refactored to allow setting a schema metadata service
Dec 12, 2022
c5899db
Exposed two methods for subclasses
Jan 4, 2023
9a61559
Changed setGetTableFunction() to be a noop if tables is already set.
Jan 30, 2023
e7998c8
Fix a compile error in a test file.
Jan 30, 2023
42438a1
Merge branch 'override-schema-service' of https://github.com/normanj-…
birschick-bq Jan 30, 2023
61640dd
Fix merge issues
birschick-bq Jan 30, 2023
3328ff5
Fix merge issues - #2
birschick-bq Jan 30, 2023
0612175
Added a null check on the DocumentDbMetadataService
Jan 31, 2023
bb115b9
Added a comment for when setGetTablesFunction is a noop.
Jan 31, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/badges/jacoco.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import software.amazon.documentdb.jdbc.common.utilities.SqlError;
import software.amazon.documentdb.jdbc.common.utilities.SqlState;
import software.amazon.documentdb.jdbc.metadata.DocumentDbDatabaseSchemaMetadata;
import software.amazon.documentdb.jdbc.metadata.DocumentDbMetadataService;
import software.amazon.documentdb.jdbc.sshtunnel.DocumentDbSshTunnelClient;

import java.sql.DatabaseMetaData;
Expand Down Expand Up @@ -65,14 +66,18 @@ public class DocumentDbConnection extends Connection
private MongoClient mongoClient = null;
private MongoDatabase mongoDatabase = null;
private DocumentDbSshTunnelClient sshTunnelClient;
private final DocumentDbMetadataService metadataService;

/**
* DocumentDbConnection constructor, initializes super class.
*/
DocumentDbConnection(final DocumentDbConnectionProperties connectionProperties)
DocumentDbConnection(
final DocumentDbConnectionProperties connectionProperties,
final DocumentDbMetadataService metadataService)
throws SQLException {
super(connectionProperties);
this.connectionProperties = connectionProperties;
this.metadataService = metadataService;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we might have to check for nulls since the public interface from DocumentDbDriver might return a null from the Supplier.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be sufficient to add a not null check in the DocumentDbDriver constructor so that the DocumentDbMetadataService is never null?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a null check in DocumentDbDriver just before this constructor is called.

if (LOGGER.isDebugEnabled()) {
final StringBuilder sb = new StringBuilder();
sb.append("Creating connection with following properties:");
Expand Down Expand Up @@ -212,6 +217,7 @@ private void setMetadata(final int version) throws SQLException {
connectionProperties,
connectionProperties.getSchemaName(),
version,
metadataService,
getMongoClient());
metadata = new DocumentDbDatabaseMetaData(this, databaseMetadata, connectionProperties);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,15 @@
import org.checkerframework.checker.nullness.qual.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import software.amazon.documentdb.jdbc.metadata.DocumentDbMetadataService;
import software.amazon.documentdb.jdbc.metadata.DocumentDbMetadataServiceImpl;

import java.io.InputStream;
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.SQLException;
import java.util.Properties;
import java.util.function.Supplier;

import static software.amazon.documentdb.jdbc.DocumentDbConnectionProperties.DOCUMENT_DB_SCHEME;

Expand Down Expand Up @@ -69,6 +73,23 @@ public class DocumentDbDriver extends software.amazon.documentdb.jdbc.common.Dri
new DocumentDbDriver().register();
}

private final Supplier<DocumentDbMetadataService> metadataServiceProvider;

/**
* Constructs a new DocumentDbDriver and uses the default metadata service.
*/
public DocumentDbDriver() {
metadataServiceProvider = () -> new DocumentDbMetadataServiceImpl();
}

/**
* Constructs a new DocumentDbDriver with the provided metadata service.
* @param metadataServiceProvider metadata service to use
*/
public DocumentDbDriver(final Supplier<DocumentDbMetadataService> metadataServiceProvider) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@normanj-bitquill I'm assuming this will be the entry point to override the schema service for a future local file implementation. Correct?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@birschick-bq That is correct. The goal is to supply my own implementation of the DocumentDbMetadataService that will be used for all connections made using the driver.

this.metadataServiceProvider = metadataServiceProvider;
}

@SneakyThrows
protected void register() {
DriverManager.registerDriver(this);
Expand All @@ -91,7 +112,11 @@ protected void register() {
throw new SQLException(exception.getMessage(), exception);
}

return new DocumentDbConnection(properties);
final DocumentDbMetadataService metadataService = metadataServiceProvider.get();
if (metadataService == null) {
throw new SQLException("Unable to retrieve the DocumentDbMetadataService");
}
return new DocumentDbConnection(properties, metadataService);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@
import org.slf4j.LoggerFactory;
import software.amazon.documentdb.jdbc.common.utilities.SqlError;
import software.amazon.documentdb.jdbc.metadata.DocumentDbDatabaseSchemaMetadata;
import software.amazon.documentdb.jdbc.metadata.DocumentDbMetadataService;
import software.amazon.documentdb.jdbc.metadata.DocumentDbMetadataServiceImpl;
import software.amazon.documentdb.jdbc.metadata.DocumentDbSchema;
import software.amazon.documentdb.jdbc.metadata.DocumentDbSchemaColumn;
import software.amazon.documentdb.jdbc.metadata.DocumentDbSchemaTable;
Expand Down Expand Up @@ -235,6 +237,7 @@ public class DocumentDbMain {
private static final String REMOVED_SCHEMA_MESSAGE = "Removed schema '%s'.";

private static MongoClient client;
private static final DocumentDbMetadataService METADATA_SERVICE;

static {
ARCHIVE_VERSION = getArchiveVersion();
Expand All @@ -255,6 +258,8 @@ public class DocumentDbMain {
HELP_VERSION_OPTIONS = new Options()
.addOption(HELP_OPTION)
.addOption(VERSION_OPTION);

METADATA_SERVICE = new DocumentDbMetadataServiceImpl();
}

/**
Expand Down Expand Up @@ -438,6 +443,7 @@ private static void updateTableSchema(
properties,
properties.getSchemaName(),
schemaTableList,
METADATA_SERVICE,
getMongoClient(properties));
} catch (SQLException | DocumentDbSchemaSecurityException e) {
output.append(e.getClass().getSimpleName())
Expand Down Expand Up @@ -536,6 +542,7 @@ private static void performExport(
properties,
properties.getSchemaName(),
VERSION_LATEST_OR_NONE,
METADATA_SERVICE,
getMongoClient(properties));
if (schema == null) {
// No schema to export.
Expand Down Expand Up @@ -610,7 +617,7 @@ private static void performListSchema(
final DocumentDbConnectionProperties properties,
final StringBuilder output) throws SQLException {
final List<DocumentDbSchema> schemas = DocumentDbDatabaseSchemaMetadata.getSchemaList(
properties, getMongoClient(properties));
properties, METADATA_SERVICE, getMongoClient(properties));
for (DocumentDbSchema schema : schemas) {
output.append(String.format("Name=%s, Version=%d, SQL Name=%s, Modified=%s%n",
maybeQuote(schema.getSchemaName()),
Expand All @@ -629,6 +636,7 @@ private static void performListTables(
properties,
properties.getSchemaName(),
VERSION_LATEST_OR_NONE,
METADATA_SERVICE,
getMongoClient(properties));
if (schema != null) {
final List<String> sortedTableNames = schema.getTableSchemaMap().keySet().stream()
Expand All @@ -651,6 +659,7 @@ private static void performRemove(
DocumentDbDatabaseSchemaMetadata.remove(
properties,
properties.getSchemaName(),
METADATA_SERVICE,
getMongoClient(properties));
output.append(String.format(REMOVED_SCHEMA_MESSAGE, properties.getSchemaName()));
}
Expand All @@ -662,6 +671,7 @@ private static void performGenerateNew(
properties,
properties.getSchemaName(),
VERSION_NEW,
METADATA_SERVICE,
getMongoClient(properties));
if (schema != null) {
output.append(String.format(NEW_SCHEMA_VERSION_GENERATED_MESSAGE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,10 @@ protected DocumentDbDatabaseSchemaMetadata(
public static DocumentDbDatabaseSchemaMetadata get(
final DocumentDbConnectionProperties properties,
final String schemaName,
final DocumentDbMetadataService metadataService,
final MongoClient client)
throws SQLException {
return get(properties, schemaName, VERSION_LATEST_OR_NEW, client);
return get(properties, schemaName, VERSION_LATEST_OR_NEW, metadataService, client);
}


Expand All @@ -106,15 +107,16 @@ public static DocumentDbDatabaseSchemaMetadata get(
public static DocumentDbDatabaseSchemaMetadata get(
final DocumentDbConnectionProperties properties, final String schemaName,
final int schemaVersion,
final DocumentDbMetadataService metadataService,
final MongoClient client) throws SQLException {

// Try to get it from the service.
final DocumentDbDatabaseSchemaMetadata databaseMetadata;
final DocumentDbSchema schema = DocumentDbMetadataService
final DocumentDbSchema schema = metadataService
.get(properties, schemaName, schemaVersion, client);
if (schema != null) {
// Setup lazy load based on table ID.
setSchemaGetTableFunction(properties, schemaName, schemaVersion, schema, client);
setSchemaGetTableFunction(properties, schemaName, schemaVersion, schema, metadataService, client);
databaseMetadata = new DocumentDbDatabaseSchemaMetadata(schema);
} else {
databaseMetadata = null;
Expand All @@ -134,8 +136,9 @@ public static DocumentDbDatabaseSchemaMetadata get(
public static void remove(
final DocumentDbConnectionProperties properties,
final String schemaName,
final DocumentDbMetadataService metadataService,
final MongoClient client) throws SQLException {
DocumentDbMetadataService.remove(properties, schemaName, client);
metadataService.remove(properties, schemaName, client);
}

/**
Expand All @@ -152,8 +155,9 @@ public static void remove(
final DocumentDbConnectionProperties properties,
final String schemaName,
final int schemaVersion,
final DocumentDbMetadataService metadataService,
final MongoClient client) throws SQLException {
DocumentDbMetadataService.remove(properties, schemaName, schemaVersion, client);
metadataService.remove(properties, schemaName, schemaVersion, client);
}

/**
Expand All @@ -166,11 +170,12 @@ public static void remove(
*/
public static List<DocumentDbSchema> getSchemaList(
final DocumentDbConnectionProperties properties,
final DocumentDbMetadataService metadataService,
final MongoClient client) throws SQLException {
final List<DocumentDbSchema> schemas = DocumentDbMetadataService
final List<DocumentDbSchema> schemas = metadataService
.getSchemaList(properties, client);
schemas.forEach(schema -> setSchemaGetTableFunction(
properties, schema.getSchemaName(), schema.getSchemaVersion(), schema, client));
properties, schema.getSchemaName(), schema.getSchemaVersion(), schema, metadataService, client));
return schemas;
}

Expand All @@ -190,21 +195,23 @@ public static void update(
final DocumentDbConnectionProperties properties,
final String schemaName,
final Collection<DocumentDbSchemaTable> schemaTables,
final DocumentDbMetadataService metadataService,
final MongoClient client)
throws SQLException, DocumentDbSchemaSecurityException {
DocumentDbMetadataService.update(properties, schemaName, schemaTables, client);
metadataService.update(properties, schemaName, schemaTables, client);
}

private static void setSchemaGetTableFunction(
final DocumentDbConnectionProperties properties,
final String schemaName,
final int schemaVersion,
final DocumentDbSchema schema,
final DocumentDbMetadataService metadataService,
final MongoClient client) {
schema.setGetTableFunction(
tableId -> DocumentDbMetadataService
tableId -> metadataService
.getTable(properties, schemaName, schemaVersion, tableId, client),
remainingTableIds -> DocumentDbMetadataService
remainingTableIds -> metadataService
.getTables(properties, schemaName, schemaVersion, remainingTableIds, client));
}

Expand Down
Loading