Skip to content

Commit

Permalink
Perform non-critical checks only in debug mode
Browse files Browse the repository at this point in the history
- Fix missing edge case checks if reminder is changed after notification appears, then note is postponed.
  • Loading branch information
maltaisn committed Sep 5, 2021
1 parent 15ed58b commit 1dd8f06
Show file tree
Hide file tree
Showing 11 changed files with 73 additions and 24 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
- Fixed simultaneous notifications all opening the same note (#43).
- Fixed notification click not working if already editing a note.
- Fixed notification creating new note if clicked after note is deleted.
- Fixed postpone check failing if note reminder is changed before postponing.
- Remove check for internal list note consistency between content and metadata.

## v1.4.0
Expand Down
28 changes: 28 additions & 0 deletions app/src/debug/kotlin/com/maltaisn/notes/DebugExtensions.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright 2021 Nicolas Maltais
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/


/**
* For checks only performed in debug mode.
* A failing check should be handled correctly in release mode.
*/
inline fun debugCheck(value: Boolean, message: () -> String = { "Check failed" }) {
check(value, message)
}

inline fun debugRequire(value: Boolean, message: () -> String = { "Failed requirement" }) {
require(value, message)
}
12 changes: 7 additions & 5 deletions app/src/main/kotlin/com/maltaisn/notes/model/entity/Note.kt
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import androidx.room.Junction
import androidx.room.PrimaryKey
import androidx.room.Relation
import com.maltaisn.notes.model.converter.NoteMetadataConverter
import debugCheck
import debugRequire
import kotlinx.serialization.Transient
import java.util.Date

Expand Down Expand Up @@ -104,18 +106,18 @@ data class Note(
NoteType.LIST -> metadata is ListNoteMetadata
})

require(addedDate.time <= lastModifiedDate.time) {
debugRequire(addedDate.time <= lastModifiedDate.time) {
"Note added date must be before or on last modified date."
}

require(status != NoteStatus.ACTIVE || pinned != PinnedStatus.CANT_PIN) {
debugRequire(status != NoteStatus.ACTIVE || pinned != PinnedStatus.CANT_PIN) {
"Active note must be pinnable."
}
require(status == NoteStatus.ACTIVE || pinned == PinnedStatus.CANT_PIN) {
debugRequire(status == NoteStatus.ACTIVE || pinned == PinnedStatus.CANT_PIN) {
"Archived or deleted note must not be pinnable."
}

require(status != NoteStatus.DELETED || reminder == null) {
debugRequire(status != NoteStatus.DELETED || reminder == null) {
"Deleted note cannot have a reminder."
}
}
Expand All @@ -142,7 +144,7 @@ data class Note(
return mutableListOf()
}

// check(checked.size == items.size) { "Invalid list note data." }
debugCheck(checked.size == items.size) { "Invalid list note data." }

return items.mapIndexedTo(mutableListOf()) { i, text ->
ListNoteItem(text.trim(), checked.getOrElse(i) { false })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ class AlarmReceiver : BroadcastReceiver() {
val postponeIntent = Intent(context, NotificationActivity::class.java).apply {
action = NotificationActivity.INTENT_ACTION_POSTPONE
putExtra(EXTRA_NOTE_ID, noteId)
setFlags(Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TASK)
flags = Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TASK
}
builder.addAction(R.drawable.ic_calendar,
context.getString(R.string.action_postpone),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import com.maltaisn.notes.ui.note.adapter.NoteListItem
import com.maltaisn.notes.ui.send
import com.squareup.inject.assisted.Assisted
import com.squareup.inject.assisted.AssistedInject
import debugCheck
import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.launch
import java.util.Calendar
Expand Down Expand Up @@ -185,7 +186,7 @@ class HomeViewModel @AssistedInject constructor(
} else {
// If one note is active, consider all active.
// Otherwise consider archived.
check(selectedNotes.none { it.status == NoteStatus.DELETED })
debugCheck(selectedNotes.none { it.status == NoteStatus.DELETED })
when {
selectedNotes.isEmpty() -> null
selectedNotes.any { it.status == NoteStatus.ACTIVE } -> NoteStatus.ACTIVE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import com.maltaisn.notes.sync.R
import com.maltaisn.notes.sync.databinding.DialogLabelEditBinding
import com.maltaisn.notes.ui.observeEvent
import com.maltaisn.notes.ui.viewModel
import debugCheck
import javax.inject.Inject

class LabelEditDialog : DialogFragment() {
Expand All @@ -51,7 +52,7 @@ class LabelEditDialog : DialogFragment() {
val binding = DialogLabelEditBinding.inflate(LayoutInflater.from(context), null, false)

// Using `this` as lifecycle owner, cannot show dialog twice with same instance to avoid double observation.
check(!viewModel.setLabelEvent.hasObservers()) { "Dialog was shown twice with same instance." }
debugCheck(!viewModel.setLabelEvent.hasObservers()) { "Dialog was shown twice with same instance." }

val nameInput = binding.labelInput
val nameInputLayout = binding.labelInputLayout
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,10 @@ class NotificationViewModel @AssistedInject constructor(
viewModelScope.launch {
val note = notesRepository.getNoteById(noteId)
val reminder = note?.reminder
if (note != null && reminder != null) {
// There's hardly a way note or reminder can be null at this point, but
// let's assume it's possible and do nothing in that case.
if (note != null && reminder != null && reminder.recurrence == null
&& !reminder.done && calendar.timeInMillis > reminder.next.time) {
// Reminder can be null or be recurring if user changed it between the
// notification and the postpone action.
val newNote = note.copy(reminder = reminder.postponeTo(calendar.time))
notesRepository.updateNote(newNote)
reminderAlarmManager.setNoteReminderAlarm(newNote)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import com.maltaisn.recurpicker.list.RecurrenceListCallback
import com.maltaisn.recurpicker.list.RecurrenceListDialog
import com.maltaisn.recurpicker.picker.RecurrencePickerCallback
import com.maltaisn.recurpicker.picker.RecurrencePickerDialog
import debugCheck
import java.text.DateFormat
import javax.inject.Inject
import javax.inject.Provider
Expand Down Expand Up @@ -103,7 +104,7 @@ class ReminderDialog : DialogFragment(), RecurrenceListCallback, RecurrencePicke

private fun setupViewModelObservers(binding: DialogReminderBinding) {
// Using `this` as lifecycle owner, cannot show dialog twice with same instance to avoid double observation.
check(!viewModel.details.hasObservers()) { "Dialog was shown twice with same instance." }
debugCheck(!viewModel.details.hasObservers()) { "Dialog was shown twice with same instance." }

viewModel.details.observe(this) { details ->
binding.dateInput.setText(dateFormat.format(details.date))
Expand Down
3 changes: 2 additions & 1 deletion app/src/main/kotlin/com/maltaisn/notes/ui/sort/SortDialog.kt
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import com.maltaisn.notes.ui.SharedViewModel
import com.maltaisn.notes.ui.navGraphViewModel
import com.maltaisn.notes.ui.observeEvent
import com.maltaisn.notes.ui.viewModel
import debugCheck
import javax.inject.Inject
import javax.inject.Provider

Expand Down Expand Up @@ -85,7 +86,7 @@ class SortDialog : DialogFragment() {

private fun setupViewModelObservers(binding: DialogSortBinding) {
// Using `this` as lifecycle owner, cannot show dialog twice with same instance to avoid double observation.
check(!viewModel.sortField.hasObservers()) { "Dialog was shown twice with same instance." }
debugCheck(!viewModel.sortField.hasObservers()) { "Dialog was shown twice with same instance." }

viewModel.sortField.observeEvent(this) { field ->
when (field) {
Expand Down
24 changes: 24 additions & 0 deletions app/src/release/kotlin/com/maltaisn/notes/DebugExtensions.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
* Copyright 2021 Nicolas Maltais
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/


/**
* For checks only performed in debug mode.
* A failing check should be handled correctly in release mode.
*/
fun debugCheck(value: Boolean, message: () -> String = { "" }) = Unit

fun debugRequire(value: Boolean, message: () -> String = { "" }) = Unit
11 changes: 0 additions & 11 deletions app/src/test/kotlin/com/maltaisn/notes/model/entity/NoteTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,6 @@ class NoteTest {
}
}

@Test
fun `should handle mismatch in list note metadata and content`() {
val items = listOf(
ListNoteItem("0", false),
ListNoteItem("1", true),
ListNoteItem("2", true))
var note = listNote(items)
note = note.copy(content = note.content + "\n3")
assertEquals(items + ListNoteItem("3", false), note.listItems)
}

@Test
fun `should fail to create deleted note with reminder`() {
assertFailsWith<IllegalArgumentException> {
Expand Down

0 comments on commit 1dd8f06

Please sign in to comment.