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

feat: new middleware redirect_as2 #632

Merged
merged 9 commits into from
Jul 15, 2024
Merged

feat: new middleware redirect_as2 #632

merged 9 commits into from
Jul 15, 2024

Conversation

kwaa
Copy link
Member

@kwaa kwaa commented Jul 9, 2024

Description

This is an updated version of the same middleware in Aoba.

Check List

  • Have you read the
    CODE OF CONDUCT
  • Have you read the document
    CONTRIBUTING
    • One pull request per feature. If you want to do more than one thing,
      send multiple pull request.
    • Write tests.
    • Run deno fmt to fix the code format before commit.
    • Document any change in the CHANGELOG.md.

@kwaa kwaa added the middleware label Jul 9, 2024
@kwaa kwaa requested a review from oscarotero July 9, 2024 06:22
@oscarotero oscarotero mentioned this pull request Jul 10, 2024
6 tasks
@kwaa
Copy link
Member Author

kwaa commented Jul 13, 2024

@oscarotero For this middleware, what do you think about renaming it to redirect_as2 (redirectAS2) and providing the following options?

export type Options = (url: URL) => Promise<URL> | URL | undefined
export default (options: Options): Middleware => async (req, next) => {
  if (req.headers.get('accept')?.includes("application/activity+json")) {
    const dest = await options(req.url)
    if (dest) return Response.redirect(dest);
  }
  return await next(req);
};
// manual
server.use(redirectAS2(url => new URL(url.href, 'https://hatsu.local/posts/')))
// hatsu preset
export const hatsu = (instance: string) => (url: URL) => new URL(`${url.origin}${url.pathname}`, `https://${instance}/posts/`)

import Server from 'lume/core/server.ts'
import redirectAS2, { hatsu } from 'lume/middlewares/redirect_as2.ts'
const server = new Server()
server.use(redirectAS2(hatsu('hatsu.local')))

This would also be compatible with Bridgy Fed (not sure of the effect since I'm not currently using it):

// bridgy-fed preset
export const bridgyFed = () => (url: URL) => new URL(`${url.origin}${url.pathname}`, 'https://fed.brid.gy/r/')

import Server from 'lume/core/server.ts'
import redirectAS2, { bridgyFed } from 'lume/middlewares/redirect_as2.ts'
const server = new Server()
server.use(redirectAS2(bridgyFed()))

@oscarotero
Copy link
Member

Yep, I like the idea!
BTW, what does as2 mean? 😄

For consistency with other middlewares, I'd use an options object as the first argument, instead a function.
Also, why don't simply pass the url, instead of a function? something like:

server.use(redirectAS2({
  dest: new URL("https://hatsu.local/posts/"),
}));

If the logic for calculate the final destination is different for hatsu and bridgy, maybe it can be configured in the mode option (name may change). For example:

server.use(redirectAS2({
  dest: new URL("https://hatsu.local/posts/"),
  mode: "hatsu"
}));

(or automatically detect it from the domain name).
What do you think?

@kwaa
Copy link
Member Author

kwaa commented Jul 14, 2024

Yep, I like the idea! BTW, what does as2 mean? 😄

Activity Streams 2.0

Also, why don't simply pass the url, instead of a function?

Using functions allows users who need it to write custom logic, for example:

server.use(redirectAS2(url => {
  const { origin, pathname } = new URL(URL)
  if (pathname === '/') return new URL('https:/hatsu.local/users/example.com')
  else if (pathname.includes('/posts/') return new URL(`${origin}${pathname}`, 'https://hatsu.local/posts/')
}))

@oscarotero
Copy link
Member

Using functions allows users who need it to write custom logic

Yes, I know. My only concern is about the value provided by this middleware. Internally, it only has an if to check if the request is an activity stream request. That's why I was thinking of something more straightforward.
Maybe something like? url: URL | (url: URL) => URL (allowing to provide a url or a function returning an url).

@kwaa
Copy link
Member Author

kwaa commented Jul 15, 2024

Maybe something like? url: URL | (url: URL) => URL (allowing to provide a url or a function returning an URL).

If the URL is provided directly, options like instance and mode must also be included. I think this might be more cumbersome than using functions.

@oscarotero
Copy link
Member

Okay, let's start with the function and we can see later if an url option makes sense.

@kwaa
Copy link
Member Author

kwaa commented Jul 15, 2024

Okay, let's start with the function and we can see later if an url option makes sense.

I have updated PR.

middlewares/redirect_as2.ts Outdated Show resolved Hide resolved
middlewares/redirect_as2.ts Outdated Show resolved Hide resolved
@kwaa kwaa requested a review from oscarotero July 15, 2024 11:06
@kwaa kwaa changed the title feat: new middleware hatsu feat: new middleware redirect_as2 Jul 15, 2024
@oscarotero
Copy link
Member

Great.
Thanks for your patience in the back and forth 🙌

@oscarotero oscarotero merged commit 20eb2ad into main Jul 15, 2024
6 checks passed
@oscarotero oscarotero deleted the middleware/hatsu branch July 15, 2024 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants