Skip to content

feat(core): add AssetManager::iter #8288

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.

Sign up for GitHub

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 1 commit into from
Nov 23, 2023
Merged

feat(core): add AssetManager::iter #8288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changes/asset-iter.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"tauri": patch:enhance
---

Added `AssetResolver::iter` to iterate on all embedded assets.
7 changes: 7 additions & 0 deletions core/tauri-utils/src/assets.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ pub trait Assets: Send + Sync + 'static {
/// Get the content of the passed [`AssetKey`].
fn get(&self, key: &AssetKey) -> Option<Cow<'_, [u8]>>;

/// Iterator for the assets.
fn iter(&self) -> Box<dyn Iterator<Item = (&&str, &&[u8])> + '_>;

/// Gets the hashes for the CSP tag of the HTML on the given path.
fn csp_hashes(&self, html_path: &AssetKey) -> Box<dyn Iterator<Item = CspHash<'_>> + '_>;
}
Expand Down Expand Up @@ -163,6 +166,10 @@ impl Assets for EmbeddedAssets {
.map(|a| Cow::Owned(a.to_vec()))
}

fn iter(&self) -> Box<dyn Iterator<Item = (&&str, &&[u8])> + '_> {
Box::new(self.assets.into_iter())
Copy link
Member

@amrbashir amrbashir Nov 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Do we really need the box here? can't we implement InotIterator directly on the type and it construct a new type AssetEntries that is an iterator?

Copy link
Contributor Author

@lucasfernog-crabnebula lucasfernog-crabnebula Nov 22, 2023

Choose a reason for hiding this comment

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

it needs to be dyn because we're in the context of a trait.. so it needs Box too to be sized. Writing another type doesn't help this. It's also the solution used for csp_hashes.

Copy link
Member

@amrbashir amrbashir Nov 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

If we return an AssetsEntries struct, we don't need to deal with dyn and can just implement Iterator on AssetsEntries

      fn iter<'a>(&'a self) -> AssetsEntries<'a, &'_ str, &'_ [u8]>;

or we can return phf::map::Entries directly

      fn iter<'a>(&'a self) -> phf::map::Entries<'a, &'_ str, &'_ [u8]>;

This should also be done for csp_hashes as well but to avoid breaking changes, we can delay that change to v2

Copy link
Contributor Author

@lucasfernog-crabnebula lucasfernog-crabnebula Nov 23, 2023

Choose a reason for hiding this comment

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

but then the AssetEntries need to hold the iterator anyway, unless we force the type to be a HashMap which isn't a good idea really for a trait

Copy link
Member

@amrbashir amrbashir Nov 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The only downside I see with this, is that AssetsEntries will need to use phf::map::Entries as a concrete type under the hood and now the trait Assets is not so generic and that makes me wonder, do we need this trait? seems like our usage of it, could be just replaced with the concrete EmbeddedAssets type

edit: GitHub didn't show me your previous message about the HashMap

Copy link
Contributor Author

@lucasfernog-crabnebula lucasfernog-crabnebula Nov 23, 2023

Choose a reason for hiding this comment

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

for tests it's nice to have this trait

but we can also put the author @chippers on the line

Copy link
Member

@amrbashir amrbashir Nov 23, 2023

Choose a reason for hiding this comment

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

this is not a blocker btw, just would be a nice improvement for v2 (unless the trait is really required)

Copy link
Member

@chippers chippers Dec 12, 2023

Choose a reason for hiding this comment

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

Late on seeing this @amrbashir @lucasfernog-crabnebula , the asset trait allows people to implement their own asset handling, such as if they want to fetch them from disk or network. it's one of the original reasons it became possible to change the items in a Context

}

fn csp_hashes(&self, html_path: &AssetKey) -> Box<dyn Iterator<Item = CspHash<'_>> + '_> {
Box::new(
self
Expand Down
5 changes: 5 additions & 0 deletions core/tauri/src/app.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,11 @@ impl<R: Runtime> AssetResolver<R> {
pub fn get(&self, path: String) -> Option<Asset> {
self.manager.get_asset(path).ok()
}

/// Iterate on all assets.
pub fn iter(&self) -> Box<dyn Iterator<Item = (&&str, &&[u8])> + '_> {
self.manager.asset_iter()
}
}

/// A handle to the currently running application.
Expand Down
4 changes: 4 additions & 0 deletions core/tauri/src/manager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,10 @@ impl<R: Runtime> WindowManager<R> {
})
}

pub fn asset_iter(&self) -> Box<dyn Iterator<Item = (&&str, &&[u8])> + '_> {
self.inner.assets.iter()
}

pub fn get_asset(&self, mut path: String) -> Result<Asset, Box<dyn std::error::Error>> {
let assets = &self.inner.assets;
if path.ends_with('/') {
Expand Down
6 changes: 6 additions & 0 deletions core/tauri/src/test/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ struct Ipc(Mutex<HashMap<IpcKey, Sender<std::result::Result<JsonValue, JsonValue

/// An empty [`Assets`] implementation.
pub struct NoopAsset {
assets: HashMap<&'static str, &'static [u8]>,
csp_hashes: Vec<CspHash<'static>>,
}

Expand All @@ -107,6 +108,10 @@ impl Assets for NoopAsset {
None
}

fn iter(&self) -> Box<dyn Iterator<Item = (&&str, &&[u8])> + '_> {
Box::new(self.assets.iter())
}

fn csp_hashes(&self, html_path: &AssetKey) -> Box<dyn Iterator<Item = CspHash<'_>> + '_> {
Box::new(self.csp_hashes.iter().copied())
}
Expand All @@ -115,6 +120,7 @@ impl Assets for NoopAsset {
/// Creates a new empty [`Assets`] implementation.
pub fn noop_assets() -> NoopAsset {
NoopAsset {
assets: Default::default(),
csp_hashes: Default::default(),
}
}
Expand Down