Port actor data and account.json to application.db #138

johnholdun · 2023-10-04T08:13:42Z

This PR introduces a new database, application.db, which will eventually replace all the other data stores used by this application. For now, it is only replacing activitypub.db and account.json. Naturally, this PR also updates any business logic that used to reference those stores to instead fetch from the new database.

Still to do:

Address TODOs in diff
Move remaining data from actor table in activitypub.db to application.db
Update docs

Closes #135.

johnholdun · 2023-10-04T08:14:21Z

server.js

  res.locals.loggedIn = req.session.loggedIn;
+  const { displayName } = await getActorInfo();


Feels not great to make this database call before every page load but I think this is just the world we live in

johnholdun · 2023-10-04T08:16:01Z

src/activity-pub-db.js

@@ -221,7 +222,7 @@ async function firstTimeSetup(actorName) {

 function setup() {
  // activitypub not set up yet, skip until we have the data we need
-  if (actorInfo.disabled) {
+  if (false) {


I have temporarily removed the disabled flag across this PR. Still thinking about what this should be—maybe some UI onboarding on first run, or we disable AP until it's enabled in admin settings?

I think I'd like a non-AP mode to be the default for at least a little longer because it's useful to not get in your way when you're running on localhost to work on some non-federated part of the app. I think it's fine to kick that into the admin settings, update the instructions to send people there as quickly as possible, and possibly add some pointers in the future like a localstorage-dismissable banner at the top of the screen that tells you you're in non-AP mode when you're logged in as admin? 🤔

@ckolderup what you are describing is very similar to the Owncast configuration. Get all the bits and bobbles setup first, then you can flip on AP mode if you want.

https://owncast.online/docs/social/

@jeffsikes ooh, interesting, thank you! This is a great comparison.

src/database.js

johnholdun · 2023-10-04T08:21:10Z

src/database.js

+  const { value } = result;
+
+  if (typeof value === 'string') {
+    return JSON.parse(value);


Every value in the settings table is assumed to be serialized as JSON. I'm opting not to actually use SQLite's built-in JSON functions for this, though, because we'll never want to treat the values as anything but blobs of text at the database level. The only reason I can think of to actually use native JSON is if we can get multiple rows deserialized all at once somehow—I'll check if this is an option.

johnholdun · 2023-10-04T08:21:19Z

src/database.js

+  // be safe!
+  if (!names.every(name => name.match(/^[a-z0-9-_ .\/]+$/i))) {
+    throw new Error('Names contain unexpected characters');
+  }


johnholdun · 2023-10-04T08:22:28Z

src/schema.sql

@@ -0,0 +1,12 @@
+create table if not exists settings (
+  name text unique not null check (name <> ""),


This probably deserves an index

yes, especially with getActorInfo on every page load?

johnholdun · 2023-10-04T08:23:00Z

src/schema.sql

+  ('avatar', '"https://cdn.glitch.global/8eaf209c-2fa9-4353-9b99-e8d8f3a5f8d4/postmarks-logo-white-small.png?v=1693610556689"'),
+  ('displayName', '"Postmarks"'),
+  ('description', '"An ActivityPub bookmarking and sharing site built with Postmarks"')
+on conflict (name) do nothing;


This upsert syntax means we'll only insert these default values if there aren't already rows in the settings table with these names

ckolderup · 2023-10-09T05:17:14Z

src/util.js


 dotenv.config();

+const ACTOR_SETTING_NAMES = ['username', 'avatar', 'displayName', 'description'];
+const IS_ACCOUNT_FILE_IMPORTED = 'isAccountFileImported';


would it make sense for "devex" to move stuff related to upgrades/migrations into a separate util-like file? Like, we basically never want to get rid of code that relates to making sure that upgrades are smooth, but on the other hand we also want to minimize any given dev's exposure to this kind of code because theoretically it should be set-and-forget.

I'm aware that in general util.js as a junk drawer is a problem brewing, but this feels like a clean delineation for "a kind of thing to put in a separate place"

I've added a new file since this comment was made called boot.js which, as the name implies, is a bunch of procedures that should run once when the application boots. It's only used for migrating data around and I don't anticipate it having much use beyond that, but it does mean that util.js can be dedicated to utilities (which is still kind of a junk drawer, but a less crowded one).

johnholdun · 2023-11-15T03:53:35Z

Just pushed a really big commit whoa. Pulling on the thread of refactoring the database just lead to more and more unraveling. I ended up inlining all of the database statements that used to touch activitypub.db in their respective locations—the ORM-ish named functions were each only ever used once, and separating them the way they were lead to some unnecessary queries that I was able to combine into single statements in some cases. I recognize that this is maybe an unpopular pattern. I personally feel good about it but I'm happy to revise this if desired!

In addition, this is way too much change to be merged without serious testing, so I'm going to work on adding test coverage for all of this separately from this PR to ensure I'm not breaking stuff. We'll also want to really carefully QA the various states a user can be in while their data is being imported and migrated—I've only very cursorily tested this so far.

That said, I think all this code is in good shape and ready for review, so I'm going to mark the PR as open!

johnholdun commented Oct 4, 2023

View reviewed changes

src/database.js Show resolved Hide resolved

johnholdun commented Oct 4, 2023

View reviewed changes

src/database.js Outdated Show resolved Hide resolved

johnholdun commented Oct 4, 2023

View reviewed changes

ckolderup reviewed Oct 9, 2023

View reviewed changes

johnholdun added 3 commits November 4, 2023 11:55

Port actor data and account.json to application.db

d7470aa

lint

09a4bc7

Additional refactoring whoa

3aa54a9

johnholdun force-pushed the actor-application branch from 21eaf53 to 3aa54a9 Compare November 15, 2023 03:44

Install dependencies

71d985a

johnholdun force-pushed the actor-application branch from aafd2d2 to 71d985a Compare November 15, 2023 03:50

johnholdun marked this pull request as ready for review November 15, 2023 03:53

TomCasavant mentioned this pull request May 15, 2024

follow chains of replies #28

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port actor data and account.json to application.db #138

Port actor data and account.json to application.db #138

johnholdun commented Oct 4, 2023 •

edited

Loading

johnholdun Oct 4, 2023

johnholdun Oct 4, 2023

ckolderup Oct 9, 2023

jeffsikes Oct 9, 2023

ckolderup Oct 9, 2023

johnholdun Oct 4, 2023

johnholdun Oct 4, 2023

johnholdun Oct 4, 2023

ckolderup Oct 9, 2023

johnholdun Oct 4, 2023

ckolderup Oct 9, 2023

johnholdun Nov 15, 2023

johnholdun commented Nov 15, 2023

		res.locals.loggedIn = req.session.loggedIn;
		const { displayName } = await getActorInfo();

		@@ -0,0 +1,12 @@
		create table if not exists settings (
		name text unique not null check (name <> ""),

Port actor data and account.json to application.db #138

Are you sure you want to change the base?

Port actor data and account.json to application.db #138

Conversation

johnholdun commented Oct 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnholdun commented Nov 15, 2023

johnholdun commented Oct 4, 2023 •

edited

Loading