From cd42ce56d8fff58a67e16e6f7283a1dc15d7934e Mon Sep 17 00:00:00 2001 From: Olivier de Broqueville Date: Tue, 30 Jan 2024 19:15:17 +0100 Subject: [PATCH 01/32] Add Rules menu item to dropdown menu --- .../components/templates/PrimaryDropdown.tsx | 618 +++++++++--------- 1 file changed, 313 insertions(+), 305 deletions(-) diff --git a/packages/web/components/templates/PrimaryDropdown.tsx b/packages/web/components/templates/PrimaryDropdown.tsx index f28aaa415..7a01cb2cd 100644 --- a/packages/web/components/templates/PrimaryDropdown.tsx +++ b/packages/web/components/templates/PrimaryDropdown.tsx @@ -6,9 +6,9 @@ import { currentTheme, updateTheme } from '../../lib/themeUpdater' import { Avatar } from '../elements/Avatar' import { AvatarDropdown } from '../elements/AvatarDropdown' import { - Dropdown, - DropdownOption, - DropdownSeparator, + Dropdown, + DropdownOption, + DropdownSeparator, } from '../elements/DropdownElements' import GridLayoutIcon from '../elements/images/GridLayoutIcon' import ListLayoutIcon from '../elements/images/ListLayoutIcon' @@ -18,327 +18,335 @@ import { styled, theme, ThemeId } from '../tokens/stitches.config' import { LayoutType } from './homeFeed/HomeFeedContainer' type PrimaryDropdownProps = { - children?: ReactNode - showThemeSection: boolean + children?: ReactNode + showThemeSection: boolean - layout?: LayoutType - updateLayout?: (layout: LayoutType) => void + layout?: LayoutType + updateLayout?: (layout: LayoutType) => void - showAddLinkModal?: () => void + showAddLinkModal?: () => void } export type HeaderDropdownAction = - | 'navigate-to-install' - | 'navigate-to-feeds' - | 'navigate-to-emails' - | 'navigate-to-labels' - | 'navigate-to-profile' - | 'navigate-to-subscriptions' - | 'navigate-to-api' - | 'navigate-to-integrations' - | 'navigate-to-saved-searches' - | 'increaseFontSize' - | 'decreaseFontSize' - | 'logout' + | 'navigate-to-install' + | 'navigate-to-feeds' + | 'navigate-to-emails' + | 'navigate-to-labels' + | 'navigate-to-rules' + | 'navigate-to-profile' + | 'navigate-to-subscriptions' + | 'navigate-to-api' + | 'navigate-to-integrations' + | 'navigate-to-saved-searches' + | 'increaseFontSize' + | 'decreaseFontSize' + | 'logout' export function PrimaryDropdown(props: PrimaryDropdownProps): JSX.Element { - const { viewerData } = useGetViewerQuery() - const router = useRouter() + const { viewerData } = useGetViewerQuery() + const router = useRouter() - const headerDropdownActionHandler = useCallback( - (action: HeaderDropdownAction) => { - switch (action) { - case 'navigate-to-install': - router.push('/settings/installation') - break - case 'navigate-to-feeds': - router.push('/settings/feeds') - break - case 'navigate-to-emails': - router.push('/settings/emails') - break - case 'navigate-to-labels': - router.push('/settings/labels') - break - case 'navigate-to-subscriptions': - router.push('/settings/subscriptions') - break - case 'navigate-to-api': - router.push('/settings/api') - break - case 'navigate-to-integrations': - router.push('/settings/integrations') - break - case 'navigate-to-saved-searches': - router.push('/settings/saved-searches') - break - case 'logout': - document.dispatchEvent(new Event('logout')) - break - default: - break - } - }, - [router] - ) + const headerDropdownActionHandler = useCallback( + (action: HeaderDropdownAction) => { + switch (action) { + case 'navigate-to-install': + router.push('/settings/installation') + break + case 'navigate-to-feeds': + router.push('/settings/feeds') + break + case 'navigate-to-emails': + router.push('/settings/emails') + break + case 'navigate-to-labels': + router.push('/settings/labels') + break + case 'navigate-to-rules': + router.push('/settings/rules') + break + case 'navigate-to-subscriptions': + router.push('/settings/subscriptions') + break + case 'navigate-to-api': + router.push('/settings/api') + break + case 'navigate-to-integrations': + router.push('/settings/integrations') + break + case 'navigate-to-saved-searches': + router.push('/settings/saved-searches') + break + case 'logout': + document.dispatchEvent(new Event('logout')) + break + default: + break + } + }, + [router] + ) - if (!viewerData?.me) { - return <> - } + if (!viewerData?.me) { + return <> + } - return ( - - ) - } - css={{ width: '240px' }} - > - { - router.push('/settings/account') - event.preventDefault() - }} - > - - + ) + } + css={{ width: '240px' }} > - {viewerData.me && ( - <> - - {viewerData.me.name} - - { + router.push('/settings/account') + event.preventDefault() }} - > - {`@${viewerData.me.profile.username}`} - - - )} - - - - {props.showThemeSection && } - headerDropdownActionHandler('navigate-to-install')} - title="Install" - /> - headerDropdownActionHandler('navigate-to-feeds')} - title="Feeds" - /> - headerDropdownActionHandler('navigate-to-emails')} - title="Emails" - /> - headerDropdownActionHandler('navigate-to-labels')} - title="Labels" - /> - {props.showAddLinkModal && ( - <> - + > + + + {viewerData.me && ( + <> + + {viewerData.me.name} + + + {`@${viewerData.me.profile.username}`} + + + )} + + + + {props.showThemeSection && } + headerDropdownActionHandler('navigate-to-install')} + title="Install" + /> + headerDropdownActionHandler('navigate-to-feeds')} + title="Feeds" + /> + headerDropdownActionHandler('navigate-to-emails')} + title="Emails" + /> + headerDropdownActionHandler('navigate-to-labels')} + title="Labels" + /> + headerDropdownActionHandler('navigate-to-rules')} + title="Rules" + /> + {props.showAddLinkModal && ( + <> + - props.showAddLinkModal && props.showAddLinkModal()} - title="Add Link" - /> - - )} - headerDropdownActionHandler('navigate-to-api')} - title="API Keys" - /> - headerDropdownActionHandler('navigate-to-integrations')} - title="Integrations" - /> - window.open('https://docs.omnivore.app', '_blank')} - title="Documentation" - /> - window.Intercom('show')} - title="Feedback" - /> - - headerDropdownActionHandler('logout')} - title="Logout" - /> - - ) + props.showAddLinkModal && props.showAddLinkModal()} + title="Add Link" + /> + + )} + headerDropdownActionHandler('navigate-to-api')} + title="API Keys" + /> + headerDropdownActionHandler('navigate-to-integrations')} + title="Integrations" + /> + window.open('https://docs.omnivore.app', '_blank')} + title="Documentation" + /> + window.Intercom('show')} + title="Feedback" + /> + + headerDropdownActionHandler('logout')} + title="Logout" + /> + + ) } export const StyledToggleButton = styled('button', { - display: 'flex', - alignItems: 'center', - justifyContent: 'center', - color: '$thTextContrast2', - backgroundColor: 'transparent', - border: 'none', - cursor: 'pointer', - width: '70px', - height: '100%', - borderRadius: '5px', - fontSize: '12px', - fontFamily: '$inter', - gap: '5px', - m: '2px', - '&:hover': { - opacity: 0.8, - }, - '&[data-state="on"]': { - bg: '$thBackground', - }, + display: 'flex', + alignItems: 'center', + justifyContent: 'center', + color: '$thTextContrast2', + backgroundColor: 'transparent', + border: 'none', + cursor: 'pointer', + width: '70px', + height: '100%', + borderRadius: '5px', + fontSize: '12px', + fontFamily: '$inter', + gap: '5px', + m: '2px', + '&:hover': { + opacity: 0.8, + }, + '&[data-state="on"]': { + bg: '$thBackground', + }, }) function ThemeSection(props: PrimaryDropdownProps): JSX.Element { - return ( - <> - - - - Mode - - - { - updateTheme(ThemeId.Light) - }} - > - Light - - - { - updateTheme(ThemeId.Dark) - }} - > - Dark - - - - - {props.layout && ( - - - Layout - - - { - props.updateLayout && props.updateLayout('LIST_LAYOUT') - }} - > - - - { - props.updateLayout && props.updateLayout('GRID_LAYOUT') - }} - > - - - - - )} - - - - ) + return ( + <> + + + + Mode + + + { + updateTheme(ThemeId.Light) + }} + > + Light + + + { + updateTheme(ThemeId.Dark) + }} + > + Dark + + + + + {props.layout && ( + + + Layout + + + { + props.updateLayout && props.updateLayout('LIST_LAYOUT') + }} + > + + + { + props.updateLayout && props.updateLayout('GRID_LAYOUT') + }} + > + + + + + )} + + + + ) } From 6d484884daf6a70df0b3b4a4cea258013e7844de Mon Sep 17 00:00:00 2001 From: kawamataryo Date: Wed, 31 Jan 2024 13:38:00 +0900 Subject: [PATCH 02/32] Prevent add label if press enter in composition session --- pkg/extension/src/scripts/content/toast.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/extension/src/scripts/content/toast.js b/pkg/extension/src/scripts/content/toast.js index a8d17d823..72c3d7b34 100644 --- a/pkg/extension/src/scripts/content/toast.js +++ b/pkg/extension/src/scripts/content/toast.js @@ -575,7 +575,7 @@ } case 'enter': { if (event.target.id == 'omnivore-edit-label-input') { - if (event.target.value) { + if (event.target.value && !event.isComposing) { const labelList = event.target.form.querySelector('#label-list') addLabel(labelList, event.target, event.target.value) } From 1fa061ee0a36cc8a8588ef80bbba8ca7f0eda67a Mon Sep 17 00:00:00 2001 From: Stefano Sansone Date: Wed, 31 Jan 2024 12:55:55 +0000 Subject: [PATCH 03/32] bump compose version --- android/Omnivore/app/build.gradle | 6 +++--- android/Omnivore/build.gradle | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/android/Omnivore/app/build.gradle b/android/Omnivore/app/build.gradle index 21a202d33..4bf7ef0f3 100644 --- a/android/Omnivore/app/build.gradle +++ b/android/Omnivore/app/build.gradle @@ -13,12 +13,12 @@ if (keystorePropertiesFile.exists()) { } android { - compileSdk 33 + compileSdk 34 defaultConfig { applicationId "app.omnivore.omnivore" minSdk 26 - targetSdk 33 + targetSdk 34 versionCode 188 versionName "0.0.188" @@ -92,7 +92,7 @@ dependencies { implementation "androidx.compose.ui:ui-tooling-preview:$compose_version" implementation "androidx.compose.material:material-icons-extended:$compose_version" implementation 'androidx.lifecycle:lifecycle-runtime-ktx:2.5.1' - implementation 'androidx.activity:activity-compose:1.6.1' + implementation 'androidx.activity:activity-compose:1.8.2' implementation 'androidx.appcompat:appcompat:1.5.1' implementation 'com.google.android.gms:play-services-base:18.1.0' diff --git a/android/Omnivore/build.gradle b/android/Omnivore/build.gradle index 9965e4843..62770128d 100644 --- a/android/Omnivore/build.gradle +++ b/android/Omnivore/build.gradle @@ -1,6 +1,6 @@ buildscript { ext { - compose_version = '1.3.1' + compose_version = '1.6.0' lifecycle_version = '2.5.1' hilt_version = '2.44.2' gradle_plugin_version = '7.4.2' From d101952deeb05426c21b06dd5f35b89dd1387334 Mon Sep 17 00:00:00 2001 From: Stefano Sansone Date: Wed, 31 Jan 2024 13:23:39 +0000 Subject: [PATCH 04/32] allow full window draw --- .../Omnivore/app/src/main/AndroidManifest.xml | 1 + .../app/omnivore/omnivore/MainActivity.kt | 95 +++---- .../omnivore/ui/auth/WelcomeScreen.kt | 250 ++++++++++-------- .../omnivore/ui/library/LibraryView.kt | 6 +- .../app/omnivore/omnivore/ui/root/RootView.kt | 153 +++++------ .../app/src/main/res/values/themes.xml | 2 - 6 files changed, 255 insertions(+), 252 deletions(-) diff --git a/android/Omnivore/app/src/main/AndroidManifest.xml b/android/Omnivore/app/src/main/AndroidManifest.xml index 3cea932cb..4ebf91f5d 100644 --- a/android/Omnivore/app/src/main/AndroidManifest.xml +++ b/android/Omnivore/app/src/main/AndroidManifest.xml @@ -22,6 +22,7 @@ diff --git a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/MainActivity.kt b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/MainActivity.kt index 408d285a3..8996416ad 100644 --- a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/MainActivity.kt +++ b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/MainActivity.kt @@ -4,12 +4,11 @@ import android.os.Bundle import android.view.View import androidx.activity.ComponentActivity import androidx.activity.compose.setContent +import androidx.activity.enableEdgeToEdge import androidx.activity.viewModels -import androidx.compose.foundation.background import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.ui.Modifier -import androidx.compose.ui.graphics.Color import androidx.core.view.ViewCompat import androidx.core.view.WindowCompat import androidx.core.view.WindowInsetsCompat @@ -24,61 +23,65 @@ import app.omnivore.omnivore.ui.settings.SettingsViewModel import app.omnivore.omnivore.ui.theme.OmnivoreTheme import com.pspdfkit.PSPDFKit import dagger.hilt.android.AndroidEntryPoint +import kotlinx.coroutines.DelicateCoroutinesApi import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.launch +@OptIn(DelicateCoroutinesApi::class) @AndroidEntryPoint class MainActivity : ComponentActivity() { - override fun onCreate(savedInstanceState: Bundle?) { - super.onCreate(savedInstanceState) + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) - val loginViewModel: LoginViewModel by viewModels() - val libraryViewModel: LibraryViewModel by viewModels() - val settingsViewModel: SettingsViewModel by viewModels() - val searchViewModel: SearchViewModel by viewModels() - val labelsViewModel: LabelsViewModel by viewModels() - val saveViewModel: SaveViewModel by viewModels() - val editInfoViewModel: EditInfoViewModel by viewModels() + val loginViewModel: LoginViewModel by viewModels() + val libraryViewModel: LibraryViewModel by viewModels() + val settingsViewModel: SettingsViewModel by viewModels() + val searchViewModel: SearchViewModel by viewModels() + val labelsViewModel: LabelsViewModel by viewModels() + val saveViewModel: SaveViewModel by viewModels() + val editInfoViewModel: EditInfoViewModel by viewModels() - val context = this + val context = this - GlobalScope.launch(Dispatchers.IO) { - val licenseKey = getString(R.string.pspdfkit_license_key) + GlobalScope.launch(Dispatchers.IO) { + val licenseKey = getString(R.string.pspdfkit_license_key) - if (licenseKey.length > 30) { - PSPDFKit.initialize(context, licenseKey) - } else { - PSPDFKit.initialize(context, null) - } - } - - setContent { - OmnivoreTheme { - Box( - modifier = Modifier - .fillMaxSize() - .background(color = Color.Black) - ) { - RootView( - loginViewModel, - searchViewModel, - libraryViewModel, - settingsViewModel, - labelsViewModel, - saveViewModel, - editInfoViewModel) + if (licenseKey.length > 30) { + PSPDFKit.initialize(context, licenseKey) + } else { + PSPDFKit.initialize(context, null) + } } - } - } - // animate the view up when keyboard appears - WindowCompat.setDecorFitsSystemWindows(window, false) - val rootView = findViewById(android.R.id.content).rootView - ViewCompat.setOnApplyWindowInsetsListener(rootView) { _, insets -> - val imeHeight = insets.getInsets(WindowInsetsCompat.Type.ime()).bottom - rootView.setPadding(0, 0, 0, imeHeight) - insets + enableEdgeToEdge() + + setContent { + OmnivoreTheme { + Box( + modifier = Modifier + .fillMaxSize() + ) { + RootView( + loginViewModel, + searchViewModel, + libraryViewModel, + settingsViewModel, + labelsViewModel, + saveViewModel, + editInfoViewModel + ) + } + } + } + + // animate the view up when keyboard appears + WindowCompat.setDecorFitsSystemWindows(window, false) + val rootView = findViewById(android.R.id.content).rootView + ViewCompat.setOnApplyWindowInsetsListener(rootView) { _, insets -> + val imeHeight = insets.getInsets(WindowInsetsCompat.Type.ime()).bottom + rootView.setPadding(0, 0, 0, imeHeight) + insets + } } - } } diff --git a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/ui/auth/WelcomeScreen.kt b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/ui/auth/WelcomeScreen.kt index 6d1e2e138..45dfe5095 100644 --- a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/ui/auth/WelcomeScreen.kt +++ b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/ui/auth/WelcomeScreen.kt @@ -4,7 +4,7 @@ import android.annotation.SuppressLint import android.content.Intent import android.net.Uri import androidx.compose.foundation.Image -import androidx.compose.foundation.clickable +import androidx.compose.foundation.isSystemInDarkTheme import androidx.compose.foundation.layout.* import androidx.compose.foundation.text.ClickableText import androidx.compose.material3.* @@ -14,7 +14,6 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color import androidx.compose.ui.platform.LocalContext -import androidx.compose.ui.platform.LocalFocusManager import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.AnnotatedString @@ -23,154 +22,173 @@ import androidx.compose.ui.text.style.TextDecoration import androidx.compose.ui.unit.dp import app.omnivore.omnivore.R import app.omnivore.omnivore.ui.theme.OmnivoreTheme +import com.google.accompanist.systemuicontroller.rememberSystemUiController import com.google.android.gms.common.GoogleApiAvailability import kotlinx.coroutines.launch @Composable fun WelcomeScreen(viewModel: LoginViewModel) { - OmnivoreTheme(darkTheme = false) { - Surface(modifier = Modifier.fillMaxSize(), color = Color(0xFFFCEBA8)) { - WelcomeScreenContent(viewModel = viewModel) + + val systemUiController = rememberSystemUiController() + val useDarkIcons = !isSystemInDarkTheme() + + DisposableEffect(systemUiController, useDarkIcons) { + systemUiController.setSystemBarsColor( + color = Color.Black, + darkIcons = true + ) + onDispose { + systemUiController.setSystemBarsColor( + color = Color.Black, + darkIcons = useDarkIcons + ) + } + } + + OmnivoreTheme(darkTheme = false) { + Surface(modifier = Modifier.fillMaxSize(), color = Color(0xFFFCEBA8)) { + WelcomeScreenContent(viewModel = viewModel) + } } - } } @SuppressLint("CoroutineCreationDuringComposition") @Composable fun WelcomeScreenContent(viewModel: LoginViewModel) { - val registrationState: RegistrationState by viewModel - .registrationStateLiveData - .observeAsState(RegistrationState.SocialLogin) + val registrationState: RegistrationState by viewModel + .registrationStateLiveData + .observeAsState(RegistrationState.SocialLogin) - val snackBarHostState = remember { SnackbarHostState() } - val coroutineScope = rememberCoroutineScope() - val focusManager = LocalFocusManager.current + val snackBarHostState = remember { SnackbarHostState() } + val coroutineScope = rememberCoroutineScope() - Column( - verticalArrangement = Arrangement.SpaceAround, - horizontalAlignment = Alignment.Start, - modifier = Modifier - .fillMaxSize() - .padding(horizontal = 16.dp) - .clickable { focusManager.clearFocus() } - ) { - Spacer(modifier = Modifier.height(50.dp)) - Image( - painter = painterResource(id = R.drawable.ic_omnivore_name_logo), - contentDescription = "Omnivore Icon with Name" - ) - Spacer(modifier = Modifier.height(50.dp)) - - when(registrationState) { - RegistrationState.EmailSignIn -> { - EmailLoginView(viewModel = viewModel) - } - RegistrationState.EmailSignUp -> { - EmailSignUpView(viewModel = viewModel) - } - RegistrationState.SelfHosted -> { - SelfHostedView(viewModel = viewModel) - } - RegistrationState.SocialLogin -> { - Text( - text = stringResource(id = R.string.welcome_title), - style = MaterialTheme.typography.headlineLarge + Column( + verticalArrangement = Arrangement.SpaceAround, + horizontalAlignment = Alignment.Start, + modifier = Modifier + .fillMaxSize() + .padding(16.dp) + ) { + Spacer(modifier = Modifier.height(50.dp)) + Image( + painter = painterResource(id = R.drawable.ic_omnivore_name_logo), + contentDescription = "Omnivore Icon with Name" ) - - Text( - text = stringResource(id = R.string.welcome_subtitle), - style = MaterialTheme.typography.titleSmall - ) - - MoreInfoButton() - Spacer(modifier = Modifier.height(50.dp)) - AuthProviderView(viewModel = viewModel) - } - RegistrationState.PendingUser -> { - CreateUserProfileView(viewModel = viewModel) - } + when (registrationState) { + RegistrationState.EmailSignIn -> { + EmailLoginView(viewModel = viewModel) + } + + RegistrationState.EmailSignUp -> { + EmailSignUpView(viewModel = viewModel) + } + + RegistrationState.SelfHosted -> { + SelfHostedView(viewModel = viewModel) + } + + RegistrationState.SocialLogin -> { + Text( + text = stringResource(id = R.string.welcome_title), + style = MaterialTheme.typography.headlineLarge + ) + + Text( + text = stringResource(id = R.string.welcome_subtitle), + style = MaterialTheme.typography.titleSmall + ) + + MoreInfoButton() + + Spacer(modifier = Modifier.height(50.dp)) + + AuthProviderView(viewModel = viewModel) + } + + RegistrationState.PendingUser -> { + CreateUserProfileView(viewModel = viewModel) + } + } + + Spacer(modifier = Modifier.weight(1.0F)) } - Spacer(modifier = Modifier.weight(1.0F)) - } + if (viewModel.errorMessage != null) { + coroutineScope.launch { + val result = snackBarHostState + .showSnackbar( + viewModel.errorMessage!!, + actionLabel = "Dismiss", + duration = SnackbarDuration.Indefinite + ) + when (result) { + SnackbarResult.ActionPerformed -> viewModel.resetErrorMessage() + else -> {} + } + } - if (viewModel.errorMessage != null) { - coroutineScope.launch { - val result = snackBarHostState - .showSnackbar( - viewModel.errorMessage!!, - actionLabel = "Dismiss", - duration = SnackbarDuration.Indefinite - ) - when (result) { - SnackbarResult.ActionPerformed -> viewModel.resetErrorMessage() - else -> {} - } + SnackbarHost(hostState = snackBarHostState) } - - SnackbarHost(hostState = snackBarHostState) - } } @Composable fun AuthProviderView(viewModel: LoginViewModel) { - val isGoogleAuthAvailable: Boolean = GoogleApiAvailability - .getInstance() - .isGooglePlayServicesAvailable(LocalContext.current) == 0 + val isGoogleAuthAvailable: Boolean = GoogleApiAvailability + .getInstance() + .isGooglePlayServicesAvailable(LocalContext.current) == 0 - Row( - horizontalArrangement = Arrangement.Center - ) { - Spacer(modifier = Modifier.weight(1.0F)) - Column( - verticalArrangement = Arrangement.spacedBy(8.dp), - horizontalAlignment = Alignment.CenterHorizontally + Row( + horizontalArrangement = Arrangement.Center ) { - if (isGoogleAuthAvailable) { - GoogleAuthButton(viewModel) - } + Spacer(modifier = Modifier.weight(1.0F)) + Column( + verticalArrangement = Arrangement.spacedBy(8.dp), + horizontalAlignment = Alignment.CenterHorizontally + ) { + if (isGoogleAuthAvailable) { + GoogleAuthButton(viewModel) + } - AppleAuthButton(viewModel) + AppleAuthButton(viewModel) - ClickableText( - text = AnnotatedString(stringResource(R.string.welcome_screen_action_continue_with_email)), - style = MaterialTheme.typography.titleMedium - .plus(TextStyle(textDecoration = TextDecoration.Underline)), - onClick = { viewModel.showEmailSignIn() } - ) + ClickableText( + text = AnnotatedString(stringResource(R.string.welcome_screen_action_continue_with_email)), + style = MaterialTheme.typography.titleMedium + .plus(TextStyle(textDecoration = TextDecoration.Underline)), + onClick = { viewModel.showEmailSignIn() } + ) - Spacer(modifier = Modifier.weight(1.0F)) + Spacer(modifier = Modifier.weight(1.0F)) - ClickableText( - text = AnnotatedString(stringResource(R.string.welcome_screen_action_self_hosting_options)), - style = MaterialTheme.typography.titleMedium - .plus(TextStyle(textDecoration = TextDecoration.Underline)), - onClick = { viewModel.showSelfHostedSettings() }, - modifier = Modifier - .padding(vertical = 10.dp) - ) + ClickableText( + text = AnnotatedString(stringResource(R.string.welcome_screen_action_self_hosting_options)), + style = MaterialTheme.typography.titleMedium + .plus(TextStyle(textDecoration = TextDecoration.Underline)), + onClick = { viewModel.showSelfHostedSettings() }, + modifier = Modifier + .padding(vertical = 10.dp) + ) + } + Spacer(modifier = Modifier.weight(1.0F)) } - Spacer(modifier = Modifier.weight(1.0F)) - } } @Composable fun MoreInfoButton() { - val context = LocalContext.current - val intent = remember { Intent(Intent.ACTION_VIEW, Uri.parse("https://omnivore.app/about")) } + val context = LocalContext.current + val intent = remember { Intent(Intent.ACTION_VIEW, Uri.parse("https://omnivore.app/about")) } - ClickableText( - text = AnnotatedString( - stringResource(id = R.string.learn_more), - ), - style = MaterialTheme.typography.titleSmall - .plus(TextStyle(textDecoration = TextDecoration.Underline)), - onClick = { - context.startActivity(intent) - }, - modifier = Modifier.padding(vertical = 6.dp) - ) + ClickableText( + text = AnnotatedString( + stringResource(id = R.string.learn_more), + ), + style = MaterialTheme.typography.titleSmall + .plus(TextStyle(textDecoration = TextDecoration.Underline)), + onClick = { + context.startActivity(intent) + }, + modifier = Modifier.padding(vertical = 6.dp) + ) } - diff --git a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/ui/library/LibraryView.kt b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/ui/library/LibraryView.kt index caf8816b2..32e0d66a4 100644 --- a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/ui/library/LibraryView.kt +++ b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/ui/library/LibraryView.kt @@ -20,7 +20,6 @@ import androidx.compose.material.DismissValue import androidx.compose.material.ExperimentalMaterialApi import androidx.compose.material.FractionalThreshold import androidx.compose.material.Icon -import androidx.compose.material.ModalBottomSheetValue import androidx.compose.material.Scaffold import androidx.compose.material.ScaffoldState import androidx.compose.material.SwipeToDismiss @@ -32,11 +31,11 @@ import androidx.compose.material.pullrefresh.PullRefreshIndicator import androidx.compose.material.pullrefresh.pullRefresh import androidx.compose.material.pullrefresh.rememberPullRefreshState import androidx.compose.material.rememberDismissState -import androidx.compose.material3.rememberModalBottomSheetState import androidx.compose.material.rememberScaffoldState import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.MaterialTheme import androidx.compose.material3.ModalBottomSheet +import androidx.compose.material3.rememberModalBottomSheetState import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.derivedStateOf @@ -70,7 +69,6 @@ import app.omnivore.omnivore.ui.savedItemViews.SavedItemCard import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.launch -@OptIn(ExperimentalMaterialApi::class) @Composable fun LibraryView( libraryViewModel: LibraryViewModel, @@ -146,7 +144,7 @@ fun showAddLinkBottomSheet(libraryViewModel: LibraryViewModel) { libraryViewModel.bottomSheetState.value = LibraryBottomSheetState.ADD_LINK } -@OptIn(ExperimentalMaterial3Api::class, ExperimentalMaterialApi::class) +@OptIn(ExperimentalMaterial3Api::class) @Composable fun LabelBottomSheet( libraryViewModel: LibraryViewModel, diff --git a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/ui/root/RootView.kt b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/ui/root/RootView.kt index e411f9d12..cdb12f527 100644 --- a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/ui/root/RootView.kt +++ b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/ui/root/RootView.kt @@ -1,14 +1,10 @@ package app.omnivore.omnivore.ui.root -import androidx.compose.foundation.isSystemInDarkTheme import androidx.compose.foundation.layout.Box -import androidx.compose.foundation.layout.systemBarsPadding import androidx.compose.runtime.Composable import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.livedata.observeAsState -import androidx.compose.ui.Modifier -import androidx.compose.ui.graphics.Color import androidx.navigation.compose.NavHost import androidx.navigation.compose.composable import androidx.navigation.compose.rememberNavController @@ -18,109 +14,98 @@ import app.omnivore.omnivore.ui.auth.WelcomeScreen import app.omnivore.omnivore.ui.components.LabelsViewModel import app.omnivore.omnivore.ui.editinfo.EditInfoViewModel import app.omnivore.omnivore.ui.library.LibraryView -import app.omnivore.omnivore.ui.library.SearchView import app.omnivore.omnivore.ui.library.LibraryViewModel +import app.omnivore.omnivore.ui.library.SearchView import app.omnivore.omnivore.ui.library.SearchViewModel import app.omnivore.omnivore.ui.save.SaveViewModel import app.omnivore.omnivore.ui.settings.PolicyWebView import app.omnivore.omnivore.ui.settings.SettingsView import app.omnivore.omnivore.ui.settings.SettingsViewModel -import com.google.accompanist.systemuicontroller.rememberSystemUiController @Composable fun RootView( - loginViewModel: LoginViewModel, - searchViewModel: SearchViewModel, - libraryViewModel: LibraryViewModel, - settingsViewModel: SettingsViewModel, - labelsViewModel: LabelsViewModel, - saveViewModel: SaveViewModel, - editInfoViewModel: EditInfoViewModel, + loginViewModel: LoginViewModel, + searchViewModel: SearchViewModel, + libraryViewModel: LibraryViewModel, + settingsViewModel: SettingsViewModel, + labelsViewModel: LabelsViewModel, + saveViewModel: SaveViewModel, + editInfoViewModel: EditInfoViewModel, ) { - val hasAuthToken: Boolean by loginViewModel.hasAuthTokenLiveData.observeAsState(false) - val systemUiController = rememberSystemUiController() - val useDarkIcons = !isSystemInDarkTheme() + val hasAuthToken: Boolean by loginViewModel.hasAuthTokenLiveData.observeAsState(false) - DisposableEffect(systemUiController, useDarkIcons) { - systemUiController.setSystemBarsColor( - color = Color.Black, - darkIcons = false - ) + Box { + if (hasAuthToken) { + PrimaryNavigator( + loginViewModel = loginViewModel, + searchViewModel = searchViewModel, + libraryViewModel = libraryViewModel, + settingsViewModel = settingsViewModel, + labelsViewModel = labelsViewModel, + saveViewModel = saveViewModel, + editInfoViewModel = editInfoViewModel, + ) + } else { + WelcomeScreen(viewModel = loginViewModel) + } - onDispose {} - } - - Box( - modifier = Modifier - .systemBarsPadding() - ) { - if (hasAuthToken) { - PrimaryNavigator( - loginViewModel = loginViewModel, - searchViewModel = searchViewModel, - libraryViewModel = libraryViewModel, - settingsViewModel = settingsViewModel, - labelsViewModel = labelsViewModel, - saveViewModel = saveViewModel, - editInfoViewModel = editInfoViewModel, - ) - } else { - WelcomeScreen(viewModel = loginViewModel) + DisposableEffect(hasAuthToken) { + if (hasAuthToken) { + loginViewModel.registerUser() + } + onDispose {} + } } - - DisposableEffect(hasAuthToken) { - if (hasAuthToken) { - loginViewModel.registerUser() - } - onDispose {} - } - } } @Composable fun PrimaryNavigator( - loginViewModel: LoginViewModel, - libraryViewModel: LibraryViewModel, - searchViewModel: SearchViewModel, - settingsViewModel: SettingsViewModel, - labelsViewModel: LabelsViewModel, - saveViewModel: SaveViewModel, - editInfoViewModel: EditInfoViewModel, + loginViewModel: LoginViewModel, + libraryViewModel: LibraryViewModel, + searchViewModel: SearchViewModel, + settingsViewModel: SettingsViewModel, + labelsViewModel: LabelsViewModel, + saveViewModel: SaveViewModel, + editInfoViewModel: EditInfoViewModel, ) { - val navController = rememberNavController() + val navController = rememberNavController() - NavHost(navController = navController, startDestination = Routes.Library.route) { - composable(Routes.Library.route) { - LibraryView( - libraryViewModel = libraryViewModel, - navController = navController, - labelsViewModel = labelsViewModel, - saveViewModel = saveViewModel, - editInfoViewModel = editInfoViewModel, - ) - } + NavHost(navController = navController, startDestination = Routes.Library.route) { + composable(Routes.Library.route) { + LibraryView( + libraryViewModel = libraryViewModel, + navController = navController, + labelsViewModel = labelsViewModel, + saveViewModel = saveViewModel, + editInfoViewModel = editInfoViewModel, + ) + } - composable(Routes.Search.route) { - SearchView( - viewModel = searchViewModel, - navController = navController - ) - } + composable(Routes.Search.route) { + SearchView( + viewModel = searchViewModel, + navController = navController + ) + } - composable(Routes.Settings.route) { - SettingsView(loginViewModel = loginViewModel, settingsViewModel = settingsViewModel, navController = navController) - } + composable(Routes.Settings.route) { + SettingsView( + loginViewModel = loginViewModel, + settingsViewModel = settingsViewModel, + navController = navController + ) + } - composable(Routes.Documentation.route) { - PolicyWebView(navController = navController, url = "https://docs.omnivore.app") - } + composable(Routes.Documentation.route) { + PolicyWebView(navController = navController, url = "https://docs.omnivore.app") + } - composable(Routes.PrivacyPolicy.route) { - PolicyWebView(navController = navController, url = "https://omnivore.app/privacy") - } + composable(Routes.PrivacyPolicy.route) { + PolicyWebView(navController = navController, url = "https://omnivore.app/privacy") + } - composable(Routes.TermsAndConditions.route) { - PolicyWebView(navController = navController, url = "https://omnivore.app/app/terms") + composable(Routes.TermsAndConditions.route) { + PolicyWebView(navController = navController, url = "https://omnivore.app/app/terms") + } } - } } diff --git a/android/Omnivore/app/src/main/res/values/themes.xml b/android/Omnivore/app/src/main/res/values/themes.xml index fa82596de..f08194bdc 100644 --- a/android/Omnivore/app/src/main/res/values/themes.xml +++ b/android/Omnivore/app/src/main/res/values/themes.xml @@ -18,7 +18,5 @@ true false true - - @android:color/transparent From 86c80d991f97bdfa93add75fcc78929bef531545 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Tue, 30 Jan 2024 13:02:32 +0800 Subject: [PATCH 05/32] Write behing cache for reading progress --- packages/api/src/apollo.ts | 4 + .../reading_progress_data_source.ts | 86 +++++++++++++++++++ packages/api/src/resolvers/article/index.ts | 7 +- .../api/src/resolvers/function_resolvers.ts | 86 ++++++++++++++++--- packages/api/src/resolvers/types.ts | 4 + 5 files changed, 172 insertions(+), 15 deletions(-) create mode 100644 packages/api/src/datasources/reading_progress_data_source.ts diff --git a/packages/api/src/apollo.ts b/packages/api/src/apollo.ts index 7d4c8ea02..ef8ea5cb5 100644 --- a/packages/api/src/apollo.ts +++ b/packages/api/src/apollo.ts @@ -25,6 +25,7 @@ import { tracer } from './tracing' import { getClaimsByToken, setAuthInCookie } from './utils/auth' import { SetClaimsRole } from './utils/dictionary' import { logger } from './utils/logger' +import { ReadingProgressDataSource } from './datasources/reading_progress_data_source' const signToken = promisify(jwt.sign) const pubsub = createPubSubClient() @@ -84,6 +85,9 @@ const contextFunc: ContextFunction = async ({ return cb(tx) }), tracingSpan: tracer.startSpan('apollo.request'), + dataSources: { + readingProgress: new ReadingProgressDataSource(), + }, } return ctx diff --git a/packages/api/src/datasources/reading_progress_data_source.ts b/packages/api/src/datasources/reading_progress_data_source.ts new file mode 100644 index 000000000..1155aef1a --- /dev/null +++ b/packages/api/src/datasources/reading_progress_data_source.ts @@ -0,0 +1,86 @@ +import { redisDataSource } from '../redis_data_source' + +type ReadingProgressCacheItem = { + readingProgressPercent: number + readingProgressTopPercent: number | undefined + readingProgressAnchorIndex: number | undefined + updatedAt: Date +} + +export class ReadingProgressDataSource { + private cacheItems: { [id: string]: ReadingProgressCacheItem } = {} + + constructor() {} + + async getReadingProgress( + libraryItemID: string + ): Promise { + const cacheKey = `omnivore:reading-progress:${libraryItemID}` + const cached = this.cacheItems[cacheKey] + if (cached) { + return cached + } + return this.valueFromRedis(libraryItemID) + } + + async updateReadingProgress( + libraryItemID: string, + progress: { + readingProgressPercent: number + readingProgressTopPercent: number | undefined | null + readingProgressAnchorIndex: number | undefined | null + } + ): Promise { + const cacheKey = `omnivore:reading-progress:${libraryItemID}` + const existingItem = await this.valueFromRedis(cacheKey) + const cacheItem = { + readingProgressPercent: Math.max( + progress.readingProgressPercent, + existingItem?.readingProgressPercent ?? 0 + ), + readingProgressTopPercent: Math.max( + progress.readingProgressTopPercent ?? 0, + existingItem?.readingProgressTopPercent ?? 0 + ), + readingProgressAnchorIndex: Math.max( + progress.readingProgressAnchorIndex ?? 0, + existingItem?.readingProgressAnchorIndex ?? 0 + ), + updatedAt: new Date(), + } + + this.cacheItems[cacheKey] = cacheItem + if (await redisDataSource.redisClient?.hmset(cacheKey, cacheItem)) { + console.log('cached reading progress') + } else { + console.log('failed to cache reading progress') + } + } + + async valueFromRedis( + libraryItemID: string + ): Promise { + const cacheKey = `omnivore:reading-progress:${libraryItemID}` + const redisCached = await redisDataSource.redisClient?.hgetall(cacheKey) + if (redisCached) { + const readingProgressPercent = parseInt( + redisCached.readingProgressPercent, + 10 + ) + const updatedAt = new Date(parseInt(redisCached.updatedAt, 10)) + if (!Number.isNaN(readingProgressPercent) && updatedAt) { + return { + readingProgressPercent, + readingProgressTopPercent: redisCached.readingProgressTopPercent + ? parseInt(redisCached.readingProgressTopPercent, 10) + : undefined, + readingProgressAnchorIndex: redisCached.readingProgressAnchorIndex + ? parseInt(redisCached.readingProgressAnchorIndex, 10) + : undefined, + updatedAt, + } + } + } + return undefined + } +} diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index 921a01cf8..5a5aa38f9 100644 --- a/packages/api/src/resolvers/article/index.ts +++ b/packages/api/src/resolvers/article/index.ts @@ -607,7 +607,7 @@ export const saveArticleReadingProgressResolver = authorized< force, }, }, - { log, pubsub, uid } + { log, pubsub, uid, dataSources } ) => { if ( readingProgressPercent < 0 || @@ -640,6 +640,11 @@ export const saveArticleReadingProgressResolver = authorized< } } + dataSources.readingProgress.updateReadingProgress(id, { + readingProgressPercent, + readingProgressTopPercent, + readingProgressAnchorIndex, + }) // update reading progress only if the current value is lower const updatedItem = await updateLibraryItemReadingProgress( id, diff --git a/packages/api/src/resolvers/function_resolvers.ts b/packages/api/src/resolvers/function_resolvers.ts index 871782068..bcc17360b 100644 --- a/packages/api/src/resolvers/function_resolvers.ts +++ b/packages/api/src/resolvers/function_resolvers.ts @@ -312,20 +312,6 @@ export const functionResolvers = { publishedAt(article: { publishedAt: Date }) { return validatedDate(article.publishedAt) }, - // async shareInfo( - // article: { id: string; sharedBy?: User; shareInfo?: LinkShareInfo }, - // __: unknown, - // ctx: WithDataSourcesContext - // ): Promise { - // if (article.shareInfo) return article.shareInfo - // if (!ctx.claims?.uid) return undefined - // return getShareInfoForArticle( - // ctx.kx, - // ctx.claims?.uid, - // article.id, - // ctx.models - // ) - // }, image(article: { image?: string }): string | undefined { return article.image && createImageProxyUrl(article.image, 320, 320) }, @@ -342,6 +328,42 @@ export const functionResolvers = { return findLabelsByLibraryItemId(article.id, ctx.uid) }, + async readingProgressPercent( + article: { id: string; readingProgressPercent?: number }, + _: unknown, + ctx: WithDataSourcesContext + ) { + const readingProgress = + await ctx.dataSources.readingProgress.getReadingProgress(article.id) + if (readingProgress) { + return readingProgress.readingProgressPercent + } + return article.readingProgressPercent + }, + async readingProgressAnchorIndex( + article: { id: string; readingProgressAnchorIndex?: number }, + _: unknown, + ctx: WithDataSourcesContext + ) { + const readingProgress = + await ctx.dataSources.readingProgress.getReadingProgress(article.id) + if (readingProgress) { + return readingProgress.readingProgressAnchorIndex + } + return article.readingProgressAnchorIndex + }, + async readingProgressTopPercent( + article: { id: string; readingProgressTopPercent?: number }, + _: unknown, + ctx: WithDataSourcesContext + ) { + const readingProgress = + await ctx.dataSources.readingProgress.getReadingProgress(article.id) + if (readingProgress) { + return readingProgress.readingProgressTopPercent + } + return article.readingProgressTopPercent + }, }, Highlight: { // async reactions( @@ -447,6 +469,42 @@ export const functionResolvers = { const highlights = await findHighlightsByLibraryItemId(item.id, ctx.uid) return highlights.map(highlightDataToHighlight) }, + async readingProgressPercent( + article: { id: string; readingProgressPercent?: number }, + _: unknown, + ctx: WithDataSourcesContext + ) { + const readingProgress = + await ctx.dataSources.readingProgress.getReadingProgress(article.id) + if (readingProgress) { + return readingProgress.readingProgressPercent + } + return article.readingProgressPercent + }, + async readingProgressAnchorIndex( + article: { id: string; readingProgressAnchorIndex?: number }, + _: unknown, + ctx: WithDataSourcesContext + ) { + const readingProgress = + await ctx.dataSources.readingProgress.getReadingProgress(article.id) + if (readingProgress) { + return readingProgress.readingProgressAnchorIndex + } + return article.readingProgressAnchorIndex + }, + async readingProgressTopPercent( + article: { id: string; readingProgressTopPercent?: number }, + _: unknown, + ctx: WithDataSourcesContext + ) { + const readingProgress = + await ctx.dataSources.readingProgress.getReadingProgress(article.id) + if (readingProgress) { + return readingProgress.readingProgressTopPercent + } + return article.readingProgressTopPercent + }, }, Subscription: { newsletterEmail(subscription: Subscription) { diff --git a/packages/api/src/resolvers/types.ts b/packages/api/src/resolvers/types.ts index 6c842d650..6fc24405f 100644 --- a/packages/api/src/resolvers/types.ts +++ b/packages/api/src/resolvers/types.ts @@ -5,6 +5,7 @@ import * as jwt from 'jsonwebtoken' import { EntityManager } from 'typeorm' import winston from 'winston' import { PubsubClient } from '../pubsub' +import { ReadingProgressDataSource } from '../datasources/reading_progress_data_source' export interface Claims { uid: string @@ -37,6 +38,9 @@ export interface RequestContext { userRole?: string ) => Promise tracingSpan: Span + dataSources: { + readingProgress: ReadingProgressDataSource + } } export type ResolverContext = ApolloContext From c2327781e1e8f34597c72ca83efae5c75e12ddbf Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Tue, 30 Jan 2024 13:17:26 +0800 Subject: [PATCH 06/32] Remove empty ctor --- packages/api/src/datasources/reading_progress_data_source.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/api/src/datasources/reading_progress_data_source.ts b/packages/api/src/datasources/reading_progress_data_source.ts index 1155aef1a..1421bedcf 100644 --- a/packages/api/src/datasources/reading_progress_data_source.ts +++ b/packages/api/src/datasources/reading_progress_data_source.ts @@ -10,8 +10,6 @@ type ReadingProgressCacheItem = { export class ReadingProgressDataSource { private cacheItems: { [id: string]: ReadingProgressCacheItem } = {} - constructor() {} - async getReadingProgress( libraryItemID: string ): Promise { From fbfa93447996ed862830a7e7dc9f78aacfc2a27e Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Wed, 31 Jan 2024 09:28:09 +0800 Subject: [PATCH 07/32] Write behind reading progress as a list that can be reduced at commit or read time --- .../reading_progress_data_source.ts | 72 ++++------ packages/api/src/queue-processor.ts | 19 +++ packages/api/src/resolvers/article/index.ts | 46 +++++-- .../api/src/resolvers/function_resolvers.ts | 128 ++++++++---------- packages/api/src/utils/createTask.ts | 1 + .../components/templates/article/Article.tsx | 2 +- 6 files changed, 137 insertions(+), 131 deletions(-) diff --git a/packages/api/src/datasources/reading_progress_data_source.ts b/packages/api/src/datasources/reading_progress_data_source.ts index 1421bedcf..5f9e7358b 100644 --- a/packages/api/src/datasources/reading_progress_data_source.ts +++ b/packages/api/src/datasources/reading_progress_data_source.ts @@ -4,80 +4,64 @@ type ReadingProgressCacheItem = { readingProgressPercent: number readingProgressTopPercent: number | undefined readingProgressAnchorIndex: number | undefined - updatedAt: Date + updatedAt: string } export class ReadingProgressDataSource { private cacheItems: { [id: string]: ReadingProgressCacheItem } = {} + constructor() {} + async getReadingProgress( + uid: string, libraryItemID: string ): Promise { - const cacheKey = `omnivore:reading-progress:${libraryItemID}` + const cacheKey = `omnivore:reading-progress:${uid}:${libraryItemID}` const cached = this.cacheItems[cacheKey] if (cached) { return cached } - return this.valueFromRedis(libraryItemID) + return this.valueFromRedis(cacheKey) } async updateReadingProgress( + uid: string, libraryItemID: string, progress: { readingProgressPercent: number - readingProgressTopPercent: number | undefined | null - readingProgressAnchorIndex: number | undefined | null + readingProgressTopPercent: number | undefined + readingProgressAnchorIndex: number | undefined } ): Promise { - const cacheKey = `omnivore:reading-progress:${libraryItemID}` - const existingItem = await this.valueFromRedis(cacheKey) - const cacheItem = { - readingProgressPercent: Math.max( - progress.readingProgressPercent, - existingItem?.readingProgressPercent ?? 0 - ), - readingProgressTopPercent: Math.max( - progress.readingProgressTopPercent ?? 0, - existingItem?.readingProgressTopPercent ?? 0 - ), - readingProgressAnchorIndex: Math.max( - progress.readingProgressAnchorIndex ?? 0, - existingItem?.readingProgressAnchorIndex ?? 0 - ), - updatedAt: new Date(), + const cacheKey = `omnivore:reading-progress:${uid}:${libraryItemID}` + const cacheItem: ReadingProgressCacheItem = { + ...progress, + updatedAt: new Date().toISOString(), } this.cacheItems[cacheKey] = cacheItem - if (await redisDataSource.redisClient?.hmset(cacheKey, cacheItem)) { - console.log('cached reading progress') + if ( + await redisDataSource.redisClient?.lpush( + cacheKey, + JSON.stringify(cacheItem) + ) + ) { + console.log('cached reading progress', cacheKey) } else { console.log('failed to cache reading progress') } } async valueFromRedis( - libraryItemID: string + cacheKey: string ): Promise { - const cacheKey = `omnivore:reading-progress:${libraryItemID}` - const redisCached = await redisDataSource.redisClient?.hgetall(cacheKey) - if (redisCached) { - const readingProgressPercent = parseInt( - redisCached.readingProgressPercent, - 10 - ) - const updatedAt = new Date(parseInt(redisCached.updatedAt, 10)) - if (!Number.isNaN(readingProgressPercent) && updatedAt) { - return { - readingProgressPercent, - readingProgressTopPercent: redisCached.readingProgressTopPercent - ? parseInt(redisCached.readingProgressTopPercent, 10) - : undefined, - readingProgressAnchorIndex: redisCached.readingProgressAnchorIndex - ? parseInt(redisCached.readingProgressAnchorIndex, 10) - : undefined, - updatedAt, - } - } + const redisCached = await redisDataSource.redisClient?.lrange( + cacheKey, + 0, + 0 + ) + if (redisCached && redisCached.length > 0) { + return JSON.parse(redisCached[0]) } return undefined } diff --git a/packages/api/src/queue-processor.ts b/packages/api/src/queue-processor.ts index ebf6fcfdf..afaa5b3ab 100644 --- a/packages/api/src/queue-processor.ts +++ b/packages/api/src/queue-processor.ts @@ -28,6 +28,10 @@ import { import { updatePDFContentJob } from './jobs/update_pdf_content' import { redisDataSource } from './redis_data_source' import { CustomTypeOrmLogger } from './utils/logger' +import { + SYNC_READ_POSITIONS_JOB_NAME, + syncReadPositionsJob, +} from './jobs/sync_read_positions' export const QUEUE_NAME = 'omnivore-backend-queue' @@ -152,6 +156,21 @@ const main = async () => { const worker = createWorker(workerRedisClient) + const queue = await getBackendQueue() + if (queue) { + await queue.add( + SYNC_READ_POSITIONS_JOB_NAME, + {}, + { + priority: 1, + repeat: { + every: 10000, + limit: 100, + }, + } + ) + } + const queueEvents = new QueueEvents(QUEUE_NAME, { connection: workerRedisClient, }) diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index 5a5aa38f9..2b95913f8 100644 --- a/packages/api/src/resolvers/article/index.ts +++ b/packages/api/src/resolvers/article/index.ts @@ -60,7 +60,7 @@ import { UpdatesSinceError, UpdatesSinceSuccess, } from '../../generated/graphql' -import { getColumns } from '../../repository' +import { authTrx, getColumns } from '../../repository' import { getInternalLabelWithColor } from '../../repository/label' import { libraryItemRepository } from '../../repository/library_item' import { userRepository } from '../../repository/user' @@ -640,19 +640,37 @@ export const saveArticleReadingProgressResolver = authorized< } } - dataSources.readingProgress.updateReadingProgress(id, { - readingProgressPercent, - readingProgressTopPercent, - readingProgressAnchorIndex, - }) - // update reading progress only if the current value is lower - const updatedItem = await updateLibraryItemReadingProgress( - id, - uid, - readingProgressPercent, - readingProgressTopPercent, - readingProgressAnchorIndex - ) + let updatedItem: LibraryItem | null + if (env.redis.cache && env.redis.mq) { + // If redis caching and queueing are available we delay this write + dataSources.readingProgress.updateReadingProgress(uid, id, { + readingProgressPercent, + readingProgressTopPercent: readingProgressTopPercent ?? undefined, + readingProgressAnchorIndex: readingProgressAnchorIndex ?? undefined, + }) + + updatedItem = await authTrx( + async (t) => { + return t.getRepository(LibraryItem).findOne({ + where: { + id, + }, + }) + }, + undefined, + uid + ) + } else { + // update reading progress only if the current value is lower + updatedItem = await updateLibraryItemReadingProgress( + id, + uid, + readingProgressPercent, + readingProgressTopPercent, + readingProgressAnchorIndex + ) + } + if (!updatedItem) { return { errorCodes: [SaveArticleReadingProgressErrorCode.BadData] } } diff --git a/packages/api/src/resolvers/function_resolvers.ts b/packages/api/src/resolvers/function_resolvers.ts index bcc17360b..8d3fb3187 100644 --- a/packages/api/src/resolvers/function_resolvers.ts +++ b/packages/api/src/resolvers/function_resolvers.ts @@ -158,6 +158,60 @@ const resultResolveTypeResolver = ( }, }) +const readingProgressHandlers = { + async readingProgressPercent( + article: { id: string; readingProgressPercent?: number }, + _: unknown, + ctx: WithDataSourcesContext + ) { + if (ctx.claims?.uid) { + const readingProgress = + await ctx.dataSources.readingProgress.getReadingProgress( + ctx.claims?.uid, + article.id + ) + if (readingProgress) { + return readingProgress.readingProgressPercent + } + } + return article.readingProgressPercent + }, + async readingProgressAnchorIndex( + article: { id: string; readingProgressAnchorIndex?: number }, + _: unknown, + ctx: WithDataSourcesContext + ) { + if (ctx.claims?.uid) { + const readingProgress = + await ctx.dataSources.readingProgress.getReadingProgress( + ctx.claims?.uid, + article.id + ) + if (readingProgress) { + return readingProgress.readingProgressAnchorIndex + } + } + return article.readingProgressAnchorIndex + }, + async readingProgressTopPercent( + article: { id: string; readingProgressTopPercent?: number }, + _: unknown, + ctx: WithDataSourcesContext + ) { + if (ctx.claims?.uid) { + const readingProgress = + await ctx.dataSources.readingProgress.getReadingProgress( + ctx.claims?.uid, + article.id + ) + if (readingProgress) { + return readingProgress.readingProgressTopPercent + } + } + return article.readingProgressTopPercent + }, +} + // Provide resolver functions for your schema fields export const functionResolvers = { Mutation: { @@ -328,42 +382,7 @@ export const functionResolvers = { return findLabelsByLibraryItemId(article.id, ctx.uid) }, - async readingProgressPercent( - article: { id: string; readingProgressPercent?: number }, - _: unknown, - ctx: WithDataSourcesContext - ) { - const readingProgress = - await ctx.dataSources.readingProgress.getReadingProgress(article.id) - if (readingProgress) { - return readingProgress.readingProgressPercent - } - return article.readingProgressPercent - }, - async readingProgressAnchorIndex( - article: { id: string; readingProgressAnchorIndex?: number }, - _: unknown, - ctx: WithDataSourcesContext - ) { - const readingProgress = - await ctx.dataSources.readingProgress.getReadingProgress(article.id) - if (readingProgress) { - return readingProgress.readingProgressAnchorIndex - } - return article.readingProgressAnchorIndex - }, - async readingProgressTopPercent( - article: { id: string; readingProgressTopPercent?: number }, - _: unknown, - ctx: WithDataSourcesContext - ) { - const readingProgress = - await ctx.dataSources.readingProgress.getReadingProgress(article.id) - if (readingProgress) { - return readingProgress.readingProgressTopPercent - } - return article.readingProgressTopPercent - }, + ...readingProgressHandlers, }, Highlight: { // async reactions( @@ -469,42 +488,7 @@ export const functionResolvers = { const highlights = await findHighlightsByLibraryItemId(item.id, ctx.uid) return highlights.map(highlightDataToHighlight) }, - async readingProgressPercent( - article: { id: string; readingProgressPercent?: number }, - _: unknown, - ctx: WithDataSourcesContext - ) { - const readingProgress = - await ctx.dataSources.readingProgress.getReadingProgress(article.id) - if (readingProgress) { - return readingProgress.readingProgressPercent - } - return article.readingProgressPercent - }, - async readingProgressAnchorIndex( - article: { id: string; readingProgressAnchorIndex?: number }, - _: unknown, - ctx: WithDataSourcesContext - ) { - const readingProgress = - await ctx.dataSources.readingProgress.getReadingProgress(article.id) - if (readingProgress) { - return readingProgress.readingProgressAnchorIndex - } - return article.readingProgressAnchorIndex - }, - async readingProgressTopPercent( - article: { id: string; readingProgressTopPercent?: number }, - _: unknown, - ctx: WithDataSourcesContext - ) { - const readingProgress = - await ctx.dataSources.readingProgress.getReadingProgress(article.id) - if (readingProgress) { - return readingProgress.readingProgressTopPercent - } - return article.readingProgressTopPercent - }, + ...readingProgressHandlers, }, Subscription: { newsletterEmail(subscription: Subscription) { diff --git a/packages/api/src/utils/createTask.ts b/packages/api/src/utils/createTask.ts index 6a30a9ccd..b05387574 100644 --- a/packages/api/src/utils/createTask.ts +++ b/packages/api/src/utils/createTask.ts @@ -666,6 +666,7 @@ export const enqueueTriggerRuleJob = async (data: TriggerRuleJobData) => { attempts: 1, removeOnComplete: true, removeOnFail: true, + priority: 1, }) } diff --git a/packages/web/components/templates/article/Article.tsx b/packages/web/components/templates/article/Article.tsx index 2c3f4a5e4..091ad7e92 100644 --- a/packages/web/components/templates/article/Article.tsx +++ b/packages/web/components/templates/article/Article.tsx @@ -92,7 +92,7 @@ export function Article(props: ArticleProps): JSX.Element { setReadingProgress(bottomProgress * 100) } - }, 2500) + }, 3500) // Scroll to initial anchor position useEffect(() => { From a40f5bed55786b0c3dda3a7afe14372091ee54f9 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Wed, 31 Jan 2024 10:17:51 +0800 Subject: [PATCH 08/32] Create a service for interacting with cached read positions --- .../reading_progress_data_source.ts | 38 ++----- .../src/services/cached_reading_position.ts | 106 ++++++++++++++++++ 2 files changed, 118 insertions(+), 26 deletions(-) create mode 100644 packages/api/src/services/cached_reading_position.ts diff --git a/packages/api/src/datasources/reading_progress_data_source.ts b/packages/api/src/datasources/reading_progress_data_source.ts index 5f9e7358b..3c860556f 100644 --- a/packages/api/src/datasources/reading_progress_data_source.ts +++ b/packages/api/src/datasources/reading_progress_data_source.ts @@ -1,17 +1,14 @@ import { redisDataSource } from '../redis_data_source' - -type ReadingProgressCacheItem = { - readingProgressPercent: number - readingProgressTopPercent: number | undefined - readingProgressAnchorIndex: number | undefined - updatedAt: string -} +import { + ReadingProgressCacheItem, + fetchCachedReadingPosition, + keyForCachedReadingPosition, + pushCachedReadingPosition, +} from '../services/cached_reading_position' export class ReadingProgressDataSource { private cacheItems: { [id: string]: ReadingProgressCacheItem } = {} - constructor() {} - async getReadingProgress( uid: string, libraryItemID: string @@ -21,7 +18,7 @@ export class ReadingProgressDataSource { if (cached) { return cached } - return this.valueFromRedis(cacheKey) + return fetchCachedReadingPosition(uid, libraryItemID) } async updateReadingProgress( @@ -33,11 +30,14 @@ export class ReadingProgressDataSource { readingProgressAnchorIndex: number | undefined } ): Promise { - const cacheKey = `omnivore:reading-progress:${uid}:${libraryItemID}` const cacheItem: ReadingProgressCacheItem = { - ...progress, + uid, + libraryItemID, updatedAt: new Date().toISOString(), + ...progress, } + const cacheKey = keyForCachedReadingPosition(uid, libraryItemID) + pushCachedReadingPosition(uid, libraryItemID, cacheItem) this.cacheItems[cacheKey] = cacheItem if ( @@ -51,18 +51,4 @@ export class ReadingProgressDataSource { console.log('failed to cache reading progress') } } - - async valueFromRedis( - cacheKey: string - ): Promise { - const redisCached = await redisDataSource.redisClient?.lrange( - cacheKey, - 0, - 0 - ) - if (redisCached && redisCached.length > 0) { - return JSON.parse(redisCached[0]) - } - return undefined - } } diff --git a/packages/api/src/services/cached_reading_position.ts b/packages/api/src/services/cached_reading_position.ts new file mode 100644 index 000000000..3ae1965ac --- /dev/null +++ b/packages/api/src/services/cached_reading_position.ts @@ -0,0 +1,106 @@ +import { redisDataSource } from '../redis_data_source' +import { logger } from '../utils/logger' + +export type ReadingProgressCacheItem = { + uid: string + libraryItemID: string + readingProgressPercent: number + readingProgressTopPercent: number | undefined + readingProgressAnchorIndex: number | undefined + updatedAt: string | undefined +} + +export const keyForCachedReadingPosition = ( + uid: string, + libraryItemID: string +): string => { + return `omnivore:reading-progress:${uid}:${libraryItemID}` +} + +// Reading positions are cached as an array of positions, when +// we fetch them from the cache we find the maximum values +export const clearCachedReadingPosition = async ( + uid: string, + libraryItemID: string +): Promise => { + const cacheKey = keyForCachedReadingPosition(uid, libraryItemID) + try { + const res = await redisDataSource.redisClient?.del(cacheKey) + return res ? res > 0 : false + } catch (error) { + logger.error('exception clearing cached reading position', { + cacheKey, + error, + }) + } + return false +} + +export const pushCachedReadingPosition = async ( + uid: string, + libraryItemID: string, + position: ReadingProgressCacheItem +): Promise => { + const cacheKey = keyForCachedReadingPosition(uid, libraryItemID) + try { + const result = await redisDataSource.redisClient?.lpush( + cacheKey, + JSON.stringify(position) + ) + return result ? result > 0 : false + } catch (error) { + logger.error('error writing cached reading position', { cacheKey, error }) + } + return false +} + +// Reading positions are cached as an array of positions, when +// we fetch them from the cache we find the maximum values +export const fetchCachedReadingPosition = async ( + uid: string, + libraryItemID: string +): Promise => { + const cacheKey = keyForCachedReadingPosition(uid, libraryItemID) + try { + const cacheItemList = await redisDataSource.redisClient?.lrange( + cacheKey, + 0, + -1 + ) + const items = cacheItemList?.map((item) => JSON.parse(item)) + if (!items || items.length < 1) { + return undefined + } + + const percent = Math.max( + ...items.map((o) => + 'readingProgressPercent' in o ? o.readingProgressPercent : 0 + ) + ) + const top = Math.max( + ...items.map((o) => + 'readingProgressTopPercent' in o ? o.readingProgressTopPercent : 0 + ) + ) + const anchor = Math.max( + ...items.map((o) => + 'readingProgressAnchorIndex' in o ? o.readingProgressAnchorIndex : 0 + ) + ) + + return { + uid, + libraryItemID, + readingProgressPercent: percent, + readingProgressTopPercent: top, + readingProgressAnchorIndex: anchor, + updatedAt: undefined, + } + } catch (error) { + logger.error('exception looking up cached reading position', { + cacheKey, + error, + }) + } + return undefined +} From 7bb3718a8f4bf371fb84a7fb7bd509e48d04f4f7 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Wed, 31 Jan 2024 12:15:32 +0800 Subject: [PATCH 09/32] Add job to sync read position data --- packages/api/src/jobs/sync_read_positions.ts | 68 +++++++++++++++++++ .../src/services/cached_reading_position.ts | 22 +++++- 2 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 packages/api/src/jobs/sync_read_positions.ts diff --git a/packages/api/src/jobs/sync_read_positions.ts b/packages/api/src/jobs/sync_read_positions.ts new file mode 100644 index 000000000..a91182048 --- /dev/null +++ b/packages/api/src/jobs/sync_read_positions.ts @@ -0,0 +1,68 @@ +import Redis from 'ioredis' +import { redisDataSource } from '../redis_data_source' +import { + CACHED_READING_POSITION_PREFIX, + componentsForCachedReadingPositionKey, + fetchCachedReadingPosition, +} from '../services/cached_reading_position' +import { logger } from '../utils/logger' +import { updateLibraryItemReadingProgress } from '../services/library_item' + +export const SYNC_READ_POSITIONS_JOB_NAME = 'sync-read-positions' + +async function* getSyncUpdatesIterator(redis: Redis) { + const match = `${CACHED_READING_POSITION_PREFIX}:*` + let [cursor, batch]: [string | number, string[]] = [0, []] + do { + ;[cursor, batch] = await redis.scan(cursor, 'MATCH', match, 'COUNT', 100) + if (batch.length) { + for (const key of batch) { + yield key + } + } + } while (cursor !== '0') + return +} + +const syncReadPosition = async (cacheKey: string) => { + const components = componentsForCachedReadingPositionKey(cacheKey) + const position = components + ? await fetchCachedReadingPosition(components.uid, components.libraryItemID) + : undefined + if (components && position) { + const result = await updateLibraryItemReadingProgress( + components.libraryItemID, + components.uid, + position.readingProgressPercent, + position.readingProgressTopPercent, + position.readingProgressAnchorIndex + ) + if (!result) { + logger.error('unable to update reading progress', { cacheKey }) + } + } else { + logger.warning( + 'potential error, reading position cache key found with no data', + { cacheKey } + ) + } + // Even if there are errors above we want to delete the key, otherwise + // in error scenarios we could accumulate a huge number of keys for + // something that is not critical (reading position) + const result = await redisDataSource.redisClient?.del(cacheKey) + if (!result || result < 1) { + logger.warning('error deleting cache key', { cacheKey }) + } +} + +export const syncReadPositionsJob = async (data: any, attempts: number) => { + const redis = redisDataSource.redisClient + if (!redis) { + throw new Error('unable to sync reading position, no redis client') + } + + const updates = getSyncUpdatesIterator(redis) + for await (const value of updates) { + await syncReadPosition(value) + } +} diff --git a/packages/api/src/services/cached_reading_position.ts b/packages/api/src/services/cached_reading_position.ts index 3ae1965ac..44e7edc72 100644 --- a/packages/api/src/services/cached_reading_position.ts +++ b/packages/api/src/services/cached_reading_position.ts @@ -10,11 +10,28 @@ export type ReadingProgressCacheItem = { updatedAt: string | undefined } +export const CACHED_READING_POSITION_PREFIX = `omnivore:reading-progress` + export const keyForCachedReadingPosition = ( uid: string, libraryItemID: string ): string => { - return `omnivore:reading-progress:${uid}:${libraryItemID}` + return `${CACHED_READING_POSITION_PREFIX}:${uid}:${libraryItemID}` +} + +export const componentsForCachedReadingPositionKey = ( + cacheKey: string +): { uid: string; libraryItemID: string } | undefined => { + try { + const [_owner, _prefix, uid, libraryItemID] = cacheKey.split(':') + return { + uid, + libraryItemID, + } + } catch (error) { + logger.log('exception getting cache key components', { cacheKey, error }) + } + return undefined } // Reading positions are cached as an array of positions, when @@ -60,6 +77,7 @@ export const fetchCachedReadingPosition = async ( uid: string, libraryItemID: string ): Promise => { + console.log('checking uid', uid, 'libraryItemId', libraryItemID) const cacheKey = keyForCachedReadingPosition(uid, libraryItemID) try { const cacheItemList = await redisDataSource.redisClient?.lrange( @@ -67,7 +85,9 @@ export const fetchCachedReadingPosition = async ( 0, -1 ) + console.log('cacheItemList: ', cacheKey, cacheItemList) const items = cacheItemList?.map((item) => JSON.parse(item)) + console.log(' items[]: ', items) if (!items || items.length < 1) { return undefined } From d8ea4ff1c5a496098d691841543e19ca48a53a48 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Wed, 31 Jan 2024 12:39:09 +0800 Subject: [PATCH 10/32] Add reading position messages to exported metrics for queue processor --- packages/api/src/queue-processor.ts | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/packages/api/src/queue-processor.ts b/packages/api/src/queue-processor.ts index afaa5b3ab..ea458a6fe 100644 --- a/packages/api/src/queue-processor.ts +++ b/packages/api/src/queue-processor.ts @@ -32,6 +32,7 @@ import { SYNC_READ_POSITIONS_JOB_NAME, syncReadPositionsJob, } from './jobs/sync_read_positions' +import { CACHED_READING_POSITION_PREFIX } from './services/cached_reading_position' export const QUEUE_NAME = 'omnivore-backend-queue' @@ -136,6 +137,26 @@ const main = async () => { output += `omnivore_queue_messages_${metric}{queue="${QUEUE_NAME}"} ${counts[metric]}\n` }) + if (redisDataSource.redisClient) { + // Add read-position count, if its more than 10K items just denote + // 10_001. As this should never occur and means there is some + // other serious issue occurring. + const [cursor, batch] = await redisDataSource.redisClient.scan( + 0, + 'MATCH', + `${CACHED_READING_POSITION_PREFIX}:*`, + 'COUNT', + 10_000 + ) + if (cursor != '0') { + output += `# TYPE omnivore_read_position_messages gauge\n` + output += `omnivore_read_position_messages{queue="${QUEUE_NAME}"} ${10_001}\n` + } else if (batch) { + output += `# TYPE omnivore_read_position_messages gauge\n` + output += `omnivore_read_position_messages{queue="${QUEUE_NAME}"} ${batch.length}\n` + } + } + res.status(200).setHeader('Content-Type', 'text/plain').send(output) }) From 9dc070243a92e8c7a28388c8293081a9539a942a Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Wed, 31 Jan 2024 12:40:02 +0800 Subject: [PATCH 11/32] Revert temp --- packages/api/src/utils/createTask.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/api/src/utils/createTask.ts b/packages/api/src/utils/createTask.ts index b05387574..6a30a9ccd 100644 --- a/packages/api/src/utils/createTask.ts +++ b/packages/api/src/utils/createTask.ts @@ -666,7 +666,6 @@ export const enqueueTriggerRuleJob = async (data: TriggerRuleJobData) => { attempts: 1, removeOnComplete: true, removeOnFail: true, - priority: 1, }) } From a060aaf4961fd9e9db8ffbd398c4eb52786417ac Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Wed, 31 Jan 2024 13:32:14 +0800 Subject: [PATCH 12/32] Make reading position parsing safer --- .../reading_progress_data_source.ts | 14 ++------ .../src/services/cached_reading_position.ts | 33 +++++++++++++++---- 2 files changed, 28 insertions(+), 19 deletions(-) diff --git a/packages/api/src/datasources/reading_progress_data_source.ts b/packages/api/src/datasources/reading_progress_data_source.ts index 3c860556f..6c08eca35 100644 --- a/packages/api/src/datasources/reading_progress_data_source.ts +++ b/packages/api/src/datasources/reading_progress_data_source.ts @@ -37,18 +37,8 @@ export class ReadingProgressDataSource { ...progress, } const cacheKey = keyForCachedReadingPosition(uid, libraryItemID) - pushCachedReadingPosition(uid, libraryItemID, cacheItem) - - this.cacheItems[cacheKey] = cacheItem - if ( - await redisDataSource.redisClient?.lpush( - cacheKey, - JSON.stringify(cacheItem) - ) - ) { - console.log('cached reading progress', cacheKey) - } else { - console.log('failed to cache reading progress') + if (await pushCachedReadingPosition(uid, libraryItemID, cacheItem)) { + this.cacheItems[cacheKey] = cacheItem } } } diff --git a/packages/api/src/services/cached_reading_position.ts b/packages/api/src/services/cached_reading_position.ts index 44e7edc72..4b47a49a5 100644 --- a/packages/api/src/services/cached_reading_position.ts +++ b/packages/api/src/services/cached_reading_position.ts @@ -1,6 +1,8 @@ import { redisDataSource } from '../redis_data_source' import { logger } from '../utils/logger' +export const CACHED_READING_POSITION_PREFIX = `omnivore:reading-progress` + export type ReadingProgressCacheItem = { uid: string libraryItemID: string @@ -10,7 +12,23 @@ export type ReadingProgressCacheItem = { updatedAt: string | undefined } -export const CACHED_READING_POSITION_PREFIX = `omnivore:reading-progress` +export const isReadingProgressCacheItem = ( + item: any +): item is ReadingProgressCacheItem => { + return ( + 'uid' in item && 'libraryItemID' in item && 'readingProgressPercent' in item + ) +} + +export const parseReadingProgressCacheItem = ( + item: any +): ReadingProgressCacheItem | undefined => { + const result = JSON.parse(item) as unknown + if (isReadingProgressCacheItem(result)) { + return result + } + return undefined +} export const keyForCachedReadingPosition = ( uid: string, @@ -77,7 +95,6 @@ export const fetchCachedReadingPosition = async ( uid: string, libraryItemID: string ): Promise => { - console.log('checking uid', uid, 'libraryItemId', libraryItemID) const cacheKey = keyForCachedReadingPosition(uid, libraryItemID) try { const cacheItemList = await redisDataSource.redisClient?.lrange( @@ -85,9 +102,9 @@ export const fetchCachedReadingPosition = async ( 0, -1 ) - console.log('cacheItemList: ', cacheKey, cacheItemList) - const items = cacheItemList?.map((item) => JSON.parse(item)) - console.log(' items[]: ', items) + const items = cacheItemList + ?.map((item) => parseReadingProgressCacheItem(item)) + .filter(isReadingProgressCacheItem) if (!items || items.length < 1) { return undefined } @@ -99,12 +116,14 @@ export const fetchCachedReadingPosition = async ( ) const top = Math.max( ...items.map((o) => - 'readingProgressTopPercent' in o ? o.readingProgressTopPercent : 0 + 'readingProgressTopPercent' in o ? o.readingProgressTopPercent ?? 0 : 0 ) ) const anchor = Math.max( ...items.map((o) => - 'readingProgressAnchorIndex' in o ? o.readingProgressAnchorIndex : 0 + 'readingProgressAnchorIndex' in o + ? o.readingProgressAnchorIndex ?? 0 + : 0 ) ) From 97efbe4487e8f5b44f4a2cd1804e9a40eb9c71c4 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Wed, 31 Jan 2024 14:15:09 +0800 Subject: [PATCH 13/32] Force read position writes when user explicitly sets it --- packages/web/lib/networking/queries/useGetLibraryItemsQuery.tsx | 2 ++ packages/web/pages/[username]/[slug]/index.tsx | 1 + 2 files changed, 3 insertions(+) diff --git a/packages/web/lib/networking/queries/useGetLibraryItemsQuery.tsx b/packages/web/lib/networking/queries/useGetLibraryItemsQuery.tsx index cddf3d6f4..c289b3f7c 100644 --- a/packages/web/lib/networking/queries/useGetLibraryItemsQuery.tsx +++ b/packages/web/lib/networking/queries/useGetLibraryItemsQuery.tsx @@ -385,6 +385,7 @@ export function useGetLibraryItemsQuery({ }) articleReadingProgressMutation({ id: item.node.id, + force: true, readingProgressPercent: 100, readingProgressTopPercent: 100, readingProgressAnchorIndex: 0, @@ -402,6 +403,7 @@ export function useGetLibraryItemsQuery({ }) articleReadingProgressMutation({ id: item.node.id, + force: true, readingProgressPercent: 0, readingProgressTopPercent: 0, readingProgressAnchorIndex: 0, diff --git a/packages/web/pages/[username]/[slug]/index.tsx b/packages/web/pages/[username]/[slug]/index.tsx index 0293a3da0..22237f74b 100644 --- a/packages/web/pages/[username]/[slug]/index.tsx +++ b/packages/web/pages/[username]/[slug]/index.tsx @@ -155,6 +155,7 @@ export default function Home(): JSX.Element { if (article) { articleReadingProgressMutation({ id: article.id, + force: true, readingProgressPercent: 100, readingProgressTopPercent: 100, readingProgressAnchorIndex: 0, From bdf01ddaab8da23d54087183578d6950c804c5c0 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Wed, 31 Jan 2024 14:15:29 +0800 Subject: [PATCH 14/32] Clear cache on force, return updated read position values --- packages/api/src/resolvers/article/index.ts | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index 2b95913f8..116af997f 100644 --- a/packages/api/src/resolvers/article/index.ts +++ b/packages/api/src/resolvers/article/index.ts @@ -112,6 +112,7 @@ import { parsePreparedContent, } from '../../utils/parser' import { getStorageFileDetails } from '../../utils/uploads' +import { clearCachedReadingPosition } from '../../services/cached_reading_position' export enum ArticleFormat { Markdown = 'markdown', @@ -621,7 +622,10 @@ export const saveArticleReadingProgressResolver = authorized< } try { if (force) { - // update reading progress without checking the current value + // update reading progress without checking the current value, also + // clear any cached values. + await clearCachedReadingPosition(uid, id) + const updatedItem = await updateLibraryItem( id, { @@ -660,6 +664,17 @@ export const saveArticleReadingProgressResolver = authorized< undefined, uid ) + if (updatedItem) { + updatedItem.readAt = new Date() + updatedItem.readingProgressBottomPercent = readingProgressPercent + if (readingProgressTopPercent) { + updatedItem.readingProgressTopPercent = readingProgressTopPercent + } + if (readingProgressAnchorIndex) { + updatedItem.readingProgressLastReadAnchor = + readingProgressAnchorIndex + } + } } else { // update reading progress only if the current value is lower updatedItem = await updateLibraryItemReadingProgress( From c47a720fe4b15c00708c4569b6aca902dba90a29 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Wed, 31 Jan 2024 15:09:40 +0800 Subject: [PATCH 15/32] Do not cache on write, so the most updated item is always available on read This is mostly an issue on tests where all the queries can be run in a single process, but if we cache on write, it means the next read will just have the cached value, instead of the calculated maximum value. --- .../reading_progress_data_source.ts | 8 ++--- packages/api/src/resolvers/article/index.ts | 31 ++++++++----------- 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/packages/api/src/datasources/reading_progress_data_source.ts b/packages/api/src/datasources/reading_progress_data_source.ts index 6c08eca35..1f1702810 100644 --- a/packages/api/src/datasources/reading_progress_data_source.ts +++ b/packages/api/src/datasources/reading_progress_data_source.ts @@ -29,16 +29,14 @@ export class ReadingProgressDataSource { readingProgressTopPercent: number | undefined readingProgressAnchorIndex: number | undefined } - ): Promise { + ): Promise { const cacheItem: ReadingProgressCacheItem = { uid, libraryItemID, updatedAt: new Date().toISOString(), ...progress, } - const cacheKey = keyForCachedReadingPosition(uid, libraryItemID) - if (await pushCachedReadingPosition(uid, libraryItemID, cacheItem)) { - this.cacheItems[cacheKey] = cacheItem - } + await pushCachedReadingPosition(uid, libraryItemID, cacheItem) + return fetchCachedReadingPosition(uid, libraryItemID) } } diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index 116af997f..5bb26b2e6 100644 --- a/packages/api/src/resolvers/article/index.ts +++ b/packages/api/src/resolvers/article/index.ts @@ -112,7 +112,10 @@ import { parsePreparedContent, } from '../../utils/parser' import { getStorageFileDetails } from '../../utils/uploads' -import { clearCachedReadingPosition } from '../../services/cached_reading_position' +import { + clearCachedReadingPosition, + fetchCachedReadingPosition, +} from '../../services/cached_reading_position' export enum ArticleFormat { Markdown = 'markdown', @@ -647,12 +650,16 @@ export const saveArticleReadingProgressResolver = authorized< let updatedItem: LibraryItem | null if (env.redis.cache && env.redis.mq) { // If redis caching and queueing are available we delay this write - dataSources.readingProgress.updateReadingProgress(uid, id, { - readingProgressPercent, - readingProgressTopPercent: readingProgressTopPercent ?? undefined, - readingProgressAnchorIndex: readingProgressAnchorIndex ?? undefined, - }) + const updatedProgress = + await dataSources.readingProgress.updateReadingProgress(uid, id, { + readingProgressPercent, + readingProgressTopPercent: readingProgressTopPercent ?? undefined, + readingProgressAnchorIndex: readingProgressAnchorIndex ?? undefined, + }) + // We don't need to update the values of reading progress here + // because the function resolver will handle that for us when + // it resolves the properties of the Article object updatedItem = await authTrx( async (t) => { return t.getRepository(LibraryItem).findOne({ @@ -664,19 +671,7 @@ export const saveArticleReadingProgressResolver = authorized< undefined, uid ) - if (updatedItem) { - updatedItem.readAt = new Date() - updatedItem.readingProgressBottomPercent = readingProgressPercent - if (readingProgressTopPercent) { - updatedItem.readingProgressTopPercent = readingProgressTopPercent - } - if (readingProgressAnchorIndex) { - updatedItem.readingProgressLastReadAnchor = - readingProgressAnchorIndex - } - } } else { - // update reading progress only if the current value is lower updatedItem = await updateLibraryItemReadingProgress( id, uid, From 0e37bfa03d62738537011d3b5bbd3e75349d4325 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Wed, 31 Jan 2024 15:23:39 +0800 Subject: [PATCH 16/32] We need to update readAt because that isnt resolved from the cache --- packages/api/src/resolvers/article/index.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index 5bb26b2e6..b68bf7024 100644 --- a/packages/api/src/resolvers/article/index.ts +++ b/packages/api/src/resolvers/article/index.ts @@ -671,6 +671,9 @@ export const saveArticleReadingProgressResolver = authorized< undefined, uid ) + if (updatedItem) { + updatedItem.readAt = new Date() + } } else { updatedItem = await updateLibraryItemReadingProgress( id, From 3ecbd3f434e41fc2b28254fcd2554aae52c5f91e Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Wed, 31 Jan 2024 15:24:09 +0800 Subject: [PATCH 17/32] Need to force this test to overwrite the cached value --- packages/api/test/resolvers/article.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/api/test/resolvers/article.test.ts b/packages/api/test/resolvers/article.test.ts index 1b49a5a83..2587e4754 100644 --- a/packages/api/test/resolvers/article.test.ts +++ b/packages/api/test/resolvers/article.test.ts @@ -628,7 +628,6 @@ describe('Article API', () => { ).expect(200) const savedItem = await findLibraryItemByUrl(url, user.id) - console.log('savedItem: ', savedItem) expect(savedItem?.archivedAt).to.not.be.null expect(savedItem?.labels?.map((l) => l.name)).to.eql(labels) }) @@ -779,7 +778,12 @@ describe('Article API', () => { it('saves topPercent as 0 if defined as 0', async () => { const topPercent = 0 - query = saveArticleReadingProgressQuery(itemId, progress, topPercent) + query = saveArticleReadingProgressQuery( + itemId, + progress, + topPercent, + true + ) const res = await graphqlRequest(query, authToken).expect(200) expect( res.body.data.saveArticleReadingProgress.updatedArticle From 38a90e39874ec91cb314b6f3a788375cee169668 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Wed, 31 Jan 2024 15:29:52 +0800 Subject: [PATCH 18/32] Revert web change --- packages/web/components/templates/article/Article.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/web/components/templates/article/Article.tsx b/packages/web/components/templates/article/Article.tsx index 091ad7e92..2c3f4a5e4 100644 --- a/packages/web/components/templates/article/Article.tsx +++ b/packages/web/components/templates/article/Article.tsx @@ -92,7 +92,7 @@ export function Article(props: ArticleProps): JSX.Element { setReadingProgress(bottomProgress * 100) } - }, 3500) + }, 2500) // Scroll to initial anchor position useEffect(() => { From 570b7a4b7826b12a5875378e95c8c1609b4b99c6 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Wed, 31 Jan 2024 20:20:27 +0800 Subject: [PATCH 19/32] Use a set in redis and push items to the set This lets us grab all available items, apply them, and then remove them from the set. Once completed the items that were applied can be removed from the set, if any new items were added during that time they wont be effected. --- packages/api/src/jobs/sync_read_positions.ts | 54 ++++++++++----- packages/api/src/queue-processor.ts | 3 +- .../src/services/cached_reading_position.ts | 67 ++++++++++++++++--- 3 files changed, 95 insertions(+), 29 deletions(-) diff --git a/packages/api/src/jobs/sync_read_positions.ts b/packages/api/src/jobs/sync_read_positions.ts index a91182048..2223ef6c7 100644 --- a/packages/api/src/jobs/sync_read_positions.ts +++ b/packages/api/src/jobs/sync_read_positions.ts @@ -3,7 +3,8 @@ import { redisDataSource } from '../redis_data_source' import { CACHED_READING_POSITION_PREFIX, componentsForCachedReadingPositionKey, - fetchCachedReadingPosition, + fetchCachedReadingPositionsAndMembers, + reduceCachedReadingPositionMembers, } from '../services/cached_reading_position' import { logger } from '../utils/logger' import { updateLibraryItemReadingProgress } from '../services/library_item' @@ -26,19 +27,43 @@ async function* getSyncUpdatesIterator(redis: Redis) { const syncReadPosition = async (cacheKey: string) => { const components = componentsForCachedReadingPositionKey(cacheKey) - const position = components - ? await fetchCachedReadingPosition(components.uid, components.libraryItemID) + const positions = components + ? await fetchCachedReadingPositionsAndMembers( + components.uid, + components.libraryItemID + ) : undefined - if (components && position) { - const result = await updateLibraryItemReadingProgress( - components.libraryItemID, + if ( + components && + positions && + positions.positionItems && + positions.positionItems.length > 0 + ) { + const position = await reduceCachedReadingPositionMembers( components.uid, - position.readingProgressPercent, - position.readingProgressTopPercent, - position.readingProgressAnchorIndex + components.libraryItemID, + positions.positionItems ) - if (!result) { - logger.error('unable to update reading progress', { cacheKey }) + if (position) { + // this will throw if there is an error + await updateLibraryItemReadingProgress( + components.libraryItemID, + components.uid, + position.readingProgressPercent, + position.readingProgressTopPercent, + position.readingProgressAnchorIndex + ) + } + + const removed = await redisDataSource.redisClient?.srem( + cacheKey, + ...positions.members + ) + if (!removed || removed < positions.members.length) { + logger.warning( + 'potential error, reading position cache key members not removed', + { cacheKey } + ) } } else { logger.warning( @@ -46,13 +71,6 @@ const syncReadPosition = async (cacheKey: string) => { { cacheKey } ) } - // Even if there are errors above we want to delete the key, otherwise - // in error scenarios we could accumulate a huge number of keys for - // something that is not critical (reading position) - const result = await redisDataSource.redisClient?.del(cacheKey) - if (!result || result < 1) { - logger.warning('error deleting cache key', { cacheKey }) - } } export const syncReadPositionsJob = async (data: any, attempts: number) => { diff --git a/packages/api/src/queue-processor.ts b/packages/api/src/queue-processor.ts index ea458a6fe..54a9c5bad 100644 --- a/packages/api/src/queue-processor.ts +++ b/packages/api/src/queue-processor.ts @@ -179,13 +179,14 @@ const main = async () => { const queue = await getBackendQueue() if (queue) { + // run every 60s await queue.add( SYNC_READ_POSITIONS_JOB_NAME, {}, { priority: 1, repeat: { - every: 10000, + every: 60_000, limit: 100, }, } diff --git a/packages/api/src/services/cached_reading_position.ts b/packages/api/src/services/cached_reading_position.ts index 4b47a49a5..c6635cb1a 100644 --- a/packages/api/src/services/cached_reading_position.ts +++ b/packages/api/src/services/cached_reading_position.ts @@ -78,7 +78,10 @@ export const pushCachedReadingPosition = async ( ): Promise => { const cacheKey = keyForCachedReadingPosition(uid, libraryItemID) try { - const result = await redisDataSource.redisClient?.lpush( + // Its critical that the date is set so the entry will be a unique + // set value. + position.updatedAt = new Date().toISOString() + const result = await redisDataSource.redisClient?.sadd( cacheKey, JSON.stringify(position) ) @@ -95,16 +98,35 @@ export const fetchCachedReadingPosition = async ( uid: string, libraryItemID: string ): Promise => { - const cacheKey = keyForCachedReadingPosition(uid, libraryItemID) try { - const cacheItemList = await redisDataSource.redisClient?.lrange( - cacheKey, - 0, - -1 + const items = await fetchCachedReadingPositionsAndMembers( + uid, + libraryItemID ) - const items = cacheItemList - ?.map((item) => parseReadingProgressCacheItem(item)) - .filter(isReadingProgressCacheItem) + if (!items) { + return undefined + } + return reduceCachedReadingPositionMembers( + uid, + libraryItemID, + items.positionItems + ) + } catch (error) { + logger.error('exception looking up cached reading position', { + uid, + libraryItemID, + error, + }) + } + return undefined +} + +export const reduceCachedReadingPositionMembers = async ( + uid: string, + libraryItemID: string, + items: ReadingProgressCacheItem[] +): Promise => { + try { if (!items || items.length < 1) { return undefined } @@ -126,7 +148,6 @@ export const fetchCachedReadingPosition = async ( : 0 ) ) - return { uid, libraryItemID, @@ -135,6 +156,32 @@ export const fetchCachedReadingPosition = async ( readingProgressAnchorIndex: anchor, updatedAt: undefined, } + } catch (error) { + logger.error('exception reducing cached reading items', { + uid, + libraryItemID, + error, + }) + } + return undefined +} + +export const fetchCachedReadingPositionsAndMembers = async ( + uid: string, + libraryItemID: string +): Promise< + { positionItems: ReadingProgressCacheItem[]; members: string[] } | undefined +> => { + const cacheKey = keyForCachedReadingPosition(uid, libraryItemID) + try { + const members = await redisDataSource.redisClient?.smembers(cacheKey) + if (!members) { + return undefined + } + const positionItems = members + ?.map((item) => parseReadingProgressCacheItem(item)) + .filter(isReadingProgressCacheItem) + return { members, positionItems } } catch (error) { logger.error('exception looking up cached reading position', { cacheKey, From ddf7d590fd22ab535c164b54f25e094afb33cc69 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Wed, 31 Jan 2024 21:04:21 +0800 Subject: [PATCH 20/32] Copy params so no extra params are sent --- .../mutations/mergeHighlightMutation.ts | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/packages/web/lib/networking/mutations/mergeHighlightMutation.ts b/packages/web/lib/networking/mutations/mergeHighlightMutation.ts index 4b7d23bda..9da67bb8d 100644 --- a/packages/web/lib/networking/mutations/mergeHighlightMutation.ts +++ b/packages/web/lib/networking/mutations/mergeHighlightMutation.ts @@ -57,7 +57,22 @@ export async function mergeHighlightMutation( ` try { - const data = await gqlFetcher(mutation, { input }) + const data = await gqlFetcher(mutation, { + input: { + id: input.id, + shortId: input.shortId, + articleId: input.articleId, + patch: input.patch, + quote: input.quote, + prefix: input.prefix, + suffix: input.suffix, + html: input.html, + annotation: input.annotation, + overlapHighlightIdList: input.overlapHighlightIdList, + highlightPositionPercent: input.highlightPositionPercent, + highlightPositionAnchorIndex: input.highlightPositionAnchorIndex, + }, + }) const output = data as MergeHighlightOutput | undefined return output?.mergeHighlight.highlight } catch { From 7d6645b87a55977c0aac32b878d9ed9bd28ce12e Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Thu, 1 Feb 2024 10:02:38 +0800 Subject: [PATCH 21/32] Rebase --- packages/api/src/queue-processor.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/api/src/queue-processor.ts b/packages/api/src/queue-processor.ts index 54a9c5bad..724a69f6e 100644 --- a/packages/api/src/queue-processor.ts +++ b/packages/api/src/queue-processor.ts @@ -82,6 +82,8 @@ export const createWorker = (connection: ConnectionOptions) => return updateLabels(job.data) case UPDATE_HIGHLIGHT_JOB: return updateHighlight(job.data) + case SYNC_READ_POSITIONS_JOB_NAME: + return syncReadPositionsJob(job.data) } }, { From 1b44cb860bea59234c3148c9d1824298e00ac76d Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Thu, 1 Feb 2024 10:33:51 +0800 Subject: [PATCH 22/32] Function for cron jobs dont include queue in the read position metrics since its not part of that queue --- packages/api/src/jobs/sync_read_positions.ts | 2 +- packages/api/src/queue-processor.ts | 40 +++++++++++--------- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/packages/api/src/jobs/sync_read_positions.ts b/packages/api/src/jobs/sync_read_positions.ts index 2223ef6c7..818347865 100644 --- a/packages/api/src/jobs/sync_read_positions.ts +++ b/packages/api/src/jobs/sync_read_positions.ts @@ -73,7 +73,7 @@ const syncReadPosition = async (cacheKey: string) => { } } -export const syncReadPositionsJob = async (data: any, attempts: number) => { +export const syncReadPositionsJob = async (_data: any) => { const redis = redisDataSource.redisClient if (!redis) { throw new Error('unable to sync reading position, no redis client') diff --git a/packages/api/src/queue-processor.ts b/packages/api/src/queue-processor.ts index 724a69f6e..d1ec1ed2b 100644 --- a/packages/api/src/queue-processor.ts +++ b/packages/api/src/queue-processor.ts @@ -27,7 +27,7 @@ import { } from './jobs/update_db' import { updatePDFContentJob } from './jobs/update_pdf_content' import { redisDataSource } from './redis_data_source' -import { CustomTypeOrmLogger } from './utils/logger' +import { logger, CustomTypeOrmLogger } from './utils/logger' import { SYNC_READ_POSITIONS_JOB_NAME, syncReadPositionsJob, @@ -91,6 +91,26 @@ export const createWorker = (connection: ConnectionOptions) => } ) +const setupCronJobs = async () => { + const queue = await getBackendQueue() + if (!queue) { + logger.error('Unable to setup cron jobs. Queue is not available.') + return + } + + await queue.add( + SYNC_READ_POSITIONS_JOB_NAME, + {}, + { + priority: 1, + repeat: { + every: 60_000, + limit: 100, + }, + } + ) +} + const main = async () => { console.log('[queue-processor]: starting queue processor') @@ -155,7 +175,7 @@ const main = async () => { output += `omnivore_read_position_messages{queue="${QUEUE_NAME}"} ${10_001}\n` } else if (batch) { output += `# TYPE omnivore_read_position_messages gauge\n` - output += `omnivore_read_position_messages{queue="${QUEUE_NAME}"} ${batch.length}\n` + output += `omnivore_read_position_messages{} ${batch.length}\n` } } @@ -179,21 +199,7 @@ const main = async () => { const worker = createWorker(workerRedisClient) - const queue = await getBackendQueue() - if (queue) { - // run every 60s - await queue.add( - SYNC_READ_POSITIONS_JOB_NAME, - {}, - { - priority: 1, - repeat: { - every: 60_000, - limit: 100, - }, - } - ) - } + await setupCronJobs() const queueEvents = new QueueEvents(QUEUE_NAME, { connection: workerRedisClient, From 792f13edfca6f228758aff67ebd4672848394106 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Thu, 1 Feb 2024 10:49:48 +0800 Subject: [PATCH 23/32] Remove async from non-async function --- packages/api/src/services/cached_reading_position.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/api/src/services/cached_reading_position.ts b/packages/api/src/services/cached_reading_position.ts index c6635cb1a..a516c4978 100644 --- a/packages/api/src/services/cached_reading_position.ts +++ b/packages/api/src/services/cached_reading_position.ts @@ -121,7 +121,7 @@ export const fetchCachedReadingPosition = async ( return undefined } -export const reduceCachedReadingPositionMembers = async ( +export const reduceCachedReadingPositionMembers = ( uid: string, libraryItemID: string, items: ReadingProgressCacheItem[] From 177a3c3f90a34d58cbce0c079fce259430738a59 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 1 Feb 2024 11:09:20 +0800 Subject: [PATCH 24/32] fix: ios add labels not working in subscription settings * creating wrong filter in the rule --- .../Sources/App/Views/Profile/SubscriptionsView.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apple/OmnivoreKit/Sources/App/Views/Profile/SubscriptionsView.swift b/apple/OmnivoreKit/Sources/App/Views/Profile/SubscriptionsView.swift index a0e1ccb2f..0f6507bfc 100644 --- a/apple/OmnivoreKit/Sources/App/Views/Profile/SubscriptionsView.swift +++ b/apple/OmnivoreKit/Sources/App/Views/Profile/SubscriptionsView.swift @@ -406,14 +406,14 @@ struct SubscriptionSettingsView: View { } var ruleName: String { - if let url = subscription.url, subscription.type == .newsletter { + if let url = subscription.url, subscription.type == .feed { return "system.autoLabel.(\(url))" } return "system.autoLabel.(\(subscription.name))" } var ruleFilter: String { - if let url = subscription.url, subscription.type == .newsletter { + if let url = subscription.url, subscription.type == .feed { return "rss:\"\(url)\"" } return "subscription:\"\(subscription.name)\"" From 02c28be403361f3906cedbac8692394b8b258f12 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Thu, 1 Feb 2024 11:24:13 +0800 Subject: [PATCH 25/32] reduce unction is no longer async --- packages/api/src/jobs/sync_read_positions.ts | 2 +- packages/api/src/services/cached_reading_position.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/api/src/jobs/sync_read_positions.ts b/packages/api/src/jobs/sync_read_positions.ts index 818347865..1b25eab13 100644 --- a/packages/api/src/jobs/sync_read_positions.ts +++ b/packages/api/src/jobs/sync_read_positions.ts @@ -39,7 +39,7 @@ const syncReadPosition = async (cacheKey: string) => { positions.positionItems && positions.positionItems.length > 0 ) { - const position = await reduceCachedReadingPositionMembers( + const position = reduceCachedReadingPositionMembers( components.uid, components.libraryItemID, positions.positionItems diff --git a/packages/api/src/services/cached_reading_position.ts b/packages/api/src/services/cached_reading_position.ts index a516c4978..b69a82ac8 100644 --- a/packages/api/src/services/cached_reading_position.ts +++ b/packages/api/src/services/cached_reading_position.ts @@ -125,7 +125,7 @@ export const reduceCachedReadingPositionMembers = ( uid: string, libraryItemID: string, items: ReadingProgressCacheItem[] -): Promise => { +): ReadingProgressCacheItem | undefined => { try { if (!items || items.length < 1) { return undefined From e13c418389676726ecee80f744802a02f400542d Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 1 Feb 2024 13:09:50 +0800 Subject: [PATCH 26/32] remove the inner join with highlights table in the sql of update-label job --- packages/api/src/jobs/update_db.ts | 34 ++++++++++++++---------------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/packages/api/src/jobs/update_db.ts b/packages/api/src/jobs/update_db.ts index dcde84383..d178ad384 100644 --- a/packages/api/src/jobs/update_db.ts +++ b/packages/api/src/jobs/update_db.ts @@ -17,16 +17,15 @@ export const updateLabels = async (data: UpdateLabelsData) => { return authTrx( async (tx) => tx.query( - `WITH labels_agg AS ( - SELECT array_agg(DISTINCT l.name) AS names_agg - FROM omnivore.labels l - INNER JOIN omnivore.entity_labels el ON el.label_id = l.id - LEFT JOIN omnivore.highlight h ON h.id = el.highlight_id - WHERE el.library_item_id = $1 OR h.library_item_id = $1 - ) - UPDATE omnivore.library_item li - SET label_names = COALESCE((SELECT names_agg FROM labels_agg), ARRAY[]::TEXT[]) - WHERE li.id = $1`, + `UPDATE omnivore.library_item + SET label_names = COALESCE(( + SELECT array_agg(DISTINCT l.name) + FROM omnivore.labels l + INNER JOIN omnivore.entity_labels el + ON el.label_id = l.id + AND el.library_item_id = $1 + ), ARRAY[]::TEXT[]) + WHERE id = $1`, [data.libraryItemId] ), undefined, @@ -38,14 +37,13 @@ export const updateHighlight = async (data: UpdateHighlightData) => { return authTrx( async (tx) => tx.query( - `WITH highlight_agg AS ( - SELECT array_agg(COALESCE(annotation, '')) AS annotation_agg - FROM omnivore.highlight - WHERE library_item_id = $1 - ) - UPDATE omnivore.library_item - SET highlight_annotations = COALESCE((SELECT annotation_agg FROM highlight_agg), ARRAY[]::TEXT[]) - WHERE id = $1`, + `UPDATE omnivore.library_item + SET highlight_annotations = COALESCE(( + SELECT array_agg(COALESCE(annotation, '')) + FROM omnivore.highlight + WHERE library_item_id = $1 + ), ARRAY[]::TEXT[]) + WHERE id = $1`, [data.libraryItemId] ), undefined, From 19fe60d27a191ebe7164fbe1e3880550202238a2 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 1 Feb 2024 13:14:01 +0800 Subject: [PATCH 27/32] change the update-db job priority to 5 and use exponential backoff strategy for retrying --- packages/api/src/utils/createTask.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/api/src/utils/createTask.ts b/packages/api/src/utils/createTask.ts index 6a30a9ccd..b4edb4e0d 100644 --- a/packages/api/src/utils/createTask.ts +++ b/packages/api/src/utils/createTask.ts @@ -679,7 +679,12 @@ export const bulkEnqueueUpdateLabels = async (data: UpdateLabelsData[]) => { name: UPDATE_LABELS_JOB, data: d, opts: { - priority: 1, + attempts: 3, + priority: 5, + backoff: { + type: 'exponential', + delay: 1000, + }, }, })) @@ -699,7 +704,12 @@ export const enqueueUpdateHighlight = async (data: UpdateHighlightData) => { try { return queue.add(UPDATE_HIGHLIGHT_JOB, data, { - priority: 1, + attempts: 3, + priority: 5, + backoff: { + type: 'exponential', + delay: 1000, + }, }) } catch (error) { logger.error('error enqueuing update highlight job', error) From abaf726044c156cd0467446896005f3fd5069ac2 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 1 Feb 2024 13:22:06 +0800 Subject: [PATCH 28/32] revert the change of priority --- packages/api/src/utils/createTask.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/api/src/utils/createTask.ts b/packages/api/src/utils/createTask.ts index b4edb4e0d..8b4d57d1a 100644 --- a/packages/api/src/utils/createTask.ts +++ b/packages/api/src/utils/createTask.ts @@ -680,7 +680,7 @@ export const bulkEnqueueUpdateLabels = async (data: UpdateLabelsData[]) => { data: d, opts: { attempts: 3, - priority: 5, + priority: 1, backoff: { type: 'exponential', delay: 1000, @@ -705,7 +705,7 @@ export const enqueueUpdateHighlight = async (data: UpdateHighlightData) => { try { return queue.add(UPDATE_HIGHLIGHT_JOB, data, { attempts: 3, - priority: 5, + priority: 1, backoff: { type: 'exponential', delay: 1000, From 3feef7ca66ddf942baef5190618285072102997a Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Thu, 1 Feb 2024 16:23:21 +0800 Subject: [PATCH 29/32] Use the DB stored reading progress when calculating maxes --- .../api/src/resolvers/function_resolvers.ts | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/packages/api/src/resolvers/function_resolvers.ts b/packages/api/src/resolvers/function_resolvers.ts index 8d3fb3187..4815da662 100644 --- a/packages/api/src/resolvers/function_resolvers.ts +++ b/packages/api/src/resolvers/function_resolvers.ts @@ -171,7 +171,10 @@ const readingProgressHandlers = { article.id ) if (readingProgress) { - return readingProgress.readingProgressPercent + return Math.max( + article.readingProgressPercent ?? 0, + readingProgress.readingProgressPercent + ) } } return article.readingProgressPercent @@ -187,8 +190,11 @@ const readingProgressHandlers = { ctx.claims?.uid, article.id ) - if (readingProgress) { - return readingProgress.readingProgressAnchorIndex + if (readingProgress && readingProgress.readingProgressAnchorIndex) { + return Math.max( + article.readingProgressAnchorIndex ?? 0, + readingProgress.readingProgressAnchorIndex + ) } } return article.readingProgressAnchorIndex @@ -204,8 +210,11 @@ const readingProgressHandlers = { ctx.claims?.uid, article.id ) - if (readingProgress) { - return readingProgress.readingProgressTopPercent + if (readingProgress && readingProgress.readingProgressTopPercent) { + return Math.max( + article.readingProgressTopPercent ?? 0, + readingProgress.readingProgressTopPercent + ) } } return article.readingProgressTopPercent From 0e9e331e18c7ef37b91b15c90c950c5a4e4e0eb0 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Thu, 1 Feb 2024 16:43:57 +0800 Subject: [PATCH 30/32] Implement a lifecycle hook for queue processor so it can shut down gracefully --- packages/api/src/queue-processor.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/api/src/queue-processor.ts b/packages/api/src/queue-processor.ts index d1ec1ed2b..241ef809f 100644 --- a/packages/api/src/queue-processor.ts +++ b/packages/api/src/queue-processor.ts @@ -142,6 +142,12 @@ const main = async () => { // respond healthy to auto-scaler. app.get('/_ah/health', (req, res) => res.sendStatus(200)) + app.get('/lifecyle/prestop', async (req, res) => { + logger.info('prestop lifecycle hook called.') + await worker.close() + res.sendStatus(200) + }) + app.get('/metrics', async (_, res) => { const queue = await getBackendQueue() if (!queue) { From 252443f1e46cf5f83bc411a6f83163f22a602589 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 1 Feb 2024 16:45:08 +0800 Subject: [PATCH 31/32] fix: update reading progress in cache when mark as read action was triggered in a rule --- packages/api/src/jobs/trigger_rule.ts | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/api/src/jobs/trigger_rule.ts b/packages/api/src/jobs/trigger_rule.ts index 72d902a90..efd3063fd 100644 --- a/packages/api/src/jobs/trigger_rule.ts +++ b/packages/api/src/jobs/trigger_rule.ts @@ -1,3 +1,4 @@ +import { ReadingProgressDataSource } from '../datasources/reading_progress_data_source' import { LibraryItem, LibraryItemState } from '../entity/library_item' import { Rule, RuleAction, RuleActionType, RuleEventType } from '../entity/rule' import { addLabelsToLibraryItem } from '../services/labels' @@ -21,10 +22,10 @@ interface RuleActionObj { action: RuleAction libraryItem: LibraryItem } +type RuleActionFunc = (obj: RuleActionObj) => Promise export const TRIGGER_RULE_JOB_NAME = 'trigger-rule' - -type RuleActionFunc = (obj: RuleActionObj) => Promise +const readingProgressDataSource = new ReadingProgressDataSource() const addLabels = async (obj: RuleActionObj) => { const labelIds = obj.action.params @@ -48,16 +49,14 @@ const archivePage = async (obj: RuleActionObj) => { } const markPageAsRead = async (obj: RuleActionObj) => { - return updateLibraryItem( + return readingProgressDataSource.updateReadingProgress( + obj.userId, obj.libraryItem.id, { + readingProgressPercent: 100, readingProgressTopPercent: 100, - readingProgressBottomPercent: 100, - readAt: new Date(), - }, - obj.userId, - undefined, - true + readingProgressAnchorIndex: undefined, + } ) } From ed7ac6c1b4d7f14723a3f60ecbdf093a09fcda52 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Thu, 1 Feb 2024 20:25:52 +0800 Subject: [PATCH 32/32] Fix typo in lifecycle endpoint path --- packages/api/src/queue-processor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/api/src/queue-processor.ts b/packages/api/src/queue-processor.ts index 241ef809f..7e796b34b 100644 --- a/packages/api/src/queue-processor.ts +++ b/packages/api/src/queue-processor.ts @@ -142,7 +142,7 @@ const main = async () => { // respond healthy to auto-scaler. app.get('/_ah/health', (req, res) => res.sendStatus(200)) - app.get('/lifecyle/prestop', async (req, res) => { + app.get('/lifecycle/prestop', async (req, res) => { logger.info('prestop lifecycle hook called.') await worker.close() res.sendStatus(200)