Skip to content

Commit

Permalink
Check whether historyFile is readable/writable
Browse files Browse the repository at this point in the history
This change updates Console::getHistory() so that if the historyFile
is either not readable or writable, then MemoryHistory is used instead
of FileHistory. Previously, in that case CLI would crash as FileHistory
would fail when flushing the history to disk.
  • Loading branch information
bivas authored and nezihyigitbasi committed Mar 7, 2018
1 parent f2f24cf commit 92e82dc
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 1 deletion.
13 changes: 12 additions & 1 deletion presto-cli/src/main/java/com/facebook/presto/cli/Console.java
Expand Up @@ -16,6 +16,7 @@
import com.facebook.presto.cli.ClientOptions.OutputFormat;
import com.facebook.presto.client.ClientSession;
import com.facebook.presto.sql.parser.StatementSplitter;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableSet;
import com.google.common.io.Files;
Expand Down Expand Up @@ -364,8 +365,18 @@ private static MemoryHistory getHistory()
return getHistory(new File(getUserHome(), ".presto_history"));
}

private static MemoryHistory getHistory(File historyFile)
@VisibleForTesting
static MemoryHistory getHistory(File historyFile)
{
if (!historyFile.canWrite() || !historyFile.canRead()) {
System.err.printf("WARNING: History file is not readable/writable: %s. " +
"History will not be available during this session.%n",
historyFile.getAbsolutePath());
MemoryHistory history = new MemoryHistory();
history.setAutoTrim(true);
return history;
}

MemoryHistory history;
try {
history = new FileHistory(historyFile);
Expand Down
@@ -0,0 +1,37 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.facebook.presto.cli;

import jline.console.history.FileHistory;
import jline.console.history.MemoryHistory;
import org.testng.annotations.Test;

import java.io.File;

import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertNotNull;

public class TestConsoleHistory
{
@Test
public void testNonExistingHomeFolder() throws Exception
{
File historyFile = new File("/?", ".history");
assertFalse(historyFile.canRead(), "historyFile is readable");
assertFalse(historyFile.canWrite(), "historyFile is writable");
MemoryHistory result = Console.getHistory(historyFile);
assertNotNull(result, "result is null");
assertFalse(result instanceof FileHistory, "result type is FileHistory");
}
}

0 comments on commit 92e82dc

Please sign in to comment.