From a72e8f1ede3dc11fb60bd1346e6c7cb07c5d126e Mon Sep 17 00:00:00 2001 From: Jean Chalard Date: Thu, 5 Dec 2013 18:02:20 +0900 Subject: [PATCH] [RF3] Cleanups Make the version number a single number on native and java side. Also, remove the hasValidContents method. It's useless since the native code already checks this when creating the dictionary (I wish I had known that when I added it). Bug: 11281748 Change-Id: I572d37429972b2f280e4bdb748b709e5d0d7737e --- .../inputmethod/latin/BinaryDictionary.java | 5 ----- .../inputmethod/latin/DictionaryFactory.java | 2 +- .../latin/ExpandableBinaryDictionary.java | 12 +++++------- .../latin/ReadOnlyBinaryDictionary.java | 9 --------- .../inputmethod/latin/makedict/FormatSpec.java | 4 ++-- .../latin/makedict/Ver4DictDecoder.java | 2 +- ...android_inputmethod_latin_BinaryDictionary.cpp | 15 +-------------- .../src/suggest/core/dictionary/dictionary.cpp | 2 +- .../jni/src/suggest/core/dictionary/dictionary.h | 6 +++--- .../dictionary_structure_with_buffer_policy.h | 4 ++-- .../policyimpl/dictionary/header/header_policy.h | 11 +++++++---- ...onary_structure_with_buffer_policy_factory.cpp | 14 +++++++------- ...tionary_structure_with_buffer_policy_factory.h | 2 +- .../dictionary/utils/dict_file_writing_utils.cpp | 2 +- .../policyimpl/dictionary/utils/format_utils.cpp | 9 ++++++--- .../policyimpl/dictionary/utils/format_utils.h | 8 ++++---- 16 files changed, 42 insertions(+), 65 deletions(-) diff --git a/java/src/com/android/inputmethod/latin/BinaryDictionary.java b/java/src/com/android/inputmethod/latin/BinaryDictionary.java index aa530ffb9..a41cc6a82 100644 --- a/java/src/com/android/inputmethod/latin/BinaryDictionary.java +++ b/java/src/com/android/inputmethod/latin/BinaryDictionary.java @@ -121,7 +121,6 @@ public final class BinaryDictionary extends Dictionary { String[] attributeKeyStringArray, String[] attributeValueStringArray); private static native long openNative(String sourceDir, long dictOffset, long dictSize, boolean isUpdatable); - private static native boolean hasValidContentsNative(long dict); private static native void flushNative(long dict, String filePath); private static native boolean needsToRunGCNative(long dict, boolean mindsBlockByGC); private static native void flushWithGCNative(long dict, String filePath); @@ -243,10 +242,6 @@ public final class BinaryDictionary extends Dictionary { return mNativeDict != 0; } - public boolean hasValidContents() { - return hasValidContentsNative(mNativeDict); - } - public int getFormatVersion() { return getFormatVersionNative(mNativeDict); } diff --git a/java/src/com/android/inputmethod/latin/DictionaryFactory.java b/java/src/com/android/inputmethod/latin/DictionaryFactory.java index bcb38da38..e09c309ea 100644 --- a/java/src/com/android/inputmethod/latin/DictionaryFactory.java +++ b/java/src/com/android/inputmethod/latin/DictionaryFactory.java @@ -63,7 +63,7 @@ public final class DictionaryFactory { final ReadOnlyBinaryDictionary readOnlyBinaryDictionary = new ReadOnlyBinaryDictionary(f.mFilename, f.mOffset, f.mLength, useFullEditDistance, locale, Dictionary.TYPE_MAIN); - if (readOnlyBinaryDictionary.hasValidContents()) { + if (readOnlyBinaryDictionary.isValidDictionary()) { dictList.add(readOnlyBinaryDictionary); } else { readOnlyBinaryDictionary.close(); diff --git a/java/src/com/android/inputmethod/latin/ExpandableBinaryDictionary.java b/java/src/com/android/inputmethod/latin/ExpandableBinaryDictionary.java index 98a18f6d3..41661573d 100644 --- a/java/src/com/android/inputmethod/latin/ExpandableBinaryDictionary.java +++ b/java/src/com/android/inputmethod/latin/ExpandableBinaryDictionary.java @@ -63,7 +63,7 @@ abstract public class ExpandableBinaryDictionary extends Dictionary { */ protected static final int MAX_WORD_LENGTH = Constants.DICTIONARY_MAX_WORD_LENGTH; - private static final int DICTIONARY_FORMAT_VERSION = 4; + private static final int DICTIONARY_FORMAT_VERSION = FormatSpec.VERSION4; /** * A static map of update controllers, each of which records the time of accesses to a single @@ -139,8 +139,8 @@ abstract public class ExpandableBinaryDictionary extends Dictionary { return formatVersion == 2; } - public boolean hasValidContents() { - return mBinaryDictionary.hasValidContents(); + public boolean isValidDictionary() { + return mBinaryDictionary.isValidDictionary(); } protected String getFileNameExtentionToOpenDict() { @@ -563,8 +563,7 @@ abstract public class ExpandableBinaryDictionary extends Dictionary { loadDictionaryAsync(); mDictionaryWriter.write(mFilename, getHeaderAttributeMap()); } else { - if (mBinaryDictionary == null || !mBinaryDictionary.isValidDictionary() - || !hasValidContents() + if (mBinaryDictionary == null || !isValidDictionary() // TODO: remove the check below || !matchesExpectedBinaryDictFormatVersionForThisType( mBinaryDictionary.getFormatVersion())) { @@ -665,8 +664,7 @@ abstract public class ExpandableBinaryDictionary extends Dictionary { // load the shared dictionary. loadBinaryDictionary(); } - if (mBinaryDictionary != null && !(mBinaryDictionary.isValidDictionary() - && hasValidContents() + if (mBinaryDictionary != null && !(isValidDictionary() // TODO: remove the check below && matchesExpectedBinaryDictFormatVersionForThisType( mBinaryDictionary.getFormatVersion()))) { diff --git a/java/src/com/android/inputmethod/latin/ReadOnlyBinaryDictionary.java b/java/src/com/android/inputmethod/latin/ReadOnlyBinaryDictionary.java index c8e4014bb..68505ce38 100644 --- a/java/src/com/android/inputmethod/latin/ReadOnlyBinaryDictionary.java +++ b/java/src/com/android/inputmethod/latin/ReadOnlyBinaryDictionary.java @@ -44,15 +44,6 @@ public final class ReadOnlyBinaryDictionary extends Dictionary { locale, dictType, false /* isUpdatable */); } - public boolean hasValidContents() { - mLock.readLock().lock(); - try { - return mBinaryDictionary.hasValidContents(); - } finally { - mLock.readLock().unlock(); - } - } - public boolean isValidDictionary() { return mBinaryDictionary.isValidDictionary(); } diff --git a/java/src/com/android/inputmethod/latin/makedict/FormatSpec.java b/java/src/com/android/inputmethod/latin/makedict/FormatSpec.java index 93ccc62b4..b81c8d8e5 100644 --- a/java/src/com/android/inputmethod/latin/makedict/FormatSpec.java +++ b/java/src/com/android/inputmethod/latin/makedict/FormatSpec.java @@ -204,8 +204,8 @@ public final class FormatSpec { static final int NOT_A_VERSION_NUMBER = -1; static final int FIRST_VERSION_WITH_DYNAMIC_UPDATE = 3; static final int FIRST_VERSION_WITH_TERMINAL_ID = 4; - static final int VERSION3 = 3; - static final int VERSION4 = 4; + public static final int VERSION3 = 3; + public static final int VERSION4 = 4; // These options need to be the same numeric values as the one in the native reading code. static final int GERMAN_UMLAUT_PROCESSING_FLAG = 0x1; diff --git a/java/src/com/android/inputmethod/latin/makedict/Ver4DictDecoder.java b/java/src/com/android/inputmethod/latin/makedict/Ver4DictDecoder.java index 8833c35aa..2cbec7cec 100644 --- a/java/src/com/android/inputmethod/latin/makedict/Ver4DictDecoder.java +++ b/java/src/com/android/inputmethod/latin/makedict/Ver4DictDecoder.java @@ -161,7 +161,7 @@ public class Ver4DictDecoder extends AbstractDictDecoder { } final FileHeader header = super.readHeader(mDictBuffer); final int version = header.mFormatOptions.mVersion; - if (version != 4) { + if (version != FormatSpec.VERSION4) { throw new UnsupportedFormatException("File header has a wrong version : " + version); } return header; diff --git a/native/jni/com_android_inputmethod_latin_BinaryDictionary.cpp b/native/jni/com_android_inputmethod_latin_BinaryDictionary.cpp index 0ef372a51..71b74b24f 100644 --- a/native/jni/com_android_inputmethod_latin_BinaryDictionary.cpp +++ b/native/jni/com_android_inputmethod_latin_BinaryDictionary.cpp @@ -86,7 +86,7 @@ static jlong latinime_BinaryDictionary_open(JNIEnv *env, jclass clazz, jstring s char sourceDirChars[sourceDirUtf8Length + 1]; env->GetStringUTFRegion(sourceDir, 0, env->GetStringLength(sourceDir), sourceDirChars); sourceDirChars[sourceDirUtf8Length] = '\0'; - DictionaryStructureWithBufferPolicy::StructurePoilcyPtr dictionaryStructureWithBufferPolicy = + DictionaryStructureWithBufferPolicy::StructurePolicyPtr dictionaryStructureWithBufferPolicy = DictionaryStructureWithBufferPolicyFactory::newDictionaryStructureWithBufferPolicy( sourceDirChars, static_cast(dictOffset), static_cast(dictSize), isUpdatable == JNI_TRUE); @@ -135,14 +135,6 @@ static void latinime_BinaryDictionary_close(JNIEnv *env, jclass clazz, jlong dic delete dictionary; } -static bool latinime_BinaryDictionary_hasValidContents(JNIEnv *env, jclass clazz, - jlong dict) { - Dictionary *dictionary = reinterpret_cast(dict); - if (!dictionary) return false; - // TODO: check format version - return true; -} - static int latinime_BinaryDictionary_getFormatVersion(JNIEnv *env, jclass clazz, jlong dict) { Dictionary *dictionary = reinterpret_cast(dict); if (!dictionary) return 0; @@ -466,11 +458,6 @@ static const JNINativeMethod sMethods[] = { const_cast("(J)V"), reinterpret_cast(latinime_BinaryDictionary_close) }, - { - const_cast("hasValidContentsNative"), - const_cast("(J)Z"), - reinterpret_cast(latinime_BinaryDictionary_hasValidContents) - }, { const_cast("getFormatVersionNative"), const_cast("(J)I"), diff --git a/native/jni/src/suggest/core/dictionary/dictionary.cpp b/native/jni/src/suggest/core/dictionary/dictionary.cpp index 8055707b7..4ee5a5ed3 100644 --- a/native/jni/src/suggest/core/dictionary/dictionary.cpp +++ b/native/jni/src/suggest/core/dictionary/dictionary.cpp @@ -34,7 +34,7 @@ namespace latinime { const int Dictionary::HEADER_ATTRIBUTE_BUFFER_SIZE = 32; -Dictionary::Dictionary(JNIEnv *env, const DictionaryStructureWithBufferPolicy::StructurePoilcyPtr +Dictionary::Dictionary(JNIEnv *env, const DictionaryStructureWithBufferPolicy::StructurePolicyPtr &dictionaryStructureWithBufferPolicy) : mDictionaryStructureWithBufferPolicy(dictionaryStructureWithBufferPolicy), mBigramDictionary(new BigramDictionary(mDictionaryStructureWithBufferPolicy.get())), diff --git a/native/jni/src/suggest/core/dictionary/dictionary.h b/native/jni/src/suggest/core/dictionary/dictionary.h index 4fef051d3..122e4fc4f 100644 --- a/native/jni/src/suggest/core/dictionary/dictionary.h +++ b/native/jni/src/suggest/core/dictionary/dictionary.h @@ -56,8 +56,8 @@ class Dictionary { static const int KIND_FLAG_POSSIBLY_OFFENSIVE = 0x80000000; static const int KIND_FLAG_EXACT_MATCH = 0x40000000; - Dictionary(JNIEnv *env, const DictionaryStructureWithBufferPolicy::StructurePoilcyPtr - &dictionaryStructureWithBufferPoilcy); + Dictionary(JNIEnv *env, const DictionaryStructureWithBufferPolicy::StructurePolicyPtr + &dictionaryStructureWithBufferPolicy); int getSuggestions(ProximityInfo *proximityInfo, DicTraverseSession *traverseSession, int *xcoordinates, int *ycoordinates, int *times, int *pointerIds, int *inputCodePoints, @@ -109,7 +109,7 @@ class Dictionary { static const int HEADER_ATTRIBUTE_BUFFER_SIZE; - const DictionaryStructureWithBufferPolicy::StructurePoilcyPtr + const DictionaryStructureWithBufferPolicy::StructurePolicyPtr mDictionaryStructureWithBufferPolicy; const BigramDictionaryPtr mBigramDictionary; const SuggestInterfacePtr mGestureSuggest; diff --git a/native/jni/src/suggest/core/policy/dictionary_structure_with_buffer_policy.h b/native/jni/src/suggest/core/policy/dictionary_structure_with_buffer_policy.h index 523aa93e6..417d22e0d 100644 --- a/native/jni/src/suggest/core/policy/dictionary_structure_with_buffer_policy.h +++ b/native/jni/src/suggest/core/policy/dictionary_structure_with_buffer_policy.h @@ -29,12 +29,12 @@ class DictionaryHeaderStructurePolicy; class DictionaryShortcutsStructurePolicy; /* - * This class abstracts structure of dictionaries. + * This class abstracts the structure of dictionaries. * Implement this policy to support additional dictionaries. */ class DictionaryStructureWithBufferPolicy { public: - typedef ExclusiveOwnershipPointer StructurePoilcyPtr; + typedef ExclusiveOwnershipPointer StructurePolicyPtr; virtual ~DictionaryStructureWithBufferPolicy() {} diff --git a/native/jni/src/suggest/policyimpl/dictionary/header/header_policy.h b/native/jni/src/suggest/policyimpl/dictionary/header/header_policy.h index fa5d5fa5a..29e937b3a 100644 --- a/native/jni/src/suggest/policyimpl/dictionary/header/header_policy.h +++ b/native/jni/src/suggest/policyimpl/dictionary/header/header_policy.h @@ -78,15 +78,18 @@ class HeaderPolicy : public DictionaryHeaderStructurePolicy { ~HeaderPolicy() {} virtual int getFormatVersionNumber() const { + // Conceptually this converts the symbolic value we use in the code into the + // hardcoded of the bytes in the file. But we want the constants to be the + // same so we use them for both here. switch (mDictFormatVersion) { case FormatUtils::VERSION_2: - return 2; + return FormatUtils::VERSION_2; case FormatUtils::VERSION_3: - return 3; + return FormatUtils::VERSION_3; case FormatUtils::VERSION_4: - return 4; + return FormatUtils::VERSION_4; default: - return 0; + return FormatUtils::UNKNOWN_VERSION; } } diff --git a/native/jni/src/suggest/policyimpl/dictionary/structure/dictionary_structure_with_buffer_policy_factory.cpp b/native/jni/src/suggest/policyimpl/dictionary/structure/dictionary_structure_with_buffer_policy_factory.cpp index f3d90f81c..f1b733a9c 100644 --- a/native/jni/src/suggest/policyimpl/dictionary/structure/dictionary_structure_with_buffer_policy_factory.cpp +++ b/native/jni/src/suggest/policyimpl/dictionary/structure/dictionary_structure_with_buffer_policy_factory.cpp @@ -28,7 +28,7 @@ namespace latinime { -/* static */ DictionaryStructureWithBufferPolicy::StructurePoilcyPtr +/* static */ DictionaryStructureWithBufferPolicy::StructurePolicyPtr DictionaryStructureWithBufferPolicyFactory ::newDictionaryStructureWithBufferPolicy(const char *const path, const int bufOffset, const int size, const bool isUpdatable) { @@ -37,12 +37,12 @@ namespace latinime { MmappedBuffer::MmappedBufferPtr mmappedBuffer = MmappedBuffer::openBuffer(path, bufOffset, size, isUpdatable); if (!mmappedBuffer.get()) { - return DictionaryStructureWithBufferPolicy::StructurePoilcyPtr(0); + return DictionaryStructureWithBufferPolicy::StructurePolicyPtr(0); } switch (FormatUtils::detectFormatVersion(mmappedBuffer.get()->getBuffer(), mmappedBuffer.get()->getBufferSize())) { case FormatUtils::VERSION_2: - return DictionaryStructureWithBufferPolicy::StructurePoilcyPtr( + return DictionaryStructureWithBufferPolicy::StructurePolicyPtr( new PatriciaTriePolicy(mmappedBuffer)); case FormatUtils::VERSION_4: { const int dictDirPathBufSize = strlen(path) + 1 /* terminator */; @@ -50,22 +50,22 @@ namespace latinime { if (!FileUtils::getFilePathWithoutSuffix(path, Ver4DictConstants::TRIE_FILE_EXTENSION, dictDirPathBufSize, dictDirPath)) { // Dictionary file name is not valid as a version 4 dictionary. - return DictionaryStructureWithBufferPolicy::StructurePoilcyPtr(0); + return DictionaryStructureWithBufferPolicy::StructurePolicyPtr(0); } const Ver4DictBuffers::Ver4DictBuffersPtr dictBuffers = Ver4DictBuffers::openVer4DictBuffers(dictDirPath, mmappedBuffer); if (!dictBuffers.get()->isValid()) { AKLOGE("DICT: The dictionary doesn't satisfy ver4 format requirements."); ASSERT(false); - return DictionaryStructureWithBufferPolicy::StructurePoilcyPtr(0); + return DictionaryStructureWithBufferPolicy::StructurePolicyPtr(0); } - return DictionaryStructureWithBufferPolicy::StructurePoilcyPtr( + return DictionaryStructureWithBufferPolicy::StructurePolicyPtr( new Ver4PatriciaTriePolicy(dictBuffers)); } default: AKLOGE("DICT: dictionary format is unknown, bad magic number"); ASSERT(false); - return DictionaryStructureWithBufferPolicy::StructurePoilcyPtr(0); + return DictionaryStructureWithBufferPolicy::StructurePolicyPtr(0); } } diff --git a/native/jni/src/suggest/policyimpl/dictionary/structure/dictionary_structure_with_buffer_policy_factory.h b/native/jni/src/suggest/policyimpl/dictionary/structure/dictionary_structure_with_buffer_policy_factory.h index 1359575f1..45237e4aa 100644 --- a/native/jni/src/suggest/policyimpl/dictionary/structure/dictionary_structure_with_buffer_policy_factory.h +++ b/native/jni/src/suggest/policyimpl/dictionary/structure/dictionary_structure_with_buffer_policy_factory.h @@ -27,7 +27,7 @@ namespace latinime { class DictionaryStructureWithBufferPolicyFactory { public: - static DictionaryStructureWithBufferPolicy::StructurePoilcyPtr + static DictionaryStructureWithBufferPolicy::StructurePolicyPtr newDictionaryStructureWithBufferPolicy(const char *const path, const int bufOffset, const int size, const bool isUpdatable); diff --git a/native/jni/src/suggest/policyimpl/dictionary/utils/dict_file_writing_utils.cpp b/native/jni/src/suggest/policyimpl/dictionary/utils/dict_file_writing_utils.cpp index ce9223158..b463d4149 100644 --- a/native/jni/src/suggest/policyimpl/dictionary/utils/dict_file_writing_utils.cpp +++ b/native/jni/src/suggest/policyimpl/dictionary/utils/dict_file_writing_utils.cpp @@ -34,7 +34,7 @@ const char *const DictFileWritingUtils::TEMP_FILE_SUFFIX_FOR_WRITING_DICT_FILE = const int dictVersion, const HeaderReadWriteUtils::AttributeMap *const attributeMap) { TimeKeeper::setCurrentTime(); switch (dictVersion) { - case 4: + case FormatUtils::VERSION_4: return createEmptyV4DictFile(filePath, attributeMap); default: AKLOGE("Cannot create dictionary %s because format version %d is not supported.", diff --git a/native/jni/src/suggest/policyimpl/dictionary/utils/format_utils.cpp b/native/jni/src/suggest/policyimpl/dictionary/utils/format_utils.cpp index 4843650ad..e3d783835 100644 --- a/native/jni/src/suggest/policyimpl/dictionary/utils/format_utils.cpp +++ b/native/jni/src/suggest/policyimpl/dictionary/utils/format_utils.cpp @@ -41,11 +41,14 @@ const int FormatUtils::DICTIONARY_MINIMUM_SIZE = 12; // Dictionary format version number (2 bytes) // Options (2 bytes) // Header size (4 bytes) : integer, big endian - if (ByteArrayUtils::readUint16(dict, 4) == 2) { + // Conceptually this converts the hardcoded value of the bytes in the file into + // the symbolic value we use in the code. But we want the constants to be the + // same so we use them for both here. + if (ByteArrayUtils::readUint16(dict, 4) == VERSION_2) { return VERSION_2; - } else if (ByteArrayUtils::readUint16(dict, 4) == 3) { + } else if (ByteArrayUtils::readUint16(dict, 4) == VERSION_3) { return VERSION_3; - } else if (ByteArrayUtils::readUint16(dict, 4) == 4) { + } else if (ByteArrayUtils::readUint16(dict, 4) == VERSION_4) { return VERSION_4; } else { return UNKNOWN_VERSION; diff --git a/native/jni/src/suggest/policyimpl/dictionary/utils/format_utils.h b/native/jni/src/suggest/policyimpl/dictionary/utils/format_utils.h index b90393a53..3d14d2607 100644 --- a/native/jni/src/suggest/policyimpl/dictionary/utils/format_utils.h +++ b/native/jni/src/suggest/policyimpl/dictionary/utils/format_utils.h @@ -29,10 +29,10 @@ namespace latinime { class FormatUtils { public: enum FORMAT_VERSION { - VERSION_2, - VERSION_3, - VERSION_4, - UNKNOWN_VERSION + VERSION_2 = 2, + VERSION_3 = 3, + VERSION_4 = 4, + UNKNOWN_VERSION = -1 }; // 32 bit magic number is stored at the beginning of the dictionary header to reject