Skip to content

Commit

Permalink
Fix issue 176 (avoid leading underscore in log file name)
Browse files Browse the repository at this point in the history
  • Loading branch information
emabee committed Oct 2, 2024
1 parent 1da74b1 commit b35af15
Show file tree
Hide file tree
Showing 9 changed files with 213 additions and 120 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this
project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [0.29.1] - 2024-10-02

Fix [issue #176](https://github.com/emabee/flexi_logger/issues/176):
leading underscore in log file name if only the infix is used (no basename, no start time, no discriminant).

## [0.29.0] - 2024-08-25

Revised `SyslogWriter` (-> version bump): introduced builder pattern,
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "flexi_logger"
version = "0.29.0"
version = "0.29.1"
authors = ["emabee <meinolf.block@sap.com>"]
categories = ["development-tools::debugging"]
description = """
Expand Down
58 changes: 43 additions & 15 deletions src/parameters/file_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,28 +4,41 @@ use std::path::{Path, PathBuf};

/// Builder object for specifying the name and path of the log output file.
///
/// The filename is built from several partially optional components, using this pattern:
/// The filename is built from several partially components, using this pattern:
///
/// ```<basename>[_<discriminant>][_<date>_<time>][_infix][.<suffix>]```
/// ```<filename> = [<basename>][_][<discriminant>][_][<starttime>][_][<infix>][.<suffix>]```
///
/// The default filename pattern without rotation uses the program name as basename,
/// no discriminant, the timestamp of the program start, and the suffix `.log`,
/// e.g.
/// - `[<basename>]`: This is by default the program's name, but can be set to a different value
/// or suppressed at all.
///
/// - `[_]`: Consecutive name parts are separated by an underscore.
/// No underscore is used at the beginning of the filename and directly before the suffix.
///
/// - `[<discriminant>]`: some optional name part that allows further differentiations.
///
/// - `[<starttime>]`: denotes the point in time when the program was started, if used.
///
/// - `[infix]`: used with rotation to differentiate consecutive files.
///
/// Without rotation, the default filename pattern uses the program name as basename,
/// no discriminant, the timestamp of the program start
/// (printed in the format "YYYY-MM-DD_hh-mm-ss"),
/// and the suffix `.log`, e.g.
///
/// ```myprog_2015-07-08_10-44-11.log```.
///
/// This ensures that with every program start a new trace file is written that can easily
/// be associated with a concrete program run.
///
/// When the timestamp is suppressed with [`FileSpec::suppress_timestamp`],
/// you get a fixed output file.
/// you get a fixed output file name.
/// It is then worth considering whether a new program start should discard
/// the content of an already existing outputfile or if it should append its new content to it
/// (see [`Logger::append`](crate::Logger::append)).
///
/// With rotation the timestamp is by default suppressed and instead the infix is used.
/// The infix always starts with "_r". For more details how its precise content can be influenced,
/// see [`Naming`](crate::Naming).
/// With rotation, the timestamp is by default suppressed and instead the infix is used.
/// The infix starts always with "r".
/// For more details how its precise content can be influenced, see [`Naming`](crate::Naming).
///
#[derive(Debug, Clone, Eq, PartialEq)]
pub struct FileSpec {
Expand Down Expand Up @@ -169,15 +182,15 @@ impl FileSpec {
self
}

/// Makes the logger not include a timestamp into the names of the log files
/// Makes the logger not include the start time into the names of the log files
///
/// Equivalent to `use_timestamp(false)`.
#[must_use]
pub fn suppress_timestamp(self) -> Self {
self.use_timestamp(false)
}

/// Defines if a timestamp should be included into the names of the log files.
/// Defines if the start time should be included into the names of the log files.
///
/// The _default_ behavior depends on the usage:
/// - without rotation, a timestamp is by default included into the name
Expand All @@ -192,6 +205,21 @@ impl FileSpec {
self
}

#[doc(hidden)]
#[must_use]
pub fn used_directory(&self) -> PathBuf {
self.directory.clone()
}
pub(crate) fn has_basename(&self) -> bool {
!self.basename.is_empty()
}
pub(crate) fn has_discriminant(&self) -> bool {
self.o_discriminant.is_some()
}
pub(crate) fn uses_timestamp(&self) -> bool {
matches!(self.timestamp_cfg, TimestampCfg::Yes)
}

// If no decision was done yet, decide now whether to include a timestamp
// into the names of the log files.
pub(crate) fn if_default_use_timestamp(&mut self, use_timestamp: bool) {
Expand All @@ -213,9 +241,6 @@ impl FileSpec {
}

/// Derives a `PathBuf` from the spec and the given infix.
///
/// It is composed like this:
/// `<directory>/<basename>_<discr>_<timestamp><infix>.<suffix>`
#[must_use]
pub fn as_pathbuf(&self, o_infix: Option<&str>) -> PathBuf {
let mut filename = self.basename.clone();
Expand All @@ -230,6 +255,7 @@ impl FileSpec {
filename.push_str(timestamp);
}
if let Some(infix) = o_infix {
FileSpec::separate_with_underscore(&mut filename);
filename.push_str(infix);
};
if let Some(suffix) = &self.o_suffix {
Expand All @@ -248,7 +274,7 @@ impl FileSpec {
}
}

// <directory>/<basename>_<discr>_<timestamp><infix_pattern>.<suffix>
// <directory>/[<basename>][_][<discriminant>][_][<starttime>][_][<infix_pattern>]
pub(crate) fn as_glob_pattern(&self, infix_pattern: &str, o_suffix: Option<&str>) -> String {
let mut filename = self.basename.clone();
filename.reserve(50);
Expand All @@ -261,6 +287,8 @@ impl FileSpec {
FileSpec::separate_with_underscore(&mut filename);
filename.push_str(timestamp);
}

FileSpec::separate_with_underscore(&mut filename);
filename.push_str(infix_pattern);

match o_suffix {
Expand Down
32 changes: 10 additions & 22 deletions src/writers/file_log_writer/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ use std::{
sync::{Arc, Mutex},
};
use timestamps::{
collision_free_infix_for_rotated_file, infix_from_timestamp, latest_timestamp_file,
rcurrents_creation_timestamp,
collision_free_infix_for_rotated_file, creation_timestamp_of_currentfile, infix_from_timestamp,
latest_timestamp_file,
};

#[cfg(feature = "async")]
Expand All @@ -34,7 +34,7 @@ use {
#[cfg(feature = "async")]
const ASYNC_WRITER: &str = "flexi_logger-fs-async_writer";

const CURRENT_INFIX: &str = "_rCURRENT";
const CURRENT_INFIX: &str = "rCURRENT";

#[derive(Debug)]
enum NamingState {
Expand Down Expand Up @@ -64,11 +64,9 @@ pub(super) enum InfixFormat {
Custom(String),
}
impl InfixFormat {
const STD_INFIX_FORMAT: &'static str = "_r%Y-%m-%d_%H-%M-%S";
pub(super) fn custom(f: &str) -> Self {
let mut fmt = "_".to_string();
fmt.push_str(f);
Self::Custom(fmt)
const STD_INFIX_FORMAT: &'static str = "r%Y-%m-%d_%H-%M-%S";
pub(super) fn custom(fmt: &str) -> Self {
Self::Custom(fmt.to_string())
}
fn format(&self) -> &str {
match self {
Expand Down Expand Up @@ -306,7 +304,7 @@ impl State {
}
Naming::Timestamps => (
NamingState::Timestamps(
rcurrents_creation_timestamp(
creation_timestamp_of_currentfile(
&self.config,
CURRENT_INFIX,
!self.config.append,
Expand All @@ -323,9 +321,9 @@ impl State {
format: ts_fmt,
} => {
if let Some(current_token) = o_current_token {
let current_infix = prepend_underscore(current_token);
let current_infix = current_token.to_string();
let naming_state = NamingState::Timestamps(
rcurrents_creation_timestamp(
creation_timestamp_of_currentfile(
&self.config,
&current_infix,
!self.config.append,
Expand Down Expand Up @@ -426,7 +424,7 @@ impl State {
let infix = match rotation_state.naming_state {
NamingState::Timestamps(ref mut ts, ref o_current_infix, ref fmt) => {
if let Some(current_infix) = o_current_infix {
*ts = rcurrents_creation_timestamp(
*ts = creation_timestamp_of_currentfile(
&self.config,
current_infix,
true,
Expand Down Expand Up @@ -551,16 +549,6 @@ impl State {
}
}

fn prepend_underscore(infix: &str) -> String {
if infix.is_empty() {
infix.to_string()
} else {
let mut infix_with_underscore = "_".to_string();
infix_with_underscore.push_str(infix);
infix_with_underscore
}
}

fn validate_logs_in_file(
reader: &mut dyn BufRead,
path: &Path,
Expand Down
2 changes: 1 addition & 1 deletion src/writers/file_log_writer/state/list_and_cleanup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::{
thread::{Builder as ThreadBuilder, JoinHandle},
};

const INFIX_PATTERN: &str = "_r[0-9]*";
const INFIX_PATTERN: &str = "r[0-9]*";

pub(super) fn existing_log_files(
file_spec: &FileSpec,
Expand Down
32 changes: 20 additions & 12 deletions src/writers/file_log_writer/state/numbers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::{writers::FileLogWriterConfig, FileSpec};
use std::cmp::max;

pub(super) fn number_infix(idx: u32) -> String {
format!("_r{idx:0>5}")
format!("r{idx:0>5}")
}

pub(super) fn index_for_rcurrent(
Expand Down Expand Up @@ -41,18 +41,26 @@ pub(super) fn index_for_rcurrent(
pub(super) fn get_highest_index(file_spec: &FileSpec) -> Option<u32> {
let mut o_highest_idx = None;
for file in super::list_and_cleanup::list_of_log_and_compressed_files(file_spec) {
let filename = file.file_stem().unwrap(/*ok*/).to_string_lossy();
let mut it = filename.rsplit("_r");
match it.next() {
Some(next) => {
let idx: u32 = next.parse().unwrap_or(0);
o_highest_idx = match o_highest_idx {
None => Some(idx),
Some(prev) => Some(max(prev, idx)),
};
let name = file.file_stem().unwrap(/*ok*/).to_string_lossy();
let infix = if file_spec.has_basename()
|| file_spec.has_discriminant()
|| file_spec.uses_timestamp()
{
// infix is the last, but not the first part of the name, starts with _r
match name.rsplit("_r").next() {
Some(infix) => infix,
None => continue, // ignore unexpected files
}
None => continue, // ignore unexpected files
}
} else {
// infix is the only part of the name, just skip over the r
&name[1..]
};

let idx: u32 = infix.parse().unwrap_or(0);
o_highest_idx = match o_highest_idx {
None => Some(idx),
Some(prev) => Some(max(prev, idx)),
};
}
o_highest_idx
}
43 changes: 24 additions & 19 deletions src/writers/file_log_writer/state/timestamps.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
use super::{get_creation_timestamp, list_and_cleanup::list_of_infix_files, InfixFormat};
use crate::{writers::FileLogWriterConfig, FileSpec};
use chrono::{DateTime, Local, NaiveDateTime, TimeZone};
use std::path::Path;
use std::{ops::Add, path::PathBuf};
use std::{
ops::Add,
path::{Path, PathBuf},
};

pub(super) fn infix_from_timestamp(
ts: &DateTime<Local>,
Expand All @@ -19,19 +21,19 @@ pub(super) fn infix_from_timestamp(

fn ts_infix_from_path(path: &Path, file_spec: &FileSpec) -> String {
let idx = file_spec
.as_pathbuf(Some("_rXXXXX"))
.as_pathbuf(Some("rXXXXX"))
.to_string_lossy()
.find("_rXXXXX")
.find("rXXXXX")
.unwrap();
String::from_utf8_lossy(&path.to_string_lossy().as_bytes()[idx..idx + 21]).to_string()
String::from_utf8_lossy(&path.to_string_lossy().as_bytes()[idx..idx + 20]).to_string()
}
fn timestamp_from_ts_infix(infix: &str, fmt: &InfixFormat) -> Option<DateTime<Local>> {
NaiveDateTime::parse_from_str(infix, fmt.format())
.ok()
.and_then(|ts| Local.from_local_datetime(&ts).single())
}

pub(super) fn rcurrents_creation_timestamp(
pub(super) fn creation_timestamp_of_currentfile(
config: &FileLogWriterConfig,
current_infix: &str,
rotate_rcurrent: bool,
Expand Down Expand Up @@ -168,16 +170,17 @@ mod test {
.suppress_timestamp();

let now = Local::now();
let now = now
let now_rounded = now
.checked_sub_signed(
Duration::from_std(std::time::Duration::from_nanos(u64::from(
now.timestamp_subsec_nanos(),
)))
.unwrap(),
)
.unwrap();

let paths: Vec<PathBuf> = (0..10)
.map(|i| now - Duration::try_seconds(i).unwrap())
.map(|i| now_rounded - Duration::try_seconds(i).unwrap())
.map(|ts| {
file_spec.as_pathbuf(Some(&super::infix_from_timestamp(
&ts,
Expand All @@ -187,21 +190,23 @@ mod test {
})
.collect();

let newest = paths
.iter()
// retrieve the infix
.map(|path| super::ts_infix_from_path(path, &file_spec))
// parse infix as date, ignore all files where this fails,
.filter_map(|infix| super::timestamp_from_ts_infix(&infix, &InfixFormat::Std))
// take the newest of these dates
.reduce(|acc, e| if acc > e { acc } else { e })
// if nothing is found, take Local::now()
.unwrap_or_else(Local::now);

assert_eq!(
now,
now_rounded,
// TODO: use mocking to avoid code duplication:
// this test is only useful if the path evaluation is the same as in
// super::latest_timestamp_file()
paths
.iter()
// retrieve the infix
.map(|path| super::ts_infix_from_path(path, &file_spec))
// parse infix as date, ignore all files where this fails,
.filter_map(|infix| super::timestamp_from_ts_infix(&infix, &InfixFormat::Std))
// take the newest of these dates
.reduce(|acc, e| if acc > e { acc } else { e })
// if nothing is found, take Local::now()
.unwrap_or_else(Local::now)
newest
);
}
}
Loading

0 comments on commit b35af15

Please sign in to comment.