Skip to content

Commit

Permalink
Fix unnecessary CachedContentIndex store() calls
Browse files Browse the repository at this point in the history
First fix, prevents forced rewriting when cipher is set but encrypt is
false.

Second, removes the store() call in SimpleCache.initialize() so
initialization doesn't fail because of CachedContentIndex write issues.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=174450586
  • Loading branch information
erdemguven authored and ojw28 committed Nov 3, 2017
1 parent d90d041 commit 872cfc1
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public CacheException(String message) {
super(message);
}

public CacheException(IOException cause) {
public CacheException(Throwable cause) {
super(cause);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ public CachedContentIndex(File cacheDir, byte[] secretKey) {
*
* @param cacheDir Directory where the index file is kept.
* @param secretKey 16 byte AES key for reading, and optionally writing, the cache index.
* @param encrypt When false, a plaintext index will be written.
* @param encrypt Whether the index will be encrypted when written. Must be false if {@code
* secretKey} is null.
*/
public CachedContentIndex(File cacheDir, byte[] secretKey, boolean encrypt) {
this.encrypt = encrypt;
Expand All @@ -105,6 +106,7 @@ public CachedContentIndex(File cacheDir, byte[] secretKey, boolean encrypt) {
throw new IllegalStateException(e); // Should never happen.
}
} else {
Assertions.checkState(!encrypt);
cipher = null;
secretKeySpec = null;
}
Expand Down Expand Up @@ -259,10 +261,8 @@ private boolean readFile() {
throw new IllegalStateException(e);
}
input = new DataInputStream(new CipherInputStream(inputStream, cipher));
} else {
if (cipher != null) {
changed = true; // Force index to be rewritten encrypted after read.
}
} else if (encrypt) {
changed = true; // Force index to be rewritten encrypted after read.
}

int count = input.readInt();
Expand Down Expand Up @@ -300,11 +300,10 @@ private void writeFile() throws CacheException {
output = new DataOutputStream(bufferedOutputStream);
output.writeInt(VERSION);

boolean writeEncrypted = encrypt && cipher != null;
int flags = writeEncrypted ? FLAG_ENCRYPTED_INDEX : 0;
int flags = encrypt ? FLAG_ENCRYPTED_INDEX : 0;
output.writeInt(flags);

if (writeEncrypted) {
if (encrypt) {
byte[] initializationVector = new byte[16];
new Random().nextBytes(initializationVector);
output.write(initializationVector);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ public final class SimpleCache implements Cache {
private final CachedContentIndex index;
private final HashMap<String, ArrayList<Listener>> listeners;
private long totalSpace = 0;
private CacheException initializationException;

/**
* Constructs the cache. The cache will delete any unrecognized files from the directory. Hence
Expand Down Expand Up @@ -71,7 +70,8 @@ public SimpleCache(File cacheDir, CacheEvictor evictor, byte[] secretKey) {
* @param evictor The evictor to be used.
* @param secretKey If not null, cache keys will be stored encrypted on filesystem using AES/CBC.
* The key must be 16 bytes long.
* @param encrypt When false, a plaintext index will be written.
* @param encrypt Whether the index will be encrypted when written. Must be false if {@code
* secretKey} is null.
*/
public SimpleCache(File cacheDir, CacheEvictor evictor, byte[] secretKey, boolean encrypt) {
this(cacheDir, evictor, new CachedContentIndex(cacheDir, secretKey, encrypt));
Expand All @@ -98,11 +98,7 @@ public SimpleCache(File cacheDir, CacheEvictor evictor, byte[] secretKey, boolea
public void run() {
synchronized (SimpleCache.this) {
conditionVariable.open();
try {
initialize();
} catch (CacheException e) {
initializationException = e;
}
initialize();
SimpleCache.this.evictor.onCacheInitialized();
}
}
Expand Down Expand Up @@ -169,10 +165,6 @@ public synchronized SimpleCacheSpan startReadWrite(String key, long position)
@Override
public synchronized SimpleCacheSpan startReadWriteNonBlocking(String key, long position)
throws CacheException {
if (initializationException != null) {
throw initializationException;
}

SimpleCacheSpan cacheSpan = getSpan(key, position);

// Read case.
Expand Down Expand Up @@ -270,7 +262,7 @@ private SimpleCacheSpan getSpan(String key, long position) throws CacheException
/**
* Ensures that the cache's in-memory representation has been initialized.
*/
private void initialize() throws CacheException {
private void initialize() {
if (!cacheDir.exists()) {
cacheDir.mkdirs();
return;
Expand All @@ -296,7 +288,7 @@ private void initialize() throws CacheException {
}

index.removeEmpty();
index.store();
// Don't call index.store() here so initialization doesn't fail because of write issues.
}

/**
Expand Down

0 comments on commit 872cfc1

Please sign in to comment.