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

Add validation to check document ids before api calling #164

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+49 −46
Open

Add validation to check document ids before api calling #164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
909e7f8
Add validation for empty ids before api calling
cp-sneh-s Jan 10, 2025
5584ffb
Revert "Add validation for empty ids before api calling"
cp-sneh-s Jan 10, 2025
a8681e8
Minor fixes
cp-sneh-s Jan 10, 2025
5d47ee4
minor changes.
cp-sneh-s Jan 16, 2025
5194516
Merge branch 'main' into sneh/refector-empty-document-id-checks-befor…
cp-sneh-s Jan 16, 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
2 changes: 1 addition & 1 deletion app/src/main/java/com/canopas/yourspace/ui/flow/settings/SettingsViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class SettingsViewModel @Inject constructor(
}

private fun getUser() = viewModelScope.launch(appDispatcher.IO) {
authService.getUserFlow().collectLatest { user ->
authService.getUserFlow()?.collectLatest { user ->
_state.emit(_state.value.copy(user = user))
}
}
Expand Down
54 changes: 30 additions & 24 deletions data/src/main/java/com/canopas/yourspace/data/repository/SpaceRepository.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ class SpaceRepository @Inject constructor(
val spaceId = currentSpaceId

if (spaceId.isEmpty()) {
val userId = authService.currentUser?.id ?: ""
return getUserSpaces(userId).firstOrNull()?.sortedBy { it.created_at }?.firstOrNull()
val userId = authService.currentUser?.id
return userId?.let { getUserSpaces(it).firstOrNull()?.sortedBy { it.created_at }?.firstOrNull() }
}
return getSpace(spaceId)
}
Expand Down Expand Up @@ -185,30 +185,26 @@ class SpaceRepository @Inject constructor(
}

suspend fun deleteUserSpaces() {
val userId = authService.currentUser?.id ?: ""
val allSpace = getUserSpaces(userId).firstOrNull() ?: emptyList()
val userId = authService.currentUser?.id
val allSpace = userId?.let { getUserSpaces(it).firstOrNull() } ?: emptyList()
val ownSpace = allSpace.filter { it.admin_id == userId }
val joinedSpace = allSpace.filter { it.admin_id != userId }

ownSpace.forEach { space ->
deleteSpace(space.id)
}

joinedSpace.forEach { space ->
spaceService.removeUserFromSpace(space.id, userId)
if (userId != null) {
joinedSpace.forEach { space ->
spaceService.removeUserFromSpace(space.id, userId)
}
}
}

suspend fun deleteSpace(spaceId: String) {
invitationService.deleteInvitations(spaceId)
spaceService.deleteSpace(spaceId)
val userId = authService.currentUser?.id ?: ""
currentSpaceId =
getUserSpaces(userId).firstOrNull()?.sortedBy { it.created_at }?.firstOrNull()?.id
?: ""
private suspend fun updateUserSpaceId(userId: String, spaceId: String) {
val user = userService.getUser(userId) ?: return

val user = userService.getUser(userId)
val updatedSpaceIds = user?.space_ids?.toMutableList()?.apply {
val updatedSpaceIds = user.space_ids?.toMutableList()?.apply {
remove(spaceId)
} ?: return

Expand All @@ -217,16 +213,26 @@ class SpaceRepository @Inject constructor(
}
}

suspend fun leaveSpace(spaceId: String) {
val userId = authService.currentUser?.id ?: ""
spaceService.removeUserFromSpace(spaceId, userId)
val user = userService.getUser(userId)
val updatedSpaceIds = user?.space_ids?.toMutableList()?.apply {
remove(spaceId)
} ?: return
suspend fun deleteSpace(spaceId: String) {
invitationService.deleteInvitations(spaceId)
spaceService.deleteSpace(spaceId)
val userId = authService.currentUser?.id
currentSpaceId =
userId?.let { getUserSpaces(it).firstOrNull()?.sortedBy { it.created_at }?.firstOrNull()?.id }
?: ""

user.copy(space_ids = updatedSpaceIds).let {
userService.updateUser(it)
if (userId != null) {
updateUserSpaceId(userId, spaceId)
}
}

suspend fun leaveSpace(spaceId: String) {
val userId = authService.currentUser?.id
if (userId != null) {
spaceService.removeUserFromSpace(spaceId, userId)
}
if (userId != null) {
updateUserSpaceId(userId, spaceId)
}
Comment on lines +229 to 236
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Remove redundant null checks

The method contains duplicate null checks for userId.

Simplify the logic:

  suspend fun leaveSpace(spaceId: String) {
      val userId = authService.currentUser?.id
-     if (userId != null) {
-         spaceService.removeUserFromSpace(spaceId, userId)
-     }
-     if (userId != null) {
-         updateUserSpaceId(userId, spaceId)
-     }
+     userId?.let { uid ->
+         spaceService.removeUserFromSpace(spaceId, uid)
+         updateUserSpaceId(uid, spaceId)
+     }
  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
suspend fun leaveSpace(spaceId: String) {
val userId = authService.currentUser?.id
if (userId != null) {
spaceService.removeUserFromSpace(spaceId, userId)
}
if (userId != null) {
updateUserSpaceId(userId, spaceId)
}
suspend fun leaveSpace(spaceId: String) {
val userId = authService.currentUser?.id
userId?.let { uid ->
spaceService.removeUserFromSpace(spaceId, uid)
updateUserSpaceId(uid, spaceId)
}

}

Expand Down
22 changes: 12 additions & 10 deletions data/src/main/java/com/canopas/yourspace/data/service/auth/AuthService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,33 +120,33 @@ class AuthService @Inject constructor(
}

suspend fun deleteAccount() {
val currentUser = currentUser ?: return
apiUserService.deleteUser(currentUser.id)
val currentUser = currentUser
currentUser?.let { apiUserService.deleteUser(it.id) }
cp-sneh-s marked this conversation as resolved.
Show resolved Hide resolved
signOut()
}

suspend fun generateAndSaveUserKeys(passKey: String) {
val user = currentUser ?: throw IllegalStateException("No user logged in")
val user = currentUser
cp-sneh-s marked this conversation as resolved.
Show resolved Hide resolved
try {
val updatedUser = apiUserService.generateAndSaveUserKeys(user, passKey)
val updatedUser = user?.let { apiUserService.generateAndSaveUserKeys(it, passKey) }
currentUser = updatedUser
} catch (e: Exception) {
throw SecurityException("Failed to generate user keys", e)
}
}

suspend fun validatePasskey(passKey: String): Boolean {
val user = currentUser ?: return false
val validationResult = apiUserService.validatePasskey(user, passKey)
val user = currentUser
val validationResult = user?.let { apiUserService.validatePasskey(it, passKey) }
cp-sneh-s marked this conversation as resolved.
Show resolved Hide resolved
if (validationResult != null) {
userPreferences.storePasskey(passKey)
userPreferences.storePrivateKey(validationResult)
}
return validationResult != null
}

suspend fun getUser(): ApiUser? = apiUserService.getUser(currentUser?.id ?: "")
suspend fun getUserFlow() = apiUserService.getUserFlow(currentUser?.id ?: "")
suspend fun getUser(): ApiUser? = currentUser?.id?.let { apiUserService.getUser(it) }
suspend fun getUserFlow() = currentUser?.id?.let { apiUserService.getUserFlow(it) }
cp-sneh-s marked this conversation as resolved.
Show resolved Hide resolved

suspend fun updateBatteryStatus(batteryPercentage: Float) {
val user = currentUser ?: return
Expand All @@ -157,8 +157,10 @@ class AuthService @Inject constructor(
}

suspend fun updateUserSessionState(state: Int) {
val currentUser = currentUser ?: return
apiUserService.updateSessionState(currentUser.id, state)
val currentUser = currentUser
if (currentUser != null) {
apiUserService.updateSessionState(currentUser.id, state)
}
cp-sneh-s marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
9 changes: 3 additions & 6 deletions data/src/main/java/com/canopas/yourspace/data/service/location/ApiJourneyService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,13 @@ class ApiJourneyService @Inject constructor(
get() = userPreferences.currentSpace ?: ""

private fun spaceMemberRef(spaceId: String) =
spaceRef.document(spaceId.takeIf { it.isNotBlank() } ?: "null")
.collection(FIRESTORE_COLLECTION_SPACE_MEMBERS)
spaceRef.document(spaceId).collection(FIRESTORE_COLLECTION_SPACE_MEMBERS)

private fun spaceMemberJourneyRef(spaceId: String, userId: String) =
spaceMemberRef(spaceId).document(userId.takeIf { it.isNotBlank() } ?: "null")
.collection(Config.FIRESTORE_COLLECTION_USER_JOURNEYS)
spaceMemberRef(spaceId).document(userId).collection(Config.FIRESTORE_COLLECTION_USER_JOURNEYS)

private fun spaceGroupKeysRef(spaceId: String) =
spaceRef.document(spaceId.takeIf { it.isNotBlank() } ?: "null")
.collection(FIRESTORE_COLLECTION_SPACE_GROUP_KEYS)
spaceRef.document(spaceId).collection(FIRESTORE_COLLECTION_SPACE_GROUP_KEYS)
.document(FIRESTORE_COLLECTION_SPACE_GROUP_KEYS)

private suspend fun getGroupKeyDoc(spaceId: String): GroupKeysDoc? {
Expand Down
8 changes: 3 additions & 5 deletions data/src/main/java/com/canopas/yourspace/data/service/location/ApiLocationService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,14 @@ class ApiLocationService @Inject constructor(
private val spaceRef by lazy { db.collection(FIRESTORE_COLLECTION_SPACES) }

private fun spaceMemberRef(spaceId: String) =
spaceRef.document(spaceId.takeIf { it.isNotBlank() } ?: "null")
.collection(FIRESTORE_COLLECTION_SPACE_MEMBERS)
spaceRef.document(spaceId).collection(FIRESTORE_COLLECTION_SPACE_MEMBERS)
cp-sneh-s marked this conversation as resolved.
Show resolved Hide resolved

private fun spaceMemberLocationRef(spaceId: String, userId: String) =
spaceMemberRef(spaceId.takeIf { it.isNotBlank() } ?: "null").document(userId)
spaceMemberRef(spaceId).document(userId)
.collection(Config.FIRESTORE_COLLECTION_USER_LOCATIONS)

private fun spaceGroupKeysRef(spaceId: String) =
spaceRef.document(spaceId.takeIf { it.isNotBlank() } ?: "null")
.collection(FIRESTORE_COLLECTION_SPACE_GROUP_KEYS)
spaceRef.document(spaceId).collection(FIRESTORE_COLLECTION_SPACE_GROUP_KEYS)
.document(FIRESTORE_COLLECTION_SPACE_GROUP_KEYS)

suspend fun saveLastKnownLocation(userId: String) {
Expand Down
Loading