Skip to content

Conversation

@gfinger
Copy link
Contributor

@gfinger gfinger commented Jan 12, 2023

What does this PR do?

A local URL like : local:<absolutePathToDB>/DB can be used within the Console to connect to a local DB, to create and drop it.

Motivation

It should be possible to use the Console not relying on the current work-directory. It might also be advantageous not to depend on a global setting like when using the -Darcadedb.server.databaseDirectory parameter at startup of the console.

Related issues

Introduction of the -Darcadedb.server.databaseDirectory parameter for the Console.

Additional Notes

The commit also contains Unit Tests for the local connect, as well as a test whether the local URL can also be used with the CREATE DB and the DROP DB commands.
I used JDK 19 for compiling and testing, but the code is compatible with Java 8.

Checklist

  • I have run the build using mvn clean package command
  • My unit tests cover both failure and success scenarios: No, this seems not necessary. There is also no negative test for the existing connect test.

private String parseLocalUrl(final String url) {
return databaseDirectory + url.replaceFirst("file://", "");
if(url.startsWith(LOCAL_PREFIX + "//")) {
return url.replaceFirst(LOCAL_PREFIX + "//", "/");
Copy link
Contributor

Choose a reason for hiding this comment

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

This forces the path tp be absolute, because it prefix the path with /. Is this wanted or a bug?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The paths are supposed to be absolute. The intention is to not rely on the current work-directory. Any not-absolute path would always be relative to the CWD. The URL without the LOCAL_PREFIX is still supported and allows to connect to a db relative to the database directory.


@Test
public void testDropCreateWithLocalUrl() throws IOException {
String localUrl = "local:/" + absoluteDBPath + "/" + DB_NAME;
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you prefixing with local:/ because the absolute path already starts with /?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, exactly.

@gfinger
Copy link
Contributor Author

gfinger commented Jan 13, 2023

The build-error is:

Error:  Errors: 
Error:    JdbcQueriesTest.simpleCypherQuery:88 » PSQL ERROR: Syntax error on parsing query: java.lang.IllegalArgumentException: Unsupported class file major version 63
Error:    JdbcQueriesTest.simpleGremlinQuery:74 » PSQL ERROR: Syntax error on parsing query: java.lang.IllegalArgumentException: Unsupported class file major version 63
Error:    RemoteDatabaseQueriesTest.simpleCypherQuery:64->lambda$simpleCypherQuery$2:65 » Remote Error on executing remote operation query (cause:java.util.concurrent.ExecutionException detail:java.lang.IllegalArgumentException: Unsupported class file major version 63)
Error:    RemoteDatabaseQueriesTest.simpleGremlinQuery:56->lambda$simpleGremlinQuery$1:57 » Remote Error on executing remote operation query (cause:java.util.concurrent.ExecutionException detail:java.lang.IllegalArgumentException: Unsupported class file major version 63)

Is it possible that the test uses a JDK version not supported by Gremlin? I cannot see any cause in my small coding additions that might be responsible for this kind of error.

@gfinger gfinger closed this Jan 20, 2023
@gfinger gfinger deleted the local-connect branch January 20, 2023 15:51
@lvca lvca added the enhancement New feature or request label Jan 20, 2023
@lvca lvca added this to the 23.1.2 milestone Jan 20, 2023
@lvca
Copy link
Contributor

lvca commented Jan 20, 2023

Merged, thanks @gfinger !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants