Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mage #1326, Mage #1252, and Mage #1356 #72

Merged
merged 4 commits into from
Feb 10, 2025
Merged

Mage #1326, Mage #1252, and Mage #1356 #72

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0c5ceda
Code Submitted Covers Mage #1326, Mage #1252, and Mage #1356
RyCarpenter Jan 30, 2025
6717e35
Add Comment for Clarity, revert min version
RyCarpenter Jan 30, 2025
d67baf3
Fix Error Created during Merge conflict resolution
RyCarpenter Jan 30, 2025
21b1aec
add tools-versions file for java open jdk
Feb 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .tool-versions
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
java openjdk-23
4 changes: 2 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,14 +124,14 @@ file ./mage/build/outputs/apk/mage-android-defaults-debug.apk

With a device connected to your machine, you can install the MAGE app with the following command:
```bash
./gradlew installDefaultsDebug
./gradlew installDebug
```

### Test

Run the tests on your device:
```bash
./gradlew connectedDefaultsDebugAndroidTest
./gradlew connectedDebugAndroidTest
```

## Pull Requests
Expand Down
1 change: 0 additions & 1 deletion gradle.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,5 @@ org.gradle.jvmargs=-Xmx4096m

# Application properties
VERSION_CODE=1
android.defaults.buildfeatures.buildconfig=true
android.nonTransitiveRClass=false
android.nonFinalResIds=false
2 changes: 1 addition & 1 deletion gradle/wrapper/gradle-wrapper.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#Thu May 06 13:38:17 MDT 2021
distributionBase=GRADLE_USER_HOME
distributionUrl=https\://services.gradle.org/distributions/gradle-8.9-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-8.10.2-all.zip
distributionPath=wrapper/dists
zipStorePath=wrapper/dists
zipStoreBase=GRADLE_USER_HOME
18 changes: 14 additions & 4 deletions mage/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ import org.ajoberstar.grgit.Grgit

plugins {
id 'com.android.application'
id 'org.jetbrains.kotlin.android'
id 'kotlin-android'
id 'kotlin-parcelize'
id 'kotlin-kapt'
id 'org.ajoberstar.grgit' version '4.1.0'
id 'org.ajoberstar.grgit' version '5.3.0'
id 'com.google.dagger.hilt.android'
}

Expand Down Expand Up @@ -33,7 +33,13 @@ def googleMapsApiReleaseKey = hasProperty('RELEASE_MAPS_API_KEY') ? RELEASE_MAPS
def googleMapsApiDebugKey = hasProperty('DEBUG_MAPS_API_KEY') ? DEBUG_MAPS_API_KEY : ''

android {
compileSdk 34
defaultConfig {
compileSdk 34
}

buildFeatures{
buildConfig = true
}
namespace 'mil.nga.giat.mage'

dataBinding {
Expand All @@ -57,6 +63,10 @@ android {
kotlinOptions {
jvmTarget = "21"
freeCompilerArgs = ["-Xcontext-receivers"]
freeCompilerArgs += [
"-P",
"plugin:androidx.compose.compiler.plugins.kotlin:suppressKotlinVersionCompatibilityCheck=1.9.25"
]
}

buildFeatures {
Expand All @@ -83,7 +93,7 @@ android {
versionName project.version
versionCode VERSION_CODE as int
minSdkVersion 27
targetSdkVersion 33
targetSdkVersion 34
multiDexEnabled true
resValue "string", "serverURLDefaultValue", serverURL
resValue "string", "recentMapXYZDefaultValue", "263.0,40.0,3"
Expand Down
1 change: 1 addition & 0 deletions mage/src/main/AndroidManifest.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
<uses-permission android:name="android.permission.RECORD_AUDIO" />
<uses-permission android:name="android.permission.VIBRATE" />
<uses-permission android:name="android.permission.FOREGROUND_SERVICE" />
<uses-permission android:name="android.permission.FOREGROUND_SERVICE_LOCATION" />
<uses-permission android:name="com.google.android.providers.gsf.permission.READ_GSERVICES" />

<uses-feature
Expand Down
31 changes: 18 additions & 13 deletions mage/src/main/java/mil/nga/giat/mage/data/repository/user/UserRepository.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,17 @@ import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext
import mil.nga.giat.mage.R
import mil.nga.giat.mage.data.datasource.permission.RoleLocalDataSource
import mil.nga.giat.mage.data.datasource.user.UserLocalDataSource
import mil.nga.giat.mage.database.dao.MageSqliteOpenHelper
import mil.nga.giat.mage.database.model.event.Event
import mil.nga.giat.mage.database.model.user.User
import mil.nga.giat.mage.di.TokenProvider
import mil.nga.giat.mage.login.AuthenticationStatus
import mil.nga.giat.mage.login.AuthorizationStatus
import mil.nga.giat.mage.network.device.DeviceService
import mil.nga.giat.mage.network.user.UserService
import mil.nga.giat.mage.sdk.Compatibility.Companion.isCompatibleWith
import mil.nga.giat.mage.database.model.event.Event
import mil.nga.giat.mage.database.model.user.User
import mil.nga.giat.mage.data.datasource.user.UserLocalDataSource
import mil.nga.giat.mage.database.dao.MageSqliteOpenHelper
import mil.nga.giat.mage.network.user.UserWithRoleTypeAdapter
import mil.nga.giat.mage.sdk.Compatibility.Companion.isCompatibleWith
import mil.nga.giat.mage.sdk.preferences.PreferenceHelper
import mil.nga.giat.mage.sdk.utils.DeviceUuidFactory
import mil.nga.giat.mage.sdk.utils.ISO8601DateFormatFactory
Expand Down Expand Up @@ -308,15 +308,20 @@ class UserRepository @Inject constructor(
private suspend fun syncIcon(user: User) {
val path = "${MediaUtility.getUserIconDirectory(application)}/${user.id}.png"
val file = File(path)
if (file.exists()) {
/// At the moment, I can't see any code that regularly updates User.lastModified
Copy link
Member

Choose a reason for hiding this comment

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

For reference, lastUpdated is tracked by the server and updated when the application fetches a user.

https://github.com/ngageoint/mage-android/blob/master/mage/src/main/java/mil/nga/giat/mage/network/user/UserWithRoleTypeAdapter.kt#L52-L55
and
https://github.com/ngageoint/mage-android/blob/master/mage/src/main/java/mil/nga/giat/mage/network/user/UserWithRoleIdTypeAdapter.kt#L48-L51

Users should only be fetched when selecting an event. This flow happens after login when selecting an event or after changing events.

Copy link
Author

Choose a reason for hiding this comment

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

Stepping through the flow, users are indeed fetched when an event is selected, but afterwards it does an additional fetch for the icons. This change is fundamentally correct for only fetching icons that are needed. That said, there is an inherent inefficiency in having users pulled every load despite having a local storage for this data, it really should be checking lastModified to see if a user is even needed to be pulled, assuming it already exists locally. The exact letter of the ticket I was given was simply to stop icons from being loaded every time, but there is a further updated needed to fix this over-pulling of information.

Copy link
Member

Choose a reason for hiding this comment

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

My comment wasn’t meant to suggest that your change was fundamentally wrong. Rather, it was intended to provide some context, if helpful, regarding the added comments.

For example, you mention:

the fact that we happen to pull users in their entirety every time we do something...

Users should currently be pulled when an event is selected, either at login or when changing events after login. Is that not the case? This would be drastically different from pulling them every time we do something.

The client has no way of determining if a user's remote properties have changed without retrieving that information. While we do have local storage for the user, it could be stale.

/// Aside from the fact that we happen to pull users in their entirety every time we
/// do something... That should be fixed and then this code will be absolutely correct.
/// Until then, this code is achieving the result of not re-pulling an icon.
if (file.exists() && user.fetchedDate.before(user.lastModified)) {
file.delete()
}

val response = userService.getIcon(user.remoteId)
val body = response.body()
if (response.isSuccessful && body != null) {
saveIcon(body, file)
compressIcon(file)
val response = userService.getIcon(user.remoteId)
val body = response.body()
if (response.isSuccessful && body != null) {
saveIcon(body, file)
compressIcon(file)
userLocalDataSource.setIconPath(user, path)
}
} else {
userLocalDataSource.setIconPath(user, path)
}
}
Expand Down
12 changes: 10 additions & 2 deletions mage/src/main/java/mil/nga/giat/mage/login/ServerUrlViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,22 @@ class ServerUrlViewModel @Inject constructor(
if (Patterns.WEB_URL.matcher(url).matches()) {
_urlState.value = UrlState.InProgress

var processedUrl:String = url
if (!processedUrl.contains("://")) {
processedUrl = "https://" + url
}
if (processedUrl.endsWith("/")) {
processedUrl = processedUrl.dropLast(1)
}

viewModelScope.launch(Dispatchers.IO) {
when (val response = apiRepository.getApi(url)) {
when (val response = apiRepository.getApi(processedUrl)) {
is ApiResponse.Success -> {
daoStore.resetDatabase()
database.destroy()
preferences
.edit()
.putString(application.getString(R.string.serverURLKey), url)
.putString(application.getString(R.string.serverURLKey), processedUrl)
.apply()

_urlState.postValue(UrlState.Valid)
Expand Down
2 changes: 1 addition & 1 deletion mage/tests/AndroidManifest.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

<uses-sdk
android:minSdkVersion="16"
android:targetSdkVersion="19" />
android:targetSdkVersion="34" />

<instrumentation
android:label="AndroidJUnitRunner"
Expand Down
4 changes: 4 additions & 0 deletions settings.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ pluginManagement {
}
}

plugins {
id 'org.gradle.toolchains.foojay-resolver-convention' version '0.8.0'
}

dependencyResolutionManagement {
repositoriesMode.set(RepositoriesMode.PREFER_SETTINGS)
repositories {
Expand Down