Feature/file type support SDK-3856 SDK-3844 #619

piyush-kukadiya · 2024-06-05T13:59:40Z

Adds implementation for file pre loading of any file type.
Adds public API for clearing files from cache and disk
Adds public API for returning absolute file path for given file url
Adds public API to check if file for given url exists or not

…ogic with callback support SDK-3845

…c, types of files storage and add file cleanup logic SDK-3856

…-3856

… in FilePreloaderCoroutine SDK-3856

… and replace existing FileResourcesRepo instance creation with factory SDK-3856

…es(bool) SDK-3869

…rl(String) SDK-3872

…l(String) SDK-3873

…rUrl(String) SDK-3872

… file preloading tests SDK-3856

…eContext, add getFile(name: String) method for retrieving file arguments SDK-3876

CTLalit · 2024-06-06T07:06:26Z

clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapAPI.java

+     * @param url the non-null url for which to check the existence of the file.
+     * @return true if a file exists for the specified url, false otherwise.
+     */
+    public boolean isFileExistsForUrl(@NonNull String url){


this can be renamed to checkFileForUrl or doesFileExistForUrl

Fixed in 49f583

CTLalit · 2024-06-06T07:23:10Z

clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/images/InAppResourceProvider.kt

    private val inAppRemoteSource: InAppImageFetchApiContract = InAppImageFetchApi()
 ) {
+
+    private var ctCaches: CTCaches = CTCaches.instance(


names to call args and formatting

fixed in 24ae903

CTLalit · 2024-06-06T12:38:22Z

clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/images/InAppResourceProvider.kt

+        val memoryAccessObjectList = listOf<MemoryAccessObject<*>>(
+            FileMemoryAccessObject(ctCaches), ImageMemoryAccessObject(ctCaches),
+            GifMemoryAccessObject(ctCaches)
+        )


can we avoid the * wildcard?

Anush-Shand · 2024-06-06T11:42:50Z

...ertap-core/src/test/java/com/clevertap/android/sdk/inapp/images/InAppResourceProviderTest.kt

 class InAppResourceProviderTest {

    private val mockCache = Mockito.mock(CTCaches::class.java)
-    private val mockBitmap = Mockito.mock(Bitmap::class.java)
+    private val mockBitmapPair = Pair(Mockito.mock(Bitmap::class.java), mockk<File>())
+    private val mockBytesPair = Pair(Mockito.mock(ByteArray::class.java), mockk<File>())


Can only mockk be used instead of Mockito?

clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapAPI.java

Anush-Shand · 2024-06-06T12:45:40Z

clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/images/InAppResourceProvider.kt

+                return true
+            }
+        }
+        return false
    }

    fun cachedImage(cacheKey: String?): Bitmap? {


Can we rename to getCachedImage. Would be more readable?

CTLalit · 2024-06-06T13:17:25Z

...-core/src/main/java/com/clevertap/android/sdk/inapp/images/memory/ImageMemoryAccessObject.kt

+        return imageInMemory.get(key)
+    }
+
+    override fun fetchInMemoryAndTransform(key: String, transformTo: Int): Any? {


can we have specific return types and not Any?

transformTo: Int can be an enum/sealed class?

can we have specific return types and not Any?

Right now return type Any produces 6 methods. Changing Any to specific return type will produce 18 methods(12 more).

transformTo: Int can be an enum/sealed class?

fixed in 2bbb5f6

clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/images/memory/ImageMemoryV1.kt

CTLalit · 2024-06-06T13:30:14Z

clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/images/repo/FileResourcesRepo.kt

+    fun cleanupStaleImages(validUrls: List<String>)
+    fun cleanupStaleFiles(validUrls: List<String>)


cleanupStaleGifs seems missing, intentional?

It's intentional because cleanupStaleImages clears both gif and images.

Anush-Shand · 2024-06-06T13:34:53Z

@piyush-kukadiya GifMemoryV1, ImageMemoryV1 and FileMemoryV2 have same implementation for same functions, can they have a default implementation in the base class itself?

…, for CustomTemplates, download all file arguments before presenting SDK-3876

…er showInApp(), for CustomTemplates, download all file arguments before presenting SDK-3876

…doesFileExistForUrl() SDK-3872

…gs SDK-3856

…ve Logger SDK-3869

…ve Logger in doesFileExistForUrl SDK-3872

…ve Logger in getFilePathForUrl SDK-3873

…o enum SDK-3856

…ore specific SDK-3856

…FilesAndCache caller and complete preloadFilesAndCache implementation SDK-3856

…on-image files SDK-3856

…feature/file_type_support

…ile_type_support

…rt_comments

… showInApp() to CTInAppNotification prepareForDisplay() for CustomTemplates, download all file arguments before presenting SDK-3876

…support

…rt_comments

- handles inapp custom code download in sync fashion

…rTap/clevertap-android-sdk into feature/file_type_support_comments

…o fix build errors SDK-3856

…o remove duplicate code for cached related methods SDK-3856

…o remove duplicate code for api call related methods SDK-3856

…o remove duplicate code for saving call related methods SDK-3856

…o remove duplicate code for fetching call related methods SDK-3856

…o remove duplicate code for delete method SDK-3856

…o remove duplicate code for isFileCached method SDK-3856

…ents File type support refactoring

vasct · 2024-06-19T07:38:44Z

clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppNotification.java

+
+                if (bytes == null) {
+                    // download fail
+                    this.error = "some error";


We need to discuss how to handle failed files with product. We could leave failed files as "null" and still display the in-app with no error.

Meanwhile could we change the error string to something more descriptive. "Failed to download file" or something.

vasct

Good work overall. Why are so many tests commented-out ?

vasct · 2024-06-20T07:38:34Z

clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapAPI.java

        } else {
-            impl.cleanupAllImages();
+            impl.cleanupStaleFiles(); // todo this also only clears expired ones. fixme


This todo should be addressed before release since this is the public API. We could also temporary remove the expiredOnly param from the function if it would be too much work to have clean up of all files.

vasct · 2024-06-20T07:44:06Z

...-core/src/main/java/com/clevertap/android/sdk/inapp/customtemplates/CustomTemplateContext.kt

@@ -179,7 +184,17 @@ sealed class CustomTemplateContext private constructor(
        return map
    }

-    //TODO CustomTemplates add getFile(name: String) method for retrieving file arguments
+    // TODO CustomTemplates add getFile(name: String) method for retrieving file arguments


You can remove the //TODO

vasct · 2024-06-20T07:44:22Z

...-core/src/main/java/com/clevertap/android/sdk/inapp/customtemplates/CustomTemplateContext.kt

@@ -253,8 +268,8 @@ sealed class CustomTemplateContext private constructor(
                        else -> overrides.getDouble(argument.name)
                    }
                }
-                //TODO CustomTemplates add FILE handling when implemented
-                FILE -> null
+                // TODO CustomTemplates add FILE handling when implemented


Remove TODO

vasct · 2024-06-20T07:49:11Z

clevertap-core/src/main/java/com/clevertap/android/sdk/bitmap/HttpBitmapLoader.kt

We use this class and HttpBitmapOperaiton for also downloading bytes (files), I think we could rename those classes to reflect that they are used in a more generic way.

vasct · 2024-06-20T08:54:36Z

clevertap-core/src/main/java/com/clevertap/android/sdk/response/InAppResponse.java

            final LegacyInAppStore legacyInAppStore = storeRegistry.getLegacyInAppStore();

-            if (impressionStore == null || inAppStore == null || inAppAssetStore == null || legacyInAppStore == null) {
+            if (impressionStore == null || inAppStore == null || inAppAssetStore == null || legacyInAppStore == null || fileStore == null) {


We only use inAppAssetStore, legacyInAppStore and fileStore to check if they are null and nowhere after that. Do we need them as variables ?

vasct · 2024-06-20T08:59:29Z

...test/java/com/clevertap/android/sdk/inapp/images/cleanup/FileCleanupStrategyExecutorsTest.kt

        executor = executors
-    )
+    )/*


vasct · 2024-06-20T08:59:42Z

...test/java/com/clevertap/android/sdk/inapp/images/preload/InAppImagePreloaderCoroutineTest.kt

        logger = logger,
        dispatchers = dispatchers
    )
-
+/*


vasct · 2024-06-20T08:59:54Z

...test/java/com/clevertap/android/sdk/inapp/images/preload/InAppImagePreloaderExecutorsTest.kt

        logger = logger,
        executor = executors
-    )
+    )/*


vasct · 2024-06-20T09:00:06Z

clevertap-core/src/test/java/com/clevertap/android/sdk/inapp/images/repo/InAppImageRepoTest.kt

-
-    private val inAppImageCleanupStrategy = mockk<InAppCleanupStrategy>(relaxed = true)
-    private val preloaderStrategy = mockk<InAppImagePreloaderStrategy>(relaxed = true)
+/*


vasct · 2024-06-20T09:00:47Z

clevertap-core/src/test/java/com/clevertap/android/sdk/utils/CTCachesTest.kt

 import org.junit.Test
 import kotlin.test.assertEquals

+@Ignore


…fore-display Fix copying of in-app before displaying of custom code template

Fix visual templates triggered as actions are not dismissed

piyush-kukadiya added 10 commits May 30, 2024 20:21

feat(file_type_download_support-v1) : add file download and caching l…

e852e5f

…ogic with callback support SDK-3845

feat(file_type_download_support-v2) : encapsulate file operation logi…

01c8172

…c, types of files storage and add file cleanup logic SDK-3856

feat(file_type_download_support-v2) : code cleanup and formatting SDK…

747eaa3

…-3856

feat(file_type_download_support-v2) : add preloadFiles implementation…

3f74812

… in FilePreloaderCoroutine SDK-3856

feat(file_type_download_support-v2) : create FileResourcesRepoFactory…

8ff39fb

… and replace existing FileResourcesRepo instance creation with factory SDK-3856

feat(file_type_download_support-v2) : add public API clearFileResourc…

57af803

…es(bool) SDK-3869

feat(file_type_download_support-v2) : add public API isFileExistsForU…

43e7693

…rl(String) SDK-3872

feat(file_type_download_support-v2) : add public API getFilePathForUr…

aa4eaa8

…l(String) SDK-3873

feat(file_type_download_support-v2) : add comments for isFileExistsFo…

63e84c0

…rUrl(String) SDK-3872

feat(file_type_download_support-v2) : fix build error temporarily for…

fdfbfeb

… file preloading tests SDK-3856

piyush-kukadiya requested review from Anush-Shand and CTLalit June 5, 2024 13:59

feat(file_type_download_support-v2) : complete TODO: in CustomTemplat…

027db57

…eContext, add getFile(name: String) method for retrieving file arguments SDK-3876

CTLalit reviewed Jun 6, 2024

View reviewed changes

Anush-Shand reviewed Jun 6, 2024

View reviewed changes

CTLalit reviewed Jun 6, 2024

View reviewed changes

Anush-Shand reviewed Jun 6, 2024

View reviewed changes

clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/images/memory/ImageMemoryV1.kt Outdated Show resolved Hide resolved

CTLalit reviewed Jun 6, 2024

View reviewed changes

piyush-kukadiya added 9 commits June 7, 2024 16:52

feat(file_type_download_support-v2) : complete TODO: in InAppResponse…

9d4b1c1

…, for CustomTemplates, download all file arguments before presenting SDK-3876

feat(file_type_download_support-v2) : complete TODO: in InAppControll…

f447027

…er showInApp(), for CustomTemplates, download all file arguments before presenting SDK-3876

feat(file_type_download_support-v2) : rename isFileExistsForUrl() to …

49f583b

…doesFileExistForUrl() SDK-3872

feat(file_type_download_support-v2) : format and add names to call ar…

24ae903

…gs SDK-3856

feat(file_type_download_support-v2) : use getConfigLogger() to retrie…

a786506

…ve Logger SDK-3869

feat(file_type_download_support-v2) : use getConfigLogger() to retrie…

b52be62

…ve Logger in doesFileExistForUrl SDK-3872

feat(file_type_download_support-v2) : use getConfigLogger() to retrie…

0442041

…ve Logger in getFilePathForUrl SDK-3873

feat(file_type_download_support-v2) : move transformation constants t…

2bbb5f6

…o enum SDK-3856

feat(file_type_download_support-v2) : rename lock identifiers to be m…

0ed03a4

…ore specific SDK-3856

piyush-kukadiya added 3 commits June 11, 2024 13:46

feat(file_type_download_support-v2) : add success callback to preload…

a310477

…FilesAndCache caller and complete preloadFilesAndCache implementation SDK-3856

feat(file_type_download_support-v2) : fix download failed issue for n…

4725e0a

…on-image files SDK-3856

Merge remote-tracking branch 'origin/feature/file_type_support' into …

98fcb39

…feature/file_type_support

CTLalit added Reviewed chained PRs core7.0.0 labels Jun 11, 2024

CTLalit changed the base branch from feature/custom_inapp_templates to task/SDK-3814/user_attribute_triggering June 11, 2024 13:28

CTLalit and others added 3 commits June 11, 2024 18:59

Merge branch 'task/SDK-3814/user_attribute_triggering' into feature/f…

c6fba2b

…ile_type_support

Merge branch 'feature/file_type_support' into feature/file_type_suppo…

7a215b0

…rt_comments

feat(file_type_download_support-v2) : move TODO: from InAppController…

1cc0e28

… showInApp() to CTInAppNotification prepareForDisplay() for CustomTemplates, download all file arguments before presenting SDK-3876

Base automatically changed from task/SDK-3814/user_attribute_triggering to feature/custom_inapp_templates June 12, 2024 13:51

CTLalit and others added 14 commits June 12, 2024 19:22

Merge branch 'feature/custom_inapp_templates' into feature/file_type_…

b941e9e

…support

Merge branch 'feature/file_type_support' into feature/file_type_suppo…

e1ee6c4

…rt_comments

Feat(SDK-3780): Prepare for display for files

434a7ac

- handles inapp custom code download in sync fashion

Merge branch 'feature/file_type_support_comments' of github.com:Cleve…

3a10c8e

…rTap/clevertap-android-sdk into feature/file_type_support_comments

Feat(SDK-3856): Fixed compilation error

9fefcfd

Chore(SDK-3856): optimise imports

320d2c2

feat(file_type_download_support-v2) : comment out tests temporarily t…

f48c456

…o fix build errors SDK-3856

feat(file_type_download_support-v2) : refactor FileResourceProvider t…

05e36f7

…o remove duplicate code for cached related methods SDK-3856

feat(file_type_download_support-v2) : refactor FileFetchApiContract t…

8aaac17

…o remove duplicate code for api call related methods SDK-3856

feat(file_type_download_support-v2) : refactor FileResourceProvider t…

90816a5

…o remove duplicate code for saving call related methods SDK-3856

feat(file_type_download_support-v2) : refactor FileResourceProvider t…

dd6d3ef

…o remove duplicate code for fetching call related methods SDK-3856

feat(file_type_download_support-v2) : refactor FileResourceProvider t…

51536bf

…o remove duplicate code for delete method SDK-3856

feat(file_type_download_support-v2) : refactor FileResourceProvider t…

02e2c1b

…o remove duplicate code for isFileCached method SDK-3856

Merge pull request #622 from CleverTap/feature/file_type_support_comm…

f8b5344

…ents File type support refactoring

vasct reviewed Jun 19, 2024

View reviewed changes

vasct reviewed Jun 20, 2024

View reviewed changes

vasct and others added 4 commits June 21, 2024 15:53

Fix copying of in-app before displaying of custom code template

dc5bd87

Merge pull request #627 from CleverTap/fix/custom-template-copying-be…

9e192ff

…fore-display Fix copying of in-app before displaying of custom code template

Fix visual templates triggered as actions are not dismissed

dc85b5b

Merge pull request #628 from CleverTap/fix/visual-actions-not-dismissed

626059a

Fix visual templates triggered as actions are not dismissed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/file type support SDK-3856 SDK-3844 #619

Feature/file type support SDK-3856 SDK-3844 #619

piyush-kukadiya commented Jun 5, 2024

CTLalit Jun 6, 2024

piyush-kukadiya Jun 7, 2024

CTLalit Jun 6, 2024

piyush-kukadiya Jun 7, 2024

CTLalit Jun 6, 2024

Anush-Shand Jun 6, 2024

Anush-Shand Jun 6, 2024

CTLalit Jun 6, 2024

CTLalit Jun 6, 2024

piyush-kukadiya Jun 7, 2024

piyush-kukadiya Jun 7, 2024

CTLalit Jun 6, 2024

piyush-kukadiya Jun 7, 2024

Anush-Shand commented Jun 6, 2024 •

edited

Loading

vasct Jun 19, 2024

vasct left a comment

vasct Jun 20, 2024

vasct Jun 20, 2024

vasct Jun 20, 2024

vasct Jun 20, 2024

vasct Jun 20, 2024

vasct Jun 20, 2024

vasct Jun 20, 2024

vasct Jun 20, 2024

vasct Jun 20, 2024

vasct Jun 20, 2024

		fun cleanupStaleImages(validUrls: List<String>)
		fun cleanupStaleFiles(validUrls: List<String>)

Feature/file type support SDK-3856 SDK-3844 #619

Are you sure you want to change the base?

Feature/file type support SDK-3856 SDK-3844 #619

Conversation

piyush-kukadiya commented Jun 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Anush-Shand commented Jun 6, 2024 • edited Loading

Choose a reason for hiding this comment

vasct left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Anush-Shand commented Jun 6, 2024 •

edited

Loading