Skip to content

Accept local Url + tests #763

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.

Sign up for GitHub

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

Closed
gfinger wants to merge 1 commit into from
Closed

Accept local Url + tests #763

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 6 additions & 1 deletion console/src/main/java/com/arcadedb/console/Console.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
public class Console {
private static final String PROMPT = "%n%s> ";
private static final String REMOTE_PREFIX = "remote:";
private static final String LOCAL_PREFIX = "local:";
private static final String SQL_LANGUAGE = "SQL";
private final boolean system = System.console() != null;
private final Terminal terminal;
Expand Down Expand Up @@ -714,7 +715,11 @@ private void checkHasSpaces(final String key, final String value) {
}

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

@lvca lvca Jan 12, 2023

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

@gfinger gfinger Jan 13, 2023

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.

} else {
return databaseDirectory + url.replaceFirst("file://", "");
}
}

private void connectToRemoteServer(final String url, final Boolean needsDatabase) {
Expand Down
21 changes: 18 additions & 3 deletions console/src/test/java/com/arcadedb/console/ConsoleTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,20 +40,30 @@
public class ConsoleTest {
private static final String DB_NAME = "console";
private static Console console;
private static String absoluteDBPath;

@BeforeEach
public void populate() throws IOException {
FileUtils.deleteRecursively(new File("./target/databases"));
File dbFile = new File("./target/databases");
absoluteDBPath = dbFile.getAbsolutePath();
FileUtils.deleteRecursively(dbFile);
GlobalConfiguration.SERVER_ROOT_PATH.setValue("./target");
console = new Console(false);
Assertions.assertTrue(console.parse("create database " + DB_NAME + "; close", false));
}

@AfterEach
public void drop() {
public void drop() throws IOException {
console.close();
TestServerHelper.checkActiveDatabases();
FileUtils.deleteRecursively(new File("target/databases"));
Assertions.assertTrue(console.parse("drop database " + DB_NAME + "; close", false));
}

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

@lvca lvca Jan 12, 2023

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

@gfinger gfinger Jan 13, 2023

Choose a reason for hiding this comment

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

yes, exactly.

Assertions.assertTrue(console.parse("drop database " + localUrl + "; close", false));
Assertions.assertTrue(console.parse("create database " + localUrl + "; close", false));
}

@Test
Expand Down Expand Up @@ -91,6 +101,11 @@ public void testConnect() throws IOException {
Assertions.assertTrue(console.parse("connect " + DB_NAME + ";info types", false));
}

@Test
public void testLocalConnect() throws IOException {
Assertions.assertTrue(console.parse("connect local:/" + absoluteDBPath + "/" + DB_NAME + ";info types", false));
}

@Test
public void testSetVerbose() throws IOException {
try {
Expand Down