Skip to content

feat: Add Open Graph tag support #195

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

Merged
merged 57 commits into from
Apr 7, 2025
Merged

Conversation

JasonLovesDoggo
Copy link
Contributor

@JasonLovesDoggo JasonLovesDoggo commented Apr 2, 2025

Adds support for OG tags.

supersedes/closes #178
closes #168
closes #168
closes #131
closes #118

NOTES:

  • This PR also adds some potentially out of scope stuff (like an air config. If @Xe want's me to remove them, I will :)
  • This is still a bit of a WIP, though it is fully functional. It needs a refactor/maybe some more through tests

Checklist:

  • Added a description of the changes to the [Unreleased] section of docs/docs/CHANGELOG.md
  • Added test cases to the relevant parts of the codebase
  • Ran integration tests npm run test:integration (unsupported on Windows, please use WSL)

Signed-off-by: Jason Cameron <git@jasoncameron.dev>
Signed-off-by: Jason Cameron <git@jasoncameron.dev>
… (WIP)

I'm going to sleep. currently tags are passed to renderIndex.

see TecharoHQ#131

Signed-off-by: Jason Cameron <git@jasoncameron.dev>
Signed-off-by: Jason Cameron <git@jasoncameron.dev>
Moves the Open Graph (OG) tags from the index template to
the base template. This allows OG tags to be set on any
page, not just the index.  Also adds a
BaseWithOGTags function to the web package to allow
passing OG tags to the base template.  Removes the
ogTags parameter from the Index function and template.

Signed-off-by: Jason Cameron <git@jasoncameron.dev>
Signed-off-by: Jason Cameron <git@jasoncameron.dev>
Signed-off-by: Jason Cameron <git@jasoncameron.dev>
Signed-off-by: Jason Cameron <git@jasoncameron.dev>
Signed-off-by: Jason Cameron <git@jasoncameron.dev>
@JasonLovesDoggo
Copy link
Contributor Author

Just a note here. The integration tests will not support OG Tags given given the way it's setup. The OGTags rely on the --target + the request's URL's Path. Yet given the ordering of the playwrite tests, the temp server's URL is only avail AFTER libanubis is configured

...
s, err := libanubis.New(libanubis.Options{
		Next:           h,
		Policy:         policy,
		ServeRobotsTXT: true, // Target would be set here
	})
	if err != nil {
		t.Fatalf("can't construct libanubis.Server: %v", err)
	}

	ts := httptest.NewServer(s) // ts URL is here 
	t.Log(ts.URL)
...

Can I leave it that way yet keep/add dedicated OG tests @Xe?

@Xe
Copy link
Contributor

Xe commented Apr 2, 2025

👍 the happy path is there for simple things, manually constructing things is always acceptable when the happy path doesn't work.

@JasonLovesDoggo
Copy link
Contributor Author

👍 the happy path is there for simple things, manually constructing things is always acceptable when the happy path doesn't work.

Awesome thanks :D

This PR should be ready tomorrow evening

@Xe
Copy link
Contributor

Xe commented Apr 2, 2025

Take all the time you need, this is gonna be awesome to see in action :D

Signed-off-by: Jason Cameron <git@jasoncameron.dev>
Signed-off-by: Jason Cameron <git@jasoncameron.dev>
Signed-off-by: Jason Cameron <git@jasoncameron.dev>
Signed-off-by: Jason Cameron <git@jasoncameron.dev>
Signed-off-by: Jason Cameron <git@jasoncameron.dev>
Signed-off-by: Jason Cameron <git@jasoncameron.dev>
Signed-off-by: Jason Cameron <git@jasoncameron.dev>
@JasonLovesDoggo
Copy link
Contributor Author

May have gone overboard with tests but the pain of the playwrite made me do this 😨

Signed-off-by: Jason Cameron <git@jasoncameron.dev>
Signed-off-by: Jason Cameron <git@jasoncameron.dev>
Signed-off-by: Jason Cameron <git@jasoncameron.dev>
Signed-off-by: Jason Cameron <git@jasoncameron.dev>
Signed-off-by: Jason Cameron <git@jasoncameron.dev>
Signed-off-by: Jason Cameron <git@jasoncameron.dev>
@JasonLovesDoggo
Copy link
Contributor Author

Will fix the rest @ ~4pm EDT unless Xe pushes them first lol

Signed-off-by: Jason Cameron <git@jasoncameron.dev>
Signed-off-by: Jason Cameron <git@jasoncameron.dev>
@JasonLovesDoggo
Copy link
Contributor Author

There should definitely be some limit to the amount of data the fetcher loads from the target.

Testing with a 4GB video file, it seems to load all of it into memory and taking quite a while to even get to the challenge part.

The main branch does not have this issue.

EDIT: by "loading into memory" I mean that Anubis's memory usage is increased by the size of the file for a long time after the request is processed, it may also have something to do with Go's GC, I don't really know.

Hmm, when the target is a raw mp4 file? I've patched this by introducing a check that limits the content length to 16 MiB to hopefully prevent this.

Adds additional approved OG tags (`keywords`, `author`), improves

Signed-off-by: Jason Cameron <git@jasoncameron.dev>
Signed-off-by: Jason Cameron <git@jasoncameron.dev>
… fetchHTMLDocument

Signed-off-by: Jason Cameron <git@jasoncameron.dev>
…Q#178)"

This reverts commit 21a9d77

Signed-off-by: Jason Cameron <git@jasoncameron.dev>
Copy link
Contributor

@jaredallard jaredallard left a comment

Choose a reason for hiding this comment

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

hype hype hype

Didn't know this was possible! wow!

Signed-off-by: Jason Cameron <git@jasoncameron.dev>
Cache a nil result for half the TTL to avoid repeatedly
requesting a timed-out URL.

Signed-off-by: Jason Cameron <git@jasoncameron.dev>
Signed-off-by: Jason Cameron <git@jasoncameron.dev>
@JasonLovesDoggo
Copy link
Contributor Author

JasonLovesDoggo commented Apr 4, 2025

don't merge yet.. forgot something We're back in business

…tags)

- Cache empty results for timeouts and non-200 status codes
  to avoid spamming the server.
- Use a non-nil empty map to represent empty results in the
  cache, as nil would be a cache miss.

Signed-off-by: Jason Cameron <git@jasoncameron.dev>
@JasonLovesDoggo
Copy link
Contributor Author

There should definitely be some limit to the amount of data the fetcher loads from the target.

Testing with a 4GB video file, it seems to load all of it into memory and taking quite a while to even get to the challenge part.

The main branch does not have this issue.

EDIT: by "loading into memory" I mean that Anubis's memory usage is increased by the size of the file for a long time after the request is processed, it may also have something to do with Go's GC, I don't really know.

Just tested with a 400mb file, ram usage of Anubis was between 3-6mb
image

Though the chrome tab tanked chal difficulty 6 and reached 4gb...

@Xe
Copy link
Contributor

Xe commented Apr 5, 2025

You want: https://pkg.go.dev/net/http#MaxBytesReader

@SlyEcho
Copy link
Contributor

SlyEcho commented Apr 5, 2025

It looks like the memory issue is resolved, one part of it is, of course, about ignoring video files now 👍

Signed-off-by: Jason Cameron <git@jasoncameron.dev>
…numbers

Signed-off-by: Jason Cameron <git@jasoncameron.dev>
Copy link
Contributor

@Xe Xe left a comment

Choose a reason for hiding this comment

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

Just opened this for testing. I think it's pretty much good to go! Thanks for sticking to it, this review cycle kinda sucked and we'll try to make sure something like this doesn't happen again.

Either way, very happy that this is gonna be in. This will be one of the headline features of the next release :)

@Xe Xe merged commit 7743620 into TecharoHQ:main Apr 7, 2025
3 checks passed
@JasonLovesDoggo JasonLovesDoggo deleted the og-tags branch April 7, 2025 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants