Skip to content

Commit

Permalink
Fix remaining issues with glob removal
Browse files Browse the repository at this point in the history
  • Loading branch information
emabee committed Oct 12, 2024
1 parent caa4331 commit 6d308ff
Show file tree
Hide file tree
Showing 6 changed files with 141 additions and 82 deletions.
10 changes: 10 additions & 0 deletions src/logger_handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,7 @@ pub struct LogfileSelector {
pub(crate) with_plain_files: bool,
pub(crate) with_r_current: bool,
pub(crate) with_compressed_files: bool,
pub(crate) with_configured_current: Option<String>,
}
impl Default for LogfileSelector {
/// Selects plain log files without the `rCURRENT` file.
Expand All @@ -422,6 +423,7 @@ impl Default for LogfileSelector {
with_plain_files: true,
with_r_current: false,
with_compressed_files: false,
with_configured_current: None,
}
}
}
Expand All @@ -433,6 +435,7 @@ impl LogfileSelector {
with_plain_files: false,
with_r_current: false,
with_compressed_files: false,
with_configured_current: None,
}
}
/// Selects additionally the `rCURRENT` file.
Expand All @@ -442,6 +445,13 @@ impl LogfileSelector {
self
}

/// Selects additionally a custom "current" file.
#[must_use]
pub fn with_custom_current(mut self, s: &str) -> Self {
self.with_configured_current = Some(s.to_string());
self
}

/// Selects additionally the compressed log files.
#[must_use]
pub fn with_compressed_files(mut self) -> Self {
Expand Down
110 changes: 56 additions & 54 deletions src/parameters/file_spec.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::{DeferredNow, FlexiLoggerError};
use std::{
ffi::OsStr,
ffi::{OsStr, OsString},
ops::Add,
path::{Path, PathBuf},
};
Expand Down Expand Up @@ -49,12 +49,12 @@ pub struct FileSpec {
pub(crate) basename: String,
pub(crate) o_discriminant: Option<String>,
timestamp_cfg: TimestampCfg,
pub(crate) o_suffix: Option<String>,
o_suffix: Option<String>,
pub(crate) use_utc: bool,
}
impl Default for FileSpec {
/// Describes a file in the current folder,
/// using, as its filestem the program name followed by the current timestamp,
/// using, as its filestem, the program name followed by the current timestamp,
/// and the suffix ".log".
#[must_use]
fn default() -> Self {
Expand Down Expand Up @@ -287,33 +287,20 @@ impl FileSpec {

// handles collisions by appending ".restart-<number>" to the infix, if necessary
pub(crate) fn collision_free_infix_for_rotated_file(&self, infix: &str) -> String {
// Some("log") -> ["log", "log.gz"], None -> [".gz"]:
let suffices = self
.o_suffix
.clone()
.into_iter()
.chain(
self.o_suffix
.as_deref()
.or(Some(""))
.map(|s| [s, ".gz"].concat()),
)
.collect::<Vec<String>>();

let mut restart_siblings = self
.list_related_files()
.read_dir_related_files()
.into_iter()
.filter(|pb| {
// ignore files with irrelevant suffixes:
// TODO this does not work correctly if o_suffix = None, because we ignore all
// non-compressed files
pb.file_name()
.map(OsStr::to_string_lossy)
.filter(|file_name| {
file_name.ends_with(&suffices[0])
|| suffices.len() > 1 && file_name.ends_with(&suffices[1])
})
.is_some()
// ignore .gz suffix
let mut pb2 = PathBuf::from(pb);
if pb2.extension() == Some(OsString::from("gz").as_ref()) {
pb2.set_extension("");
};
// suffix must match the given suffix, if one is given
match self.o_suffix {
Some(ref sfx) => pb2.extension() == Some(OsString::from(sfx).as_ref()),
None => true,
}
})
.filter(|pb| {
pb.file_name()
Expand Down Expand Up @@ -356,14 +343,47 @@ impl FileSpec {
}
}

pub(crate) fn list_of_files(
pub(crate) fn list_of_files<F>(&self, infix_filter: F, o_suffix: Option<&str>) -> Vec<PathBuf>
where
F: Fn(&str) -> bool,
{
self.filter_files(&self.read_dir_related_files(), infix_filter, o_suffix)
}

// returns an ordered list of all files in the right directory that start with the fixed_name_part
pub(crate) fn read_dir_related_files(&self) -> Vec<PathBuf> {
let fixed_name_part = self.fixed_name_part();
let mut log_files = std::fs::read_dir(&self.directory)
.unwrap(/*ignore errors from reading the directory*/)
.flatten(/*ignore errors from reading entries in the directory*/)
.filter(|entry| entry.path().is_file())
.map(|de| de.path())
.filter(|path| {
// fixed name part must match
if let Some(fln) = path.file_name() {
fln.to_string_lossy(/*good enough*/).starts_with(&fixed_name_part)
} else {
false
}
})
.collect::<Vec<PathBuf>>();
log_files.sort_unstable();
log_files.reverse();
log_files
}

pub(crate) fn filter_files<F>(
&self,
infix_filter: fn(&str) -> bool,
files: &[PathBuf],
infix_filter: F,
o_suffix: Option<&str>,
) -> Vec<PathBuf> {
) -> Vec<PathBuf>
where
F: Fn(&str) -> bool,
{
let fixed_name_part = self.fixed_name_part();
self.list_related_files()
.into_iter()
files
.iter()
.filter(|path| {
// if suffix is specified, it must match
if let Some(suffix) = o_suffix {
Expand All @@ -383,34 +403,16 @@ impl FileSpec {
} else {
fixed_name_part.len() + 1 // underscore at the end
};
if stem.len() <= infix_start {
return false;
}
let maybe_infix = &stem[infix_start..];
infix_filter(maybe_infix)
})
.map(PathBuf::clone)
.collect::<Vec<PathBuf>>()
}

// returns an ordered list of all files in the right directory that start with the fixed_name_part
fn list_related_files(&self) -> Vec<PathBuf> {
let fixed_name_part = self.fixed_name_part();
let mut log_files = std::fs::read_dir(&self.directory)
.unwrap(/*ignore errors from reading the directory*/)
.flatten(/*ignore errors from reading entries in the directory*/)
.filter(|entry| entry.path().is_file())
.map(|de| de.path())
.filter(|path| {
// fixed name part must match
if let Some(fln) = path.file_name() {
fln.to_string_lossy(/*good enough*/).starts_with(&fixed_name_part)
} else {
false
}
})
.collect::<Vec<PathBuf>>();
log_files.sort_unstable();
log_files.reverse();
log_files
}

#[cfg(test)]
pub(crate) fn get_timestamp(&self) -> Option<String> {
self.timestamp_cfg.get_timestamp()
Expand Down
2 changes: 1 addition & 1 deletion src/writers/file_log_writer/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ impl FileLogWriterConfig {
/// Returns the configured `suffix`.
#[must_use]
pub fn suffix(&self) -> Option<String> {
self.file_spec.o_suffix.clone()
self.file_spec.get_suffix()
}

/// Returns `true` if UTC is enforced.
Expand Down
45 changes: 26 additions & 19 deletions src/writers/file_log_writer/state/list_and_cleanup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,15 @@ use std::{
thread::{Builder as ThreadBuilder, JoinHandle},
};

// looks like std infix if it starts with r and a digit
fn looks_like_std_infix(s: &str) -> bool {
// looks like a standard infix if it starts with an r and a digit
pub(crate) fn looks_like_std_infix(s: &str) -> bool {
if s.len() > 2 {
let mut chars = s.chars();
chars.next().unwrap() == 'r' && chars.next().unwrap().is_ascii_digit()
} else {
false
}
}
// FIXME !! must be flexibilized
fn is_current_infix(s: &str) -> bool {
s == "rCURRENT"
}

pub(super) fn list_of_log_and_compressed_files(file_spec: &FileSpec) -> Vec<PathBuf> {
existing_log_files(
Expand All @@ -34,32 +30,43 @@ pub(super) fn existing_log_files(
selector: &LogfileSelector,
) -> Vec<PathBuf> {
let mut result = Vec::new();
let related_files = file_spec.read_dir_related_files();

if use_rotation {
if selector.with_plain_files {
result.append(
&mut file_spec
.list_of_files(looks_like_std_infix, file_spec.get_suffix().as_deref()),
);
result.append(&mut file_spec.filter_files(
&related_files,
looks_like_std_infix,
file_spec.get_suffix().as_deref(),
));
}

if selector.with_compressed_files {
result.append(&mut file_spec.list_of_files(looks_like_std_infix, Some("gz")));
result.append(&mut file_spec.filter_files(
&related_files,
looks_like_std_infix,
Some("gz"),
));
}

if selector.with_r_current {
result.append(
&mut file_spec.list_of_files(is_current_infix, file_spec.get_suffix().as_deref()),
);
result.append(&mut file_spec.filter_files(
&related_files,
|s: &str| s == super::CURRENT_INFIX,
file_spec.get_suffix().as_deref(),
));
}
if let Some(ref custom_current) = selector.with_configured_current {
result.append(&mut file_spec.filter_files(
&related_files,
|s: &str| s == custom_current,
file_spec.get_suffix().as_deref(),
));
}
} else {
result.push(file_spec.as_pathbuf(None));
}
result
}

pub(super) fn list_of_infix_files(file_spec: &FileSpec) -> Vec<PathBuf> {
file_spec.list_of_files(looks_like_std_infix, file_spec.get_suffix().as_deref())
}
pub(super) fn remove_or_compress_too_old_logfiles(
o_cleanup_thread_handle: Option<&CleanupThreadHandle>,
cleanup_config: &Cleanup,
Expand Down
9 changes: 7 additions & 2 deletions src/writers/file_log_writer/state/timestamps.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::{get_creation_timestamp, list_and_cleanup::list_of_infix_files, InfixFormat};
use super::{get_creation_timestamp, list_and_cleanup::looks_like_std_infix, InfixFormat};
use crate::{writers::FileLogWriterConfig, FileSpec};
use chrono::{DateTime, Local, NaiveDateTime, TimeZone};
use std::path::{Path, PathBuf};
Expand Down Expand Up @@ -72,7 +72,12 @@ pub(super) fn latest_timestamp_file(
Local::now()
} else {
// find all file paths that fit the pattern
list_of_infix_files(&config.file_spec)
config
.file_spec
.list_of_files(
looks_like_std_infix,
config.file_spec.get_suffix().as_deref(),
)
.into_iter()
// retrieve the infix
.map(|path| ts_infix_from_path(&path, &config.file_spec))
Expand Down
47 changes: 41 additions & 6 deletions tests/test_restart_with_no_suffix.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
mod test_utils;

use flexi_logger::{Cleanup, Criterion, FileSpec, Logger, Naming};
use std::{
fs::File,
io::{BufRead, BufReader},
};

use flexi_logger::{Cleanup, Criterion, FileSpec, LogfileSelector, Logger, Naming};
use log::*;

const COUNT: u8 = 2;
Expand All @@ -18,21 +23,51 @@ fn work(value: u8) {
let file_spec = FileSpec::default()
.directory(directory)
.o_suffix(match value {
0 => None,
1 => Some("log".to_string()),
0 => Some("log".to_string()),
1 => None,
COUNT..=u8::MAX => {
unreachable!("got unexpected value {}", value)
}
});

let _ = Logger::try_with_str("debug")
let logger = Logger::try_with_str("debug")
.unwrap()
.log_to_file(file_spec)
.rotate(Criterion::Size(100), Naming::Timestamps, Cleanup::Never)
.start()
.unwrap_or_else(|e| panic!("Logger initialization failed with {e}"));

for _ in 0..100 {
error!("This is an error message");
for i in 0..100 {
error!("This is error message {i}");
std::thread::sleep(std::time::Duration::from_millis(10));
}

let mut contents = String::new();

assert_eq!(
100,
logger
.existing_log_files(&LogfileSelector::default().with_r_current())
.unwrap()
.into_iter()
.filter(|pb| {
let extension = pb.extension().map(|s| s.to_string_lossy().into_owned());
match value {
0 => Some("log".to_string()) == extension,
1 => extension.is_none() || extension.unwrap().starts_with("restart"),
COUNT..=u8::MAX => {
unreachable!("got unexpected value {}", value)
}
}
})
.map(|path| {
let mut buf_reader = BufReader::new(File::open(path).unwrap());
let mut line_count = 0;
while buf_reader.read_line(&mut contents).unwrap() > 0 {
line_count += 1;
}
line_count
})
.sum::<u32>()
);
}

0 comments on commit 6d308ff

Please sign in to comment.