Skip to content

Commit

Permalink
fix(preferences): replace pubExtendedData with extendedData
Browse files Browse the repository at this point in the history
Fixes #1654
  • Loading branch information
growse committed Mar 25, 2024
1 parent dd09a3f commit 56ab9ab
Show file tree
Hide file tree
Showing 11 changed files with 48 additions and 16 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
- Share button added to contact sheet (#1465)
- Changing the connection details will now clear the contacts and the location message backlog (#1598)
- Messages now include a random `id` (String) field which can be used by any consumer to correlate and distinguish send/return messages
- `pubExtendedData` preference renamed to `extendedData` (#1654)

### Bug fixes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class LoadActivityTests : TestWithAnActivity<LoadActivity>(LoadActivity::class.j
"password": "password",
"ping": 30,
"port": 1883,
"pubExtendedData": true,
"extendedData": true,
"pubQos": 1,
"pubRetain": true,
"pubTopicBase": "owntracks/%u/%d",
Expand Down Expand Up @@ -149,7 +149,7 @@ class LoadActivityTests : TestWithAnActivity<LoadActivity>(LoadActivity::class.j
assertEquals("password", json["password"].asText())
assertEquals(30, json["ping"].asInt())
assertEquals(1883, json["port"].asInt())
assertTrue(json["pubExtendedData"].asBoolean())
assertTrue(json["extendedData"].asBoolean())
assertEquals(1 , json["pubQos"].asInt())
assertTrue(json["pubRetain"].asBoolean())
assertEquals("owntracks/%u/%d", json["pubTopicBase"].asText())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ class PreferencesActivityTests : TestWithAnActivity<PreferencesActivity>(Prefere
assertContains(R.id.effectiveConfiguration, "\"deviceId\" : \"testDeviceId\"")
assertContains(R.id.effectiveConfiguration, "\"tid\" : \"t1\"")
assertContains(R.id.effectiveConfiguration, "\"notificationEvents\" : true")
assertContains(R.id.effectiveConfiguration, "\"pubExtendedData\" : false")
assertContains(R.id.effectiveConfiguration, "\"extendedData\" : false")
assertContains(R.id.effectiveConfiguration, "\"ignoreInaccurateLocations\" : 950")
assertContains(R.id.effectiveConfiguration, "\"locatorInterval\" : 123")
assertContains(R.id.effectiveConfiguration, "\"locatorDisplacement\" : 567")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import org.owntracks.android.preferences.types.MonitoringMode
import org.owntracks.android.preferences.types.MqttProtocolLevel
import org.owntracks.android.preferences.types.MqttQos
import org.owntracks.android.preferences.types.StringMaxTwoAlphaNumericChars
import timber.log.Timber

interface DefaultsProvider {
@Suppress("IMPLICIT_CAST_TO_ANY", "UNCHECKED_CAST")
Expand Down Expand Up @@ -57,7 +56,7 @@ interface DefaultsProvider {
Preferences::pegLocatorFastestIntervalToInterval -> false
Preferences::ping -> 15
Preferences::port -> 8883
Preferences::pubExtendedData -> true
Preferences::extendedData -> true
Preferences::pubQos -> MqttQos.ONE
Preferences::pubRetain -> true
Preferences::pubTopicBase -> "owntracks/%u/%d"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ class Preferences @Inject constructor(
private val placeholder = Any()
private val listeners = WeakHashMap<OnPreferenceChangeListener, Any>()

private val remappedPreferenceKeys = mapOf("pubExtendedData" to Preferences::extendedData)

/*
To initialize the defaults for each property, we can simply get the property. This should set
the value in the underlying backing store to be the default, as only the backing store knows
Expand Down Expand Up @@ -107,6 +109,25 @@ class Preferences @Inject constructor(
}
}
}

remappedPreferenceKeys
.filter { configuration.containsKey(it.key) && !configuration.containsKey(it.value.name) }
.forEach { (key, property) ->
val configValue = configuration[key]
if (configValue == null) {
resetPreference(key)
} else {
Timber.d("Importing configuration key $key -> $configValue")
try {
importPreference(property, configValue)
} catch (e: java.lang.IllegalArgumentException) {
Timber.w(
"Trying to import wrong type of preference for $key. " +
"Expected ${property.getter.returnType} but given ${configValue.javaClass}. Ignoring."
)
}
}
}
}
importConfigurationIdlingResource.setIdleState(true)
}
Expand Down Expand Up @@ -275,7 +296,7 @@ class Preferences @Inject constructor(
var port: Int by preferencesStore

@Preference
var pubExtendedData: Boolean by preferencesStore
var extendedData: Boolean by preferencesStore

@Preference(exportModeHttp = false)
var pubQos: MqttQos by preferencesStore
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ class LocationProcessor @Inject constructor(
return Result.failure(Exception("message suppressed by monitoring settings: manual"))
}

val message = if (preferences.pubExtendedData) {
val message = if (preferences.extendedData) {
fromLocationAndWifiInfo(location, wifiInfoProvider).apply {
battery = deviceMetricsProvider.batteryLevel
batteryStatus = deviceMetricsProvider.batteryStatus
Expand Down
2 changes: 1 addition & 1 deletion project/app/src/main/res/xml/preferences_reporting.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
app:title="@string/preferencesReporting">
<androidx.preference.SwitchPreferenceCompat
app:iconSpaceReserved="false"
app:key="pubExtendedData"
app:key="extendedData"
app:summary="@string/preferencesPubExtendedDataSummary"
app:title="@string/preferencesPubExtendedData" />
<androidx.preference.SwitchPreferenceCompat
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.mockito.kotlin.mock
import org.owntracks.android.BuildConfig
import org.owntracks.android.model.messages.MessageConfiguration
import org.owntracks.android.preferences.types.ConnectionMode
import org.owntracks.android.preferences.types.MonitoringMode
Expand Down Expand Up @@ -77,7 +76,7 @@ class ImportExportTest {
"password": "testpassword",
"ping": 30,
"port": 1883,
"pubExtendedData": true,
"extendedData": true,
"pubQos": 1,
"pubRetain": true,
"pubTopicBase": "owntracks/%u/%d",
Expand Down Expand Up @@ -118,7 +117,7 @@ class ImportExportTest {
assertEquals("testpassword", password)
assertEquals(30, ping)
assertEquals(1883, port)
assert(pubExtendedData)
assert(extendedData)
assertEquals(MqttQos.ONE, pubQos)
assert(pubRetain)
assertEquals("owntracks/%u/%d", pubTopicBase)
Expand Down Expand Up @@ -159,7 +158,7 @@ class ImportExportTest {
password = "testpassword"
ping = 30
port = 1883
pubExtendedData = true
extendedData = true
pubQos = MqttQos.ONE
pubRetain = true
pubTopicBase = "owntracks/%u/%d"
Expand Down Expand Up @@ -212,7 +211,7 @@ class ImportExportTest {
assertFalse(jsonNode.get("pegLocatorFastestIntervalToInterval").asBoolean())
assertEquals(30, jsonNode.get("ping").asInt())
assertEquals(1883, jsonNode.get("port").asInt())
assertTrue(jsonNode.get("pubExtendedData").asBoolean())
assertTrue(jsonNode.get("extendedData").asBoolean())
assertEquals(1, jsonNode.get("pubQos").asInt())
assertTrue(jsonNode.get("pubRetain").asBoolean())
assertEquals("owntracks/%u/%d", jsonNode.get("pubTopicBase").asText())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ class PreferenceTest {
"opencageApiKey",
"password",
"ping",
"pubExtendedData",
"extendedData",
"cmd",
"remoteConfiguration",
"tid",
Expand All @@ -120,6 +120,18 @@ class PreferenceTest {
"tlsClientCrt"
)

@Test
fun `given an MQTT configuration message containing remapped values, when imported and exported the remapped values are present`() {
val preferences = Preferences(preferencesStore, mockIdlingresource)
val messageConfiguration = MessageConfiguration()
messageConfiguration["pubExtendedData"] = true

preferences.importConfiguration(messageConfiguration)

val exportedMessageConfiguration = preferences.exportToMessage()
assertTrue(exportedMessageConfiguration[Preferences::extendedData.name] as Boolean)
}

@Test
fun `given an MQTT configuration message, when imported and then exported, the config is merged and all the preference keys are present`() {
val preferences = Preferences(preferencesStore, mockIdlingresource)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ class PreferencesGettersAndSetters(private val parameter: Parameter) {
false
),
Parameter(
"pubExtendedData",
"extendedData",
true,
Boolean::class,
false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -777,7 +777,7 @@ class ParserTest {
"password": "testpassword",
"ping": 30,
"port": 1883,
"pubExtendedData": true,
"extendedData": true,
"pubQos": 1,
"pubRetain": true,
"pubTopicBase": "owntracks/%u/%d",
Expand Down

0 comments on commit 56ab9ab

Please sign in to comment.