Skip to content

Commit

Permalink
some self review
Browse files Browse the repository at this point in the history
  • Loading branch information
pgayvallet committed Dec 3, 2020
1 parent b226a63 commit 2634c76
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ const format = 'YYYY-MM-DD HH:mm:ss';
describe('TimeIntervalTriggeringPolicy', () => {
afterEach(() => {
getNextRollingTimeMock.mockReset();
jest.restoreAllMocks();
});

const createLogRecord = (timestamp: Date): LogRecord => ({
Expand Down Expand Up @@ -72,6 +73,22 @@ describe('TimeIntervalTriggeringPolicy', () => {
);
});

it('calls `getNextRollingTime` with the current time if `context.currentFileTime` is not set', () => {
const currentTime = moment('2018-06-15 04:27:12', format).toDate().getTime();
jest.spyOn(Date, 'now').mockReturnValue(currentTime);
const context = createContext(0);
const config = createConfig('15m', true);

new TimeIntervalTriggeringPolicy(config, context);

expect(getNextRollingTimeMock).toHaveBeenCalledTimes(1);
expect(getNextRollingTimeMock).toHaveBeenCalledWith(
currentTime,
config.interval,
config.modulate
);
});

describe('#isTriggeringEvent', () => {
it('returns true if the event time is after the nextRolloverTime', () => {
const eventDate = moment('2010-10-20 04:43:12', format).toDate();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,12 @@ export interface TimeIntervalTriggeringPolicyConfig {
* How often a rollover should occur.
*
* @remarks
* Due to of modulate rolling works, it is required to have an integer value for the highest time unit
* Due to how modulate rolling works, it is required to have an integer value for the highest time unit
* of the duration (you can't overflow to a higher unit).
* For example, `15m` and `4h` are valid values , but `90m` is not (as it is `1.5h`),.
* For example, `15m` and `4h` are valid values , but `90m` is not (as it is `1.5h`).
*/
interval: Duration;

/**
* Indicates whether the interval should be adjusted to cause the next rollover to occur on the interval boundary.
*
Expand Down Expand Up @@ -65,7 +66,7 @@ export const timeIntervalTriggeringPolicyConfigSchema = schema.object({
*/
export class TimeIntervalTriggeringPolicy implements TriggeringPolicy {
/**
* millisecond timestamp of when the next rollover should occur.
* milliseconds timestamp of when the next rollover should occur.
*/
private nextRolloverTime: number;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ export interface RollingFileAppenderConfig {
layout: LayoutConfigType;
/**
* The absolute path of the file to write to.
* If the file, or any of its parent directories, do not exist, they will be created.
*/
path: string;
/**
Expand Down Expand Up @@ -81,9 +80,6 @@ export class RollingFileAppender implements DisposableAppender {
private readonly strategy: RollingStrategy;
private readonly buffer: BufferAppender;

/**
* Creates FileAppender instance with specified layout and file path.
*/
constructor(config: RollingFileAppenderConfig) {
this.context = new RollingFileContext(config.path);
this.context.refreshFileInfo();
Expand All @@ -95,8 +91,8 @@ export class RollingFileAppender implements DisposableAppender {
}

/**
* Formats specified `record` and writes them to the specified file.
* @param record `LogRecord` instance to be logged.
* Formats specified `record` and writes it to the specified file. If the record
* would trigger a rollover, then it will be performed before writing to the file.
*/
public append(record: LogRecord) {
// if we are currently rolling the files, push the log record
Expand All @@ -119,7 +115,8 @@ export class RollingFileAppender implements DisposableAppender {
}

/**
* Disposes `FileAppender`. Waits for the underlying file stream to be completely flushed and closed.
* Disposes the appender.
* If a rollout is currently in progress, it will ve awaited.
*/
public async dispose() {
if (this.disposed) {
Expand All @@ -141,14 +138,13 @@ export class RollingFileAppender implements DisposableAppender {
try {
await this.strategy.rollout();
await this.fileManager.closeStream();
this.rollingPromise = undefined;
this.isRolling = false;
} catch (e) {
// eslint-disable-next-line no-console
console.log('Error while rolling file: ', e);
} finally {
this.flushBuffer();
}
this.rollingPromise = undefined;
this.isRolling = false;
this.flushBuffer();
}

private flushBuffer() {
Expand Down

0 comments on commit 2634c76

Please sign in to comment.