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

setup announcement actor, add endpoint for actor #42

Merged
merged 52 commits into from
Mar 20, 2024
Merged

Conversation

Copy link
Contributor

@RangerMauve RangerMauve left a comment

Choose a reason for hiding this comment

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

This is a good start. The AP stuff looks correct. we just need the inbox now so we can accept and track followers.

Copy link
Contributor

@RangerMauve RangerMauve left a comment

Choose a reason for hiding this comment

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

We also need to implement the webfinger protocol. Specifically we need to resolve /.well-known/webfinger to a json file with the mime type application/jrd+json.

example: https://github.com/RangerMauve/staticpub.mauve.moe/blob/default/.well-known/webfinger

This is necessary to enable AP instances to resolve the actor. We should use announcements@{hostname of the server} as the account others would follow. e.g. @[email protected]

src/client/cli.ts Outdated Show resolved Hide resolved
src/server/api/announcements.ts Outdated Show resolved Hide resolved
src/server/api/announcements.ts Outdated Show resolved Hide resolved
src/server/api/announcements.ts Outdated Show resolved Hide resolved
src/server/api/creation.ts Outdated Show resolved Hide resolved
src/server/api/creation.ts Outdated Show resolved Hide resolved
src/server/api/creation.ts Outdated Show resolved Hide resolved
src/server/api/index.ts Outdated Show resolved Hide resolved
src/server/store/index.ts Outdated Show resolved Hide resolved
src/scripts/add-post-to-announcements.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@RangerMauve RangerMauve left a comment

Choose a reason for hiding this comment

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

Excellent progress, just some minor bits left.

src/server/api/announcements.ts Outdated Show resolved Hide resolved
@catdevnull catdevnull marked this pull request as ready for review March 7, 2024 20:24
src/server/announcements.ts Outdated Show resolved Hide resolved
src/server/announcements.ts Outdated Show resolved Hide resolved
src/server/api/creation.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@RangerMauve RangerMauve left a comment

Choose a reason for hiding this comment

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

Maybe a new API should be made to check if the actor is existing, then set info, then announce if it was new. Place the logic outside of announcements and outside of the http API. Like server/creation.ts?

src/server/announcements.ts Outdated Show resolved Hide resolved
src/server/announcements.ts Outdated Show resolved Hide resolved
if (!existedAlready && info.announce) {
const activity = {
'@context': 'https://www.w3.org/ns/activitystreams',
type: 'Note',
Copy link
Contributor

Choose a reason for hiding this comment

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

ActivityStreams hold activities, not Notes. This should be wrapped in a Create activity with the Note being the object.

e.g. here's how we link to notes in the staticpub example: https://github.com/RangerMauve/staticpub.mauve.moe/blob/default/outbox.jsonld#L13

We may need to store the notes in a separate store and add a new /:actor/note/:id URL to resolve them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see a requirement in the spec that states that OrderedCollections must hold Activitys. Actually, I was mostly copying off of Sutty's Jekyll implementation, which has plain Updates inside the outbox:

Am I getting something wrong?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, notice how the update you linked wraps the note in an Update activity. We should do the same here, but with a Create.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OH lol i thought Update was similar to a Note. But the ID there is just a link to the post, so do we need an endpoint for the individual Note?

Copy link
Contributor

Choose a reason for hiding this comment

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

We might, lets see if just wrapping is enough for now though.

the same bug fixed in 0676758 applied to the test :)
src/server/announcements.ts Outdated Show resolved Hide resolved
src/server/api/creation.ts Show resolved Hide resolved
object: {
'@context': 'https://www.w3.org/ns/activitystreams',
type: 'Note',
id: `${this.outboxUrl}/${nanoid()}/note`,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can change this to be #note then the json-ld magic will resolve it? I'll test like this first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know nothing about json-ld 🤷 but sounds good to me

src/server/announcements.ts Show resolved Hide resolved
Comment on lines 7 to 14
type APActorNonStandard = APActor & {
publicKey: {
id: string
owner: string
publicKeyPem: string
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this should probably be moved elsewhere but i'm not sure where. maybe src/schemas.ts? either way it can be moved later if it's used elsewhere

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah mind moving it to schemas? Also maybe we could PR the field into the module? They accepted my orderedItems one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'm looking into doing the PR but I can't find the spec that defines keypair in APActor

object: {
'@context': 'https://www.w3.org/ns/activitystreams',
type: 'Note',
id: `${this.outboxUrl}/${nanoid()}/note`,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know nothing about json-ld 🤷 but sounds good to me

Copy link
Contributor Author

@catdevnull catdevnull left a comment

Choose a reason for hiding this comment

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

looks good!

@RangerMauve RangerMauve merged commit f61a318 into main Mar 20, 2024
2 checks passed
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