-
-
Notifications
You must be signed in to change notification settings - Fork 40
Enforce max text length for notes #799
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| package com.philkes.notallyx.data.dao | ||
|
|
||
| import android.content.ContextWrapper | ||
| import androidx.lifecycle.LiveData | ||
| import androidx.lifecycle.map | ||
| import androidx.room.Dao | ||
|
|
@@ -9,6 +10,7 @@ import androidx.room.Query | |
| import androidx.room.RawQuery | ||
| import androidx.room.Update | ||
| import androidx.sqlite.db.SupportSQLiteQuery | ||
| import com.philkes.notallyx.R | ||
| import com.philkes.notallyx.data.model.Audio | ||
| import com.philkes.notallyx.data.model.BaseNote | ||
| import com.philkes.notallyx.data.model.FileAttachment | ||
|
|
@@ -17,6 +19,10 @@ import com.philkes.notallyx.data.model.LabelsInBaseNote | |
| import com.philkes.notallyx.data.model.ListItem | ||
| import com.philkes.notallyx.data.model.Reminder | ||
| import com.philkes.notallyx.data.model.Type | ||
| import com.philkes.notallyx.presentation.showToast | ||
| import com.philkes.notallyx.utils.charLimit | ||
| import com.philkes.notallyx.utils.log | ||
| import kotlin.text.compareTo | ||
|
|
||
| data class NoteIdReminder(val id: Long, val reminders: List<Reminder>) | ||
|
|
||
|
|
@@ -27,13 +33,66 @@ data class NoteReminder( | |
| val reminders: List<Reminder>, | ||
| ) | ||
|
|
||
| /** Maximum allowed size of a note body in MB (~340,000 characters) */ | ||
| const val MAX_BODY_SIZE_MB = 1.5 | ||
|
|
||
| @Dao | ||
| interface BaseNoteDao { | ||
|
|
||
| @RawQuery fun query(query: SupportSQLiteQuery): Int | ||
|
|
||
| @Insert(onConflict = OnConflictStrategy.REPLACE) suspend fun insert(baseNote: BaseNote): Long | ||
|
|
||
| private fun BaseNote.truncated(): Pair<Boolean, BaseNote> { | ||
| return if (body.length > MAX_BODY_CHAR_LENGTH) { | ||
| return Pair(true, copy(body = body.take(MAX_BODY_CHAR_LENGTH))) | ||
| } else Pair(false, this) | ||
| } | ||
|
|
||
| suspend fun insertSafe(context: ContextWrapper, baseNote: BaseNote): Long { | ||
| val (truncated, note) = baseNote.truncated() | ||
| if (truncated) { | ||
| context.log( | ||
| TAG, | ||
| "Note (id: ${note.id}, title: '${note.title}') is too big to insert. Truncating to ${note.body.length} characters (was: ${baseNote.body.length})", | ||
| ) | ||
| context.showToast( | ||
| context.getString( | ||
| R.string.note_too_big_truncating, | ||
| note.body.length, | ||
| baseNote.body.length, | ||
| ) | ||
| ) | ||
| } | ||
| return insert(note) | ||
| } | ||
|
|
||
| suspend fun insertSafe(context: ContextWrapper, baseNotes: List<BaseNote>): List<Long> { | ||
| val truncatedNotes = mutableListOf<BaseNote>() | ||
| var truncatedCharacterSize = 0 | ||
| val notes = | ||
| baseNotes.map { baseNote -> | ||
| val (truncated, note) = baseNote.truncated() | ||
| if (truncated) { | ||
| truncatedCharacterSize += baseNote.body.length | ||
| truncatedNotes.add(note) | ||
| } | ||
| note | ||
| } | ||
| context.log( | ||
| TAG, | ||
| "${truncatedNotes.size} Notes are too big to save, they were truncated to $truncatedCharacterSize characters", | ||
| ) | ||
| context.showToast( | ||
| context.getString( | ||
| R.string.notes_too_big_truncating, | ||
| truncatedNotes.size, | ||
| truncatedCharacterSize, | ||
| ) | ||
| ) | ||
| return insert(notes) | ||
|
Comment on lines
+70
to
+93
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid logging/toasting when nothing was truncated and fix size count. The current implementation logs and shows a toast even when no note was truncated, and the size metric uses the original length rather than the truncated length. 🛠️ Suggested fix val notes =
baseNotes.map { baseNote ->
val (truncated, note) = baseNote.truncated()
if (truncated) {
- truncatedCharacterSize += baseNote.body.length
+ truncatedCharacterSize += note.body.length
truncatedNotes.add(note)
}
note
}
- context.log(
- TAG,
- "${truncatedNotes.size} Notes are too big to save, they were truncated to $truncatedCharacterSize characters",
- )
- context.showToast(
- context.getString(
- R.string.notes_too_big_truncating,
- truncatedNotes.size,
- truncatedCharacterSize,
- )
- )
+ if (truncatedNotes.isNotEmpty()) {
+ context.log(
+ TAG,
+ "${truncatedNotes.size} Notes are too big to save, they were truncated to $truncatedCharacterSize characters",
+ )
+ context.showToast(
+ context.getString(
+ R.string.notes_too_big_truncating,
+ truncatedNotes.size,
+ truncatedCharacterSize,
+ )
+ )
+ }
return insert(notes)🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| @Insert suspend fun insert(baseNotes: List<BaseNote>): List<Long> | ||
|
|
||
| @Update(entity = BaseNote::class) suspend fun update(labelsInBaseNotes: List<LabelsInBaseNote>) | ||
|
|
@@ -135,6 +194,10 @@ interface BaseNoteDao { | |
| spans: List<com.philkes.notallyx.data.model.SpanRepresentation>, | ||
| ) | ||
|
|
||
| // Truncate body at DB level without loading the row, to resolve oversized rows safely | ||
| @Query("UPDATE BaseNote SET body = substr(body, 1, :limit) WHERE id = :id") | ||
| suspend fun truncateBody(id: Long, limit: Int) | ||
|
|
||
| /** | ||
| * Both id and position can be invalid. | ||
| * | ||
|
|
@@ -248,4 +311,10 @@ interface BaseNoteDao { | |
| } | ||
| return false | ||
| } | ||
|
|
||
| companion object { | ||
| private const val TAG = "BaseNoteDao" | ||
|
|
||
| val MAX_BODY_CHAR_LENGTH = MAX_BODY_SIZE_MB.charLimit() | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,20 @@ | ||
| package com.philkes.notallyx.data.dao | ||
|
|
||
| import android.content.ContextWrapper | ||
| import androidx.room.Dao | ||
| import androidx.room.Transaction | ||
| import com.philkes.notallyx.data.NotallyDatabase | ||
| import com.philkes.notallyx.data.dao.BaseNoteDao.Companion.MAX_BODY_CHAR_LENGTH | ||
| import com.philkes.notallyx.data.model.BaseNote | ||
| import com.philkes.notallyx.data.model.Label | ||
| import com.philkes.notallyx.data.model.LabelsInBaseNote | ||
| import com.philkes.notallyx.data.model.SpanRepresentation | ||
| import com.philkes.notallyx.data.model.Type | ||
| import com.philkes.notallyx.data.model.createNoteUrl | ||
| import com.philkes.notallyx.data.model.getNoteIdFromUrl | ||
| import com.philkes.notallyx.data.model.getNoteTypeFromUrl | ||
| import com.philkes.notallyx.data.model.isNoteUrl | ||
| import com.philkes.notallyx.utils.NoteSplitUtils | ||
|
|
||
| @Dao | ||
| abstract class CommonDao(private val database: NotallyDatabase) { | ||
|
|
@@ -40,8 +45,20 @@ abstract class CommonDao(private val database: NotallyDatabase) { | |
| } | ||
|
|
||
| @Transaction | ||
| open suspend fun importBackup(baseNotes: List<BaseNote>, labels: List<Label>) { | ||
| database.getBaseNoteDao().insert(baseNotes) | ||
| open suspend fun importBackup( | ||
| context: ContextWrapper, | ||
| baseNotes: List<BaseNote>, | ||
| labels: List<Label>, | ||
| ) { | ||
| val dao = database.getBaseNoteDao() | ||
| // Insert notes, splitting oversized text notes instead of truncating | ||
| baseNotes.forEach { note -> | ||
| if (note.type == Type.NOTE && note.body.length > MAX_BODY_CHAR_LENGTH) { | ||
| NoteSplitUtils.splitAndInsertForImport(note, dao) | ||
| } else { | ||
| dao.insert(note.copy(id = 0)) | ||
| } | ||
| } | ||
|
Comment on lines
+48
to
+61
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Verify splitAndInsertForImport signature and importBackup usage
echo "=== Checking splitAndInsertForImport signature ==="
rg -n "fun splitAndInsertForImport" --type=kt -A5
echo ""
echo "=== Checking all calls to importBackup ==="
rg -n "importBackup" --type=kt -B2 -A2
echo ""
echo "=== Checking if context is used anywhere else in importBackup method ==="
# Get the full importBackup method to check for context usage
rg -n "open suspend fun importBackup" --type=kt -A20Repository: Crustack/NotallyX Length of output: 327 🏁 Script executed: #!/bin/bash
# Find Kotlin files and search for the relevant signatures
echo "=== Finding Kotlin files ==="
fd -e kt | head -20
echo ""
echo "=== Checking splitAndInsertForImport signature ==="
rg "fun splitAndInsertForImport" -A5
echo ""
echo "=== Checking all calls to importBackup ==="
rg "importBackup" -B2 -A2
echo ""
echo "=== Checking the full importBackup method signature ==="
rg "open suspend fun importBackup" -A30Repository: Crustack/NotallyX Length of output: 50373 Remove unused The 🤖 Prompt for AI Agents |
||
| database.getLabelDao().insert(labels) | ||
| } | ||
|
|
||
|
|
@@ -57,21 +74,31 @@ abstract class CommonDao(private val database: NotallyDatabase) { | |
| labels: List<Label>, | ||
| ) { | ||
| val baseNoteDao = database.getBaseNoteDao() | ||
| val newIds = baseNoteDao.insert(baseNotes) | ||
| // Build old->new mapping using positional correspondence | ||
|
|
||
| // 1) Insert notes with splitting; build mapping from original id -> first-part new id | ||
| val idMap = HashMap<Long, Long>(originalIds.size) | ||
| val count = minOf(originalIds.size, newIds.size) | ||
| for (i in 0 until count) { | ||
| idMap[originalIds[i]] = newIds[i] | ||
| } | ||
| // Keep all inserted note ids with their spans for remapping pass | ||
| val insertedParts = ArrayList<Pair<Long, List<SpanRepresentation>>>() | ||
|
|
||
| // Remap note links in spans where necessary | ||
| for (i in baseNotes.indices) { | ||
| val note = baseNotes[i] | ||
| val newId = newIds.getOrNull(i) ?: continue | ||
| val original = baseNotes[i] | ||
| val (firstId, parts) = | ||
| if (original.type == Type.NOTE && original.body.length > MAX_BODY_CHAR_LENGTH) { | ||
| NoteSplitUtils.splitAndInsertForImport(original, baseNoteDao) | ||
| } else { | ||
| val newId = baseNoteDao.insert(original.copy(id = 0)) | ||
| Pair(newId, listOf(Pair(newId, original.spans))) | ||
| } | ||
| val oldId = originalIds.getOrNull(i) | ||
| if (oldId != null) idMap[oldId] = firstId | ||
| insertedParts.addAll(parts) | ||
| } | ||
|
|
||
| // 2) Remap note links in spans for all inserted notes | ||
| for ((noteId, spans) in insertedParts) { | ||
| var changed = false | ||
| val updatedSpans = | ||
| note.spans.map { span -> | ||
| val updated = | ||
| spans.map { span -> | ||
| if (span.link && span.linkData?.isNoteUrl() == true) { | ||
| val url = span.linkData!! | ||
| val oldTargetId = url.getNoteIdFromUrl() | ||
|
|
@@ -80,13 +107,11 @@ abstract class CommonDao(private val database: NotallyDatabase) { | |
| if (newTargetId != null) { | ||
| changed = true | ||
| span.copy(linkData = newTargetId.createNoteUrl(type)) | ||
| } else { | ||
| span | ||
| } | ||
| } else span | ||
| } else span | ||
| } | ||
| if (changed) { | ||
| baseNoteDao.updateSpans(newId, updatedSpans) | ||
| baseNoteDao.updateSpans(noteId, updated) | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clamp spans when truncating bodies.
Body truncation without span clipping can leave out-of-bounds spans and corrupt rendering.
🛠️ Suggested fix
📝 Committable suggestion
🤖 Prompt for AI Agents