Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import com.instructure.horizon.database.dao.HorizonEntitySyncMetadataDao
import com.instructure.horizon.database.dao.HorizonCourseSyncPlanDao
import com.instructure.horizon.database.dao.HorizonFileSyncPlanDao
import com.instructure.horizon.database.dao.HorizonLocalImageDao
import com.instructure.horizon.database.dao.HorizonNoteDao
import com.instructure.horizon.database.dao.HorizonSubmissionDao
import com.instructure.horizon.database.dao.HorizonSyncMetadataDao
import com.instructure.horizon.database.dao.HorizonSyncSettingsDao
Expand Down Expand Up @@ -130,6 +131,9 @@ object HorizonOfflineTestModule {
@Provides
fun provideHorizonUserDao(db: HorizonDatabase): HorizonUserDao = db.userDao()

@Provides
fun provideHorizonNoteDao(db: HorizonDatabase): HorizonNoteDao = db.noteDao()

@Provides
@HorizonHtmlParserQualifier
fun provideHorizonHtmlParser(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
/*
* Copyright (C) 2026 - present Instructure, Inc.
*
* 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.
*/
package com.instructure.horizon.data.datasource

import com.google.gson.Gson
import com.instructure.canvasapi2.managers.graphql.horizon.CourseWithProgress
import com.instructure.canvasapi2.managers.graphql.horizon.redwood.NoteHighlightedData
import com.instructure.canvasapi2.managers.graphql.horizon.redwood.NoteHighlightedDataRange
import com.instructure.canvasapi2.managers.graphql.horizon.redwood.NoteHighlightedDataTextPosition
import com.instructure.canvasapi2.managers.graphql.horizon.redwood.NoteObjectType
import com.instructure.canvasapi2.managers.graphql.horizon.redwood.NoteReaction
import com.instructure.horizon.database.dao.HorizonDashboardEnrollmentDao
import com.instructure.horizon.database.dao.HorizonNoteDao
import com.instructure.horizon.database.dao.HorizonSyncMetadataDao
import com.instructure.horizon.database.entity.HorizonDashboardEnrollmentEntity
import com.instructure.horizon.database.entity.HorizonNoteEntity
import com.instructure.horizon.database.entity.HorizonSyncMetadataEntity
import com.instructure.horizon.database.entity.SyncDataType
import com.instructure.horizon.features.notebook.common.model.Note
import com.instructure.horizon.features.notebook.common.model.NotebookType
import com.instructure.pandautils.utils.toJson
import com.instructure.redwood.QueryNotesQuery
import com.instructure.redwood.type.OrderDirection
import java.util.Date
import javax.inject.Inject

data class LocalNotesPage(
val notes: List<Note>,
val hasNextPage: Boolean,
val nextOffset: Int,
)

class NotebookLocalDataSource @Inject constructor(
private val noteDao: HorizonNoteDao,
private val dashboardEnrollmentDao: HorizonDashboardEnrollmentDao,
private val syncMetadataDao: HorizonSyncMetadataDao,
) {
suspend fun replaceNotesForCourse(courseId: Long, notes: List<HorizonNoteEntity>) {
noteDao.replaceForCourse(courseId, notes)
syncMetadataDao.upsert(
HorizonSyncMetadataEntity(
dataType = SyncDataType.NOTES,
lastSyncedAtMs = System.currentTimeMillis(),
)
)
}

suspend fun upsertNotes(notes: List<HorizonNoteEntity>) {
if (notes.isNotEmpty()) noteDao.upsertAll(notes)
}

suspend fun deleteNote(noteId: String) {
noteDao.deleteById(noteId)
}

suspend fun deleteNotesByCourseId(courseId: Long) {
noteDao.deleteByCourseId(courseId)
}

suspend fun getNotes(
courseId: Long?,
filterType: NotebookType?,
objectTypeAndId: Pair<String, String>?,
orderDirection: OrderDirection?,
offset: Int,
limit: Int,
): LocalNotesPage {
val ascending = orderDirection == OrderDirection.ascending
val objectType = objectTypeAndId?.first
val objectId = objectTypeAndId?.second
val reaction = filterType?.name

val rows = noteDao.query(
courseId = courseId,
objectType = objectType,
objectId = objectId,
reaction = reaction,
ascending = ascending,
limit = limit,
offset = offset,
)
val total = noteDao.count(
courseId = courseId,
objectType = objectType,
objectId = objectId,
reaction = reaction,
)
val nextOffset = offset + rows.size
return LocalNotesPage(
notes = rows.map { it.toNote() },
hasNextPage = nextOffset < total,
nextOffset = nextOffset,
)
}

suspend fun getCourses(): List<CourseWithProgress> {
return dashboardEnrollmentDao.getAll().map { it.toCourseWithProgress() }
}

private fun HorizonNoteEntity.toNote(): Note = Note(
id = id,
courseId = courseId,
objectId = objectId,
objectType = NoteObjectType.fromValue(objectType) ?: NoteObjectType.PAGE,
userText = userText,
highlightedText = parseHighlightedData(highlightedDataJson),
type = runCatching { NotebookType.valueOf(reaction) }.getOrDefault(NotebookType.Important),
updatedAt = Date(updatedAt),
)

private fun HorizonDashboardEnrollmentEntity.toCourseWithProgress() = CourseWithProgress(
courseId = courseId,
courseName = courseName,
courseImageUrl = courseImageUrl,
courseSyllabus = courseSyllabus,
progress = completionPercentage,
)

companion object {
const val OFFLINE_CURSOR_PREFIX = "offline:"

fun encodeOfflineCursor(offset: Int): String = "$OFFLINE_CURSOR_PREFIX$offset"

fun decodeOfflineCursor(cursor: String?): Int {
if (cursor == null) return 0
return cursor.removePrefix(OFFLINE_CURSOR_PREFIX).toIntOrNull() ?: 0
}

fun parseHighlightedData(json: String): NoteHighlightedData = try {
Gson().fromJson(json, NoteHighlightedData::class.java) ?: emptyHighlight()
} catch (e: Exception) {
emptyHighlight()
}

fun toEntity(edge: QueryNotesQuery.Edge): HorizonNoteEntity {
val node = edge.node
return HorizonNoteEntity(
id = node.id,
courseId = node.courseId.toLongOrNull() ?: 0L,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Silent fallback to courseId = 0L orphans the note. If node.courseId can't be parsed as Long, the entity is persisted with courseId = 0L. That row will never be returned by a courseId = realId filter and will never be cleaned up by deleteByCourseId(realId) or CourseCleanupHelper.cleanupCourseContent(realId) — it lingers in horizon_notes indefinitely.

Course IDs are server-controlled Longs, so a parse failure here indicates a malformed payload. Preferable to skip the edge (return null from toEntity, filter at the call site) and record the malformed input rather than persist a ghost row.

objectId = node.objectId,
objectType = node.objectType,
userText = node.userText.orEmpty(),
reaction = when (node.reaction?.firstOrNull()) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

node.courseId.toLongOrNull() ?: 0L silently buckets notes with malformed courseId under 0L. If the network ever returns an unexpected value, those rows would (a) be persisted and queried as if they belonged to a fictitious course 0, and (b) be invisible to the per-course offline reads. Consider either skipping the edge (filter it out of the entities mapped into replaceNotesForCourse) or surfacing the parse failure rather than coercing.

NoteReaction.Confusing.value -> NotebookType.Confusing.name
else -> NotebookType.Important.name
},
Comment on lines +156 to +159
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unknown reactions are silently coerced to Important. Any reaction value that isn't exactly NoteReaction.Confusing.value — including null/empty list, an unexpected casing, or a future reaction the server might add — gets persisted as NotebookType.Important. After a sync the user's locally cached notes can look different from the server's truth (e.g. a "Helpful" note would render as "Important" offline).

If Important truly is the only safe default, please log/Crashlytics-record the unexpected value so we'll notice when new reaction types ship. Otherwise consider storing the raw reaction string and resolving it at read time.

highlightedDataJson = node.highlightData?.toJson() ?: "",
updatedAt = node.updatedAt.time,
)
}

private fun emptyHighlight() = NoteHighlightedData(
selectedText = "",
range = NoteHighlightedDataRange(0, 0, "", ""),
textPosition = NoteHighlightedDataTextPosition(0, 0),
)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
/*
* Copyright (C) 2026 - present Instructure, Inc.
*
* 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.
*/
package com.instructure.horizon.data.datasource

import com.apollographql.apollo.api.Optional
import com.instructure.canvasapi2.managers.graphql.horizon.CourseWithProgress
import com.instructure.canvasapi2.managers.graphql.horizon.HorizonGetCoursesManager
import com.instructure.canvasapi2.managers.graphql.horizon.redwood.NoteHighlightedData
import com.instructure.canvasapi2.managers.graphql.horizon.redwood.RedwoodApiManager
import com.instructure.canvasapi2.utils.ApiPrefs
import com.instructure.horizon.features.notebook.common.model.NotebookType
import com.instructure.redwood.QueryNotesQuery
import com.instructure.redwood.type.LearningObjectFilter
import com.instructure.redwood.type.NoteFilterInput
import com.instructure.redwood.type.OrderByInput
import com.instructure.redwood.type.OrderDirection
import javax.inject.Inject

class NotebookNetworkDataSource @Inject constructor(
private val redwoodApiManager: RedwoodApiManager,
private val horizonGetCoursesManager: HorizonGetCoursesManager,
private val apiPrefs: ApiPrefs,
) {
suspend fun getNotes(
after: String? = null,
before: String? = null,
itemCount: Int = DEFAULT_PAGE_SIZE,
filterType: NotebookType? = null,
courseId: Long? = null,
objectTypeAndId: Pair<String, String>? = null,
orderDirection: OrderDirection? = null,
forceNetwork: Boolean = false,
): QueryNotesQuery.Notes {
val filterInput = buildFilter(filterType, courseId, objectTypeAndId)
val orderByInput = OrderByInput(direction = Optional.presentIfNotNull(orderDirection))

return if (before != null) {
redwoodApiManager.getNotes(
lastN = itemCount,
before = before,
filter = filterInput,
orderBy = orderByInput,
forceNetwork = forceNetwork,
)
} else {
redwoodApiManager.getNotes(
firstN = itemCount,
after = after,
filter = filterInput,
orderBy = orderByInput,
forceNetwork = forceNetwork,
)
}
}

suspend fun getAllNotesForCourse(courseId: Long): List<QueryNotesQuery.Edge> {
val all = mutableListOf<QueryNotesQuery.Edge>()
var cursor: String? = null
do {
val page = redwoodApiManager.getNotes(
filter = NoteFilterInput(courseId = Optional.present(courseId.toString())),
firstN = SYNC_PAGE_SIZE,
after = cursor,
forceNetwork = true,
)
page.edges?.let { all.addAll(it) }
cursor = if (page.pageInfo.hasNextPage) page.pageInfo.endCursor else null
} while (cursor != null)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Potential infinite loop in pagination. If the API ever returns hasNextPage = true together with endCursor = null (server bug, partial response, etc.), cursor stays null and the same first page is fetched forever — blocking the sync coroutine and any worker calling it.

Consider gating the loop on a cursor that actually advanced, e.g.:

val next = if (page.pageInfo.hasNextPage) page.pageInfo.endCursor else null
if (next == cursor) break
cursor = next

return all
}

suspend fun getCourses(forceNetwork: Boolean): List<CourseWithProgress> {
return horizonGetCoursesManager.getCoursesWithProgress(
userId = apiPrefs.user?.id ?: 0L,
forceNetwork = forceNetwork,
).dataOrNull.orEmpty()
}

suspend fun deleteNote(noteId: String) {
redwoodApiManager.deleteNote(noteId)
}

suspend fun createNote(
courseId: String,
objectId: String,
objectType: String,
userText: String?,
notebookType: String?,
highlightData: NoteHighlightedData?,
) {
redwoodApiManager.createNote(
courseId = courseId,
objectId = objectId,
objectType = objectType,
userText = userText,
notebookType = notebookType,
highlightData = highlightData,
)
}

suspend fun updateNote(
id: String,
userText: String?,
notebookType: String?,
highlightData: NoteHighlightedData?,
) {
redwoodApiManager.updateNote(
id = id,
userText = userText,
notebookType = notebookType,
highlightData = highlightData,
)
}

private fun buildFilter(
filterType: NotebookType?,
courseId: Long?,
objectTypeAndId: Pair<String, String>?,
): NoteFilterInput = NoteFilterInput(
reactions = if (filterType != null) Optional.present(listOf(filterType.name)) else Optional.absent(),
courseId = Optional.presentIfNotNull(courseId?.toString()),
learningObject = if (objectTypeAndId != null) {
Optional.present(LearningObjectFilter(type = objectTypeAndId.first, id = objectTypeAndId.second))
} else {
Optional.absent()
},
)

companion object {
const val DEFAULT_PAGE_SIZE = 10
const val SYNC_PAGE_SIZE = 100
}
}
Loading
Loading