From 8bba2e7ef9938f2310901d341032973f6b780ed6 Mon Sep 17 00:00:00 2001 From: Aria Dhanang Date: Thu, 8 Dec 2022 18:16:53 +0700 Subject: [PATCH 1/3] Fix issue #924 caused by memory leak --- .../repository/PreviousContactRepository.java | 203 +++++++++++++----- 1 file changed, 153 insertions(+), 50 deletions(-) diff --git a/opensrp-anc/src/main/java/org/smartregister/anc/library/repository/PreviousContactRepository.java b/opensrp-anc/src/main/java/org/smartregister/anc/library/repository/PreviousContactRepository.java index d89658637..3ea175aed 100644 --- a/opensrp-anc/src/main/java/org/smartregister/anc/library/repository/PreviousContactRepository.java +++ b/opensrp-anc/src/main/java/org/smartregister/anc/library/repository/PreviousContactRepository.java @@ -1,7 +1,6 @@ package org.smartregister.anc.library.repository; import android.content.ContentValues; -import android.text.TextUtils; import android.util.Log; import com.vijay.jsonwizard.constants.JsonFormConstants; @@ -12,6 +11,7 @@ import org.apache.commons.lang3.StringUtils; import org.jeasy.rules.api.Facts; +import org.json.JSONException; import org.json.JSONObject; import org.smartregister.anc.library.AncLibrary; import org.smartregister.anc.library.model.PreviousContact; @@ -23,6 +23,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.Objects; import timber.log.Timber; @@ -307,80 +308,182 @@ public Facts getAllTestResultsForIndividualTest(String baseEntityId, String indi return allTestResults; } + /** - * Gets the Immediate previous contact's facts. It checks for both referral and normal contacts hence the recursion. + * Gets the immediate previous contact's facts. * * @param baseEntityId {@link String} * @param contactNo {@link String} - * @param checkNegative {@link Boolean} - * @return previousContactsFacts {@link Facts} + * @return Previous contact Facts object if found, otherwise returns null {@link Facts} */ - public Facts getPreviousContactFacts(String baseEntityId, String contactNo, boolean checkNegative) { - Cursor mCursor = null; - String selection = ""; - String orderBy = "MAX("+ ID + ") DESC"; - String[] selectionArgs = null; - Facts previousContactFacts = new Facts(); + public Facts getPreviousContactFacts (String baseEntityId, String contactNo) { + + // Validate input parameters + // Return null if one of the parameters is invalid + if (StringUtils.isBlank(baseEntityId) || StringUtils.isBlank(contactNo)) return null; + + // Input parameters are validated + // Get previous contact Facts try { - int contactNumber = Integer.parseInt(contactNo); + + // Prepare to get data from SQLite database + Facts previousContactFacts = new Facts(); SQLiteDatabase db = getReadableDatabase(); - if (StringUtils.isNotBlank(baseEntityId) && StringUtils.isNotBlank(contactNo)) { - selection = BASE_ENTITY_ID + " = ? AND " + CONTACT_NO + " = ?"; - selectionArgs = new String[]{baseEntityId, getContactNo(contactNo, checkNegative)}; - } + // Database query components + String selection = BASE_ENTITY_ID + " = ? AND " + CONTACT_NO + " = ?"; + String[] selectionArgs = new String[]{ baseEntityId, contactNo }; + String orderBy = "MAX(" + ID + ") DESC"; - mCursor = db.query(TABLE_NAME, projectionArgs, selection, selectionArgs, KEY, null, orderBy, null); + // Database cursor object + Cursor cursor = db.query(TABLE_NAME, projectionArgs, selection, selectionArgs, KEY, null, orderBy, null); - if (mCursor != null && mCursor.getCount() > 0) { - while (mCursor.moveToNext()) { - String previousContactValue = mCursor.getString(mCursor.getColumnIndex(VALUE)); - if (StringUtils.isNotBlank(previousContactValue) && previousContactValue.trim().charAt(0) == '{') { - JSONObject previousContactObject = new JSONObject(previousContactValue); - if (previousContactObject.has(JsonFormConstants.KEY) && previousContactObject.has(JsonFormConstants.TEXT)) { - String translated_text, text; - text = previousContactObject.optString(JsonFormConstants.TEXT).trim(); - translated_text = StringUtils.isNotBlank(text) ? NativeFormLangUtils.translateDatabaseString(text, AncLibrary.getInstance().getApplicationContext()) : ""; - previousContactFacts.put(mCursor.getString(mCursor.getColumnIndex(KEY)), translated_text); - } else { - previousContactFacts.put(mCursor.getString(mCursor.getColumnIndex(KEY)), previousContactValue); + // If the cursor found data, process it as Facts object + if (cursor != null & Objects.requireNonNull(cursor).getCount() > 0) { + + previousContactFacts.put(CONTACT_NO, contactNo); + + // Process the retrieved data + while (cursor.moveToNext()) { + + String key; + String value; + + // -- Process key + + int keyColumnIndex = cursor.getColumnIndex(KEY); + // If data doesn't have "key" column, break the while-loop process + if (keyColumnIndex == -1) break; + + key = cursor.getString(keyColumnIndex); + // If key is empty, break the while-loop process + if (StringUtils.isBlank(key)) break; + + // -- Process value + + // Get and validate the "value" column + int valueColumnIndex = cursor.getColumnIndex(VALUE); + // If data doesn't have "value" column, break the while-loop process + if (valueColumnIndex == -1) break; + + // Read the value string + value = cursor.getString(valueColumnIndex); + // If the "value" column is empty, break the while-loop process + if (StringUtils.isBlank(value)) break; + + // Check if value is a JSON, then process it accordingly + try { + JSONObject valueObject = new JSONObject(value); + if (valueObject.has(JsonFormConstants.KEY) && valueObject.has(JsonFormConstants.TEXT)) { + String text = valueObject.optString(JsonFormConstants.TEXT).trim(); + String translatedText = text; + if (StringUtils.isNotBlank(text)) { + translatedText = NativeFormLangUtils.translateDatabaseString(text, AncLibrary.getInstance().getApplicationContext()); + } + previousContactFacts.put(key, translatedText); } - } else { - previousContactFacts.put(mCursor.getString(mCursor.getColumnIndex(KEY)), previousContactValue); + } + + // Value is not a JSON, process it as is + catch (JSONException exp) { + previousContactFacts.put(key, value); } } - previousContactFacts.put(CONTACT_NO, selectionArgs[1]); + + // Return the result return previousContactFacts; - } else if (contactNumber > 0) { - return getPreviousContactFacts(baseEntityId, contactNo, false); - } - } catch (Exception e) { - Log.e(TAG, e.toString(), e); - } finally { - if (mCursor != null) { - mCursor.close(); + } + + // Data is not found, return null + return null; } - return previousContactFacts; + // Cannot get previous contact Facts, return null + catch(Exception error) { + Timber.d(error); + return null; + } } + /** - * Returns contact numbers according to the @param checkNegative. If true then it just uses the initial contact. If - * true the it would return a previous|referral contact + * Gets the immediate previous contact's facts. * - * @param previousContact {@link String} - * @param checkNegative {@link Boolean} - * @return contactNo {@link String} + * @param baseEntityId {@link String} + * @param contactNo {@link String} + * @param checkNegative {@link Boolean} + * @return Previous contact Facts object if found, otherwise returns empty Facts object {@link Facts} */ - private String getContactNo(String previousContact, boolean checkNegative) { - String contactNo = previousContact; - if (!TextUtils.isEmpty(previousContact) && checkNegative) { - contactNo = "-" + (Integer.parseInt(previousContact) + 1); + public Facts getPreviousContactFacts(String baseEntityId, String contactNo, boolean checkNegative) { + + final int negativeContactsLimit = 10; // Maximum contactNo of negative contact to search for + + // Validate input parameters + // Return empty Facts object if one of the parameters is invalid + if (StringUtils.isBlank(baseEntityId) || StringUtils.isBlank(contactNo)) return new Facts(); + + // Input parameters are validated + // Continue the process + try { + + Facts previousContactFacts = null; + + // If the checkNegative parameter is true, search for negative contacts + if (checkNegative) { + + Facts negativeContact = null; + int currentNegativeContactNo = (-1) * negativeContactsLimit; + + // Search for a negative contact numbered -10 to -1 (-10, -9, -8, ..., -1) + + while ((currentNegativeContactNo < 0) && (negativeContact == null)) { + + negativeContact = getPreviousContactFacts(baseEntityId, String.valueOf(currentNegativeContactNo)); + + // Contact found + if (negativeContact != null) { + previousContactFacts = negativeContact; + } + // Negative contact not found, update the current number + else { + currentNegativeContactNo++; + } + } + + // Return if the previousContactFacts has been set by negativeContact + if (previousContactFacts != null) return previousContactFacts; + + } + + // There is no negative contact or the checkNegative is false + // Search for current contactNo + int contactNumber = Integer.parseInt(contactNo); + + // If contactNumber is zero, return empty Facts object + if (contactNumber == 0) return new Facts(); + + // contactNumber is greater than zero + if (contactNumber > 0) { + previousContactFacts = getPreviousContactFacts(baseEntityId, contactNo); + } + + // Previous contact found, return the value + if (previousContactFacts != null) return previousContactFacts; + + // Catch all if no previous contact found, return empty Facts object + return new Facts(); + + } + + // Something happens when processing the previous contact + // Return empty Facts object + catch (Exception error) { + Timber.d(error); + return new Facts(); } - return contactNo; } /** From d593cb19bba4f23590c9de49f101cae96967f933 Mon Sep 17 00:00:00 2001 From: Aria Dhanang Date: Fri, 9 Dec 2022 11:51:56 +0700 Subject: [PATCH 2/3] Improve algorithm efficiency --- .../repository/PreviousContactRepository.java | 180 ++++++------------ 1 file changed, 62 insertions(+), 118 deletions(-) diff --git a/opensrp-anc/src/main/java/org/smartregister/anc/library/repository/PreviousContactRepository.java b/opensrp-anc/src/main/java/org/smartregister/anc/library/repository/PreviousContactRepository.java index 3ea175aed..c76336036 100644 --- a/opensrp-anc/src/main/java/org/smartregister/anc/library/repository/PreviousContactRepository.java +++ b/opensrp-anc/src/main/java/org/smartregister/anc/library/repository/PreviousContactRepository.java @@ -22,7 +22,9 @@ import org.smartregister.repository.BaseRepository; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Objects; import timber.log.Timber; @@ -320,91 +322,75 @@ public Facts getPreviousContactFacts (String baseEntityId, String contactNo) { // Validate input parameters // Return null if one of the parameters is invalid - if (StringUtils.isBlank(baseEntityId) || StringUtils.isBlank(contactNo)) return null; + if (StringUtils.isBlank(baseEntityId) || StringUtils.isBlank(contactNo) || contactNo == "0") return null; // Input parameters are validated // Get previous contact Facts - try { - // Prepare to get data from SQLite database - Facts previousContactFacts = new Facts(); - SQLiteDatabase db = getReadableDatabase(); + // Prepare to get data from SQLite database + Facts previousContactFacts = null; + SQLiteDatabase db = getReadableDatabase(); + + // Database query components + String selection = BASE_ENTITY_ID + " = ? AND " + CONTACT_NO + " = ?"; + String[] selectionArgs = new String[]{ baseEntityId, contactNo }; + String orderBy = "MAX(" + ID + ") DESC"; - // Database query components - String selection = BASE_ENTITY_ID + " = ? AND " + CONTACT_NO + " = ?"; - String[] selectionArgs = new String[]{ baseEntityId, contactNo }; - String orderBy = "MAX(" + ID + ") DESC"; + try { // Database cursor object Cursor cursor = db.query(TABLE_NAME, projectionArgs, selection, selectionArgs, KEY, null, orderBy, null); // If the cursor found data, process it as Facts object - if (cursor != null & Objects.requireNonNull(cursor).getCount() > 0) { - - previousContactFacts.put(CONTACT_NO, contactNo); - - // Process the retrieved data - while (cursor.moveToNext()) { + Map processedData = new HashMap<>(); - String key; - String value; + // Process the retrieved data + while (cursor.moveToNext()) { - // -- Process key - - int keyColumnIndex = cursor.getColumnIndex(KEY); - // If data doesn't have "key" column, break the while-loop process - if (keyColumnIndex == -1) break; + // Process key and value + String key = ""; + String value = ""; + int keyColumnIndex = cursor.getColumnIndex(KEY); + int valueColumnIndex = cursor.getColumnIndex(VALUE); + if (keyColumnIndex >= 0 && valueColumnIndex >= 0) { key = cursor.getString(keyColumnIndex); - // If key is empty, break the while-loop process - if (StringUtils.isBlank(key)) break; - - // -- Process value - - // Get and validate the "value" column - int valueColumnIndex = cursor.getColumnIndex(VALUE); - // If data doesn't have "value" column, break the while-loop process - if (valueColumnIndex == -1) break; - - // Read the value string value = cursor.getString(valueColumnIndex); - // If the "value" column is empty, break the while-loop process - if (StringUtils.isBlank(value)) break; - - // Check if value is a JSON, then process it accordingly - try { - JSONObject valueObject = new JSONObject(value); - if (valueObject.has(JsonFormConstants.KEY) && valueObject.has(JsonFormConstants.TEXT)) { - String text = valueObject.optString(JsonFormConstants.TEXT).trim(); - String translatedText = text; - if (StringUtils.isNotBlank(text)) { - translatedText = NativeFormLangUtils.translateDatabaseString(text, AncLibrary.getInstance().getApplicationContext()); - } - previousContactFacts.put(key, translatedText); - } - } + } - // Value is not a JSON, process it as is - catch (JSONException exp) { - previousContactFacts.put(key, value); - } + // Check if value is a JSON, then process it accordingly + try { + JSONObject valueObject = new JSONObject(value); + String text = valueObject.optString(JsonFormConstants.TEXT).trim(); + String translatedText = (StringUtils.isBlank(text)) ? text : NativeFormLangUtils.translateDatabaseString(text, AncLibrary.getInstance().getApplicationContext()); + value = (StringUtils.isBlank(translatedText)) ? value : translatedText; + } catch (JSONException exp) { + // Value is not JSON string, do not modify value + } + if (StringUtils.isNotBlank(key) && StringUtils.isNotBlank(value)) { + processedData.put(key, value); } - // Return the result - return previousContactFacts; + } + // Assign Facts with processedData + if (processedData.size() > 0) { + previousContactFacts = new Facts(); + previousContactFacts.put(CONTACT_NO, contactNo); + for (Map.Entry entry : processedData.entrySet()) { + previousContactFacts.put(entry.getKey(), entry.getValue()); + } } - // Data is not found, return null - return null; } - // Cannot get previous contact Facts, return null + // Cannot get previous contact Facts catch(Exception error) { Timber.d(error); - return null; } + + return previousContactFacts; } @@ -414,75 +400,33 @@ public Facts getPreviousContactFacts (String baseEntityId, String contactNo) { * @param baseEntityId {@link String} * @param contactNo {@link String} * @param checkNegative {@link Boolean} - * @return Previous contact Facts object if found, otherwise returns empty Facts object {@link Facts} + * @return Previous contact Facts if found, otherwise empty Facts object {@link Facts} */ public Facts getPreviousContactFacts(String baseEntityId, String contactNo, boolean checkNegative) { - final int negativeContactsLimit = 10; // Maximum contactNo of negative contact to search for - - // Validate input parameters - // Return empty Facts object if one of the parameters is invalid - if (StringUtils.isBlank(baseEntityId) || StringUtils.isBlank(contactNo)) return new Facts(); - - // Input parameters are validated - // Continue the process - try { - - Facts previousContactFacts = null; - - // If the checkNegative parameter is true, search for negative contacts - if (checkNegative) { - - Facts negativeContact = null; - int currentNegativeContactNo = (-1) * negativeContactsLimit; + // Maximum contactNo of negative contact to search for + final int maxNegativeContactNo = -10; - // Search for a negative contact numbered -10 to -1 (-10, -9, -8, ..., -1) + // Facts object that holds the return value + Facts previousContactFacts = null; - while ((currentNegativeContactNo < 0) && (negativeContact == null)) { - - negativeContact = getPreviousContactFacts(baseEntityId, String.valueOf(currentNegativeContactNo)); - - // Contact found - if (negativeContact != null) { - previousContactFacts = negativeContact; - } - // Negative contact not found, update the current number - else { - currentNegativeContactNo++; - } - } + // Search for current contactNo + previousContactFacts = getPreviousContactFacts(baseEntityId, contactNo); - // Return if the previousContactFacts has been set by negativeContact - if (previousContactFacts != null) return previousContactFacts; - - } - - // There is no negative contact or the checkNegative is false - // Search for current contactNo - int contactNumber = Integer.parseInt(contactNo); - - // If contactNumber is zero, return empty Facts object - if (contactNumber == 0) return new Facts(); - - // contactNumber is greater than zero - if (contactNumber > 0) { - previousContactFacts = getPreviousContactFacts(baseEntityId, contactNo); + // If the checkNegative parameter is true, search for negative (referral) contacts + if (checkNegative) { + Facts negativeContact = null; + int currentContactNo = maxNegativeContactNo; + while ((currentContactNo < 0) && (negativeContact == null)) { + negativeContact = getPreviousContactFacts(baseEntityId, String.valueOf(currentContactNo)); + currentContactNo++; } - - // Previous contact found, return the value - if (previousContactFacts != null) return previousContactFacts; - - // Catch all if no previous contact found, return empty Facts object - return new Facts(); - + previousContactFacts = (negativeContact == null) ? previousContactFacts : negativeContact; } - // Something happens when processing the previous contact - // Return empty Facts object - catch (Exception error) { - Timber.d(error); - return new Facts(); - } + // Return previous contact Facts + // Return empty Facts object if previousContactFacts is null + return (previousContactFacts == null) ? new Facts() : previousContactFacts; } From fa0a6da2ec5da970ee99f998139fd014ec947422 Mon Sep 17 00:00:00 2001 From: Aria Dhanang Date: Fri, 9 Dec 2022 11:56:05 +0700 Subject: [PATCH 3/3] Change boolean check to use .equals() --- .../anc/library/repository/PreviousContactRepository.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opensrp-anc/src/main/java/org/smartregister/anc/library/repository/PreviousContactRepository.java b/opensrp-anc/src/main/java/org/smartregister/anc/library/repository/PreviousContactRepository.java index c76336036..5a3a5cec4 100644 --- a/opensrp-anc/src/main/java/org/smartregister/anc/library/repository/PreviousContactRepository.java +++ b/opensrp-anc/src/main/java/org/smartregister/anc/library/repository/PreviousContactRepository.java @@ -322,7 +322,7 @@ public Facts getPreviousContactFacts (String baseEntityId, String contactNo) { // Validate input parameters // Return null if one of the parameters is invalid - if (StringUtils.isBlank(baseEntityId) || StringUtils.isBlank(contactNo) || contactNo == "0") return null; + if (StringUtils.isBlank(baseEntityId) || StringUtils.isBlank(contactNo) || contactNo.equals("0")) return null; // Input parameters are validated // Get previous contact Facts