Poll on page refresh; validate subscribe dialog properly; avoid save-races

web/src/app/Api.js

@@ -1,12 +1,24 @@
-import {topicUrlJsonPoll, fetchLinesIterator, topicUrl, topicUrlAuth, maybeWithBasicAuth} from "./utils";
+import {
+    topicUrlJsonPoll,
+    fetchLinesIterator,
+    topicUrl,
+    topicUrlAuth,
+    maybeWithBasicAuth,
+    topicShortUrl,
+    topicUrlJsonPollWithSince
+} from "./utils";
 
 class Api {
-    async poll(baseUrl, topic, user) {
-        const url = topicUrlJsonPoll(baseUrl, topic);
+    async poll(baseUrl, topic, since, user) {
+        const shortUrl = topicShortUrl(baseUrl, topic);
+        const url = (since > 1) // FIXME Ahh, this is >1, because we do +1 when we call this .....
+            ? topicUrlJsonPollWithSince(baseUrl, topic, since)
+            : topicUrlJsonPoll(baseUrl, topic);
         const messages = [];
         const headers = maybeWithBasicAuth({}, user);
         console.log(`[Api] Polling ${url}`);
         for await (let line of fetchLinesIterator(url, headers)) {
+            console.log(`[Api, ${shortUrl}] Received message ${line}`);
             messages.push(JSON.parse(line));
         }
         return messages;

web/src/app/NotificationManager.js

@@ -1,9 +1,9 @@
-import {formatMessage, formatTitle} from "./utils";
+import {formatMessage, formatTitleWithFallback, topicShortUrl} from "./utils";
 
 class NotificationManager {
     notify(subscription, notification, onClickFallback) {
         const message = formatMessage(notification);
-        const title = formatTitle(notification);
+        const title = formatTitleWithFallback(notification, topicShortUrl(subscription.baseUrl, subscription.topic));
         const n = new Notification(title, {
             body: message,
             icon: '/static/img/favicon.png'

web/src/app/Repository.js

@@ -7,6 +7,7 @@ class Repository {
     loadSubscriptions() {
         console.log(`[Repository] Loading subscriptions from localStorage`);
         const subscriptions = new Subscriptions();
+        subscriptions.loaded = true;
         const serialized = localStorage.getItem('subscriptions');
         if (serialized === null) {
             return subscriptions;
@@ -15,6 +16,7 @@ class Repository {
             JSON.parse(serialized).forEach(s => {
                 const subscription = new Subscription(s.baseUrl, s.topic);
                 subscription.addNotifications(s.notifications);
+                subscription.last = s.last; // Explicitly set, in case old notifications have been deleted
                 subscriptions.add(subscription);
             });
             console.log(`[Repository] Loaded ${subscriptions.size()} subscription(s) from localStorage`);
@@ -26,6 +28,9 @@ class Repository {
     }
 
     saveSubscriptions(subscriptions) {
+        if (!subscriptions.loaded) {
+            return; // Avoid saving invalid state, triggered by initial useEffect hook
+        }
         console.log(`[Repository] Saving ${subscriptions.size()} subscription(s) to localStorage`);
         const serialized = JSON.stringify(subscriptions.map( (id, subscription) => {
             return {
@@ -41,6 +46,7 @@ class Repository {
     loadUsers() {
         console.log(`[Repository] Loading users from localStorage`);
         const users = new Users();
+        users.loaded = true;
         const serialized = localStorage.getItem('users');
         if (serialized === null) {
             return users;
@@ -57,6 +63,9 @@ class Repository {
     }
 
     saveUsers(users) {
+        if (!users.loaded) {
+            return; // Avoid saving invalid state, triggered by initial useEffect hook
+        }
         console.log(`[Repository] Saving users to localStorage`);
         const serialized = JSON.stringify(users.map(user => {
             return {

web/src/app/Subscription.js

@@ -11,11 +11,11 @@ class Subscription {
 
     addNotification(notification) {
         if (this.notifications.has(notification.id) || notification.time < this.last) {
-            return this;
+            return false;
         }
         this.notifications.set(notification.id, notification);
         this.last = notification.time;
-        return this;
+        return true;
     }
 
     addNotifications(notifications) {
@@ -39,6 +39,10 @@ class Subscription {
         return Array.from(this.notifications.values());
     }
 
+    url() {
+        return topicUrl(this.baseUrl, this.topic);
+    }
+
     shortUrl() {
         return topicShortUrl(this.baseUrl, this.topic);
     }

web/src/app/Subscriptions.js

@@ -1,5 +1,6 @@
 class Subscriptions {
     constructor() {
+        this.loaded = false; // FIXME I hate this
         this.subscriptions = new Map();
     }
 
@@ -46,6 +47,7 @@ class Subscriptions {
 
     clone() {
         const c = new Subscriptions();
+        c.loaded = this.loaded;
         c.subscriptions = new Map(this.subscriptions);
         return c;
     }

web/src/app/Users.js

@@ -1,5 +1,6 @@
 class Users {
     constructor() {
+        this.loaded = false; // FIXME I hate this
         this.users = new Map();
     }
 
@@ -28,6 +29,7 @@ class Users {
 
     clone() {
         const c = new Users();
+        c.loaded = this.loaded;
         c.users = new Map(this.users);
         return c;
     }

web/src/app/utils.js

@@ -6,10 +6,15 @@ export const topicUrlWs = (baseUrl, topic) => `${topicUrl(baseUrl, topic)}/ws`
     .replaceAll("http://", "ws://");
 export const topicUrlJson = (baseUrl, topic) => `${topicUrl(baseUrl, topic)}/json`;
 export const topicUrlJsonPoll = (baseUrl, topic) => `${topicUrlJson(baseUrl, topic)}?poll=1`;
+export const topicUrlJsonPollWithSince = (baseUrl, topic, since) => `${topicUrlJson(baseUrl, topic)}?poll=1&since=${since}`;
 export const topicUrlAuth = (baseUrl, topic) => `${topicUrl(baseUrl, topic)}/auth`;
 export const topicShortUrl = (baseUrl, topic) => shortUrl(topicUrl(baseUrl, topic));
 export const shortUrl = (url) => url.replaceAll(/https?:\/\//g, "");
 
+export const validTopic = (topic) => {
+    return topic.match(/^([-_a-zA-Z0-9]{1,64})$/) // Regex must match Go & Android app!
+}
+
 // Format emojis (see emoji.js)
 const emojis = {};
 rawEmojis.forEach(emoji => {
@@ -23,6 +28,14 @@ const toEmojis = (tags) => {
     else return tags.filter(tag => tag in emojis).map(tag => emojis[tag]);
 }
 
+
+export const formatTitleWithFallback = (m, fallback) => {
+    if (m.title) {
+        return formatTitle(m);
+    }
+    return fallback;
+};
+
 export const formatTitle = (m) => {
     const emojiList = toEmojis(m.tags);
     if (emojiList.length > 0) {

web/src/components/App.js

@@ -23,15 +23,6 @@ const App = () => {
     const [users, setUsers] = useState(new Users());
     const [selectedSubscription, setSelectedSubscription] = useState(null);
     const [notificationsGranted, setNotificationsGranted] = useState(notificationManager.granted());
-    const handleNotification = (subscriptionId, notification) => {
-        setSubscriptions(prev => {
-            const newSubscription = prev.get(subscriptionId).addNotification(notification);
-            notificationManager.notify(newSubscription, notification, () => {
-                setSelectedSubscription(newSubscription);
-            })
-            return prev.update(newSubscription).clone();
-        });
-    };
     const handleSubscribeSubmit = (subscription, user) => {
         console.log(`[App] New subscription: ${subscription.id}`);
         if (user !== null) {
@@ -39,13 +30,7 @@ const App = () => {
         }
         setSubscriptions(prev => prev.add(subscription).clone());
         setSelectedSubscription(subscription);
-        api.poll(subscription.baseUrl, subscription.topic, user)
-            .then(messages => {
-                setSubscriptions(prev => {
-                    const newSubscription = prev.get(subscription.id).addNotifications(messages);
-                    return prev.update(newSubscription).clone();
-                });
-            });
+        poll(subscription, user);
         handleRequestPermission();
     };
     const handleDeleteNotification = (subscriptionId, notificationId) => {
@@ -75,15 +60,48 @@ const App = () => {
             setNotificationsGranted(granted);
         })
     };
+    const poll = (subscription, user) => {
+        const since = subscription.last + 1; // FIXME, sigh ...
+        api.poll(subscription.baseUrl, subscription.topic, since, user)
+            .then(notifications => {
+                setSubscriptions(prev => {
+                    subscription.addNotifications(notifications);
+                    return prev.update(subscription).clone();
+                });
+            });
+    };
+
+    // Define hooks: Note that the order of the hooks is important. The "loading" hooks
+    // must be before the "saving" hooks.
     useEffect(() => {
-        setSubscriptions(repository.loadSubscriptions());
-        setUsers(repository.loadUsers());
-    }, [/* initial render only */]);
+        // Load subscriptions and users
+        const subscriptions = repository.loadSubscriptions();
+        const users = repository.loadUsers();
+        setSubscriptions(subscriptions);
+        setUsers(users);
+
+        // Poll all subscriptions
+        subscriptions.forEach((subscriptionId, subscription) => {
+            const user = users.get(subscription.baseUrl); // May be null
+            poll(subscription, user);
+        });
+    }, [/* initial render */]);
     useEffect(() => {
+        const notificationClickFallback = (subscription) => setSelectedSubscription(subscription);
+        const handleNotification = (subscriptionId, notification) => {
+            setSubscriptions(prev => {
+                const subscription = prev.get(subscriptionId);
+                if (subscription.addNotification(notification)) {
+                    notificationManager.notify(subscription, notification, notificationClickFallback)
+                }
+                return prev.update(subscription).clone();
+            });
+        };
         connectionManager.refresh(subscriptions, users, handleNotification);
-        repository.saveSubscriptions(subscriptions);
-        repository.saveUsers(users);
     }, [subscriptions, users]);
+    useEffect(() => repository.saveSubscriptions(subscriptions), [subscriptions]);
+    useEffect(() => repository.saveUsers(users), [users]);
+
     return (
         <ThemeProvider theme={theme}>
             <CssBaseline/>

web/src/components/Navigation.js

@@ -111,6 +111,7 @@ const NavList = (props) => {
             <SubscribeDialog
                 key={subscribeDialogKey} // Resets dialog when canceled/closed
                 open={subscribeDialogOpen}
+                subscriptions={props.subscriptions}
                 onCancel={handleSubscribeReset}
                 onSuccess={handleSubscribeSubmit}
             />

web/src/components/SubscribeDialog.js

@@ -1,4 +1,5 @@
 import * as React from 'react';
+import {useState} from 'react';
 import Button from '@mui/material/Button';
 import TextField from '@mui/material/TextField';
 import Dialog from '@mui/material/Dialog';
@@ -6,12 +7,11 @@ import DialogActions from '@mui/material/DialogActions';
 import DialogContent from '@mui/material/DialogContent';
 import DialogContentText from '@mui/material/DialogContentText';
 import DialogTitle from '@mui/material/DialogTitle';
-import {useState} from "react";
 import Subscription from "../app/Subscription";
 import {useMediaQuery} from "@mui/material";
 import theme from "./theme";
 import api from "../app/Api";
-import {topicUrl} from "../app/utils";
+import {topicUrl, validTopic} from "../app/utils";
 import useStyles from "./styles";
 import User from "../app/User";
 
@@ -23,14 +23,9 @@ const SubscribeDialog = (props) => {
     const [topic, setTopic] = useState("");
     const [showLoginPage, setShowLoginPage] = useState(false);
     const fullScreen = useMediaQuery(theme.breakpoints.down('sm'));
-    const handleCancel = () => {
-        setTopic('');
-        props.onCancel();
-    }
     const handleSuccess = (baseUrl, topic, user) => {
         const subscription = new Subscription(baseUrl, topic);
         props.onSuccess(subscription, user);
-        setTopic('');
     }
     return (
         <Dialog open={props.open} onClose={props.onClose} fullScreen={fullScreen}>
@@ -38,7 +33,8 @@ const SubscribeDialog = (props) => {
                 baseUrl={baseUrl}
                 topic={topic}
                 setTopic={setTopic}
-                onCancel={handleCancel}
+                subscriptions={props.subscriptions}
+                onCancel={props.onCancel}
                 onNeedsLogin={() => setShowLoginPage(true)}
                 onSuccess={handleSuccess}
             />}
@@ -65,6 +61,8 @@ const SubscribePage = (props) => {
         console.log(`[SubscribeDialog] Successful login to ${topicUrl(baseUrl, topic)} for anonymous user`);
         props.onSuccess(baseUrl, topic, null);
     };
+    const existingTopicUrls = props.subscriptions.map((id, s) => s.url());
+    const subscribeButtonEnabled = validTopic(props.topic) && !existingTopicUrls.includes(topicUrl(baseUrl, topic));
     return (
         <>
             <DialogTitle>Subscribe to topic</DialogTitle>
@@ -87,7 +85,7 @@ const SubscribePage = (props) => {
             </DialogContent>
             <DialogActions>
                 <Button onClick={props.onCancel}>Cancel</Button>
-                <Button onClick={handleSubscribe} disabled={props.topic === ""}>Subscribe</Button>
+                <Button onClick={handleSubscribe} disabled={!subscribeButtonEnabled}>Subscribe</Button>
             </DialogActions>
         </>
     );