Skip to content
This repository has been archived by the owner on Oct 15, 2022. It is now read-only.

Introduce caching for page assets (js/css) #32

Merged
merged 8 commits into from Oct 26, 2013

Conversation

moollaza
Copy link
Member

cc/ @nospampleasemam @yegg @russellholt

In light of https://dub.duckduckgo.com/core/ddg/pull/252#issuecomment-796
I've re-written a portion of Web.pm to now request the SERP from DDG, then find the latest version for d####.js, s.###css and spice2_duckpan_###.js

If the file already exists in Duckpan's local cache, it won't request the file, otherwise it will get the newest one.

Please give feedback on my implementation -- there's room for improvement but for now this works and is a much safer way of getting the required assets.

@moollaza
Copy link
Member Author

**Note: In order to test this, you'll also need to deploy the core changes (https://dub.duckduckgo.com/core/ddg/pull/261) to your dev instance and make sure duckpan is pulling from your instance.

if (my $src = $_->attr('src')) {
if ($src =~ m/^\/(d\d{4}\.js)/) {
$page_js_filename = $1;
} elsif ($src =~ m/^\/spice2\/(spice2_duckpan_\d{1,3}\.js)/) {
Copy link
Member

Choose a reason for hiding this comment

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

Might as well make it 4.

@yegg
Copy link
Member

yegg commented Oct 11, 2013

Please add some explanatory inline comments.

@moollaza
Copy link
Member Author

ping.

This needs to be reviewed and merged asap. We've already merged the accompanying changes on Core (weeks ago...), so now versioned files are being provided, but DuckPAN is still grabbing the old, stale, unversioned files (i.e. d.js, s.css and spice2_duckpan.js) by requesting them directly from DDG.

# and stores locally in its own cache
# The are all necessary for DuckPAN Server to function properly
#
# Page_Root.html : DDG Homepage
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't these be lowercase?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, fixed.

moollaza added a commit that referenced this pull request Oct 26, 2013
@moollaza moollaza merged commit 93466bd into master Oct 26, 2013
@moollaza moollaza deleted the zaahir/duckpan-cache-page-assets branch October 26, 2013 20:14
@moollaza
Copy link
Member Author

Merged with @yegg's approval

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

Successfully merging this pull request may close these issues.

None yet

2 participants