RCF-1302 implemented logic for packet storage location

[MadhuMosip]

mosip.registration.registration_packet_store_location

Summary by CodeRabbit

• New Features

  • Added dedicated disk-space validation before packet creation and a configurable packet storage resolver.

• Bug Fixes

  • Improved resolution, initialization and error handling of packet storage across internal/external/removable storage; clearer failure logs when storage is unavailable.

• Documentation

  • Added "insufficient disk space" error translations in Arabic, English, French, Hindi, Kannada and Tamil.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Centralizes packet storage resolution via a new StorageUtils, adds PACKET_STORE_LOCATION and accessor, refactors disk-space validation to use the resolved packet storage directory (raising PAK_DISK_SPACE_LOW when insufficient), updates packet base path default, and adds localized PAK_DISK_SPACE_LOW messages.

Changes

Cohort / File(s)	Summary
Constants android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/RegistrationConstants.java	Adds public constant PACKET_STORE_LOCATION ("mosip.registration.registration_packet_store_location")
Global Params android/clientmanager/src/main/java/io/mosip/registration/clientmanager/repository/GlobalParamRepository.java	Adds getCachedStringPacketStoreLocation() accessor for cached PACKET_STORE_LOCATION
Registration / Disk Checks android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/RegistrationServiceImpl.java	Replaces prior disk-space check with validatingDiskSpace() using StorageUtils.getPacketStorageDir(context); throws PAK_DISK_SPACE_LOW when space is insufficient or unavailable
Pre-registration ZIP handling android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/external/impl/PreRegZipHandlingServiceImpl.java	Removes manual external-storage initialization block; relies on StorageUtils for BASE_LOCATION resolution (external init removed)
Packet Manager (POSIX) android/packetmanager/src/main/java/io/mosip/registration/packetmanager/service/PosixAdapterServiceImpl.java	Replaces external-storage-based base location resolution with StorageUtils.getPacketStorageDir(context); adds null-checks and detailed storage state logging
New Utility android/packetmanager/src/main/java/io/mosip/registration/packetmanager/util/StorageUtils.java	Adds StorageUtils.getPacketStorageDir(Context) to locate a writable packet storage dir (tries SD → shared Documents → app external → app internal) with logging and fallbacks
Config android/packetmanager/src/main/assets/packetmanagerconfig.properties	Changes default object store path from Documents to packets
Localization assets/l10n/app_ar.arb, assets/l10n/app_en.arb, assets/l10n/app_fr.arb, assets/l10n/app_hi.arb, assets/l10n/app_kn.arb, assets/l10n/app_ta.arb	Adds PAK_DISK_SPACE_LOW translation entries for low disk-space error

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant RegistrationService
    participant StorageUtils
    participant FileSystem
    Client->>RegistrationService: start pre-checks
    RegistrationService->>StorageUtils: request packet storage dir (context)
    StorageUtils->>FileSystem: probe SD / Documents / app storage paths
    FileSystem-->>StorageUtils: returns writable baseDir or null
    StorageUtils-->>RegistrationService: baseDir (or null)
    RegistrationService->>FileSystem: check usable space on baseDir
    alt usable space < required or unavailable
        RegistrationService-->>Client: throw PAK_DISK_SPACE_LOW
    else
        RegistrationService-->>Client: proceed with registration
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇
I hopped through folders, near and far,
Found packets resting where they are.
I checked the space before the store,
If small, I warned in tongues galore. 📦✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: implementing logic for packet storage location, which is reflected across multiple files including new StorageUtils class, updated repository/service methods, and configuration changes.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 6

🤖 Fix all issues with AI agents

In
`@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/RegistrationServiceImpl.java`:
- Around line 679-689: The current disk-space check uses a potentially relative
or invalid packetStoreLocation and silently falls back to context.getFilesDir(),
which can mask low-space on the real packet storage; update the logic around
globalParamRepository.getCachedStringPacketStoreLocation(), actualDiskSpace and
usableSpace to: normalize the configured path (use new
File(packetStoreLocation).getCanonicalFile()), ensure it is absolute and points
to the intended storage (create parent dirs with mkdirs() if needed), if the
configured path is invalid/unwritable treat that as an error/low-space condition
(do not silently switch to context.getFilesDir()), and only use
StorageUtils.getPacketStorageDir(context) when there is no configured value;
keep all checks against actualDiskSpace.getUsableSpace() after validation so
low-disk conditions on the configured storage are correctly detected and
reported.

In
`@android/packetmanager/src/main/java/io/mosip/registration/packetmanager/service/PosixAdapterServiceImpl.java`:
- Around line 73-77: The null-check mentioned is unreachable because
StorageUtils.getPacketStorageDir(context) always returns a File; update
initPosixAdapterService to validate the returned File instead of checking for
null: after calling StorageUtils.getPacketStorageDir(context) (in
initPosixAdapterService) verify that the File exists, is a directory and is
writable (e.g., file.exists(), file.isDirectory(), file.canWrite()) before
assigning BASE_LOCATION, and handle the error path (log/error/throw) if those
checks fail so BASE_LOCATION is only set when the directory is usable.
- Around line 207-218: The validation block in PosixAdapterServiceImpl (symbols:
BASE_LOCATION, baseDir, appContext, putObject) currently silently returns and
contains an unreachable null-check; update it to (1) remove the unnecessary
BASE_LOCATION == null early return (StorageUtils.getPacketStorageDir() is
non-null), (2) replace the startsWith logic with a robust path comparison using
canonical paths (e.g., compare baseDir.getCanonicalPath() vs
appContext.getFilesDir().getCanonicalPath() or use a utility to check whether
baseDir is a child of app files) so the external-storage check only runs when
truly outside app files, and (3) replace silent returns with logged error/warn
messages (use the existing logger in PosixAdapterServiceImpl) that include the
reason for aborting so callers like putObject can fail with actionable logs.

In
`@android/packetmanager/src/main/java/io/mosip/registration/packetmanager/util/StorageUtils.java`:
- Around line 49-53: The current StorageUtils method that creates baseDir (uses
Environment.getExternalStoragePublicDirectory and calls baseDir.mkdirs()) logs
failures but still returns a non-existent/unwritable path; change it to handle
failures and scoped-storage constraints by: attempt creation as now, then if
mkdirs() fails or the directory is not writable, fall back to app-private
storage via context.getExternalFilesDir(location) (or context.getFilesDir() if
getExternalFilesDir is null); do not use
Environment.getExternalStoragePublicDirectory for apps targeting API 30+; before
returning ensure the chosen directory exists and is writable (or return null /
throw an IOException) so callers of StorageUtils (the method that builds
baseDir) receive a valid, usable path or an explicit error.

In `@assets/l10n/app_hi.arb`:
- Line 305: The errors message for the "errors" key uses the wrong punctuation
for PAK_DISK_SPACE_LOW: replace the Japanese/Chinese ideographic full stop (。
U+3002) with the correct Hindi punctuation (देवनागरी danda '।') or a standard
ASCII period '.' as appropriate; update the PAK_DISK_SPACE_LOW segment inside
the "errors" select mapping so the string ends with the chosen correct
punctuation instead of U+3002.

In `@assets/l10n/app_kn.arb`:
- Line 305: Replace the English message for the select-case key
PAK_DISK_SPACE_LOW inside the "errors" select mapping with a Kannada
translation; locate the "errors" select string (contains keys like
REG_TRY_AGAIN, PAK_APPRVL_MAX_TIME, etc.) and change PAK_DISK_SPACE_LOW{Minimum
required space is not available to create the packet.} to a proper Kannada
sentence conveying the same meaning (e.g., PAK_DISK_SPACE_LOW{ಪಟ್ಟೆ ಸೃಷ್ಟಿಸಲು
ಅಗತ್ಯವಿರುವ ಕನಿಷ್ಠ ಖಾಲಿ ಜಾಗ ಲಭ್ಯವಿಲ್ಲ.}).

🧹 Nitpick comments (2)

android/packetmanager/src/main/java/io/mosip/registration/packetmanager/service/PosixAdapterServiceImpl.java (2)

220-222: Missing error handling for directory creation.

containerFolder.mkdirs() result is not checked. If directory creation fails (e.g., permissions issue on the newly validated external storage), subsequent file operations will fail silently.

Suggested improvement

         File containerFolder = new File(BASE_LOCATION, account);
-        if(!containerFolder.exists())
-            containerFolder.mkdirs();
+        if (!containerFolder.exists() && !containerFolder.mkdirs()) {
+            Log.e(TAG, "Failed to create container folder: " + containerFolder.getAbsolutePath());
+            return;
+        }

---

232-233: Resource leak: FileInputStream not closed.

The FileInputStream at line 232 is passed to getAllExistingEntries() which closes the stream, but if an exception occurs between creation and that call, the stream leaks. Consider using try-with-resources.

Suggested fix

             } else {
-                InputStream ios = new FileInputStream(containerZip);
-                Map<ZipEntry, ByteArrayOutputStream> entries = getAllExistingEntries(ios);
-                try (ZipOutputStream packetZip = new ZipOutputStream(out)) {
+                Map<ZipEntry, ByteArrayOutputStream> entries;
+                try (InputStream ios = new FileInputStream(containerZip)) {
+                    entries = getAllExistingEntries(ios);
+                }
+                try (ZipOutputStream packetZip = new ZipOutputStream(out)) {

Note: This requires refactoring getAllExistingEntries() to not close the stream, or adjusting the logic accordingly.

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In
`@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/external/impl/PreRegZipHandlingServiceImpl.java`:
- Around line 488-492: The initPreRegAdapter method sets BASE_LOCATION using
StorageUtils.getPacketStorageDir but BASE_LOCATION is never used; update
storePreRegPacketToDisk to use BASE_LOCATION (instead of
this.appContext.getFilesDir()) so the configured packet storage directory is
honored, or if you prefer to keep using appContext.getFilesDir(), remove the
BASE_LOCATION field and the unused Environment import; locate the symbols
initPreRegAdapter, BASE_LOCATION, storePreRegPacketToDisk, and the Environment
import to apply the change consistently.

In
`@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/RegistrationServiceImpl.java`:
- Around line 677-698: In validatingDiskSpace(), ensure you treat non‑positive
config values as default and treat non-directory or non-writable storage paths
as low/unavailable: set minSpaceRequiredMB to DEFAULT_MIN_SPACE_REQUIRED_MB when
getCachedIntegerDiskSpaceSize() <= 0; after getting actualDiskSpace from
StorageUtils.getPacketStorageDir(context), verify that either it exists and
isDirectory() and canWrite(), or that mkdirs() creates a writable directory; if
the path exists but isFile() or !isDirectory() or !canWrite(), log the path and
return true; then keep the existing usableSpace check and compare usableSpace
against allowedDiskSpaceSizeInBytes in validatingDiskSpace().

🧹 Nitpick comments (1)

android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/external/impl/PreRegZipHandlingServiceImpl.java (1)

5-5: Potentially unused import after refactor.

The android.os.Environment import may no longer be needed since the storage initialization logic has been delegated to StorageUtils. Consider removing it if unused.

#!/bin/bash
# Verify if Environment class is still used in this file
echo "=== Searching for Environment usage in PreRegZipHandlingServiceImpl ==="
rg -n "Environment\." android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/external/impl/PreRegZipHandlingServiceImpl.java

[SachinPremkumar]

reviewed