Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ActivityPub: Sprint 2 #3216

Draft
wants to merge 4 commits into
base: dev
Choose a base branch
from
Draft

Conversation

mediaformat
Copy link
Contributor

@mediaformat mediaformat commented Apr 4, 2024

ActivityPub: Sprint 2

  • Setup / Planning
  • Migrate annotations
  • Adapt/update existing annotations db querying code
  • Minimally implement http-foundation Request methods for use with landrok/activitypub

Other

  • fix: adds $wbr to parseURLs closure

Here's why I did it:

Checklist:

  • This pull request addresses a single issue
  • If this code includes interface changes, I've included screenshots in this Pull Request thread
  • I've adhered to Known's style guide (these codesniffer rules might help!)
  • My git branch is named in a descriptive way - i.e., yourname-summary-of-issue
  • I've tested my code in-browser
  • My code contains descriptive comments
  • I've added tests where applicable, and...
  • I can run the unit tests successfully.

@mediaformat
Copy link
Contributor Author

@benwerd The following describes the Annotations schema, and what I've considered:

CREATE TABLE IF NOT EXISTS `annotations` (
  `uuid` varchar(255) NOT NULL,
  `_id` varchar(36) NOT NULL,
  `siteid` varchar(36),
  `owner` varchar(255) NOT NULL,
  `entity_subtype` varchar(64) NOT NULL,
  `created` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
  `contents` longblob NOT NULL,
  `publish_status` varchar(255) NOT NULL DEFAULT 'published',
  `entity_id` varchar(255) NOT NULL,
  `parent_id` varchar(255),
  PRIMARY KEY (`uuid`),
  UNIQUE KEY `_id` (`_id`),
  KEY `siteid` (`siteid`),
  FOREIGN KEY (owner) REFERENCES entities(uuid),
  KEY `entity_subtype` (`entity_subtype`),
  KEY `publish_status` (`publish_status`),
  FOREIGN KEY (entity_id) REFERENCES entities(uuid),
  FOREIGN KEY (parent_id) REFERENCES annotations(uuid)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;

A migration script (stored procedure), would iterate each Entity row containing a (JSON) annotations object, extracting
annotations and storing their types [reply, like, share, ...] into entity_subtype. So for example a reply could be stored as Idno\\Annotations\\Reply.

I'm proposing 2 new columns here, entity_id as a foreign key linking to the Entity object it relates to, and parent_id for linking to another annotation, allowing for threads with depth.

Where an Annotation originates from a remote site (webmentions, activitypub), I would create a new Idno\\Entity\\User storing the minimum needed/available info [owner_name, owner_url, owner_icon].

Any thoughts, concerns, suggestions?

@benwerd
Copy link
Member

benwerd commented Apr 6, 2024

@mediaformat I think this looks great, and I very much see the need to make this migration. But I do have a big caveat:

Because not everyone is using the same database engine, I don't think migration should be a stored procedure. Or we should test the stored procedure for each of MySQL and Postgres, and create a command line tool for SQLite and MongoDB.

I think, actually, the latter is preferable.

It's imperative that the revised annotations methods use the database abstraction layer (presumably adding inherited methods for each db engine) rather than going straight to the SQL.

So here's what I'd suggest:

  • You continue down the stored procedure road for MySQL and Postgres.
  • I take the schema you've developed and adapt it for MongoDB and SQLite, with a command line update script.
  • We collaborate on the annotation methods.

Does that seem reasonable?

@mediaformat
Copy link
Contributor Author

@benwerd Totally reasonable!

@@ -2670,6 +2671,12 @@ function getAnnotationSubtype($uuid)
*/
function getAnnotation($uuid)
{
// Prioritize Annotation methods
$annotation = Annotation::getByUUID($uuid);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Overriding the internals here allow us to keep using the same getAnnotation() calls used in templates.

*/
static function getFromX($class, $search = array(), $fields = array(), $limit = 10, $offset = 0, $readGroups = [])
{
$result = \Idno\Core\Idno::site()->db()->getObject($search['uuid'], static::$retrieve_collection);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The result here is a different object than was previously received, notably missing are the owner_name, owner_url, owner_image.

I think it would make sense to use JOINs here to recreate the expected structure.

Copy link
Contributor Author

@mediaformat mediaformat left a comment

Choose a reason for hiding this comment

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

@benwerd Started on the Annotation methods, appreciate any early feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants