Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ public boolean onOptionsItemSelected(MenuItem item) {
return true;
} else if (item.getItemId() == R.id.action_update_keyboards) {
KMManager.getUpdateTool().executeOpenUpdates();
// Dismiss icon
// Dismiss icon - we intentionally hide the icon, even if the user cancels the update
updateUpdateCountIndicator(0);
final MenuItem _keyboardupdate = menu.findItem(R.id.action_update_keyboards);
if (_keyboardupdate != null && _keyboardupdate.isVisible()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,14 +263,14 @@ protected void onResume() {


// Determine the index to the current keyboard position to highlight as the selected keyboard
int currentKeyboardIndex;
int currentKeyboardIndex;
String currentKeyboardKey = KMKeyboard.currentKeyboard();
if (currentKeyboardKey == null) {
SharedPreferences prefs = this.getSharedPreferences(this.getString(R.string.kma_prefs_name), Context.MODE_PRIVATE);
currentKeyboardIndex = prefs.getInt(KMManager.KMKey_UserKeyboardIndex, 0);
} else {
currentKeyboardIndex = KeyboardController.getInstance().getKeyboardIndex(currentKeyboardKey);
}
}
setSelection(currentKeyboardIndex);

imeList = getIMEList(this);
Expand Down Expand Up @@ -756,6 +756,32 @@ public static int getLexicalModelIndex(Context context, String lexicalModelKey)
return index;
}

/**
* Get an array list with all lexical model hashmaps matching the lexical model ID.
* @param context
* @param lexicalModelId - ID of the lexical model
* @return Array of lexical model hashmaps with lexicalModelId. Otherwise empty.
*/
public static ArrayList<HashMap<String, String>> getLexicalModelsMatchingId(Context context, String lexicalModelId) {
ArrayList<HashMap<String, String>> lexicalModels = new ArrayList<>();

if (lexicalModelsList == null) {
lexicalModelsList = getLexicalModelsList(context);
}

if (lexicalModelsList != null) {
int length = lexicalModelsList.size();
for (int i=0; i < length; i++) {
HashMap<String, String> lmInfo = lexicalModelsList.get(i);
if (lexicalModelId.equals(lmInfo.get(KMManager.KMKey_LexicalModelID))) {
lexicalModels.add(lmInfo);
}
}
}

return lexicalModels;
}

public static HashMap<String, String> getLexicalModelInfo(Context context,int index) {
if (index < 0) {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import com.keyman.engine.KeyboardPickerActivity;
import com.keyman.engine.R;
import com.keyman.engine.cloud.CloudApiTypes;
import com.keyman.engine.data.CloudRepository;
import com.keyman.engine.data.Keyboard;
import com.keyman.engine.data.KeyboardController;
import com.keyman.engine.data.LexicalModel;
Expand Down Expand Up @@ -188,7 +189,6 @@ public static void processKeyboardPackageUpdateJSON(Context aContext, JSONObject
}

public static void processLexicalModelPackageUpdateJSON(Context aContext, JSONObject pkgData, List<Bundle> updateBundles) {
boolean saveModelsList = false;
// Parse for lexical model package updates
if (pkgData.has(CDKey_Models)) {
try {
Expand All @@ -201,32 +201,37 @@ public static void processLexicalModelPackageUpdateJSON(Context aContext, JSONOb
String cloudVersion = cloudModelObj.getString(CDKey_Version);
String cloudKMP = cloudModelObj.getString(CDKey_KMP);
// Valid lexical model package exists. See if lexical model list needs to be updated
// Valid keyboard package exists. See if keyboard list needs to be updated
int index = KeyboardPickerActivity.getLexicalModelIndex(aContext, lexicalModelID);
if (index != -1) {
HashMap<String, String> lmInfo = KeyboardPickerActivity.getLexicalModelInfo(aContext, index);
String version = lmInfo.get(KMManager.KMKey_Version);
ArrayList<HashMap<String, String>> lmInfos = KeyboardPickerActivity.getLexicalModelsMatchingId(aContext, lexicalModelID);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This change is necessary to get matching models. The way we did it before (KeyboardPickerActivity.getLexicalModelIndex(aContext, lexicalModelID)) never found anything because the the second parameter is expected to be the full model key, consisting of pkg id, language id and model id, but we passed in only the model id.

for (HashMap<String, String> lmInfo : lmInfos) {
String version = lmInfo.get(KMManager.KMKey_LexicalModelVersion);
if (lexicalModelID.equalsIgnoreCase(lmInfo.get(KMManager.KMKey_LexicalModelID)) &&
(FileUtils.compareVersions(cloudVersion, version) == FileUtils.VERSION_GREATER) &&
(!MapCompat.getOrDefault(lmInfo, KMManager.KMKey_KMPLink, "").equalsIgnoreCase(cloudKMP))) {
// Update keyboard with the latest KMP link
lmInfo.put(KMManager.KMKey_KMPLink, cloudKMP);
KeyboardPickerActivity.addLexicalModel(aContext, lmInfo);

(FileUtils.compareVersions(cloudVersion, version) == FileUtils.VERSION_GREATER)) {
// Update bundle list
LexicalModel lm = new LexicalModel(
lmInfo.get(KMManager.KMKey_PackageID),
lmInfo.get(KMManager.KMKey_LexicalModelID),
lmInfo.get(KMManager.KMKey_LexicalModelName),
lmInfo.get(KMManager.KMKey_LanguageID),
lmInfo.get(KMManager.KMKey_LanguageName),
lmInfo.get(KMManager.KMKey_Version),
lmInfo.get(KMManager.KMKey_LexicalModelVersion),
lmInfo.get(KMManager.KMKey_CustomHelpLink),
lmInfo.get(KMManager.KMKey_KMPLink));
Bundle bundle = new Bundle(lm.buildDownloadBundle());
updateBundles.add(bundle);
String updateKMP = lm.getUpdateKMP();
if (cloudLinkIsNewer(updateKMP, cloudKMP)) {
// Update lexical model info with the latest KMP link after appending languageID
String languageID = lm.getLanguageID();
String link = String.format("%s&bcp47=%s", cloudKMP, languageID);
lmInfo.put(KMManager.KMKey_KMPLink, link);
KeyboardPickerActivity.addLexicalModel(aContext, lmInfo);
lm.setUpdateKMP(link);
updateKMP = link;
}

saveModelsList = true;
if (updateKMP != null && !updateKMP.isEmpty()) {
// Update bundle list for update notifications
Bundle bundle = new Bundle(lm.buildDownloadBundle());
updateBundles.add(bundle);
}
}
}
}
Expand Down Expand Up @@ -367,14 +372,13 @@ public static boolean cloudLinkIsNewer(String updateKMP, String cloudKMP) {
return false;
}

try {
Uri cloudLink = Uri.parse(cloudKMP);

if (updateKMP == null || updateKMP.isEmpty()) {
return true;
}
if (updateKMP == null || updateKMP.isEmpty()) {
return true;
}

try {
Uri localLink = Uri.parse(updateKMP);
Uri cloudLink = Uri.parse(cloudKMP);

boolean pathsMatch = localLink.getLastPathSegment().equalsIgnoreCase(cloudLink.getLastPathSegment());
boolean cloudVersionNewer = FileUtils.compareVersions(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ public void processCloudReturns(Dataset aDataSet, CloudCatalogDownloadReturns js

// The actual update check
CloudDataJsonUtil.processKeyboardPackageUpdateJSON(context, jsonTuple.packagesJSON, updateBundles);
CloudDataJsonUtil.processLexicalModelPackageUpdateJSON(context, jsonTuple.packagesJSON, updateBundles);

// Only add installed kmp keyboards
aDataSet.keyboards.clear();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.keyman.engine.packages.JSONUtils;
import com.keyman.engine.util.BCP47;
import com.keyman.engine.util.DownloadFileUtils;
import com.keyman.engine.util.FileUtils;
import com.keyman.engine.util.KMLog;
import com.keyman.engine.util.VersionUtils;

Expand Down Expand Up @@ -253,6 +254,49 @@ public void updateDatasetIfNeeded(@NonNull Context context, UpdateHandler update
downloadMetaDataFromServer(context,updateHandler,onSuccess,onFailure);
}

/**
* Merges the new lexical models into the existing dataset models. If
* a model already exists, it compares the versions and uses the newer one.
* Otherwise the model gets added to the dataset. Models that exist
* in the dataset but not in the new models list are preserved.
* @param datasetModels Collection of lexical models in the dataset
* @param newModels List of new lexical models to merge
*/
private void mergeLexicalModels(Dataset.LexicalModels datasetModels, List<LexicalModel> newModels) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Doc-comments, please.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

if (newModels == null) {
return;
}

List<LexicalModel> existingModels = new ArrayList<>(datasetModels.asList());
List<LexicalModel> mergedModels = new ArrayList<>();

// Process all incoming models
for (LexicalModel newModel : newModels) {
LexicalModel existingMatch = null;
for (int i = 0; i < existingModels.size(); i++) {
if (newModel.equals(existingModels.get(i))) {
existingMatch = existingModels.remove(i);
break;
}
}

if (existingMatch != null) {
if (FileUtils.compareVersions(existingMatch.getVersion(), newModel.getVersion()) != FileUtils.VERSION_LOWER) {
mergedModels.add(existingMatch);
} else {
mergedModels.add(newModel);
}
} else {
mergedModels.add(newModel);
}
}

// Add remaining existing models that weren't matched
mergedModels.addAll(existingModels);

datasetModels.clear();
datasetModels.addAll(mergedModels);
}

/**
* precache dataset and notify callbacks if no update from cloud api services is necessary.
Expand Down Expand Up @@ -286,19 +330,24 @@ private void preCacheDataSet(@NonNull Context context, UpdateHandler updateHandl
languageCodes.add(installedSet.getItem(i).code);
}

// add all models that are installed locally
memCachedDataset.lexicalModels.addAll(installedSet.lexicalModels.asList());

// Get kmp.json info from installed (adhoc and cloud) models.
// Consolidate kmp.json info from packages/
JSONObject kmpLanguagesArray = wrapKmpKeyboardJSON(JSONUtils.getLanguages());
JSONArray kmpLexicalModelsArray = JSONUtils.getLexicalModels();
final boolean fromKMP = true;

try {
if (kmpLanguagesArray.getJSONObject(KMKeyboardDownloaderActivity.KMKey_Languages).
getJSONArray(KMKeyboardDownloaderActivity.KMKey_Languages).length() > 0) {
memCachedDataset.keyboards.addAll(CloudDataJsonUtil.processKeyboardJSON(kmpLanguagesArray, true));
}
if (kmpLexicalModelsArray.length() > 0) {
memCachedDataset.lexicalModels.addAll(CloudDataJsonUtil.processLexicalModelJSON(kmpLexicalModelsArray, fromKMP));
// Add all models that exist locally, whether or not they are installed.
// This set of models doesn't have the download url set, so instead of
// replacing the models in `memCacheDataSet.lexicalModels` we merge them.
mergeLexicalModels(memCachedDataset.lexicalModels, CloudDataJsonUtil.processLexicalModelJSON(kmpLexicalModelsArray, true));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I feel like this section could use some documentation.

So... this step aims to reconcile previously-cached data with the new incoming data from the cloud? Rather than replace the old data wholesale, we preserve entries for models that appear to no longer exist?

I do realize the old code was likely doing something similar here, but I feel like these lines would be well-served by comments providing additional context.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Honestly, I don't fully understand why we're doing this here. Since it was here before I kept it.

All the models that we added from installedSet above DO exist - the set is based on the locally installed models. Here we add all models that exist locally - whether installed or not. The difference to installedSet is that only installedSet has the download link for the new version.

It's possible that just using installedSet would be sufficient - at least it works with just that, but I don't know which edge cases that would break, i.e. what we would loose if models that exit locally but are not installed wouldn't be in the dataset.

}
} catch (Exception e) {
KMLog.LogException(TAG, "preCacheDataSet error ", e);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
/*
* Keyman is copyright (C) SIL Global. MIT License.
*/
package com.keyman.engine.data;

import android.content.Context;
import androidx.test.core.app.ApplicationProvider;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.robolectric.RobolectricTestRunner;

import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.List;

@RunWith(RobolectricTestRunner.class)
public class CloudRepositoryTests {

private CloudRepository repository;
private Dataset dataset;
private Context context;

private boolean containsModel(Dataset.LexicalModels models, String modelID) {
for (int i = 0; i < models.getCount(); i++) {
if (models.getItem(i).getLexicalModelID().equals(modelID)) {
return true;
}
}
return false;
}

private void setMemCachedDataset(Dataset dataset) throws Exception {
Field field = CloudRepository.class.getDeclaredField("memCachedDataset");
field.setAccessible(true);
field.set(repository, dataset);
}

@Before
public void setUp() {
context = ApplicationProvider.getApplicationContext();
repository = CloudRepository.shared;
dataset = new Dataset(context);
}

@After
public void tearDown() throws Exception {
// Reset singleton state
setMemCachedDataset(null);
}

@Test
public void testMergeLexicalModels_PreservesUniqueExistingModels() throws Exception {
// Setup
dataset.lexicalModels.add(new LexicalModel("pkg1", "model1", "Model 1", "en", "English", "1.0", "", ""));
List<LexicalModel> newModels = new ArrayList<>();
newModels.add(new LexicalModel("pkg2", "model2", "Model 2", "fr", "French", "1.0", "", ""));

// Execute
Method mergeMethod = CloudRepository.class.getDeclaredMethod("mergeLexicalModels", Dataset.LexicalModels.class, List.class);
mergeMethod.setAccessible(true);
mergeMethod.invoke(repository, dataset.lexicalModels, newModels);

// Verify
Assert.assertEquals(2, dataset.lexicalModels.getCount());
Assert.assertTrue(containsModel(dataset.lexicalModels, "model1"));
Assert.assertTrue(containsModel(dataset.lexicalModels, "model2"));
}

@Test
public void testMergeLexicalModels_UpdatesVersion() throws Exception {
// Setup
dataset.lexicalModels.add(new LexicalModel("pkg1", "model1", "Model 1", "en", "English", "1.0", "", ""));

List<LexicalModel> newModels = new ArrayList<>();
// New model (same ID, newer version v1.1)
newModels.add(new LexicalModel("pkg1", "model1", "Model 1", "en", "English", "1.1", "", ""));

// Execute
Method mergeMethod = CloudRepository.class.getDeclaredMethod("mergeLexicalModels", Dataset.LexicalModels.class, List.class);
mergeMethod.setAccessible(true);
mergeMethod.invoke(repository, dataset.lexicalModels, newModels);

// Verify
Assert.assertEquals(1, dataset.lexicalModels.getCount());
LexicalModel result = dataset.lexicalModels.getItem(0);
Assert.assertEquals("1.1", result.getVersion());
}

@Test
public void testMergeLexicalModels_KeepsOlderIfNewerIsLowerVersion() throws Exception {
// Setup
dataset.lexicalModels.add(new LexicalModel("pkg1", "model1", "Model 1", "en", "English", "1.2", "", ""));

List<LexicalModel> newModels = new ArrayList<>();
// New model (same ID, older version v1.1)
newModels.add(new LexicalModel("pkg1", "model1", "Model 1", "en", "English", "1.1", "", ""));

// Execute
Method mergeMethod = CloudRepository.class.getDeclaredMethod("mergeLexicalModels", Dataset.LexicalModels.class, List.class);
mergeMethod.setAccessible(true);
mergeMethod.invoke(repository, dataset.lexicalModels, newModels);

// Verify
Assert.assertEquals(1, dataset.lexicalModels.getCount());
LexicalModel result = dataset.lexicalModels.getItem(0);
Assert.assertEquals("1.2", result.getVersion());
}

@Test
public void testGetAssociatedLexicalModel_ReturnsCorrectModel() throws Exception {
// Setup
LexicalModel model1 = new LexicalModel("pkg1", "model1", "Model 1", "en", "English", "1.0", "", "");
LexicalModel model2 = new LexicalModel("pkg2", "model2", "Model 2", "fr", "French", "1.0", "", "");
dataset.lexicalModels.add(model1);
dataset.lexicalModels.add(model2);
setMemCachedDataset(dataset);

// Execute & Verify
Assert.assertEquals(model1, repository.getAssociatedLexicalModel(context, "en"));
Assert.assertEquals(model2, repository.getAssociatedLexicalModel(context, "fr"));
Assert.assertEquals(model1, repository.getAssociatedLexicalModel(context, "EN")); // Case insensitivity
}

@Test
public void testGetAssociatedLexicalModel_ReturnsNullWhenDatasetIsNull() throws Exception {
// Setup
setMemCachedDataset(null);

// Execute & Verify
Assert.assertNull(repository.getAssociatedLexicalModel(context, "en"));
}

@Test
public void testGetAssociatedLexicalModel_ReturnsFirstMatch() throws Exception {
// Setup
LexicalModel model1 = new LexicalModel("pkg1", "model1", "Model 1", "en", "English", "1.0", "", "");
LexicalModel model2 = new LexicalModel("pkg2", "model2", "Model 2", "en", "English", "1.0", "", "");
dataset.lexicalModels.add(model1);
dataset.lexicalModels.add(model2);
setMemCachedDataset(dataset);

// Execute & Verify
Assert.assertEquals(model1, repository.getAssociatedLexicalModel(context, "en"));
}
}