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

Add internal resolution for assets in get_url function #2726

Open
wants to merge 11 commits into
base: next
Choose a base branch
from
36 changes: 35 additions & 1 deletion components/content/src/file_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,18 @@ impl FileInfo {
format!("{}.md", name)
};
let grand_parent = parent.parent().map(|p| p.to_path_buf());
let mut colocated_path = None;

// If we have a folder with an asset, don't consider it as a component
// Splitting on `.` as we might have a language so it isn't *only* index but also index.fr
// etc
if !components.is_empty() && name.split('.').collect::<Vec<_>>()[0] == "_index" {
colocated_path = Some({
let mut val = components.join("/");
val.push('/');
val
});
}

FileInfo {
filename: file_path.file_name().unwrap().to_string_lossy().to_string(),
Expand All @@ -119,7 +131,7 @@ impl FileInfo {
name,
components,
relative,
colocated_path: None,
colocated_path: colocated_path,
}
}

Expand Down Expand Up @@ -289,4 +301,26 @@ mod tests {
Path::new("/home/vincent/code/site/content/posts/tutorials/python/index")
);
}

#[test]
fn correct_colocated_path() {
let files = vec![
FileInfo::new_page(
Path::new("/home/vincent/code/site/content/posts/tutorials/python/index.md"),
&PathBuf::new(),
),
FileInfo::new_section(
Path::new("/home/vincent/code/site/content/posts/tutorials/_index.fr.md"),
&PathBuf::new(),
),
];

for file in files {
assert!(file.colocated_path.is_some());
if let Some(colocated_path) = file.colocated_path {
assert!(!colocated_path.starts_with('/'));
assert!(colocated_path.ends_with('/'));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you assert what the actual path it?

Copy link
Author

Choose a reason for hiding this comment

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

I just asserted what is really important for a colocated path which is not starting with a '/' and ending with a '/'.
I wanted to emphasize that because if you change the way you compute colocated path in the future but don't respect those two assertions, you'll break the current feature.

I definitely can split the check (one for the page and one for the section as they have different colocated path) in this test while adding a comment on what is really important to keep through time.
Would it be ok for you?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes that would be fine

}
}
}
}
82 changes: 58 additions & 24 deletions components/content/src/page.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::front_matter::{split_page_content, PageFrontMatter};
use crate::library::Library;
use crate::ser::SerializingPage;
use crate::utils::get_reading_analytics;
use crate::utils::{find_related_assets, has_anchor};
use crate::utils::{find_related_assets, get_assets_permalinks, has_anchor, serialize_assets};
use utils::anchors::has_anchor_id;
use utils::fs::read_file;

Expand All @@ -45,6 +45,8 @@ pub struct Page {
pub assets: Vec<PathBuf>,
/// All the non-md files we found next to the .md file
pub serialized_assets: Vec<String>,
/// The permalinks of all the non-md files we found next to the .md file
pub assets_permalinks: HashMap<String, String>,
/// The HTML rendered of the page
pub content: String,
/// The slug of that page.
Expand Down Expand Up @@ -195,7 +197,16 @@ impl Page {
if page.file.name == "index" {
let parent_dir = path.parent().unwrap();
page.assets = find_related_assets(parent_dir, config, true);
page.serialized_assets = page.serialize_assets(base_path);
page.serialized_assets = serialize_assets(
&page.assets,
page.file.path.parent(),
page.file.colocated_path.as_ref(),
);
page.assets_permalinks = get_assets_permalinks(
&page.serialized_assets,
&page.permalink,
page.file.colocated_path.as_ref(),
);
} else {
page.assets = vec![];
}
Expand Down Expand Up @@ -255,28 +266,6 @@ impl Page {
.with_context(|| format!("Failed to render page '{}'", self.file.path.display()))
}

/// Creates a vectors of asset URLs.
fn serialize_assets(&self, base_path: &Path) -> Vec<String> {
self.assets
.iter()
.filter_map(|asset| asset.strip_prefix(self.file.path.parent().unwrap()).ok())
.filter_map(|filename| filename.to_str())
.map(|filename| {
let mut path = self.file.path.clone();
// Popping the index.md from the path since file.parent would be one level too high
// for our need here
path.pop();
path.push(filename);
path = path
.strip_prefix(&base_path.join("content"))
.expect("Should be able to stripe prefix")
.to_path_buf();
path
})
.map(|path| format!("/{}", path.display()))
.collect()
}

pub fn has_anchor(&self, anchor: &str) -> bool {
has_anchor(&self.toc, anchor)
}
Expand Down Expand Up @@ -589,8 +578,19 @@ And here's another. [^3]
assert_eq!(page.file.parent, path.join("content").join("posts"));
assert_eq!(page.slug, "with-assets");
assert_eq!(page.assets.len(), 3);
assert_eq!(page.serialized_assets.len(), 3);
assert!(page.serialized_assets[0].starts_with('/'));
assert_eq!(page.permalink, "http://a-website.com/posts/with-assets/");
assert_eq!(page.assets_permalinks.len(), 3);
let random_assets_permalinks_key =
page.assets_permalinks.keys().next().expect("assets permalinks key should be present");
assert!(!random_assets_permalinks_key.starts_with('/'));
let random_assets_permalinks_value = page
.assets_permalinks
.values()
.next()
.expect("assets permalinks value should be present");
assert!(random_assets_permalinks_value.starts_with(&page.permalink));
}

#[test]
Expand All @@ -614,6 +614,13 @@ And here's another. [^3]
assert_eq!(page.slug, "hey");
assert_eq!(page.assets.len(), 3);
assert_eq!(page.permalink, "http://a-website.com/posts/hey/");
assert_eq!(page.assets_permalinks.len(), 3);
let random_assets_permalinks_value = page
.assets_permalinks
.values()
.next()
.expect("assets permalinks value should be present");
assert!(random_assets_permalinks_value.starts_with(&page.permalink));
}

// https://github.com/getzola/zola/issues/674
Expand All @@ -640,6 +647,17 @@ And here's another. [^3]
// We should not get with-assets since that's the slugified version
assert!(page.serialized_assets[0].contains("with_assets"));
assert_eq!(page.permalink, "http://a-website.com/posts/with-assets/");
assert_eq!(page.assets_permalinks.len(), 3);
let random_assets_permalinks_key =
page.assets_permalinks.keys().next().expect("assets permalinks key should be present");
// We should not get with-assets since that's the slugified version
assert!(random_assets_permalinks_key.contains("with_assets"));
let random_assets_permalinks_value = page
.assets_permalinks
.values()
.next()
.expect("assets permalinks value should be present");
assert!(random_assets_permalinks_value.starts_with(&page.permalink));
}

// https://github.com/getzola/zola/issues/607
Expand All @@ -664,7 +682,21 @@ And here's another. [^3]
assert_eq!(page.slug, "with-assets");
assert_eq!(page.meta.date, Some("2013-06-02".to_string()));
assert_eq!(page.assets.len(), 3);
assert_eq!(page.serialized_assets.len(), 3);
// We should not get with-assets since that's the slugified version
assert!(page.serialized_assets[0].contains("2013-06-02"));
assert_eq!(page.permalink, "http://a-website.com/posts/with-assets/");
assert_eq!(page.assets_permalinks.len(), 3);
let random_assets_permalinks_key =
page.assets_permalinks.keys().next().expect("assets permalinks key should be present");
// We should not get with-assets since that's the slugified version
assert!(random_assets_permalinks_key.contains("2013-06-02"));
let random_assets_permalinks_value = page
.assets_permalinks
.values()
.next()
.expect("assets permalinks value should be present");
assert!(random_assets_permalinks_value.starts_with(&page.permalink));
}

#[test]
Expand Down Expand Up @@ -692,6 +724,8 @@ And here's another. [^3]
let page = res.unwrap();
assert_eq!(page.assets.len(), 1);
assert_eq!(page.assets[0].file_name().unwrap().to_str(), Some("graph.jpg"));
assert_eq!(page.serialized_assets.len(), 1);
assert_eq!(page.assets_permalinks.len(), 1);
}

// https://github.com/getzola/zola/issues/1566
Expand Down
49 changes: 34 additions & 15 deletions components/content/src/section.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ use crate::file_info::FileInfo;
use crate::front_matter::{split_section_content, SectionFrontMatter};
use crate::library::Library;
use crate::ser::{SectionSerMode, SerializingSection};
use crate::utils::{find_related_assets, get_reading_analytics, has_anchor};
use crate::utils::{
find_related_assets, get_assets_permalinks, get_reading_analytics, has_anchor, serialize_assets,
};

// Default is used to create a default index section if there is no _index.md in the root content directory
#[derive(Clone, Debug, Default, PartialEq, Eq)]
Expand All @@ -38,6 +40,8 @@ pub struct Section {
pub assets: Vec<PathBuf>,
/// All the non-md files we found next to the .md file as string
pub serialized_assets: Vec<String>,
/// The permalinks of all the non-md files we found next to the .md file
pub assets_permalinks: HashMap<String, String>,
/// All direct pages of that section
pub pages: Vec<PathBuf>,
/// All pages that cannot be sorted in this section
Expand Down Expand Up @@ -125,7 +129,16 @@ impl Section {

let parent_dir = path.parent().unwrap();
section.assets = find_related_assets(parent_dir, config, false);
section.serialized_assets = section.serialize_assets();
section.serialized_assets = serialize_assets(
&section.assets,
section.file.path.parent(),
section.file.colocated_path.as_ref(),
);
section.assets_permalinks = get_assets_permalinks(
&section.serialized_assets,
&section.permalink,
section.file.colocated_path.as_ref(),
);

Ok(section)
}
Expand Down Expand Up @@ -202,16 +215,6 @@ impl Section {
self.file.components.is_empty()
}

/// Creates a vectors of asset URLs.
fn serialize_assets(&self) -> Vec<String> {
self.assets
.iter()
.filter_map(|asset| asset.strip_prefix(self.file.path.parent().unwrap()).ok())
.filter_map(|filename| filename.to_str())
.map(|filename| format!("{}{}", self.path, filename))
.collect()
}

pub fn has_anchor(&self, anchor: &str) -> bool {
has_anchor(&self.toc, anchor)
}
Expand Down Expand Up @@ -269,8 +272,22 @@ mod tests {
assert!(res.is_ok());
let section = res.unwrap();
assert_eq!(section.assets.len(), 3);
assert_eq!(section.serialized_assets.len(), 3);
assert!(section.serialized_assets[0].starts_with('/'));
assert_eq!(section.permalink, "http://a-website.com/posts/with-assets/");
assert_eq!(section.assets_permalinks.len(), 3);
let random_assets_permalinks_key = section
.assets_permalinks
.keys()
.next()
.expect("assets permalinks key should be present");
assert!(!random_assets_permalinks_key.starts_with('/'));
let random_assets_permalinks_value = section
.assets_permalinks
.values()
.next()
.expect("assets permalinks value should be present");
assert!(random_assets_permalinks_value.starts_with(&section.permalink));
}

#[test]
Expand Down Expand Up @@ -300,9 +317,11 @@ mod tests {
Section::from_file(article_path.join("_index.md").as_path(), &config, &PathBuf::new());

assert!(res.is_ok());
let page = res.unwrap();
assert_eq!(page.assets.len(), 1);
assert_eq!(page.assets[0].file_name().unwrap().to_str(), Some("graph.jpg"));
let section = res.unwrap();
assert_eq!(section.assets.len(), 1);
assert_eq!(section.assets[0].file_name().unwrap().to_str(), Some("graph.jpg"));
assert_eq!(section.serialized_assets.len(), 1);
assert_eq!(section.assets_permalinks.len(), 1);
}

#[test]
Expand Down
Loading