RCF-1302 implemented config properties

[MadhuMosip]

mosip.registration.jobs.offline
mosip.registration.jobs.unTagged
mosip.registration.jobs.restart

Summary by CodeRabbit

• New Features

  • Runtime exclusion of specific sync jobs from scheduling.
  • User restart prompt after manual synchronization completes (localized strings added).
  • Completion callbacks for packet sync/upload to signal end of processing.

• Bug Fixes

  • Improved disk-space validation to reduce pre-check failures and better handle storage availability.

• Refactoring

  • Consolidated job tracking and sync flow to better manage restartable and excluded jobs.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Adds runtime job exclusion when scheduling and during manual sync, introduces coordinated post-sync restart prompting via a Flutter MethodChannel, exposes new global param constants/getters for excluded/restart/restartable jobs, adds completion callbacks to batch upload/sync, and propagates jobId into ID schema sync APIs across native and Dart layers.

Changes

Cohort / File(s)	Summary
Constants & Global Params android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/RegistrationConstants.java, android/clientmanager/src/main/java/io/mosip/registration/clientmanager/repository/GlobalParamRepository.java	Added JOBS_OFFLINE, JOBS_UNTAGGED, JOBS_RESTART constants and new getters returning cached comma-separated job ID strings.
MainActivity — Scheduling & Flutter callback android/app/src/main/java/io/mosip/registration_client/MainActivity.java	Builds excluded jobId set from global params, skips scheduling jobs with null or excluded IDs, adds helpers to parse comma-separated job lists, and passes BinaryMessenger when setting sync callback for Flutter interaction.
MasterDataSyncApi — Sync lifecycle, exclusions & restart coordination android/app/src/main/java/io/mosip/registration_client/api_services/MasterDataSyncApi.java	Adds restart coordination state (locks, counters, handlers), per-job exclusion checks, onSyncJobStart/onSyncJobComplete lifecycle tracking, restart-prompt scheduling/cancellation, MethodChannel bridge to show Flutter restart dialog, and propagates jobId through many sync flows; updates method signatures accordingly.
BatchJob — Completion callback android/app/src/main/java/io/mosip/registration_client/utils/BatchJob.java	Adds Runnable onComplete parameter to syncRegistrationPackets and uploadRegistrationPackets and invokes the callback on success/error completion paths, including empty-list early-return handling.
Disk / Posix changes android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/RegistrationServiceImpl.java, android/packetmanager/src/main/java/io/mosip/registration/packetmanager/service/PosixAdapterServiceImpl.java	Renamed/refactored disk-space check to isDiskSpaceAvailable; removed explicit failure branch/log when packet storage dir creation fails.
Pigeon / Dart API & provider changes pigeon/master_data_sync.dart, lib/platform_android/sync_response_service_impl.dart, lib/platform_spi/sync_response_service.dart, lib/provider/sync_provider.dart	Changed getIDSchemaSync signature to accept jobId; updated platform SPI/impl and provider call sites to pass jobId.
Flutter app restart UI & localization lib/main.dart, assets/l10n/app_*.arb	Adds MethodChannel _syncRestartChannel handling to show localized restart dialog and restart app via Restart.restartApp(); adds sync_restart_dialog_message and sync_restart_button keys in multiple locales.

Sequence Diagram(s)

sequenceDiagram
    participant UI as MainActivity
    participant Params as GlobalParamRepository
    participant SyncApi as MasterDataSyncApi
    participant Flutter as Flutter Engine / Dart UI

    UI->>Params: getCachedStringJobsOffline()/Untagged()/Restart()
    Params-->>UI: comma-separated jobId strings
    UI->>SyncApi: scheduleAllActiveJobs()
    SyncApi->>Params: getExcludedJobIds()
    Params-->>SyncApi: Set<jobId>
    SyncApi->>SyncApi: iterate active jobs
    alt jobId null or excluded
        SyncApi-->>UI: log skip
    else eligible job
        SyncApi->>SyncApi: onSyncJobStart(jobId)
        SyncApi->>SyncApi: execute job (various API calls)
        SyncApi->>SyncApi: onSyncJobComplete(jobId, result)
    end
    opt restart prompt needed
        SyncApi->>Flutter: MethodChannel invoke showRestartDialog
        Flutter-->>UI: display localized dialog
        UI->>UI: Restart.restartApp() on confirm
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰
I nudge the jobs that sleep offline,
Trim the list, pass IDs in line,
When syncs finish soft I knock the door—
“Shall we restart?”—the fluttered score,
Callbacks hum, the app hops new. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 8.70% 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 'RCF-1302 implemented config properties' accurately describes the main changes—adding three new configuration property constants to RegistrationConstants and implementing their usage throughout the codebase for job exclusion and restart handling.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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: 1

🤖 Fix all issues with AI agents

In
`@android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/RegistrationServiceImpl.java`:
- Around line 577-579: The method isDiskSpaceAvailable() currently returns true
when disk space is insufficient, causing the condition in
RegistrationServiceImpl (the if (isDiskSpaceAvailable()) throw new
ClientCheckedException("PAK_DISK_SPACE_LOW");) to be misleading; fix this by
either renaming isDiskSpaceAvailable() to reflect its behavior (e.g.,
isDiskSpaceInsufficient()) and update all call sites, or invert the method's
return logic so isDiskSpaceAvailable() returns true only when space is actually
sufficient, then adjust the condition in RegistrationServiceImpl accordingly;
ensure you update all references to the method to avoid inconsistent semantics.

🧹 Nitpick comments (7)

android/clientmanager/src/main/java/io/mosip/registration/clientmanager/constant/RegistrationConstants.java (1)

3-5: Pre-existing: duplicate and unused imports.

java.util.ArrayList is imported twice (lines 4-5), and java.sql.Array (line 3) appears unused. Consider cleaning these up in a follow-up.

android/app/src/main/java/io/mosip/registration_client/MainActivity.java (1)

288-305: Consider extracting shared utility to reduce duplication.

getExcludedJobIds() and addJobIdsFromString() are nearly identical to methods in MasterDataSyncApi.java (lines 717-734). Consider extracting these into a shared utility class (e.g., JobIdUtils) to follow DRY principles.

♻️ Example shared utility

// In a new utility class or RegistrationConstants
public static Set<String> parseJobIds(String... values) {
    Set<String> result = new HashSet<>();
    for (String value : values) {
        if (value == null || value.trim().isEmpty()) continue;
        for (String jobId : value.split(RegistrationConstants.COMMA)) {
            String trimmed = jobId.trim();
            if (!trimmed.isEmpty()) {
                result.add(trimmed);
            }
        }
    }
    return result;
}

android/app/src/main/java/io/mosip/registration_client/api_services/MasterDataSyncApi.java (3)

798-799: Consider extracting the delay as a named constant.

The 2000ms delay is a magic number. Consider extracting it for clarity and easier tuning.

♻️ Suggested change

+    private static final long RESTART_PROMPT_DELAY_MS = 2000;
+
     private void scheduleRestartPrompt() {
         cancelPendingRestartPrompt();
         pendingRestartPrompt = () -> {
             // ...
         };
-        restartHandler.postDelayed(pendingRestartPrompt, 2000);
+        restartHandler.postDelayed(pendingRestartPrompt, RESTART_PROMPT_DELAY_MS);
     }

---

809-826: Non-cancellable dialog may trap users.

The dialog uses setCancelable(false) with only one "Restart" button. If something prevents the restart from working, users have no way to dismiss the dialog. Consider adding a "Later" option or making the dialog cancellable.

♻️ Suggested change

                 new AlertDialog.Builder(activity, android.R.style.Theme_Material_Light_Dialog_Alert)
                         .setTitle("Sync Completed Successfully")
                         .setMessage("The app will restart once you click Restart, and you will be redirected to the login page.")
-                        .setCancelable(false)
                         .setPositiveButton("Restart", (dialog, which) -> requestAppRestart())
+                        .setNegativeButton("Later", (dialog, which) -> dialog.dismiss())
                         .show();

---

717-734: Code duplication with MainActivity.

These helper methods (getExcludedJobIds(), addJobIdsFromString()) are duplicated from MainActivity.java (lines 288-305). As noted in the MainActivity review, consider extracting to a shared utility.

android/clientmanager/src/main/java/io/mosip/registration/clientmanager/service/RegistrationServiceImpl.java (2)

618-621: Directory creation in validation method.

The validation method creates directories as a side effect (line 618), which is unexpected for a method whose name suggests pure validation. While this ensures the directory exists before checking space (which is practical), consider whether this side effect should be documented or moved to initialization code.

---

628-631: Redundant zero space check.

The explicit check for usableSpace == 0 may be redundant since line 633 (return usableSpace < allowedDiskSpaceSizeInBytes) would already return true when usable space is zero (assuming allowedDiskSpaceSizeInBytes is always positive, which it is given lines 611-614).

♻️ Simplification option

You could remove the explicit zero check and rely on the comparison at line 633:

-        long usableSpace = actualDiskSpace.getUsableSpace();
-        if (usableSpace == 0) {
-            Log.e(TAG, "Usable space is 0 for path: " + actualDiskSpace.getAbsolutePath());
-            return true; // treat as low space/unavailable
-        }
-
-        return usableSpace < allowedDiskSpaceSizeInBytes;
+        long usableSpace = actualDiskSpace.getUsableSpace();
+        return usableSpace < allowedDiskSpaceSizeInBytes;

However, keeping the explicit check provides clearer logging for the zero-space edge case, which may be valuable for troubleshooting.

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In
`@android/app/src/main/java/io/mosip/registration_client/api_services/MasterDataSyncApi.java`:
- Around line 349-364: getPreRegIds currently only calls result.success on the
happy path, leaving the Pigeon callback unresolved on network-failure or
exception; update getPreRegIds (and its catch/else branches) to complete the
Pigeon result (call result.error with an error code and message, or
result.success with a failure string) whenever
NetworkUtils.isNetworkConnected(this.context) is false or an exception occurs,
and keep calling onSyncJobComplete(jobId, false, false) as before; ensure the
same unique symbols are used: getPreRegIds, result
(MasterDataSyncPigeon.Result), NetworkUtils.isNetworkConnected,
preRegistrationDataSyncService.fetchPreRegistrationIds, and onSyncJobComplete so
callers never hang.
- Around line 614-618: The completion signal is invoked too early: change
MasterDataSyncApi's call to batchJob.syncRegistrationPackets(context) so it
passes onSyncJobComplete(jobId, false, false) as a completion callback instead
of calling onSyncJobComplete immediately; update
BatchJob.syncRegistrationPackets(...) to accept that callback and invoke it from
inside the async flow (where packetService.syncRegistration(...) decrements
remainingPack[0]) when remainingPack[0] reaches zero (mirror how
uploadRegistrationPackets(...) calls its final-callback), ensuring
onSyncJobComplete is executed only after all packet callbacks finish.
- Around line 264-279: The getIDSchemaSync method is missing the jobId parameter
used by other sync methods; update the getIDSchemaSync(`@NonNull` Boolean
isManualSync, ...) signature to include the jobId (same type/name used
elsewhere), and pass that jobId into onSyncJobComplete instead of the empty
string in both the success callback and the catch block; ensure the lambda
passed to masterDataService.syncLatestIdSchema still calls result.success(...)
as before and that any callers/interface declarations are updated to match the
new getIDSchemaSync(jobId, isManualSync, ...) signature.

[coderabbitai]

Actionable comments posted: 4

🤖 Fix all issues with AI agents

In
`@android/app/src/main/java/io/mosip/registration_client/api_services/MasterDataSyncApi.java`:
- Around line 827-844: Extract the hardcoded strings in showRestartDialog() into
string resources (e.g., strings.xml keys like sync_completed_title,
sync_completed_message, restart_button, later_button) and use
activity.getString(...) when building the AlertDialog; replace
setCancelable(false) with setCancelable(true) and add a negative button via
.setNegativeButton(getString(R.string.later_button), (dialog, which) ->
dialog.dismiss()) so the user can defer restart while keeping the existing
positive button to call requestAppRestart(); ensure the AlertDialog.Builder
usage inside activity.runOnUiThread(...) still references these resources.
- Around line 326-334: Wrap the call to masterDataService.syncCACertificates
(the block that currently calls onSyncJobStart(), resetAlarm(...), reads
masterDataService.onResponseComplete(), calls onSyncJobComplete(jobId, ...), and
result.success(...)) in a try/catch so any thrown exception still calls
onSyncJobComplete to decrement runningSyncJobs and completes the Pigeon result;
also protect the async callback body with its own try/catch so exceptions inside
the callback log the error, compute an appropriate errorCode (e.g., from
exception message), call onSyncJobComplete(jobId, false, isManualSync), and call
result.success(syncResult("CACertificatesSync", 6, errorCode)) to ensure the
Flutter caller is always resolved.

In `@android/app/src/main/java/io/mosip/registration_client/utils/BatchJob.java`:
- Around line 132-141: The catch block currently sets
syncAndUploadInProgressStatus = false immediately, which can prematurely clear
the in-progress flag while other async syncRegistration calls are still running;
change the catch handling to only clear syncAndUploadInProgressStatus when
remainingPack[0] == 0 (same behavior as the success path) and otherwise leave
the flag true, decrement remainingPack[0], log the exception (including
stack/message) and then call uploadRegistrationPackets(context, onComplete) or
uploadRegistrationPackets(context, null) only when remainingPack[0] reaches 0;
update the catch in the BatchJob method where remainingPack and
uploadRegistrationPackets are referenced to match the success-path logic.
- Around line 210-216: In uploadRegistrationPackets' catch block, decrement the
shared counter remainingPack[0] (same way syncRegistrationPackets does), then
check if remainingPack[0] == 0 before invoking onComplete so the final summary
toast and proper completion semantics occur; keep resetting
syncAndUploadInProgressStatus = false and logging the exception, but do not call
onComplete unconditionally—mirror the exception-handling pattern used in
syncRegistrationPackets to ensure asynchronous callbacks and the toast run only
when all packets have finished.

🧹 Nitpick comments (1)

android/app/src/main/java/io/mosip/registration_client/api_services/MasterDataSyncApi.java (1)

119-123: pendingRestartPrompt is accessed from multiple threads without synchronization.

scheduleRestartPrompt assigns pendingRestartPrompt on whatever thread calls onSyncJobComplete, while cancelPendingRestartPrompt reads/clears it from onSyncJobStart on potentially different threads. Although Handler.removeCallbacks itself is thread-safe, the field reference can have visibility issues (stale reads) without volatile or guarding under restartLock.

Consider either declaring pendingRestartPrompt as volatile or guarding all accesses under restartLock.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In
`@android/app/src/main/java/io/mosip/registration_client/api_services/MasterDataSyncApi.java`:
- Around line 119-123: The field pendingRestartPrompt is accessed from multiple
threads (used in cancelPendingRestartPrompt() called by onSyncJobStart(), and in
scheduleRestartPrompt()), causing race conditions; fix by serializing
access—either wrap all reads/writes of pendingRestartPrompt and calls to
Handler.removeCallbacks/post inside synchronized(restartLock) blocks (update
cancelPendingRestartPrompt(), scheduleRestartPrompt(), and any other access
sites), or move all access to the main thread by changing
cancelPendingRestartPrompt() and scheduleRestartPrompt() to post Runnables to
restartHandler that perform the get/set/remove operations on
pendingRestartPrompt; pick one approach and apply it consistently so
pendingRestartPrompt is never accessed unsynchronized from background threads.

🧹 Nitpick comments (3)

android/app/src/main/java/io/mosip/registration_client/MainActivity.java (1)

258-305: Duplicate helper methods across MainActivity and MasterDataSyncApi.

getExcludedJobIds() and addJobIdsFromString() are duplicated verbatim in MasterDataSyncApi.java (lines 735–752). Consider extracting them into a shared utility class (e.g., JobFilterUtils) to keep the logic in one place.

android/app/src/main/java/io/mosip/registration_client/api_services/MasterDataSyncApi.java (2)

715-718: "Completed" log is misleading for async job branches.

Line 715 logs "Completed: " + jobApiName immediately after the switch, but for async cases (e.g., registrationPacketUploadJob, masterSyncJob), the actual work hasn't finished yet. This can confuse debugging. Consider moving this log into each case's completion callback, or changing the message to "Dispatched: " for clarity.

---

754-757: isExcludedJob rebuilds the exclusion set on every call.

Each invocation triggers getExcludedJobIds() which reads from globalParamRepository twice. During manual sync this runs ~8 times. Consider caching the set for the duration of a manual sync cycle, or at minimum within the same method chain.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

android/app/src/main/java/io/mosip/registration_client/api_services/MasterDataSyncApi.java (2)

464-466: ⚠️ Potential issue | 🟡 Minor

Misleading log message — references CA certificates instead of audit logs.

Line 465 says "Failed to store CA certificates sync last sync time" but this is inside deleteAuditLogs. Likely a copy-paste artifact.

Fix

-                Log.e(TAG, "Failed to store CA certificates sync last sync time", e);
+                Log.e(TAG, "Failed to store audit log deletion last sync time", e);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@android/app/src/main/java/io/mosip/registration_client/api_services/MasterDataSyncApi.java`
around lines 464 - 466, In the deleteAuditLogs method of class
MasterDataSyncApi, update the misleading log and user message: replace the
Log.e(TAG, "Failed to store CA certificates sync last sync time", e) with a
message that correctly references audit logs (e.g., "Failed to delete audit
logs") and fix the Toast.makeText text from "Failed to Deleted Audit logs" to a
grammatically correct string like "Failed to delete audit logs"; ensure both
statements reference the caught exception where appropriate.

---

622-733: ⚠️ Potential issue | 🟠 Major

Unguarded async callbacks in executeJobByApiName could leak the job counter.

For async branches (e.g., masterSyncJob, publicKeySyncJob, syncCertificateJob), if the callback itself throws (e.g., onResponseComplete() throws), the exception propagates inside the service's internal thread — it is not caught by the outer try/catch at line 729. This means onSyncJobComplete is never called and runningSyncJobs permanently stays elevated, blocking restart prompts forever.

Consider wrapping the callback bodies with try/catch:

Example for masterSyncJob

                     masterDataService.syncMasterData(() -> {
+                      try {
                         Log.d(getClass().getSimpleName(), "Master data sync callback");
                         String errorCode = masterDataService.onResponseComplete();
                         boolean success = errorCode == null || errorCode.isEmpty();
                         onSyncJobComplete(jobId, success, false);
+                      } catch (Exception e) {
+                        Log.e(TAG, "Error in masterSyncJob callback", e);
+                        onSyncJobComplete(jobId, false, false);
+                      }
                     }, 0, false, jobId);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@android/app/src/main/java/io/mosip/registration_client/api_services/MasterDataSyncApi.java`
around lines 622 - 733, The async callbacks passed to executeJobByApiName (e.g.,
callbacks for masterDataService.syncMasterData, syncGlobalParamsData,
syncUserDetails, syncCertificate, syncCACertificates,
batchJob.syncRegistrationPackets,
preRegistrationDataSyncService.fetchPreRegistrationIds) are unguarded and any
exception inside those callbacks (for example from
masterDataService.onResponseComplete()) will escape the outer try/catch and
prevent onSyncJobComplete from being called; wrap each callback body in a
try/catch (or try/finally) that catches Throwable, logs the error, and always
calls onSyncJobComplete(jobId, false, false) on failure (or
onSyncJobComplete(jobId, success, false) on success) so runningSyncJobs is
decremented reliably; ensure the same pattern is applied to every lambda passed
into those service methods in executeJobByApiName.

♻️ Duplicate comments (2)

android/app/src/main/java/io/mosip/registration_client/api_services/MasterDataSyncApi.java (2)

366-383: ⚠️ Potential issue | 🔴 Critical

getPreRegIds still does not complete the Pigeon result on failure or no-network.

The catch block (line 377) and the no-network else block (line 380) call onSyncJobComplete but never invoke result.success(...) or result.error(...), leaving the Flutter caller's Future unresolved. This was flagged in a previous review and marked as addressed, but the current code still exhibits the issue.

Proposed fix

         } catch (Exception e) {
             e.printStackTrace();
             onSyncJobComplete(jobId, false, false);
+            result.error(e);
         }
         } else {
             onSyncJobComplete(jobId, false, false);
+            result.success("Network unavailable");
         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@android/app/src/main/java/io/mosip/registration_client/api_services/MasterDataSyncApi.java`
around lines 366 - 383, In getPreRegIds, ensure the Pigeon callback is completed
on failure/no-network: in the catch block and the NetworkUtils false branch,
call result.error(...) (or result.success with a failure string if your API
prefers) including a descriptive message and the exception message for the
catch, then invoke onSyncJobComplete(jobId, false, false); update the code paths
in getPreRegIds so every exit path either calls result.success(...) on success
or result.error(...) on failure/no-network (use the
MasterDataSyncPigeon.Result<String> result parameter) to avoid leaving the
Flutter Future unresolved.

---

213-216: ⚠️ Potential issue | 🔴 Critical

Missing result completion in catch/else blocks leaves Flutter callers hanging.

Six Pigeon-exposed sync methods fail to complete their Result callback in exception paths:

• getPolicyKeySync (line 213–216)
• getGlobalParamsSync (line 239–242)
• getUserDetailsSync (line 264–267)
• getMasterDataSync (line 310–314)
• getKernelCertsSync (line 401–404)
• getPreRegIds (line 376–382, both catch and else blocks)

When the synchronous setup before the async callback throws, the caller's Flutter Future never resolves. Both getIDSchemaSync (line 289) and getCaCertsSync (line 346) correctly call result.error(e) in their catch blocks—apply this pattern to all six methods.

Example fix (apply to all six methods)

         } catch (Exception e) {
             e.printStackTrace();
             onSyncJobComplete(jobId, false, isManualSync);
+            result.error(e);
         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@android/app/src/main/java/io/mosip/registration_client/api_services/MasterDataSyncApi.java`
around lines 213 - 216, The six Pigeon-exposed sync methods (getPolicyKeySync,
getGlobalParamsSync, getUserDetailsSync, getMasterDataSync, getKernelCertsSync,
and getPreRegIds) sometimes fail to call the Pigeon Result in exception/else
paths causing Flutter Futures to hang; update each method so that in every catch
block and any else branch that currently returns early you call
result.error(...) (mirroring the pattern used in getIDSchemaSync and
getCaCertsSync) passing the exception message and details (or a generic error
code/message plus e.toString()), and keep existing onSyncJobComplete calls
intact where present so the Result is always completed on error or unexpected
branches.

🧹 Nitpick comments (3)

android/app/src/main/java/io/mosip/registration_client/utils/BatchJob.java (2)

140-144: Redundant if/else — simplify to a direct call.

Both branches call uploadRegistrationPackets(context, ...) with the same value: the if branch passes onComplete (which is non-null there) and the else branch passes null (which is exactly what onComplete already holds). The whole block collapses to a single call.

♻️ Proposed fix

-                    if (onComplete != null) {
-                        uploadRegistrationPackets(context, onComplete);
-                    } else {
-                        uploadRegistrationPackets(context, null);
-                    }
+                    uploadRegistrationPackets(context, onComplete);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@android/app/src/main/java/io/mosip/registration_client/utils/BatchJob.java`
around lines 140 - 144, The if/else in BatchJob is redundant: both branches call
uploadRegistrationPackets(context, ...) so replace the conditional with a single
call to uploadRegistrationPackets(context, onComplete) (allowing onComplete to
be null); update the block containing uploadRegistrationPackets(...) to remove
the if/else and pass onComplete directly to simplify the code.

---

214-238: Redundant double-assignment of syncAndUploadInProgressStatus = false in catch block.

Line 215 sets the flag unconditionally on entry to the catch block. Line 221 sets it again inside the if (remainingPack[0] == 0) guard — this is dead code since the flag is already false by that point.

♻️ Proposed fix

             } catch (Exception e) {
-                syncAndUploadInProgressStatus = false;
                 Log.e(getClass().getSimpleName(), e.getMessage());
                 remainingPack[0] -= 1;
                 if (remainingPack[0] == 0) {
-                    syncAndUploadInProgressStatus = false;
+                    syncAndUploadInProgressStatus = false;
                     Integer failed = packetSize - remainingPack[1];

Note: If the intent is to keep line 215 (fail-fast flag reset on any exception), consider whether that premature reset while other packets are still in-flight is acceptable — it mirrors the pattern accepted in the previous review cycle. If the goal is to only reset when all outstanding work is done, line 215 should move inside the if (remainingPack[0] == 0) block, consistent with how syncRegistrationPackets handles this.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@android/app/src/main/java/io/mosip/registration_client/utils/BatchJob.java`
around lines 214 - 238, The catch block in BatchJob currently sets
syncAndUploadInProgressStatus = false twice; remove the redundant unconditional
assignment and instead set syncAndUploadInProgressStatus = false only when all
packets are processed (inside the if (remainingPack[0] == 0) block) so the flag
reflects true in-flight work; locate the catch in the method that updates
remainingPack[0] and packetSize, move the syncAndUploadInProgressStatus
assignment into that if branch (near where failed, newToast, and
onComplete.run() are handled) or, if intentional to fail-fast, remove the second
assignment inside the if and keep the first—choose one consistent behavior and
apply it to the syncAndUploadInProgressStatus updates.

android/app/src/main/java/io/mosip/registration_client/api_services/MasterDataSyncApi.java (1)

748-770: getExcludedJobIds() rebuilds the set on every call — consider caching per manual-sync cycle.

isExcludedJob is called once per sync method during a manual sync flow (potentially 7+ times). Each call rebuilds a HashSet and re-parses the comma-separated config strings from the repository. This is functionally correct but wasteful for a tight call sequence.

A simple improvement: compute the excluded set once at the start of the manual sync and pass it through, or cache it as a field that is invalidated on global param changes.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@android/app/src/main/java/io/mosip/registration_client/api_services/MasterDataSyncApi.java`
around lines 748 - 770, getExcludedJobIds currently rebuilds the HashSet on
every call causing repeated parsing in isExcludedJob; change this to compute
once and reuse by introducing a cached field (e.g., private volatile Set<String>
cachedExcludedJobIds) and a method to refresh it when global params change or at
start of a manual sync; update getExcludedJobIds to return the cached set
(initializing it lazily by calling addJobIdsFromString with
globalParamRepository.getCachedStringJobsOffline() and
getCachedStringJobsUntagged()), ensure cache invalidation when global params
update, and have isExcludedJob continue to trim jobId but use the
cachedExcludedJobIds for containment checks.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/main.dart`:
- Around line 46-82: The showRestartDialog flow in _setupSyncRestartChannel can
silently fail because showRestartDialog re-checks
rootNavigatorKey.currentContext and AppLocalizations.of(context) once and
returns if not ready; change it to retry a few times with backoff and emit a
warning if it still fails. Specifically, replace the single addPostFrameCallback
retry with a small retry loop inside the _setupSyncRestartChannel handler (or
inside showRestartDialog) that attempts to obtain
rootNavigatorKey.currentContext and AppLocalizations.of(context) up to N times
(e.g., 3) with short delays/next-frame waits between attempts, logging via your
logger/warn when attempts are exhausted, and only then give up; keep the
existing behavior of showing the AlertDialog and calling Restart.restartApp()
when successful. Ensure you reference _setupSyncRestartChannel,
showRestartDialog, rootNavigatorKey, AppLocalizations.of, and
WidgetsBinding.instance.addPostFrameCallback when applying the fix.

---

Outside diff comments:
In
`@android/app/src/main/java/io/mosip/registration_client/api_services/MasterDataSyncApi.java`:
- Around line 464-466: In the deleteAuditLogs method of class MasterDataSyncApi,
update the misleading log and user message: replace the Log.e(TAG, "Failed to
store CA certificates sync last sync time", e) with a message that correctly
references audit logs (e.g., "Failed to delete audit logs") and fix the
Toast.makeText text from "Failed to Deleted Audit logs" to a grammatically
correct string like "Failed to delete audit logs"; ensure both statements
reference the caught exception where appropriate.
- Around line 622-733: The async callbacks passed to executeJobByApiName (e.g.,
callbacks for masterDataService.syncMasterData, syncGlobalParamsData,
syncUserDetails, syncCertificate, syncCACertificates,
batchJob.syncRegistrationPackets,
preRegistrationDataSyncService.fetchPreRegistrationIds) are unguarded and any
exception inside those callbacks (for example from
masterDataService.onResponseComplete()) will escape the outer try/catch and
prevent onSyncJobComplete from being called; wrap each callback body in a
try/catch (or try/finally) that catches Throwable, logs the error, and always
calls onSyncJobComplete(jobId, false, false) on failure (or
onSyncJobComplete(jobId, success, false) on success) so runningSyncJobs is
decremented reliably; ensure the same pattern is applied to every lambda passed
into those service methods in executeJobByApiName.

---

Duplicate comments:
In
`@android/app/src/main/java/io/mosip/registration_client/api_services/MasterDataSyncApi.java`:
- Around line 366-383: In getPreRegIds, ensure the Pigeon callback is completed
on failure/no-network: in the catch block and the NetworkUtils false branch,
call result.error(...) (or result.success with a failure string if your API
prefers) including a descriptive message and the exception message for the
catch, then invoke onSyncJobComplete(jobId, false, false); update the code paths
in getPreRegIds so every exit path either calls result.success(...) on success
or result.error(...) on failure/no-network (use the
MasterDataSyncPigeon.Result<String> result parameter) to avoid leaving the
Flutter Future unresolved.
- Around line 213-216: The six Pigeon-exposed sync methods (getPolicyKeySync,
getGlobalParamsSync, getUserDetailsSync, getMasterDataSync, getKernelCertsSync,
and getPreRegIds) sometimes fail to call the Pigeon Result in exception/else
paths causing Flutter Futures to hang; update each method so that in every catch
block and any else branch that currently returns early you call
result.error(...) (mirroring the pattern used in getIDSchemaSync and
getCaCertsSync) passing the exception message and details (or a generic error
code/message plus e.toString()), and keep existing onSyncJobComplete calls
intact where present so the Result is always completed on error or unexpected
branches.

---

Nitpick comments:
In
`@android/app/src/main/java/io/mosip/registration_client/api_services/MasterDataSyncApi.java`:
- Around line 748-770: getExcludedJobIds currently rebuilds the HashSet on every
call causing repeated parsing in isExcludedJob; change this to compute once and
reuse by introducing a cached field (e.g., private volatile Set<String>
cachedExcludedJobIds) and a method to refresh it when global params change or at
start of a manual sync; update getExcludedJobIds to return the cached set
(initializing it lazily by calling addJobIdsFromString with
globalParamRepository.getCachedStringJobsOffline() and
getCachedStringJobsUntagged()), ensure cache invalidation when global params
update, and have isExcludedJob continue to trim jobId but use the
cachedExcludedJobIds for containment checks.

In `@android/app/src/main/java/io/mosip/registration_client/utils/BatchJob.java`:
- Around line 140-144: The if/else in BatchJob is redundant: both branches call
uploadRegistrationPackets(context, ...) so replace the conditional with a single
call to uploadRegistrationPackets(context, onComplete) (allowing onComplete to
be null); update the block containing uploadRegistrationPackets(...) to remove
the if/else and pass onComplete directly to simplify the code.
- Around line 214-238: The catch block in BatchJob currently sets
syncAndUploadInProgressStatus = false twice; remove the redundant unconditional
assignment and instead set syncAndUploadInProgressStatus = false only when all
packets are processed (inside the if (remainingPack[0] == 0) block) so the flag
reflects true in-flight work; locate the catch in the method that updates
remainingPack[0] and packetSize, move the syncAndUploadInProgressStatus
assignment into that if branch (near where failed, newToast, and
onComplete.run() are handled) or, if intentional to fail-fast, remove the second
assignment inside the if and keep the first—choose one consistent behavior and
apply it to the syncAndUploadInProgressStatus updates.

---

ℹ️ Review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f183b69 and 24453ef.

📒 Files selected for processing (10)

• android/app/src/main/java/io/mosip/registration_client/MainActivity.java
• android/app/src/main/java/io/mosip/registration_client/api_services/MasterDataSyncApi.java
• android/app/src/main/java/io/mosip/registration_client/utils/BatchJob.java
• 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
• lib/main.dart