Skip to content

Add MQTT dependencies, settings UI, and test coverage (CATROID-1670, CATROID-1672) - #5220

Merged
wslany merged 14 commits into
Catrobat:developfrom
Paras-ydv:mqtt-settings-entry-task-tdd
Aug 3, 2026
Merged

wslany merged 14 commits into
Catrobat:developfrom
Paras-ydv:mqtt-settings-entry-task-tdd

Conversation

@Paras-ydv

@Paras-ydv Paras-ydv commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR introduces the initial MQTT foundation for Catroid by combining:

  • CATROID-1670: Eclipse Paho MQTT dependency and Android configuration
  • CATROID-1672: MQTT settings UI, persistence layer, validation, and automated tests

CATROID-1670 — MQTT Dependency and Android Configuration

Changes Implemented

catroid/build.gradle

  • Added org.eclipse.paho.client.mqttv3:1.2.5
  • Added org.eclipse.paho.android.service:1.1.1
  • Added FEATURE_MQTT_ENABLED build config flag

AndroidManifest.xml

  • Registered org.eclipse.paho.android.service.MqttService
  • Verified existing permissions:
    • INTERNET
    • ACCESS_NETWORK_STATE

Testing

Added dependency sanity tests verifying:

  • MqttClient
  • MqttConnectOptions
  • MqttMessage
  • MqttException
  • MqttCallback

are available on the test classpath.

CATROID-1672 — MQTT Settings UI and Persistence

UI Demo

Watch UI Demo

Features Implemented

Added MQTT settings screen with:

  • MQTT enable toggle
  • Broker host
  • Broker port
  • TLS option
  • Username
  • Password
  • Client ID

Persistence

Implemented persistent storage using SharedPreferences.

Added accessor APIs:

  • isMqttSharedPreferenceEnabled()
  • getMqttHost()
  • getMqttPort()
  • isMqttTlsEnabled()
  • getMqttUsername()
  • getMqttPassword()
  • getMqttClientId()

Password Storage Refactor (MqttPasswordRepository)

Following a discussion with Harsh and Prof. Wolfgang's review feedback, the MQTT password is no longer stored in plain SharedPreferences.

Instead, a MqttPasswordRepository interface was introduced (modelled after LocalHashVersionRepository) with a
DefaultMqttPasswordRepository implementation backed by EncryptedSharedPreferences. It is registered as a Koin singleton and injected wherever the password is read or written.

This keeps SettingsFragment.getMqttPassword(Context) as the existing public API — it now delegates to the injected repository internally. Unit tests mock the interface via a Koin test module, so no Android
Keystore is required in the test environment.

Validation

  • Port range: 1–65535
  • Invalid values rejected with UI feedback

Testing

Added automated tests covering:

  • Default values
  • Persistence
  • Validation
  • Edge cases
  • UI integration

28 unit tests added.

Followed TDD workflow:

  • Red
  • Green
  • Refactor

Your Checklist

  • Include the name of the Jira ticket in the PR’s title
  • Include a summary of the changes plus the relevant context
  • Choose the proper base branch (develop)
  • Confirm that the changes follow the project’s coding guidelines
  • Verify that the changes generate no compiler or linter warnings
  • Perform a self-review of the changes
  • Verify to commit no other files than the intentionally changed ones
  • Include reasonable and readable tests verifying the added or changed behavior
  • Confirm that new and existing unit tests pass locally
  • Check that the commits’ message style matches the project’s guideline
  • Stick to the project’s gitflow workflow
  • Verify that your changes do not have any conflicts with the base branch
  • After the PR, verify that all CI checks have passed
  • Post a message in the catroid-stage or catroid-ide Slack channel and ask for a code reviewer

Add the Eclipse Paho MQTT Java library dependency to the Catroid
module's build configuration. This lays the groundwork for the
connection manager and the new MQTT bricks.

- Declare org.eclipse.paho.client.mqttv3:1.2.5 via Maven Central
  (Eclipse Paho repo not required)
- Declare org.eclipse.paho.android.service:1.1.1 via Maven Central
- Add FEATURE_MQTT_ENABLED build config flag in build.gradle
- Register MqttService in AndroidManifest.xml
- Add MqttDependencySanityTest to verify Paho classes are
  accessible on the test classpath

INTERNET and ACCESS_NETWORK_STATE permissions were already present
in the manifest — no changes required.
@harshsomankar123-tech harshsomankar123-tech added the GSoC-2026 This ticket is assigned to the GSoC contributor. label Jun 21, 2026

@harshsomankar123-tech harshsomankar123-tech left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @Paras-ydv. I only gave this a quick first pass, but I’ll do a full review ASAP. PTAL at the comments in the meantime.

Comment thread catroid/src/main/AndroidManifest.xml Outdated
Comment thread catroid/build.gradle Outdated
…d default host, disable MQTT settings if feature is not enabled
@harshsomankar123-tech
harshsomankar123-tech self-requested a review July 2, 2026 19:46

@harshsomankar123-tech harshsomankar123-tech left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Paras-ydv PTAL the review and questions!
Thanks!

@wslany wslany left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks!!

MQTT password is stored in default SharedPreferences
mqtt_preferences.xml (line 63) defines the broker password as an EditTextPreference, and SettingsFragment.java (line 456) reads it from default shared preferences. That means broker credentials are persisted in plaintext app prefs. Since Catroid already has androidx.security:security-crypto and an EncryptedSharedPreferences pattern, this should either move to encrypted storage or avoid persisting the password.

MQTT Espresso smoke test toggles the navigation entry instead of the actual checkbox
In SettingsFragmentTest.java (line 264), the first click opens the MQTT settings screen using the title preference_title_enable_mqtt_bricks (“MQTT extension”). Then line 268 (line 268) passes the same title into checkPreference, whose helper expects a checkbox row to toggle. Inside the MQTT screen, the checkbox title is preference_title_mqtt_enabled (“Enable MQTT bricks”), so the test either fails to find the row or does not verify SETTINGS_SHOW_MQTT_BRICKS. It should navigate once, then call checkPreference(R.string.preference_title_mqtt_enabled, SETTINGS_SHOW_MQTT_BRICKS).

…encrypt password storage

- Fix mqttSettingsTest() to use preference_title_mqtt_enabled instead of preference_title_enable_mqtt_bricks so Espresso finds the actual checkbox inside MqttSettingsFragment

- Remove redundant password onPreferenceChangeListener that only returned true

- Store MQTT password in EncryptedSharedPreferences (AES256) instead of plaintext default SharedPreferences
@Paras-ydv

Paras-ydv commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Hi @harshsomankar123-tech and @wslany,

I've addressed the review comments:

  • Updated the Espresso test to use R.string.preference_title_mqtt_enabled for the actual checkbox in MqttSettingsFragment.
  • Removed the redundant password preference listener that only returned true.
  • Changed MQTT password storage to use EncryptedSharedPreferences instead of the default SharedPreferences, and updated SettingsFragment.getMqttPassword() accordingly.

PTAL. Thanks!

@Paras-ydv

Copy link
Copy Markdown
Contributor Author

Hi @wslany and @harshsomankar123-tech ,

I've implemented the MqttPasswordRepository approach we discussed.

Here's a summary of what was done:

  • Added MqttPasswordRepository interface with getPassword(), setPassword(), and clearPassword()
  • DefaultMqttPasswordRepository wraps EncryptedSharedPreferences (lazy-initialised), following the same pattern as LocalHashVersionRepository
  • Registered as a Koin singleton in repositoryModules inside CatroidKoinHelper
  • MqttSettingsFragment injects it via by inject() - password preference is non-persistent and reads/writes through the repository
  • SettingsFragment.getMqttPassword(Context) retains its existing signature but delegates to the injected repository internally, so no other callers needed to change
  • Unit tests mock the interface via a Koin test module (no mockkStatic, no Android Keystore dependency) and use verify(exactly = 1) to assert both the read and write paths

Let me know if you'd like any adjustments!!!

@harshsomankar123-tech harshsomankar123-tech left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Paras-ydv Thanks! Sorry for the delay. I was actually a bit busy working on a new feature for the organization. I just have a few final comments; otherwise, this PR is good to go from my side.

Comment thread catroid/src/test/java/org/catrobat/catroid/test/mqtt/MqttSettingsTest.kt Outdated
@sonarqubecloud

Copy link
Copy Markdown

@harshsomankar123-tech harshsomankar123-tech left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Paras-ydv Thanks! LGTM
cc @wslany @reichli

@wslany
wslany merged commit f634463 into Catrobat:develop Aug 3, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

GSoC-2026 This ticket is assigned to the GSoC contributor.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants