From b9634df0fea0b2b6b94923da42f2c8902c980ed6 Mon Sep 17 00:00:00 2001 From: Arif Burak Demiray Date: Tue, 5 Dec 2023 11:40:03 +0300 Subject: [PATCH 1/6] feat: make user profile byte data as base64 store --- .../java/internal/ImmediateRequestMaker.java | 2 +- .../sdk/java/internal/ModuleUserProfile.java | 5 +- .../ly/count/sdk/java/internal/SDKCore.java | 11 +- .../ly/count/sdk/java/internal/Transport.java | 54 +++---- .../sdk/java/internal/UserEditorImpl.java | 1 - .../ly/count/sdk/java/internal/UserImpl.java | 153 +----------------- .../ly/count/sdk/java/internal/TestUtils.java | 8 +- .../sdk/java/internal/UserEditorTests.java | 2 +- 8 files changed, 38 insertions(+), 198 deletions(-) diff --git a/sdk-java/src/main/java/ly/count/sdk/java/internal/ImmediateRequestMaker.java b/sdk-java/src/main/java/ly/count/sdk/java/internal/ImmediateRequestMaker.java index 5d8bf8f2c..abced9ad7 100644 --- a/sdk-java/src/main/java/ly/count/sdk/java/internal/ImmediateRequestMaker.java +++ b/sdk-java/src/main/java/ly/count/sdk/java/internal/ImmediateRequestMaker.java @@ -67,7 +67,7 @@ private JSONObject doInBackground(String requestData, String customEndpoint, Tra request.endpoint(customEndpoint); //getting connection ready try { - connection = cp.connection(request, null); + connection = cp.connection(request); } catch (IOException e) { L.e("[ImmediateRequestMaker] IOException while preparing remote config update request :[" + e + "]"); return null; diff --git a/sdk-java/src/main/java/ly/count/sdk/java/internal/ModuleUserProfile.java b/sdk-java/src/main/java/ly/count/sdk/java/internal/ModuleUserProfile.java index 2560aff1f..6ed7d3522 100644 --- a/sdk-java/src/main/java/ly/count/sdk/java/internal/ModuleUserProfile.java +++ b/sdk-java/src/main/java/ly/count/sdk/java/internal/ModuleUserProfile.java @@ -13,7 +13,6 @@ public class ModuleUserProfile extends ModuleBase { static final String CUSTOM_KEY = "custom"; - static final String PICTURE_IN_USER_PROFILE = "[CLY]_USER_PROFILE_PICTURE"; boolean isSynced = true; UserProfile userProfileInterface; private final Map sets; @@ -114,7 +113,7 @@ void perform(JSONObject changes) throws JSONException { } else if (value instanceof byte[]) { internalConfig.sdk.user().picture = (byte[]) value; //set a special value to indicate that the picture information is already stored in memory - changes.put(PredefinedUserPropertyKeys.PICTURE_PATH, PICTURE_IN_USER_PROFILE); + changes.put(PredefinedUserPropertyKeys.PICTURE_PATH, Utils.Base64.encode((byte[]) value)); } break; case PredefinedUserPropertyKeys.PICTURE_PATH: @@ -220,7 +219,7 @@ private Params prepareRequestParamsForUserProfile() { L.w("Won't send picturePath" + e); } } - if (!json.isEmpty() || internalConfig.sdk.user().picturePath != null || internalConfig.sdk.user().picture != null) { + if (!json.isEmpty() || params.has(PredefinedUserPropertyKeys.PICTURE_PATH)) { params.add("user_details", json.toString()); return params; } else { diff --git a/sdk-java/src/main/java/ly/count/sdk/java/internal/SDKCore.java b/sdk-java/src/main/java/ly/count/sdk/java/internal/SDKCore.java index d435745ff..31b0fc68a 100644 --- a/sdk-java/src/main/java/ly/count/sdk/java/internal/SDKCore.java +++ b/sdk-java/src/main/java/ly/count/sdk/java/internal/SDKCore.java @@ -557,16 +557,7 @@ public Integer remaningRequests() { } } - try { - user = Storage.read(config, new UserImpl(config)); - if (user == null) { - user = new UserImpl(config); - } - } catch (Throwable e) { - L.e("[SDKCore] Cannot happen" + e); - user = new UserImpl(config); - } - + user = new UserImpl(config); initFinished(config); } diff --git a/sdk-java/src/main/java/ly/count/sdk/java/internal/Transport.java b/sdk-java/src/main/java/ly/count/sdk/java/internal/Transport.java index 3a24a92b2..8c4230c4a 100644 --- a/sdk-java/src/main/java/ly/count/sdk/java/internal/Transport.java +++ b/sdk-java/src/main/java/ly/count/sdk/java/internal/Transport.java @@ -26,6 +26,7 @@ import java.security.spec.X509EncodedKeySpec; import java.util.ArrayList; import java.util.Arrays; +import java.util.Base64; import java.util.List; import java.util.Map; import java.util.Set; @@ -36,7 +37,6 @@ import javax.net.ssl.TrustManagerFactory; import javax.net.ssl.X509TrustManager; import ly.count.sdk.java.PredefinedUserPropertyKeys; -import ly.count.sdk.java.User; import org.json.JSONObject; /** @@ -118,11 +118,10 @@ public HttpURLConnection openConnection(String url, String params, boolean using * set SSL context, calculate and add checksum, load and send user picture if needed. * * @param request request to send - * @param user user to check for picture * @return connection, not {@link HttpURLConnection} yet * @throws IOException from {@link HttpURLConnection} in case of error */ - HttpURLConnection connection(final Request request, final User user) throws IOException { + HttpURLConnection connection(final Request request) throws IOException { String endpoint = request.params.remove(Request.ENDPOINT); if (!request.params.has("device_id") && config.getDeviceId() != null) { @@ -160,7 +159,7 @@ HttpURLConnection connection(final Request request, final User user) throws IOEx PrintWriter writer = null; try { L.d("[network] Picture path value " + picturePathValue); - byte[] pictureByteData = picturePathValue == null ? null : getPictureDataFromGivenValue(user, picturePathValue); + byte[] pictureByteData = picturePathValue == null ? null : getPictureDataFromGivenValue(picturePathValue); if (pictureByteData != null) { String boundary = Long.toHexString(System.currentTimeMillis()); @@ -240,36 +239,35 @@ void addMultipart(OutputStream output, PrintWriter writer, String boundary, Stri * If we have the bytes, give them * Otherwise load them from disk * - * @param user - * @param picture - * @return + * @param picture picture path or base64 encoded byte array + * @return picture data */ - byte[] getPictureDataFromGivenValue(User user, String picture) { - if (user == null) { - return null; - } - - byte[] data = null; - if (ModuleUserProfile.PICTURE_IN_USER_PROFILE.equals(picture)) { - //if the value is this special value then we know that we will send over bytes that are already provided by the integrator - //those stored bytes are already in a internal data structure, use them - data = user.picture(); - } else { - //otherwise we assume it is a local path, and we try to read it from disk - try { - File file = new File(picture); - if (!file.exists()) { - return null; - } - data = Files.readAllBytes(file.toPath()); - } catch (Throwable t) { - L.w("[Transport] getPictureDataFromGivenValue, Error while reading picture from disk " + t); + byte[] getPictureDataFromGivenValue(String picture) { + byte[] data; + //firstly, we assume it is a local path, and we try to read it from disk + try { + File file = new File(picture); + if (!file.exists()) { + return null; } + data = Files.readAllBytes(file.toPath()); + } catch (Throwable t) { + //if we can't read it from disk, we assume it is a base64 encoded byte array + data = readBase64String(picture); } return data; } + private byte[] readBase64String(String string) { + try { + return Base64.getDecoder().decode(string); + } catch (IllegalArgumentException e) { + L.w("[Transport] readBase64String, Error while reading base64 string " + e); + return null; + } + } + String response(HttpURLConnection connection) { BufferedReader reader = null; try { @@ -319,7 +317,7 @@ public Boolean send() { Class requestOwner = request.owner(); request.params.remove(Request.MODULE); - connection = connection(request, SDKCore.instance.user()); + connection = connection(request); connection.connect(); int code = connection.getResponseCode(); diff --git a/sdk-java/src/main/java/ly/count/sdk/java/internal/UserEditorImpl.java b/sdk-java/src/main/java/ly/count/sdk/java/internal/UserEditorImpl.java index b57495c15..4d988eaad 100644 --- a/sdk-java/src/main/java/ly/count/sdk/java/internal/UserEditorImpl.java +++ b/sdk-java/src/main/java/ly/count/sdk/java/internal/UserEditorImpl.java @@ -275,7 +275,6 @@ public User commit() { try { Countly.instance().userProfile().save(); - Storage.push(SDKCore.instance.config, user); // todo this is not need it is for another task } catch (JSONException e) { L.e("[UserEditorImpl] Exception while committing changes to User profile" + e); } diff --git a/sdk-java/src/main/java/ly/count/sdk/java/internal/UserImpl.java b/sdk-java/src/main/java/ly/count/sdk/java/internal/UserImpl.java index 8e04eb311..31686ec2e 100644 --- a/sdk-java/src/main/java/ly/count/sdk/java/internal/UserImpl.java +++ b/sdk-java/src/main/java/ly/count/sdk/java/internal/UserImpl.java @@ -1,12 +1,8 @@ package ly.count.sdk.java.internal; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.ObjectInputStream; -import java.io.ObjectOutputStream; -import java.util.*; - +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; import ly.count.sdk.java.User; import ly.count.sdk.java.UserEditor; @@ -14,7 +10,7 @@ * Class for user profile data access & manipulation */ -public class UserImpl extends User implements Storable { +public class UserImpl extends User { private Log L = null; String id; @@ -102,131 +98,6 @@ public UserEditor edit() { return new UserEditorImpl(this, L); } - @Override - public byte[] store(Log L) { - ByteArrayOutputStream bytes = null; - ObjectOutputStream stream = null; - try { - bytes = new ByteArrayOutputStream(); - stream = new ObjectOutputStream(bytes); - stream.writeObject(name); - stream.writeObject(username); - stream.writeObject(email); - stream.writeObject(org); - stream.writeObject(phone); - stream.writeInt(picture == null ? 0 : picture.length); - if (picture != null) { - stream.write(picture); - } - stream.writeObject(picturePath); - stream.writeObject(gender == null ? null : gender.toString()); - stream.writeInt(birthyear == null ? -1 : birthyear); - stream.writeObject(locale); - stream.writeObject(country); - stream.writeObject(city); - stream.writeObject(location); - stream.writeObject(null);//this is for the removed "cohorts" functionality. just to keep the correct order. Throw away in the future - stream.writeObject(custom); - stream.close(); - return bytes.toByteArray(); - } catch (IOException e) { - if (L != null) { - L.e("[UserImpl Cannot serialize session" + e); - } - } finally { - if (stream != null) { - try { - stream.close(); - } catch (IOException e) { - if (L != null) { - L.e("[UserImpl Cannot happen" + e); - } - } - } - if (bytes != null) { - try { - bytes.close(); - } catch (IOException e) { - if (L != null) { - L.e("[UserImpl Cannot happen" + e); - } - } - } - } - return null; - } - - @SuppressWarnings("unchecked") - public boolean restore(byte[] data, Log L) { - ByteArrayInputStream bytes = null; - ObjectInputStream stream = null; - - try { - bytes = new ByteArrayInputStream(data); - stream = new ObjectInputStream(bytes); - name = (String) stream.readObject(); - username = (String) stream.readObject(); - email = (String) stream.readObject(); - org = (String) stream.readObject(); - phone = (String) stream.readObject(); - - int picLength = stream.readInt(); - if (picLength != 0) { - picture = new byte[picLength]; - stream.readFully(picture); - } - picturePath = (String) stream.readObject(); - - String g = (String) stream.readObject(); - if (g != null) { - gender = Gender.fromString(g); - } - - int y = stream.readInt(); - if (y != -1) { - birthyear = y; - } - locale = (String) stream.readObject(); - country = (String) stream.readObject(); - city = (String) stream.readObject(); - location = (String) stream.readObject(); - - Set throwawayCohorts = (Set) stream.readObject();//this is for keeping backwards compatibility. Throw away in the future - - custom = (Map) stream.readObject(); - if (custom == null) { - custom = new HashMap<>(); - } - - return true; - } catch (IOException | ClassNotFoundException e) { - if (L != null) { - L.e("[UserImpl Cannot deserialize session" + e); - } - } finally { - if (stream != null) { - try { - stream.close(); - } catch (IOException e) { - if (L != null) { - L.e("[UserImpl Cannot happen" + e); - } - } - } - if (bytes != null) { - try { - bytes.close(); - } catch (IOException e) { - if (L != null) { - L.e("[UserImpl Cannot happen" + e); - } - } - } - } - - return false; - } - @Override public String toString() { return "UserImpl{" + @@ -247,20 +118,4 @@ public String toString() { ", custom=" + custom + '}'; } - - @Override - public Long storageId() { - return 0L; - } - - @Override - public String storagePrefix() { - return "user"; - } - - @Override - public void setId(Long id) { - this.id = id.toString(); - //todo remove this - } } diff --git a/sdk-java/src/test/java/ly/count/sdk/java/internal/TestUtils.java b/sdk-java/src/test/java/ly/count/sdk/java/internal/TestUtils.java index fda18add6..92efc262d 100644 --- a/sdk-java/src/test/java/ly/count/sdk/java/internal/TestUtils.java +++ b/sdk-java/src/test/java/ly/count/sdk/java/internal/TestUtils.java @@ -270,18 +270,16 @@ private static File[] getRequestFiles(File targetFolder) { private static Map parseRequestParams(File file) throws IOException { try (Scanner scanner = new Scanner(file)) { String firstLine = scanner.nextLine(); - String urlDecodedStr = Utils.urldecode(firstLine); - - if (urlDecodedStr == null) { + if (Utils.isEmptyOrNull(firstLine)) { return new HashMap<>(); } - String[] params = urlDecodedStr.split("&"); + String[] params = firstLine.split("&"); Map paramMap = new HashMap<>(); for (String param : params) { String[] pair = param.split("="); - paramMap.put(pair[0], pair.length == 1 ? "" : pair[1]); + paramMap.put(Utils.urldecode(pair[0]), pair.length == 1 ? "" : Utils.urldecode(pair[1])); } return paramMap; diff --git a/sdk-java/src/test/java/ly/count/sdk/java/internal/UserEditorTests.java b/sdk-java/src/test/java/ly/count/sdk/java/internal/UserEditorTests.java index a4dab7afa..e4278c6b9 100644 --- a/sdk-java/src/test/java/ly/count/sdk/java/internal/UserEditorTests.java +++ b/sdk-java/src/test/java/ly/count/sdk/java/internal/UserEditorTests.java @@ -112,7 +112,7 @@ public void setPicture_binaryData() { sessionHandler(() -> Countly.instance().user().edit().setPicture(imgData).commit()); validatePictureAndPath(null, imgData); Countly.session().end(); - validateUserDetailsRequestInRQ(map("user_details", "{}", "picturePath", ModuleUserProfile.PICTURE_IN_USER_PROFILE)); + validateUserDetailsRequestInRQ(map("user_details", "{}", "picturePath", Utils.Base64.encode(imgData))); } /** From 93af50b91365f91116bad5d4a4471fe8b8195751 Mon Sep 17 00:00:00 2001 From: Arif Burak Demiray Date: Thu, 7 Dec 2023 10:14:00 +0300 Subject: [PATCH 2/6] fix: remove unneeded check --- .../java/ly/count/sdk/java/internal/Transport.java | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/sdk-java/src/main/java/ly/count/sdk/java/internal/Transport.java b/sdk-java/src/main/java/ly/count/sdk/java/internal/Transport.java index 8c4230c4a..3c979b73c 100644 --- a/sdk-java/src/main/java/ly/count/sdk/java/internal/Transport.java +++ b/sdk-java/src/main/java/ly/count/sdk/java/internal/Transport.java @@ -26,7 +26,6 @@ import java.security.spec.X509EncodedKeySpec; import java.util.ArrayList; import java.util.Arrays; -import java.util.Base64; import java.util.List; import java.util.Map; import java.util.Set; @@ -253,21 +252,12 @@ byte[] getPictureDataFromGivenValue(String picture) { data = Files.readAllBytes(file.toPath()); } catch (Throwable t) { //if we can't read it from disk, we assume it is a base64 encoded byte array - data = readBase64String(picture); + data = Utils.Base64.decode(picture, L); } return data; } - private byte[] readBase64String(String string) { - try { - return Base64.getDecoder().decode(string); - } catch (IllegalArgumentException e) { - L.w("[Transport] readBase64String, Error while reading base64 string " + e); - return null; - } - } - String response(HttpURLConnection connection) { BufferedReader reader = null; try { From 03113520edd78ed83cec4adea8e9b04ac90ddfd2 Mon Sep 17 00:00:00 2001 From: Arif Burak Demiray Date: Thu, 7 Dec 2023 12:08:55 +0300 Subject: [PATCH 3/6] fix: perform flow --- .../sdk/java/internal/ModuleUserProfile.java | 20 +++++++------------ .../sdk/java/internal/UserEditorTests.java | 2 +- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/sdk-java/src/main/java/ly/count/sdk/java/internal/ModuleUserProfile.java b/sdk-java/src/main/java/ly/count/sdk/java/internal/ModuleUserProfile.java index 6ed7d3522..cd3e6ccd8 100644 --- a/sdk-java/src/main/java/ly/count/sdk/java/internal/ModuleUserProfile.java +++ b/sdk-java/src/main/java/ly/count/sdk/java/internal/ModuleUserProfile.java @@ -14,6 +14,7 @@ public class ModuleUserProfile extends ModuleBase { static final String CUSTOM_KEY = "custom"; boolean isSynced = true; + static final String PICTURE_BYTES = "[CLY]_picture_bytes"; UserProfile userProfileInterface; private final Map sets; private final List ops; @@ -92,9 +93,10 @@ private Object optString(String key, Object value) { * Transforming changes in "sets" into a json contained in "changes" * * @param changes JSONObject to store changes + * @param params Params to store changes * @throws JSONException if something goes wrong */ - void perform(JSONObject changes) throws JSONException { + void perform(JSONObject changes, Params params) throws JSONException { for (String key : sets.keySet()) { Object value = sets.get(key); switch (key) { @@ -113,7 +115,7 @@ void perform(JSONObject changes) throws JSONException { } else if (value instanceof byte[]) { internalConfig.sdk.user().picture = (byte[]) value; //set a special value to indicate that the picture information is already stored in memory - changes.put(PredefinedUserPropertyKeys.PICTURE_PATH, Utils.Base64.encode((byte[]) value)); + params.add(PICTURE_BYTES, Utils.Base64.encode((byte[]) value)); } break; case PredefinedUserPropertyKeys.PICTURE_PATH: @@ -127,7 +129,7 @@ void perform(JSONObject changes) throws JSONException { changes.put(PredefinedUserPropertyKeys.PICTURE, value); } else { //if we get here then that means it is a local file path which we would send over as bytes to the server - changes.put(PredefinedUserPropertyKeys.PICTURE_PATH, value); + params.add(PredefinedUserPropertyKeys.PICTURE_PATH, value); } internalConfig.sdk.user().picturePath = value.toString(); } else { @@ -210,16 +212,8 @@ private Params prepareRequestParamsForUserProfile() { isSynced = true; Params params = new Params(); final JSONObject json = new JSONObject(); - perform(json); - if (json.has(PredefinedUserPropertyKeys.PICTURE_PATH)) { - try { - params.add(PredefinedUserPropertyKeys.PICTURE_PATH, json.getString(PredefinedUserPropertyKeys.PICTURE_PATH)); - json.remove(PredefinedUserPropertyKeys.PICTURE_PATH); - } catch (JSONException e) { - L.w("Won't send picturePath" + e); - } - } - if (!json.isEmpty() || params.has(PredefinedUserPropertyKeys.PICTURE_PATH)) { + perform(json, params); + if (!json.isEmpty() || params.has(PredefinedUserPropertyKeys.PICTURE_PATH) || params.has(PICTURE_BYTES)) { params.add("user_details", json.toString()); return params; } else { diff --git a/sdk-java/src/test/java/ly/count/sdk/java/internal/UserEditorTests.java b/sdk-java/src/test/java/ly/count/sdk/java/internal/UserEditorTests.java index e4278c6b9..b4086032d 100644 --- a/sdk-java/src/test/java/ly/count/sdk/java/internal/UserEditorTests.java +++ b/sdk-java/src/test/java/ly/count/sdk/java/internal/UserEditorTests.java @@ -112,7 +112,7 @@ public void setPicture_binaryData() { sessionHandler(() -> Countly.instance().user().edit().setPicture(imgData).commit()); validatePictureAndPath(null, imgData); Countly.session().end(); - validateUserDetailsRequestInRQ(map("user_details", "{}", "picturePath", Utils.Base64.encode(imgData))); + validateUserDetailsRequestInRQ(map("user_details", "{}", ModuleUserProfile.PICTURE_BYTES, Utils.Base64.encode(imgData))); } /** From 43c8654189fb513ca534826ebbac82633f96f617 Mon Sep 17 00:00:00 2001 From: Arif Burak Demiray Date: Thu, 7 Dec 2023 12:31:54 +0300 Subject: [PATCH 4/6] refactor: image getting from the request --- .../ly/count/sdk/java/internal/Transport.java | 42 ++++++++++--------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/sdk-java/src/main/java/ly/count/sdk/java/internal/Transport.java b/sdk-java/src/main/java/ly/count/sdk/java/internal/Transport.java index 3c979b73c..cfb002ea5 100644 --- a/sdk-java/src/main/java/ly/count/sdk/java/internal/Transport.java +++ b/sdk-java/src/main/java/ly/count/sdk/java/internal/Transport.java @@ -133,10 +133,10 @@ HttpURLConnection connection(final Request request) throws IOException { } String path = config.getServerURL().toString() + endpoint; - String picturePathValue = request.params.remove(PredefinedUserPropertyKeys.PICTURE_PATH); - boolean usingGET = !config.isHTTPPostForced() && request.isGettable(config.getServerURL()) && Utils.isEmptyOrNull(picturePathValue); + byte[] maybePictureData = getPictureDataFromRequest(request); + boolean usingGET = !config.isHTTPPostForced() && request.isGettable(config.getServerURL()) && maybePictureData == null; - if (!usingGET && !Utils.isEmptyOrNull(picturePathValue)) { + if (!usingGET && maybePictureData != null) { path = setProfilePicturePathRequestParams(path, request.params); } @@ -157,10 +157,7 @@ HttpURLConnection connection(final Request request) throws IOException { OutputStream output = null; PrintWriter writer = null; try { - L.d("[network] Picture path value " + picturePathValue); - byte[] pictureByteData = picturePathValue == null ? null : getPictureDataFromGivenValue(picturePathValue); - - if (pictureByteData != null) { + if (maybePictureData != null) { String boundary = Long.toHexString(System.currentTimeMillis()); connection.setRequestProperty("Content-Type", "multipart/form-data; boundary=" + boundary); @@ -168,7 +165,7 @@ HttpURLConnection connection(final Request request) throws IOException { output = connection.getOutputStream(); writer = new PrintWriter(new OutputStreamWriter(output, Utils.UTF8), true); - addMultipart(output, writer, boundary, "image/jpeg", "binaryFile", "image", pictureByteData); + addMultipart(output, writer, boundary, "image/jpeg", "binaryFile", "image", maybePictureData); StringBuilder salting = new StringBuilder(); Map map = request.params.map(); @@ -238,21 +235,26 @@ void addMultipart(OutputStream output, PrintWriter writer, String boundary, Stri * If we have the bytes, give them * Otherwise load them from disk * - * @param picture picture path or base64 encoded byte array + * @param request picture path or base64 encoded byte array * @return picture data */ - byte[] getPictureDataFromGivenValue(String picture) { - byte[] data; - //firstly, we assume it is a local path, and we try to read it from disk - try { - File file = new File(picture); - if (!file.exists()) { - return null; + byte[] getPictureDataFromRequest(Request request) { + String maybeLocalPath = request.params.remove(PredefinedUserPropertyKeys.PICTURE_PATH); + String maybePictureData = request.params.remove(ModuleUserProfile.PICTURE_BYTES); + + byte[] data = null; + + //first check for pure bytes + if (!Utils.isEmptyOrNull(maybePictureData)) { + data = Utils.Base64.decode(maybePictureData, L); + } else if (!Utils.isEmptyOrNull(maybeLocalPath)) { + //if no bytes, check for local path + File file = new File(maybeLocalPath); + try { + data = Files.readAllBytes(file.toPath()); + } catch (IOException e) { + L.e("[Transport] getPictureDataFromRequest, Error while reading picture, wont send from path:[ " + maybeLocalPath + "], " + e); } - data = Files.readAllBytes(file.toPath()); - } catch (Throwable t) { - //if we can't read it from disk, we assume it is a base64 encoded byte array - data = Utils.Base64.decode(picture, L); } return data; From a6fd2114f66921be3b0cc26b5fb9b3db8a1b78eb Mon Sep 17 00:00:00 2001 From: Arif Burak Demiray Date: Thu, 7 Dec 2023 14:44:24 +0300 Subject: [PATCH 5/6] fix: user picture things --- .../sdk/java/internal/ModuleUserProfile.java | 7 +++--- .../count/sdk/java/internal/SessionImpl.java | 2 +- .../ly/count/sdk/java/internal/Transport.java | 22 ++++++++++--------- .../sdk/java/internal/UserEditorTests.java | 4 ++-- 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/sdk-java/src/main/java/ly/count/sdk/java/internal/ModuleUserProfile.java b/sdk-java/src/main/java/ly/count/sdk/java/internal/ModuleUserProfile.java index cd3e6ccd8..72cd09a53 100644 --- a/sdk-java/src/main/java/ly/count/sdk/java/internal/ModuleUserProfile.java +++ b/sdk-java/src/main/java/ly/count/sdk/java/internal/ModuleUserProfile.java @@ -213,12 +213,11 @@ private Params prepareRequestParamsForUserProfile() { Params params = new Params(); final JSONObject json = new JSONObject(); perform(json, params); - if (!json.isEmpty() || params.has(PredefinedUserPropertyKeys.PICTURE_PATH) || params.has(PICTURE_BYTES)) { + if (!json.isEmpty()) { params.add("user_details", json.toString()); - return params; - } else { - return new Params(); } + + return params; } /** diff --git a/sdk-java/src/main/java/ly/count/sdk/java/internal/SessionImpl.java b/sdk-java/src/main/java/ly/count/sdk/java/internal/SessionImpl.java index 7c9a7f3ae..7dbb2cb55 100644 --- a/sdk-java/src/main/java/ly/count/sdk/java/internal/SessionImpl.java +++ b/sdk-java/src/main/java/ly/count/sdk/java/internal/SessionImpl.java @@ -122,7 +122,7 @@ Future begin(Long now) { if (pushOnChange) { Storage.pushAsync(config, this); } - if (hasConsent(CoreFeature.Location)) { + if (hasConsent(CoreFeature.Location) && config.sdk.module(ModuleLocation.class) != null) { params.add(config.sdk.module(ModuleLocation.class).prepareLocationParams()); } Future ret = ModuleRequests.sessionBegin(config, this); diff --git a/sdk-java/src/main/java/ly/count/sdk/java/internal/Transport.java b/sdk-java/src/main/java/ly/count/sdk/java/internal/Transport.java index cfb002ea5..fb21dd9e7 100644 --- a/sdk-java/src/main/java/ly/count/sdk/java/internal/Transport.java +++ b/sdk-java/src/main/java/ly/count/sdk/java/internal/Transport.java @@ -506,22 +506,24 @@ public X509Certificate[] getAcceptedIssuers() { private String setProfilePicturePathRequestParams(String path, Params params) { Params tempParams = new Params(); - tempParams.add("device_id", params.get("device_id")); - tempParams.add("app_key", params.get("app_key")); - tempParams.add("timestamp", params.get("timestamp")); - tempParams.add("sdk_name", params.get("sdk_name")); - tempParams.add("sdk_version", params.get("sdk_version")); - tempParams.add("tz", params.get("tz")); - tempParams.add("hour", params.get("hour")); - tempParams.add("dow", params.get("dow")); - tempParams.add("rr", params.get("rr")); + tempParams.add("device_id", params.remove("device_id")); + tempParams.add("app_key", params.remove("app_key")); + tempParams.add("timestamp", params.remove("timestamp")); + tempParams.add("sdk_name", params.remove("sdk_name")); + tempParams.add("sdk_version", params.remove("sdk_version")); + tempParams.add("tz", params.remove("tz")); + tempParams.add("hour", params.remove("hour")); + tempParams.add("dow", params.remove("dow")); + tempParams.add("rr", params.remove("rr")); if (params.has("av")) { - tempParams.add("av", params.get("av")); + tempParams.add("av", params.remove("av")); } //if no user details, add empty user details to indicate that we are sending a picture if (!params.has("user_details")) { tempParams.add("user_details", "{}"); + } else { + tempParams.add("user_details", params.remove("user_details")); } return path + tempParams; diff --git a/sdk-java/src/test/java/ly/count/sdk/java/internal/UserEditorTests.java b/sdk-java/src/test/java/ly/count/sdk/java/internal/UserEditorTests.java index b4086032d..86ac58ae9 100644 --- a/sdk-java/src/test/java/ly/count/sdk/java/internal/UserEditorTests.java +++ b/sdk-java/src/test/java/ly/count/sdk/java/internal/UserEditorTests.java @@ -64,7 +64,7 @@ public void setPicturePath_localPath() { //set profile picture url and commit it sessionHandler(() -> Countly.instance().user().edit().setPicturePath(imgFile.getAbsolutePath()).commit()); validatePictureAndPath(imgFile.getAbsolutePath(), null); - validateUserDetailsRequestInRQ(map("user_details", "{}", "picturePath", imgFile.getAbsolutePath())); + validateUserDetailsRequestInRQ(map("picturePath", imgFile.getAbsolutePath())); } /** @@ -112,7 +112,7 @@ public void setPicture_binaryData() { sessionHandler(() -> Countly.instance().user().edit().setPicture(imgData).commit()); validatePictureAndPath(null, imgData); Countly.session().end(); - validateUserDetailsRequestInRQ(map("user_details", "{}", ModuleUserProfile.PICTURE_BYTES, Utils.Base64.encode(imgData))); + validateUserDetailsRequestInRQ(map(ModuleUserProfile.PICTURE_BYTES, Utils.Base64.encode(imgData))); } /** From 4a32ee8babc4b8ccc0a59f7d9c8d5e3a4777c9ad Mon Sep 17 00:00:00 2001 From: Arif Burak Demiray <57103426+arifBurakDemiray@users.noreply.github.com> Date: Wed, 3 Jan 2024 09:37:15 +0000 Subject: [PATCH 6/6] [Java] Transition User profile away from bytables - migration (#189) * feat: add migration for bytetables * fix: revert migration changes1 * fix: rever additioan lchekc * feat: null check test --- .../sdk/java/internal/MigrationHelper.java | 43 ++++++++++ .../java/internal/MigrationHelperTests.java | 80 ++++++++++++++++--- 2 files changed, 113 insertions(+), 10 deletions(-) diff --git a/sdk-java/src/main/java/ly/count/sdk/java/internal/MigrationHelper.java b/sdk-java/src/main/java/ly/count/sdk/java/internal/MigrationHelper.java index db3e65e83..a70b7a79f 100644 --- a/sdk-java/src/main/java/ly/count/sdk/java/internal/MigrationHelper.java +++ b/sdk-java/src/main/java/ly/count/sdk/java/internal/MigrationHelper.java @@ -40,6 +40,7 @@ protected MigrationHelper(final Log logger) { // add migrations below migrations.add(this::migration_DeleteConfigFile_01); + migrations.add(this::migration_UserImplFile_02); latestMigrationVersion = migrations.size(); } @@ -127,6 +128,48 @@ protected boolean migration_DeleteConfigFile_01(final Map migrat return true; } + /** + * Deletes the timed event file, the user impl file and the session files and extracts events + * stored inside the timed event file and the session files + * + * @param migrationParams parameters to pass to migrations + * @return true if the migration was successful, false otherwise + */ + protected boolean migration_UserImplFile_02(final Map migrationParams) { + if (currentDataModelVersion >= 2) { + logger.d("[MigrationHelper] migration_UserImplFile_02, Migration already applied"); + return true; + } + logger.i("[MigrationHelper] migration_UserImplFile_02, Deleting user impl file migrating from 01 to 02"); + currentDataModelVersion += 1; + + File sdkPath = (File) migrationParams.get("sdk_path"); + if (sdkPath == null) { // this is not expected, but just in case and for null safety, why 2? because we expect 1 file to be the json config file + logger.d("[MigrationHelper] migration_UserImplFile_02, No files to delete, returning"); + return false; + } + + //SDK_FOLDER/[CLY]_user_0 + File userFile = new File(sdkPath, SDKStorage.FILE_NAME_PREFIX + SDKStorage.FILE_NAME_SEPARATOR + "user" + SDKStorage.FILE_NAME_SEPARATOR + 0); + //delete user file + deleteFileIfExist(userFile, "migration_UserImplFile_02, Cannot delete user file "); + return true; + } + + /** + * Deletes the file if it exists, logs the error if it cannot be deleted + * + * @param file to delete + * @param log to log if the file cannot be deleted + */ + private void deleteFileIfExist(File file, String log) { + try { // if we cannot delete the config file, we cannot continue + Files.deleteIfExists(file.toPath()); + } catch (IOException e) { + logger.e("[MigrationHelper] " + log + e); + } + } + /** * Reads unnecessary parts of the config file for the 1st migration * diff --git a/sdk-java/src/test/java/ly/count/sdk/java/internal/MigrationHelperTests.java b/sdk-java/src/test/java/ly/count/sdk/java/internal/MigrationHelperTests.java index e327b580d..1a7cd7905 100644 --- a/sdk-java/src/test/java/ly/count/sdk/java/internal/MigrationHelperTests.java +++ b/sdk-java/src/test/java/ly/count/sdk/java/internal/MigrationHelperTests.java @@ -6,6 +6,7 @@ import java.nio.file.Files; import java.util.HashMap; import java.util.Map; +import java.util.Objects; import ly.count.sdk.java.Countly; import org.junit.After; import org.junit.Assert; @@ -20,7 +21,7 @@ @RunWith(JUnit4.class) public class MigrationHelperTests { - final int expectedLatestSchemaVersion = 1; + final int expectedLatestSchemaVersion = 2; //example config file contains old type of data //has 'SDK_GENERATED' device id type and id value of 'CLY_0c54e5e7-eb86-4c17-81f0-4d7910d8ab0es' @@ -99,7 +100,7 @@ public void migrationHelper_defaults() { /** * "setupMigrations" * Fresh-install. That means nothing in storage - * Migration version should be 1 (the latest version), before calling "applyMigrations" because the SDK is already at the latest data schema version + * Migration version should be {@code expectedLatestVersion} (the latest version), before calling "applyMigrations" because the SDK is already at the latest data schema version */ @Test public void setupMigrations_freshInstall() { @@ -132,7 +133,7 @@ public void setupMigrations_legacyState() { @Test public void setupMigrations_latestVersion() throws IOException { setDataVersionInConfigFile(expectedLatestSchemaVersion); - initStorage(); // to initialize json storage after data version is set to 1 + initStorage(); // to initialize json storage after data version is set to expectedLatestVersion MigrationHelper migrationHelper = new MigrationHelper(mock(Log.class)); migrationHelper.logger = spy(migrationHelper.logger); @@ -145,7 +146,7 @@ public void setupMigrations_latestVersion() throws IOException { /** * "applyMigrations" in a legacy state * Upgrading from legacy state to latest version, no new config object, just old type of data. - * Migration version should be 0 before applying migrations, and 1 after applying migrations. + * Migration version should be 0 before applying migrations, and {@code expectedLatestVersion} after applying migrations. */ @Test public void applyMigrations_legacyToLatest() { @@ -155,9 +156,10 @@ public void applyMigrations_legacyToLatest() { MigrationHelper migrationHelper = new MigrationHelper(mock(Log.class)); migrationHelper.setupMigrations(storageProvider); Assert.assertEquals(0, migrationHelper.currentDataModelVersion); //legacy state - + Map migrationParams = new HashMap<>(); + migrationParams.put("sdk_path", TestUtils.getTestSDirectory()); //apply migrations - migrationHelper.applyMigrations(new HashMap<>()); + migrationHelper.applyMigrations(migrationParams); //check migration version is 1 after apply both from class and file Assert.assertEquals(expectedLatestSchemaVersion, migrationHelper.currentDataModelVersion); Assert.assertEquals(expectedLatestSchemaVersion, TestUtils.getJsonStorageProperty(SDKStorage.key_migration_version)); @@ -166,7 +168,7 @@ public void applyMigrations_legacyToLatest() { /** * "applyMigrations" with already at the latest version * All migrations are already applied, setting data version to the latest - * Migration version should be 1 before applying migrations, and 1 after applying migrations and expected log must be logged. + * Migration version should be {@code expectedLatestVersion} before applying migrations, and {@code expectedLatestVersion} after applying migrations and expected log must be logged. */ @Test public void applyMigrations_latestToLatest() throws IOException { @@ -185,7 +187,7 @@ public void applyMigrations_latestToLatest() throws IOException { /** * "applyMigrations" from 0 to 1 - * Upgrading from legacy state to the latest version, mock config file, just old type of data. + * Upgrading from legacy state to the 1st version, mock config file, just old type of data. * Data version must be 1 after applying migrations and expected log must be logged. and expected device id type must match */ @Test @@ -208,8 +210,6 @@ public void applyMigrations_0to1() throws IOException { Assert.assertEquals("CLY_0c54e5e7-eb86-4c17-81f0-4d7910d8ab0e", storageProvider.getDeviceID()); Assert.assertEquals(DeviceIdType.SDK_GENERATED.name(), storageProvider.getDeviceIdType()); - //check migration version is at the latest after apply both from class and file - Assert.assertEquals(1, migrationHelper.currentDataModelVersion); Mockito.verify(migrationHelper.logger, Mockito.times(1)).i("[MigrationHelper] migration_DeleteConfigFile_01, Deleting config file migrating from 00 to 01"); } @@ -281,6 +281,66 @@ public void applyMigrations_0to1_initCountlyNoDeviceId_customDeviceId() throws I Assert.assertEquals(DeviceIdType.DEVELOPER_SUPPLIED, Countly.instance().getDeviceIdType()); } + /** + * "applyMigrations" from 1 to 2 by init Countly with migration version as 1 + * Upgrading from 1 to 2, empty storage + * Data version must be 2 after applying migrations and expected log must be logged + */ + @Test + public void applyMigrations_1to2_nothingToMigrate() throws IOException { + setDataVersionInConfigFile(1); // set previous data version + initStorage(); + + Map migrationParams = new HashMap<>(); + migrationParams.put("sdk_path", TestUtils.getTestSDirectory()); + + MigrationHelper migrationHelper = new MigrationHelper(mock(Log.class)); + migrationHelper.setupMigrations(storageProvider); + Assert.assertEquals(1, migrationHelper.currentDataModelVersion); + migrationHelper.logger = Mockito.spy(migrationHelper.logger); + + migrationHelper.migration_UserImplFile_02(migrationParams); + Assert.assertEquals(2, migrationHelper.currentDataModelVersion); + Mockito.verify(migrationHelper.logger, Mockito.times(0)).d("[MigrationHelper] migration_UserImplFile_02, No files to delete, returning"); + } + + /** + * "applyMigrations" from 1 to 2 by init Countly with migration version as 1 + * Upgrading from 1 to 2, empty storage + * Data version must be 1 and migration should fail because sdk path is null + */ + @Test + public void applyMigrations_1to2_nullMigrationParams() throws IOException { + setDataVersionInConfigFile(1); // set previous data version + initStorage(); + + MigrationHelper migrationHelper = new MigrationHelper(mock(Log.class)); + migrationHelper.setupMigrations(storageProvider); + Assert.assertEquals(1, migrationHelper.currentDataModelVersion); + migrationHelper.logger = Mockito.spy(migrationHelper.logger); + + Assert.assertFalse(migrationHelper.migration_UserImplFile_02(new HashMap<>())); + Assert.assertEquals(2, migrationHelper.currentDataModelVersion); + Mockito.verify(migrationHelper.logger, Mockito.times(1)).d("[MigrationHelper] migration_UserImplFile_02, No files to delete, returning"); + } + + /** + * "applyMigrations" from 1 to 2 with mock user file + * Upgrading from legacy state to the latest version, mock user file, just old type of data. + * Data version must be 2 after applying migrations and no request should be generated + */ + @Test + public void applyMigrations_1to2_mockFiles() throws IOException { + TestUtils.createFile("user_0"); // empty user file + Assert.assertEquals(1, Objects.requireNonNull(TestUtils.getTestSDirectory().listFiles()).length); + Countly.instance().init(TestUtils.getConfigEvents(1)); + + //check that files are deleted + Assert.assertNotNull(TestUtils.getTestSDirectory().listFiles()); + Assert.assertEquals(1, Objects.requireNonNull(TestUtils.getTestSDirectory().listFiles()).length); + Assert.assertEquals(0, TestUtils.getCurrentRQ().length); // no events to send + } + void setDataVersionInConfigFile(final int targetDataVersion) throws IOException { //prepare storage in case we need to TestUtils.checkSdkStorageRootDirectoryExist(TestUtils.getTestSDirectory());