[ENG-461] Location creation fails for '/' (root) and 'C:' ([A-Z] drives) (#710)

* Fix issue with root locations failing to be added
 - Add `normpath` dependency to the core
 - Improve automatic name resolution for new locations

* Add `MetadataExt` trait to extend the `created` and `modified` methods of `Metadata`
- Prevent failure in the Indexer job caused by paths without associated created or modified times; assume current time for both when unavailable
- Rename possible location in root path from `/` to `Root`

* Improve hadling of location path on Windows
 - Normalize location's path to ensure a valid format on Windows
 - Improve error handling when adding a path
 - Avoid consuming symlinks on Linux by not normalizing location's path
 - Improve location naming logic
This commit is contained in:
Vítor Vasconcellos 2023-04-18 06:12:02 +00:00 committed by GitHub
parent 98d846827d
commit 3fbc2b909c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 178 additions and 40 deletions

86
Cargo.lock generated
View file

@ -4608,6 +4608,15 @@ dependencies = [
"minimal-lexical",
]
[[package]]
name = "normpath"
version = "1.1.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ec60c60a693226186f5d6edf073232bfb6464ed97eb22cf3b01c1e8198fd97f5"
dependencies = [
"windows-sys 0.48.0",
]
[[package]]
name = "notify"
version = "5.0.0"
@ -6644,6 +6653,7 @@ dependencies = [
"int-enum",
"itertools",
"mini-moka",
"normpath",
"notify",
"once_cell",
"prisma-client-rust",
@ -9364,12 +9374,12 @@ version = "0.42.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5a3e1820f08b8513f676f7ab6c1f99ff312fb97b553d30ff4dd86f9f15728aa7"
dependencies = [
"windows_aarch64_gnullvm",
"windows_aarch64_gnullvm 0.42.1",
"windows_aarch64_msvc 0.42.1",
"windows_i686_gnu 0.42.1",
"windows_i686_msvc 0.42.1",
"windows_x86_64_gnu 0.42.1",
"windows_x86_64_gnullvm",
"windows_x86_64_gnullvm 0.42.1",
"windows_x86_64_msvc 0.42.1",
]
@ -9379,7 +9389,16 @@ version = "0.45.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "75283be5efb2831d37ea142365f009c02ec203cd29a3ebecbc093d52315b66d0"
dependencies = [
"windows-targets",
"windows-targets 0.42.1",
]
[[package]]
name = "windows-sys"
version = "0.48.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "677d2418bec65e3338edb076e806bc1ec15693c5d0104683f2efe857f61056a9"
dependencies = [
"windows-targets 0.48.0",
]
[[package]]
@ -9388,15 +9407,30 @@ version = "0.42.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "8e2522491fbfcd58cc84d47aeb2958948c4b8982e9a2d8a2a35bbaed431390e7"
dependencies = [
"windows_aarch64_gnullvm",
"windows_aarch64_gnullvm 0.42.1",
"windows_aarch64_msvc 0.42.1",
"windows_i686_gnu 0.42.1",
"windows_i686_msvc 0.42.1",
"windows_x86_64_gnu 0.42.1",
"windows_x86_64_gnullvm",
"windows_x86_64_gnullvm 0.42.1",
"windows_x86_64_msvc 0.42.1",
]
[[package]]
name = "windows-targets"
version = "0.48.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "7b1eb6f0cd7c80c79759c929114ef071b87354ce476d9d94271031c0497adfd5"
dependencies = [
"windows_aarch64_gnullvm 0.48.0",
"windows_aarch64_msvc 0.48.0",
"windows_i686_gnu 0.48.0",
"windows_i686_msvc 0.48.0",
"windows_x86_64_gnu 0.48.0",
"windows_x86_64_gnullvm 0.48.0",
"windows_x86_64_msvc 0.48.0",
]
[[package]]
name = "windows-tokens"
version = "0.39.0"
@ -9409,6 +9443,12 @@ version = "0.42.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "8c9864e83243fdec7fc9c5444389dcbbfd258f745e7853198f365e3c4968a608"
[[package]]
name = "windows_aarch64_gnullvm"
version = "0.48.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "91ae572e1b79dba883e0d315474df7305d12f569b400fcf90581b06062f7e1bc"
[[package]]
name = "windows_aarch64_msvc"
version = "0.32.0"
@ -9445,6 +9485,12 @@ version = "0.42.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "4c8b1b673ffc16c47a9ff48570a9d85e25d265735c503681332589af6253c6c7"
[[package]]
name = "windows_aarch64_msvc"
version = "0.48.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b2ef27e0d7bdfcfc7b868b317c1d32c641a6fe4629c171b8928c7b08d98d7cf3"
[[package]]
name = "windows_i686_gnu"
version = "0.32.0"
@ -9481,6 +9527,12 @@ version = "0.42.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "de3887528ad530ba7bdbb1faa8275ec7a1155a45ffa57c37993960277145d640"
[[package]]
name = "windows_i686_gnu"
version = "0.48.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "622a1962a7db830d6fd0a69683c80a18fda201879f0f447f065a3b7467daa241"
[[package]]
name = "windows_i686_msvc"
version = "0.32.0"
@ -9517,6 +9569,12 @@ version = "0.42.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "bf4d1122317eddd6ff351aa852118a2418ad4214e6613a50e0191f7004372605"
[[package]]
name = "windows_i686_msvc"
version = "0.48.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "4542c6e364ce21bf45d69fdd2a8e455fa38d316158cfd43b3ac1c5b1b19f8e00"
[[package]]
name = "windows_x86_64_gnu"
version = "0.32.0"
@ -9553,12 +9611,24 @@ version = "0.42.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c1040f221285e17ebccbc2591ffdc2d44ee1f9186324dd3e84e99ac68d699c45"
[[package]]
name = "windows_x86_64_gnu"
version = "0.48.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ca2b8a661f7628cbd23440e50b05d705db3686f894fc9580820623656af974b1"
[[package]]
name = "windows_x86_64_gnullvm"
version = "0.42.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "628bfdf232daa22b0d64fdb62b09fcc36bb01f05a3939e20ab73aaf9470d0463"
[[package]]
name = "windows_x86_64_gnullvm"
version = "0.48.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "7896dbc1f41e08872e9d5e8f8baa8fdd2677f29468c4e156210174edc7f7b953"
[[package]]
name = "windows_x86_64_msvc"
version = "0.32.0"
@ -9595,6 +9665,12 @@ version = "0.42.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "447660ad36a13288b1db4d4248e857b510e8c3a225c822ba4fb748c0aafecffd"
[[package]]
name = "windows_x86_64_msvc"
version = "0.48.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "1a515f5799fe4961cb532f983ce2b23082366b898e52ffbce459c86f67c8378a"
[[package]]
name = "winreg"
version = "0.10.1"

View file

@ -18,9 +18,9 @@ sync-messages = []
[dependencies]
sd-ffmpeg = { path = "../crates/ffmpeg", optional = true }
sd-crypto = { path = "../crates/crypto", features = [
"rspc",
"serde",
"keymanager",
"rspc",
"serde",
"keymanager",
] }
sd-file-ext = { path = "../crates/file-ext" }
sd-sync = { path = "../crates/sync" }
@ -31,11 +31,11 @@ httpz = { workspace = true }
prisma-client-rust = { workspace = true }
specta = { workspace = true }
tokio = { workspace = true, features = [
"sync",
"rt-multi-thread",
"io-util",
"macros",
"time",
"sync",
"rt-multi-thread",
"io-util",
"macros",
"time",
] }
base64 = "0.21.0"
@ -70,10 +70,11 @@ serde_with = "2.2.0"
dashmap = { version = "5.4.0", features = ["serde"] }
ffmpeg-next = { version = "6.0.0", optional = true, features = [] }
notify = { version = "5.0.0", default-features = false, features = [
"macos_fsevent",
"macos_fsevent",
], optional = true }
static_assertions = "1.1.0"
serde-hashkey = "0.4.5"
normpath = { version = "1.1.1", features = ["localization"] }
[target.'cfg(windows)'.dependencies.winapi-util]
version = "0.1.5"

View file

@ -5,6 +5,7 @@ use std::{
fmt::{Display, Formatter},
fs::Metadata,
path::{Path, PathBuf, MAIN_SEPARATOR, MAIN_SEPARATOR_STR},
time::SystemTime,
};
use chrono::{DateTime, Utc};
@ -724,3 +725,19 @@ pub async fn get_inode_and_device_from_path(
Ok((info.file_index(), info.volume_serial_number()))
}
}
pub trait MetadataExt {
fn created_or_now(&self) -> SystemTime;
fn modified_or_now(&self) -> SystemTime;
}
impl MetadataExt for Metadata {
fn created_or_now(&self) -> SystemTime {
self.created().unwrap_or_else(|_| SystemTime::now())
}
fn modified_or_now(&self) -> SystemTime {
self.modified().unwrap_or_else(|_| SystemTime::now())
}
}

View file

@ -1,4 +1,4 @@
use crate::location::file_path_helper::FilePathMetadata;
use crate::location::file_path_helper::{FilePathMetadata, MetadataExt};
#[cfg(target_family = "unix")]
use crate::location::file_path_helper::get_inode_and_device;
@ -291,8 +291,8 @@ async fn inner_walk_single_dir(
inode,
device,
size_in_bytes: metadata.len(),
created_at: metadata.created()?.into(),
modified_at: metadata.modified()?.into(),
created_at: metadata.created_or_now().into(),
modified_at: metadata.modified_or_now().into(),
},
},
);
@ -327,8 +327,8 @@ async fn inner_walk_single_dir(
inode,
device,
size_in_bytes: metadata.len(),
created_at: metadata.created()?.into(),
modified_at: metadata.modified()?.into(),
created_at: metadata.created_or_now().into(),
modified_at: metadata.modified_or_now().into(),
},
},
);
@ -372,8 +372,8 @@ async fn prepared_indexed_paths(
inode,
device,
size_in_bytes: metadata.len(),
created_at: metadata.created()?.into(),
modified_at: metadata.modified()?.into(),
created_at: metadata.created_or_now().into(),
modified_at: metadata.modified_or_now().into(),
},
});
}

View file

@ -6,7 +6,7 @@ use crate::{
file_path_helper::{
extract_materialized_path, file_path_with_object, filter_existing_file_path_params,
get_parent_dir, get_parent_dir_id, loose_find_existing_file_path_params, FilePathError,
FilePathMetadata, MaterializedPath,
FilePathMetadata, MaterializedPath, MetadataExt,
},
find_location, location_with_indexer_rules,
manager::LocationManagerError,
@ -117,8 +117,8 @@ pub(super) async fn create_dir(
inode,
device,
size_in_bytes: metadata.len(),
created_at: metadata.created()?.into(),
modified_at: metadata.modified()?.into(),
created_at: metadata.created_or_now().into(),
modified_at: metadata.modified_or_now().into(),
},
)
.await?;
@ -191,8 +191,8 @@ pub(super) async fn create_file(
inode,
device,
size_in_bytes: metadata.len(),
created_at: metadata.created()?.into(),
modified_at: metadata.modified()?.into(),
created_at: metadata.created_or_now().into(),
modified_at: metadata.modified_or_now().into(),
},
)
.await?;
@ -217,7 +217,7 @@ pub(super) async fn create_file(
Uuid::new_v4().as_bytes().to_vec(),
vec![
object::date_created::set(
DateTime::<Local>::from(fs_metadata.created().unwrap()).into(),
DateTime::<Local>::from(fs_metadata.created_or_now()).into(),
),
object::kind::set(kind.int_value()),
],
@ -373,7 +373,7 @@ async fn inner_update_file(
file_path::size_in_bytes::set(fs_metadata.len().to_string()),
),
{
let date = DateTime::<Local>::from(fs_metadata.modified()?).into();
let date = DateTime::<Local>::from(fs_metadata.modified_or_now()).into();
(
("date_modified", json!(date)),

View file

@ -17,11 +17,11 @@ use crate::{
use std::{
collections::HashSet,
ffi::OsStr,
path::{Path, PathBuf},
path::{Component, Path, PathBuf},
};
use futures::future::TryFutureExt;
use normpath::PathExt;
use prisma_client_rust::QueryError;
use rspc::Type;
use serde::Deserialize;
@ -452,18 +452,62 @@ async fn create_location(
) -> Result<location_with_indexer_rules::Data, LocationError> {
let Library { db, sync, .. } = &library;
let location_path = location_path.as_ref();
let mut location_path = location_path.as_ref().to_path_buf();
let name = location_path
.file_name()
.and_then(OsStr::to_str)
.map(str::to_string)
.unwrap();
let (path, normalized_path) = location_path
// Normalize path and also check if it exists
.normalize()
.and_then(|normalized_path| {
if cfg!(windows) {
// Use normalized path as location path on Windows
// This ensures we always receive a valid windows formated path
// ex: /Users/JohnDoe/Downloads will become C:\Users\JohnDoe\Downloads
// Internally `normalize` calls `GetFullPathNameW` on Windows
// https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getfullpathnamew
location_path = normalized_path.as_path().to_path_buf();
}
let path = location_path
.to_str()
.map(str::to_string)
.expect("Found non-UTF-8 path");
Ok((
// TODO: Maybe save the path bytes instead of the string representation to avoid depending on UTF-8
location_path
.to_str()
.map(str::to_string)
.ok_or(io::Error::new(
io::ErrorKind::InvalidInput,
"Found non-UTF-8 path",
))?,
normalized_path,
))
})
.map_err(|_| {
LocationError::DirectoryNotFound(location_path.to_string_lossy().to_string())
})?;
// Not needed on Windows because the normalization already handles it
if cfg!(not(windows)) {
// Replace location_path with normalize_path, when the first one ends in `.` or `..`
// This is required so localize_name doesn't panic
if let Some(component) = location_path.components().next_back() {
match component {
Component::CurDir | Component::ParentDir => {
location_path = normalized_path.as_path().to_path_buf();
}
_ => {}
}
}
}
// Use `to_string_lossy` because a partially corrupted but identifiable name is better than nothing
let mut name = location_path.localize_name().to_string_lossy().to_string();
// Windows doesn't have a root directory
if cfg!(not(windows)) && name == "/" {
name = "Root".to_string()
}
if name.replace(char::REPLACEMENT_CHARACTER, "") == "" {
name = "Unknown".to_string()
}
let location = sync
.write_op(