You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "erikmav (via GitHub)" <gi...@apache.org> on 2024/03/20 16:30:22 UTC
[PR] Part 4 of nullable and modernization (logging-log4net)
erikmav opened a new pull request, #129:
URL: https://github.com/apache/logging-log4net/pull/129
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Part 4 of nullable and modernization (logging-log4net)
Posted by "erikmav (via GitHub)" <gi...@apache.org>.
erikmav commented on code in PR #129:
URL: https://github.com/apache/logging-log4net/pull/129#discussion_r1535848543
##########
src/log4net.Tests/DateFormatter/DateTimeDateFormatterTest.cs:
##########
@@ -0,0 +1,59 @@
+#region Apache License
+//
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to you under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+#endregion
+
+using System;
+using System.Globalization;
+using System.IO;
+using System.Text;
+using log4net.DateFormatter;
+using NUnit.Framework;
+
+namespace log4net.Tests.DateFormatter
+{
+ [TestFixture]
+ public class DateTimeDateFormatterTest
+ {
+ [Test]
+ public void TestFormattingResults()
+ {
+ var sb = new StringBuilder();
+ using var writer = new StringWriter(sb, CultureInfo.InvariantCulture);
+
+ // Tests for prepended 0 characters for 2-digit and 3-digit portions.
+ var formatter = new DateTimeDateFormatter();
+ formatter.FormatDate(new DateTime(1970, 1, 1, 1, 1, 1).AddMilliseconds(1), writer);
+ Assert.AreEqual("01 Jan 1970 01:01:01,001", sb.ToString());
+ sb.Clear();
+
+ // Non-zero-prepend case.
+ var formatter2 = new DifferentTypeNameDateTimeDateFormatter();
+ formatter.FormatDate(new DateTime(2100, 12, 30, 11, 59, 59).AddMilliseconds(100), writer);
Review Comment:
Quite right! Which means when I reran the test suite it happened to work with the internal cache. Fixed in next push.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Part 4 of nullable and modernization (logging-log4net)
Posted by "erikmav (via GitHub)" <gi...@apache.org>.
erikmav commented on code in PR #129:
URL: https://github.com/apache/logging-log4net/pull/129#discussion_r1534403615
##########
src/log4net/Appender/BufferingAppenderSkeleton.cs:
##########
@@ -459,59 +411,59 @@ protected override void Append(LoggingEvent loggingEvent)
// sent immediately because there is not enough space in the buffer
// to buffer up more than 1 event. Therefore as a special case
// we don't use the buffer at all.
- if (m_cb == null || m_bufferSize <= 1)
+ if (m_cb == null || BufferSize <= 1)
{
- // Only send the event if we are in non lossy mode or the event is a triggering event
- if ((!m_lossy) ||
- (m_evaluator != null && m_evaluator.IsTriggeringEvent(loggingEvent)) ||
- (m_lossyEvaluator != null && m_lossyEvaluator.IsTriggeringEvent(loggingEvent)))
+ // Only send the event if we are in non-lossy mode or the event is a triggering event
+ if ((!Lossy) ||
+ (Evaluator is not null && Evaluator.IsTriggeringEvent(loggingEvent)) ||
+ (LossyEvaluator is not null && LossyEvaluator.IsTriggeringEvent(loggingEvent)))
{
if (m_eventMustBeFixed)
{
// Derive class expects fixed events
- loggingEvent.Fix = this.Fix;
+ loggingEvent.Fix = Fix;
}
// Not buffering events, send immediately
- SendBuffer(new LoggingEvent[] { loggingEvent });
+ SendBuffer(new[] { loggingEvent });
Review Comment:
I think downlevel .NET is interfering with [] conversions so I've been avoiding them:
3>C:\log4net2\src\log4net\Appender\BufferingAppenderSkeleton.cs(429,22,429,23): error CS1026: ) expected
3>C:\log4net2\src\log4net\Appender\BufferingAppenderSkeleton.cs(429,36,429,37): error CS1002: ; expected
3>C:\log4net2\src\log4net\Appender\BufferingAppenderSkeleton.cs(429,36,429,37): error CS1513: } expected
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Part 4 of nullable and modernization (logging-log4net)
Posted by "erikmav (via GitHub)" <gi...@apache.org>.
erikmav commented on PR #129:
URL: https://github.com/apache/logging-log4net/pull/129#issuecomment-2009999492
For #124
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Part 4 of nullable and modernization (logging-log4net)
Posted by "erikmav (via GitHub)" <gi...@apache.org>.
erikmav commented on code in PR #129:
URL: https://github.com/apache/logging-log4net/pull/129#discussion_r1534412278
##########
src/log4net/Core/LogImpl.cs:
##########
@@ -247,7 +241,7 @@ public virtual void DebugFormat(string format, object arg0)
{
if (IsDebugEnabled)
{
- Logger.Log(ThisDeclaringType, m_levelDebug, new SystemStringFormat(CultureInfo.InvariantCulture, format, new object[] { arg0 }), null);
+ Logger.Log(ThisDeclaringType, m_levelDebug, new SystemStringFormat(CultureInfo.InvariantCulture, format, new[] { arg0 }), null);
Review Comment:
Not fixing per previous note
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Part 4 of nullable and modernization (logging-log4net)
Posted by "erikmav (via GitHub)" <gi...@apache.org>.
erikmav commented on code in PR #129:
URL: https://github.com/apache/logging-log4net/pull/129#discussion_r1534448073
##########
src/log4net/DateFormatter/AbsoluteTimeDateFormatter.cs:
##########
@@ -95,78 +87,52 @@ protected virtual void FormatDateWithoutMillis(DateTime dateToFormat, StringBuil
/// per second.
/// </para>
/// <para>
- /// Sub classes should override <see cref="FormatDateWithoutMillis"/>
+ /// Subclasses should override <see cref="FormatDateWithoutMillis"/>
/// rather than <see cref="FormatDate"/>.
/// </para>
/// </remarks>
public virtual void FormatDate(DateTime dateToFormat, TextWriter writer)
{
- lock (s_lastTimeStrings)
- {
- // Calculate the current time precise only to the second
- long currentTimeToTheSecond = (dateToFormat.Ticks - (dateToFormat.Ticks % TimeSpan.TicksPerSecond));
-
- string timeString = null;
- // Compare this time with the stored last time
- // If we are in the same second then append
- // the previously calculated time string
- if (s_lastTimeToTheSecond != currentTimeToTheSecond)
- {
- s_lastTimeStrings.Clear();
- }
- else
- {
- timeString = (string)s_lastTimeStrings[GetType()];
- }
-
- if (timeString == null)
+ string timeString = s_lastTimeStrings.AddOrUpdate(GetType(),
Review Comment:
There was a class already but it was explicitly excluded from compilation. Restored and added more coverage.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Part 4 of nullable and modernization (logging-log4net)
Posted by "FreeAndNil (via GitHub)" <gi...@apache.org>.
FreeAndNil merged PR #129:
URL: https://github.com/apache/logging-log4net/pull/129
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Part 4 of nullable and modernization (logging-log4net)
Posted by "FreeAndNil (via GitHub)" <gi...@apache.org>.
FreeAndNil commented on code in PR #129:
URL: https://github.com/apache/logging-log4net/pull/129#discussion_r1534615314
##########
src/log4net.Tests/DateFormatter/DateTimeDateFormatterTest.cs:
##########
@@ -0,0 +1,59 @@
+#region Apache License
+//
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to you under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+#endregion
+
+using System;
+using System.Globalization;
+using System.IO;
+using System.Text;
+using log4net.DateFormatter;
+using NUnit.Framework;
+
+namespace log4net.Tests.DateFormatter
+{
+ [TestFixture]
+ public class DateTimeDateFormatterTest
+ {
+ [Test]
+ public void TestFormattingResults()
+ {
+ var sb = new StringBuilder();
+ using var writer = new StringWriter(sb, CultureInfo.InvariantCulture);
+
+ // Tests for prepended 0 characters for 2-digit and 3-digit portions.
+ var formatter = new DateTimeDateFormatter();
+ formatter.FormatDate(new DateTime(1970, 1, 1, 1, 1, 1).AddMilliseconds(1), writer);
+ Assert.AreEqual("01 Jan 1970 01:01:01,001", sb.ToString());
+ sb.Clear();
+
+ // Non-zero-prepend case.
+ var formatter2 = new DifferentTypeNameDateTimeDateFormatter();
+ formatter.FormatDate(new DateTime(2100, 12, 30, 11, 59, 59).AddMilliseconds(100), writer);
Review Comment:
```suggestion
formatter2.FormatDate(new DateTime(2100, 12, 30, 11, 59, 59).AddMilliseconds(100), writer);
```
Shouldn't it be formatter2.FormatDate?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Part 4 of nullable and modernization (logging-log4net)
Posted by "FreeAndNil (via GitHub)" <gi...@apache.org>.
FreeAndNil commented on code in PR #129:
URL: https://github.com/apache/logging-log4net/pull/129#discussion_r1532722957
##########
src/log4net/Appender/AppenderCollection.cs:
##########
@@ -292,8 +234,11 @@ public virtual void Clear()
/// <returns>A new <see cref="AppenderCollection"/> with a shallow copy of the collection data.</returns>
public virtual object Clone()
{
- AppenderCollection newCol = new AppenderCollection(m_count);
- Array.Copy(m_array, 0, newCol.m_array, 0, m_count);
+ var newCol = new AppenderCollection(m_count);
+ if (m_count > 0)
Review Comment:
This check is probably not needed, because new AppenderCollection(m_count) would throw.
##########
src/log4net/Appender/AppenderCollection.cs:
##########
@@ -843,34 +702,20 @@ public override void RemoveAt(int pos)
throw new NotSupportedException("This is a Read Only Collection and can not be modified");
}
- public override bool IsFixedSize
- {
- get { return true; }
- }
-
- public override bool IsReadOnly
- {
- get { return true; }
- }
-
- #endregion
+ public override bool IsFixedSize => true;
- #region Type-safe IEnumerable
+ public override bool IsReadOnly => true;
- public override IAppenderCollectionEnumerator GetEnumerator()
+ public override IEnumerator<IAppender> GetEnumerator()
{
return m_collection.GetEnumerator();
}
- #endregion
-
- #region Public Helpers
-
// (just to mimic some nice features of ArrayList)
public override int Capacity
{
- get { return m_collection.Capacity; }
- set { throw new NotSupportedException("This is a Read Only Collection and can not be modified"); }
+ get => m_collection.Capacity;
+ set => throw new NotSupportedException("This is a Read Only Collection and can not be modified");
Review Comment:
Can you put `new NotSupportedException("This is a Read Only Collection and can not be modified")` in an ExceptionBuilder-class (Util namespace) and call this from the 20 places?
##########
src/log4net/Appender/BufferingAppenderSkeleton.cs:
##########
@@ -388,25 +344,21 @@ public override void ActivateOptions()
// If the appender is in Lossy mode then we will
// only send the buffer when the Evaluator triggers
// therefore check we have an evaluator.
- if (m_lossy && m_evaluator == null)
+ if (Lossy && Evaluator == null)
{
ErrorHandler.Error("Appender [" + Name + "] is Lossy but has no Evaluator. The buffer will never be sent!");
Review Comment:
Interpolated string?
##########
src/log4net/Appender/BufferingAppenderSkeleton.cs:
##########
@@ -388,25 +344,21 @@ public override void ActivateOptions()
// If the appender is in Lossy mode then we will
// only send the buffer when the Evaluator triggers
// therefore check we have an evaluator.
- if (m_lossy && m_evaluator == null)
+ if (Lossy && Evaluator == null)
Review Comment:
```suggestion
if (Lossy && Evaluator is null)
```
##########
src/log4net/Appender/BufferingAppenderSkeleton.cs:
##########
@@ -313,27 +273,27 @@ public virtual void Flush()
/// </remarks>
public virtual void Flush(bool flushLossyBuffer)
{
- // This method will be called outside of the AppenderSkeleton DoAppend() method
+ // This method will be called outside the AppenderSkeleton DoAppend() method
// therefore it needs to be protected by its own lock. This will block any
// Appends while the buffer is flushed.
- lock (this)
+ lock (LockObj)
{
- if (m_cb != null && m_cb.Length > 0)
+ if (m_cb is not null && m_cb.Length > 0)
{
- if (m_lossy)
+ if (Lossy)
{
// If we are allowed to eagerly flush from the lossy buffer
if (flushLossyBuffer)
{
- if (m_lossyEvaluator != null)
+ if (LossyEvaluator != null)
{
// Test the contents of the buffer against the lossy evaluator
LoggingEvent[] bufferedEvents = m_cb.PopAll();
ArrayList filteredEvents = new ArrayList(bufferedEvents.Length);
Review Comment:
Canyou switch to List<LoggingEvent>?
##########
src/log4net/Appender/BufferingAppenderSkeleton.cs:
##########
@@ -119,19 +113,15 @@ protected BufferingAppenderSkeleton(bool eventMustBeFixed) : base()
/// <para>
/// This appender uses a buffer to store logging events before
/// delivering them. A triggering event causes the whole buffer
- /// to be send to the remote sink. If the buffer overruns before
+ /// to be sent to the remote sink. If the buffer overruns before
/// a triggering event then logging events could be lost. Set
/// <see cref="Lossy"/> to <c>false</c> to prevent logging events
/// from being lost.
/// </para>
/// <para>If <see cref="Lossy"/> is set to <c>true</c> then an
/// <see cref="Evaluator"/> must be specified.</para>
/// </remarks>
- public bool Lossy
- {
- get { return m_lossy; }
- set { m_lossy = value; }
- }
+ public bool Lossy { get; set; } = false;
Review Comment:
```suggestion
public bool Lossy { get; set; }
```
##########
src/log4net/Appender/BufferingAppenderSkeleton.cs:
##########
@@ -459,59 +411,59 @@ protected override void Append(LoggingEvent loggingEvent)
// sent immediately because there is not enough space in the buffer
// to buffer up more than 1 event. Therefore as a special case
// we don't use the buffer at all.
- if (m_cb == null || m_bufferSize <= 1)
+ if (m_cb == null || BufferSize <= 1)
Review Comment:
```suggestion
if (m_cb is null || BufferSize <= 1)
```
##########
src/log4net/Appender/BufferingAppenderSkeleton.cs:
##########
@@ -520,39 +472,35 @@ protected override void Append(LoggingEvent loggingEvent)
// Buffer is not yet full
// Check if the event should trigger the whole buffer to be sent
- if (m_evaluator != null && m_evaluator.IsTriggeringEvent(loggingEvent))
+ if (Evaluator is not null && Evaluator.IsTriggeringEvent(loggingEvent))
{
SendFromBuffer(null, m_cb);
}
}
}
}
- #endregion Override implementation of AppenderSkeleton
-
- #region Protected Instance Methods
-
/// <summary>
/// Sends the contents of the buffer.
/// </summary>
/// <param name="firstLoggingEvent">The first logging event.</param>
- /// <param name="buffer">The buffer containing the events that need to be send.</param>
+ /// <param name="buffer">The buffer containing the events that need to be sent.</param>
/// <remarks>
/// <para>
/// The subclass must override <see cref="M:SendBuffer(LoggingEvent[])"/>.
/// </para>
/// </remarks>
- protected virtual void SendFromBuffer(LoggingEvent firstLoggingEvent, CyclicBuffer buffer)
+ protected virtual void SendFromBuffer(LoggingEvent? firstLoggingEvent, CyclicBuffer buffer)
{
LoggingEvent[] bufferEvents = buffer.PopAll();
- if (firstLoggingEvent == null)
+ if (firstLoggingEvent is null)
{
SendBuffer(bufferEvents);
}
else if (bufferEvents.Length == 0)
{
- SendBuffer(new LoggingEvent[] { firstLoggingEvent });
+ SendBuffer(new[] { firstLoggingEvent });
Review Comment:
```suggestion
SendBuffer([firstLoggingEvent]);
```
##########
src/log4net/Appender/SmtpAppender.cs:
##########
@@ -547,15 +410,14 @@ public enum SmtpAuthentication
/// </remarks>
Ntlm
}
-
- #endregion // SmtpAuthentication Enum
-
- private static readonly char[] ADDRESS_DELIMITERS = new char[] { ',', ';' };
+
+ // Allow semicolon delimiters for backward compatibility.
+ private static readonly char[] ADDRESS_DELIMITERS = { ',', ';' };
/// <summary>
/// trims leading and trailing commas or semicolons
/// </summary>
- private static string MaybeTrimSeparators(string s)
+ private static string? MaybeTrimSeparators(string? s)
{
return string.IsNullOrEmpty(s) ? s : s.Trim(ADDRESS_DELIMITERS);
Review Comment:
```suggestion
return s?.Trim(ADDRESS_DELIMITERS);
```
##########
src/log4net/Core/LogImpl.cs:
##########
@@ -278,7 +272,7 @@ public virtual void DebugFormat(string format, object arg0, object arg1)
{
if (IsDebugEnabled)
{
- Logger.Log(ThisDeclaringType, m_levelDebug, new SystemStringFormat(CultureInfo.InvariantCulture, format, new object[] { arg0, arg1 }), null);
+ Logger.Log(ThisDeclaringType, m_levelDebug, new SystemStringFormat(CultureInfo.InvariantCulture, format, new[] { arg0, arg1 }), null);
Review Comment:
same
##########
src/log4net/Appender/ForwardingAppender.cs:
##########
@@ -164,14 +129,7 @@ public virtual AppenderCollection Appenders
{
lock (this)
{
- if (m_appenderAttachedImpl == null)
- {
- return AppenderCollection.EmptyCollection;
- }
- else
- {
- return m_appenderAttachedImpl.Appenders;
- }
+ return m_appenderAttachedImpl is null ? AppenderCollection.EmptyCollection : m_appenderAttachedImpl.Appenders;
Review Comment:
```suggestion
return m_appenderAttachedImpl?.Appenders ?? AppenderCollection.EmptyCollection;
```
##########
src/log4net/Appender/EventLogAppender.cs:
##########
@@ -380,38 +345,37 @@ protected override void Append(LoggingEvent loggingEvent)
// Look for the EventID property
object eventIDPropertyObj = loggingEvent.LookupProperty("EventID");
- if (eventIDPropertyObj != null)
+ if (eventIDPropertyObj is not null)
{
- if (eventIDPropertyObj is int)
+ if (eventIDPropertyObj is int eventIdInt)
{
- eventID = (int)eventIDPropertyObj;
+ eventID = eventIdInt;
}
else
{
- string eventIDPropertyString = eventIDPropertyObj as string;
- if (eventIDPropertyString == null)
+ if (eventIDPropertyObj is not string eventIDPropertyString)
{
eventIDPropertyString = eventIDPropertyObj.ToString();
}
- if (eventIDPropertyString != null && eventIDPropertyString.Length > 0)
+ if (eventIDPropertyString.Length > 0)
{
// Read the string property into a number
- int intVal;
- if (SystemInfo.TryParse(eventIDPropertyString, out intVal))
+ if (SystemInfo.TryParse(eventIDPropertyString, out int intVal))
{
eventID = intVal;
}
else
{
- ErrorHandler.Error("Unable to parse event ID property [" + eventIDPropertyString + "].");
+ ErrorHandler.Error($"Unable to parse event ID property [{eventIDPropertyString}].");
}
}
}
}
short category = m_category;
// Look for the Category property
- if (loggingEvent.LookupProperty("Category") is object categoryPropertyObj)
+ object? categoryPropertyObj = loggingEvent.LookupProperty("Category");
+ if (categoryPropertyObj is not null)
Review Comment:
```suggestion
if (loggingEvent.LookupProperty("Category") is object categoryPropertyObj)
```
##########
src/log4net/Core/LogImpl.cs:
##########
@@ -643,7 +637,7 @@ public virtual void WarnFormat(string format, object arg0)
{
if (IsWarnEnabled)
{
- Logger.Log(ThisDeclaringType, m_levelWarn, new SystemStringFormat(CultureInfo.InvariantCulture, format, new object[] { arg0 }), null);
+ Logger.Log(ThisDeclaringType, m_levelWarn, new SystemStringFormat(CultureInfo.InvariantCulture, format, new[] { arg0 }), null);
Review Comment:
same
##########
src/log4net/Core/LogImpl.cs:
##########
@@ -247,7 +241,7 @@ public virtual void DebugFormat(string format, object arg0)
{
if (IsDebugEnabled)
{
- Logger.Log(ThisDeclaringType, m_levelDebug, new SystemStringFormat(CultureInfo.InvariantCulture, format, new object[] { arg0 }), null);
+ Logger.Log(ThisDeclaringType, m_levelDebug, new SystemStringFormat(CultureInfo.InvariantCulture, format, new[] { arg0 }), null);
Review Comment:
```suggestion
Logger.Log(ThisDeclaringType, m_levelDebug, new SystemStringFormat(CultureInfo.InvariantCulture, format, [arg0]), null);
```
##########
src/log4net/Appender/BufferingAppenderSkeleton.cs:
##########
@@ -589,71 +533,16 @@ protected virtual void SendFromBuffer(LoggingEvent firstLoggingEvent, CyclicBuff
/// </remarks>
private const int DEFAULT_BUFFER_SIZE = 512;
- #endregion Private Static Fields
-
- #region Private Instance Fields
-
- /// <summary>
- /// The size of the cyclic buffer used to hold the logging events.
- /// </summary>
- /// <remarks>
- /// Set to <see cref="DEFAULT_BUFFER_SIZE"/> by default.
- /// </remarks>
- private int m_bufferSize = DEFAULT_BUFFER_SIZE;
-
/// <summary>
/// The cyclic buffer used to store the logging events.
/// </summary>
- private CyclicBuffer m_cb;
-
- /// <summary>
- /// The triggering event evaluator that causes the buffer to be sent immediately.
- /// </summary>
- /// <remarks>
- /// The object that is used to determine if an event causes the entire
- /// buffer to be sent immediately. This field can be <c>null</c>, which
- /// indicates that event triggering is not to be done. The evaluator
- /// can be set using the <see cref="Evaluator"/> property. If this appender
- /// has the <see cref="m_lossy"/> (<see cref="Lossy"/> property) set to
- /// <c>true</c> then an <see cref="Evaluator"/> must be set.
- /// </remarks>
- private ITriggeringEventEvaluator m_evaluator;
-
- /// <summary>
- /// Indicates if the appender should overwrite events in the cyclic buffer
- /// when it becomes full, or if the buffer should be flushed when the
- /// buffer is full.
- /// </summary>
- /// <remarks>
- /// If this field is set to <c>true</c> then an <see cref="Evaluator"/> must
- /// be set.
- /// </remarks>
- private bool m_lossy = false;
-
- /// <summary>
- /// The triggering event evaluator filters discarded events.
- /// </summary>
- /// <remarks>
- /// The object that is used to determine if an event that is discarded should
- /// really be discarded or if it should be sent to the appenders.
- /// This field can be <c>null</c>, which indicates that all discarded events will
- /// be discarded.
- /// </remarks>
- private ITriggeringEventEvaluator m_lossyEvaluator;
-
- /// <summary>
- /// Value indicating which fields in the event should be fixed
- /// </summary>
- /// <remarks>
- /// By default all fields are fixed
- /// </remarks>
- private FixFlags m_fixFlags = FixFlags.All;
+ private CyclicBuffer? m_cb;
/// <summary>
/// The events delivered to the subclass must be fixed.
/// </summary>
private readonly bool m_eventMustBeFixed;
- #endregion Private Instance Fields
+ private readonly object m_ockObj = new();
Review Comment:
Seems do be misspelled and unused :wink:
##########
src/log4net/Core/LogImpl.cs:
##########
@@ -674,7 +668,7 @@ public virtual void WarnFormat(string format, object arg0, object arg1)
{
if (IsWarnEnabled)
{
- Logger.Log(ThisDeclaringType, m_levelWarn, new SystemStringFormat(CultureInfo.InvariantCulture, format, new object[] { arg0, arg1 }), null);
+ Logger.Log(ThisDeclaringType, m_levelWarn, new SystemStringFormat(CultureInfo.InvariantCulture, format, new[] { arg0, arg1 }), null);
Review Comment:
same
##########
src/log4net/Appender/SmtpAppender.cs:
##########
@@ -60,8 +60,6 @@ namespace log4net.Appender
/// <author>Gert Driesen</author>
Review Comment:
Please remove the `<note type="caution">` node regarding .NET 1.1
And every other cation/.NET 1.1 note in this file.
##########
src/log4net/Core/LogImpl.cs:
##########
@@ -872,7 +866,7 @@ public virtual void ErrorFormat(string format, object arg0, object arg1)
{
if (IsErrorEnabled)
{
- Logger.Log(ThisDeclaringType, m_levelError, new SystemStringFormat(CultureInfo.InvariantCulture, format, new object[] { arg0, arg1 }), null);
+ Logger.Log(ThisDeclaringType, m_levelError, new SystemStringFormat(CultureInfo.InvariantCulture, format, new[] { arg0, arg1 }), null);
Review Comment:
same
##########
src/log4net/DateFormatter/AbsoluteTimeDateFormatter.cs:
##########
@@ -95,78 +87,52 @@ protected virtual void FormatDateWithoutMillis(DateTime dateToFormat, StringBuil
/// per second.
/// </para>
/// <para>
- /// Sub classes should override <see cref="FormatDateWithoutMillis"/>
+ /// Subclasses should override <see cref="FormatDateWithoutMillis"/>
/// rather than <see cref="FormatDate"/>.
/// </para>
/// </remarks>
public virtual void FormatDate(DateTime dateToFormat, TextWriter writer)
{
- lock (s_lastTimeStrings)
- {
- // Calculate the current time precise only to the second
- long currentTimeToTheSecond = (dateToFormat.Ticks - (dateToFormat.Ticks % TimeSpan.TicksPerSecond));
-
- string timeString = null;
- // Compare this time with the stored last time
- // If we are in the same second then append
- // the previously calculated time string
- if (s_lastTimeToTheSecond != currentTimeToTheSecond)
- {
- s_lastTimeStrings.Clear();
- }
- else
- {
- timeString = (string)s_lastTimeStrings[GetType()];
- }
-
- if (timeString == null)
+ string timeString = s_lastTimeStrings.AddOrUpdate(GetType(),
Review Comment:
Can you please add some tests?
The whole class seems to be uncovered.
##########
src/log4net/DateFormatter/DateTimeDateFormatter.cs:
##########
@@ -79,28 +60,13 @@ protected override void FormatDateWithoutMillis(DateTime dateToFormat, StringBui
{
buffer.Append('0');
}
- buffer.Append(day);
- buffer.Append(' ');
-
- buffer.Append(m_dateTimeFormatInfo.GetAbbreviatedMonthName(dateToFormat.Month));
- buffer.Append(' ');
-
- buffer.Append(dateToFormat.Year);
- buffer.Append(' ');
+ buffer
Review Comment:
Adding a test for this would also be great.
##########
src/log4net/Core/StackFrameItem.cs:
##########
@@ -24,40 +25,40 @@
namespace log4net.Core
{
/// <summary>
- /// provides stack frame information without actually referencing a System.Diagnostics.StackFrame
+ /// Provides stack frame information without actually referencing a System.Diagnostics.StackFrame
/// as that would require that the containing assembly is loaded.
/// </summary>
- ///
[Serializable]
public class StackFrameItem
{
- #region Public Instance Constructors
-
/// <summary>
- /// returns a stack frame item from a stack frame. This
+ /// Creates a stack frame item from a stack frame.
/// </summary>
/// <param name="frame"></param>
- /// <returns></returns>
public StackFrameItem(StackFrame frame)
{
// set default values
- m_lineNumber = NA;
- m_fileName = NA;
- m_method = new MethodItem();
- m_className = NA;
+ LineNumber = NA;
+ FileName = NA;
+ Method = new MethodItem();
+ ClassName = NA;
try
{
// get frame values
- m_lineNumber = frame.GetFileLineNumber().ToString(System.Globalization.NumberFormatInfo.InvariantInfo);
- m_fileName = frame.GetFileName();
+ LineNumber = frame.GetFileLineNumber().ToString(System.Globalization.NumberFormatInfo.InvariantInfo);
+ FileName = frame.GetFileName();
+
// get method values
- MethodBase method = frame.GetMethod();
- if (method != null)
+ MethodBase? method = frame.GetMethod();
+ if (method is not null)
{
- if (method.DeclaringType != null)
- m_className = method.DeclaringType.FullName;
- m_method = new MethodItem(method);
+ if (method.DeclaringType is not null)
Review Comment:
```suggestion
if (method.DeclaringType?.FullName is string className)
```
##########
src/log4net/Appender/BufferingAppenderSkeleton.cs:
##########
@@ -459,59 +411,59 @@ protected override void Append(LoggingEvent loggingEvent)
// sent immediately because there is not enough space in the buffer
// to buffer up more than 1 event. Therefore as a special case
// we don't use the buffer at all.
- if (m_cb == null || m_bufferSize <= 1)
+ if (m_cb == null || BufferSize <= 1)
{
- // Only send the event if we are in non lossy mode or the event is a triggering event
- if ((!m_lossy) ||
- (m_evaluator != null && m_evaluator.IsTriggeringEvent(loggingEvent)) ||
- (m_lossyEvaluator != null && m_lossyEvaluator.IsTriggeringEvent(loggingEvent)))
+ // Only send the event if we are in non-lossy mode or the event is a triggering event
+ if ((!Lossy) ||
+ (Evaluator is not null && Evaluator.IsTriggeringEvent(loggingEvent)) ||
+ (LossyEvaluator is not null && LossyEvaluator.IsTriggeringEvent(loggingEvent)))
{
if (m_eventMustBeFixed)
{
// Derive class expects fixed events
- loggingEvent.Fix = this.Fix;
+ loggingEvent.Fix = Fix;
}
// Not buffering events, send immediately
- SendBuffer(new LoggingEvent[] { loggingEvent });
+ SendBuffer(new[] { loggingEvent });
Review Comment:
```suggestion
SendBuffer([loggingEvent]);
```
##########
src/log4net/Core/LogImpl.cs:
##########
@@ -445,7 +439,7 @@ public virtual void InfoFormat(string format, object arg0)
{
if (IsInfoEnabled)
{
- Logger.Log(ThisDeclaringType, m_levelInfo, new SystemStringFormat(CultureInfo.InvariantCulture, format, new object[] { arg0 }), null);
+ Logger.Log(ThisDeclaringType, m_levelInfo, new SystemStringFormat(CultureInfo.InvariantCulture, format, new[] { arg0 }), null);
Review Comment:
same
##########
src/log4net/Core/LogImpl.cs:
##########
@@ -310,7 +304,7 @@ public virtual void DebugFormat(string format, object arg0, object arg1, object
{
if (IsDebugEnabled)
{
- Logger.Log(ThisDeclaringType, m_levelDebug, new SystemStringFormat(CultureInfo.InvariantCulture, format, new object[] { arg0, arg1, arg2 }), null);
+ Logger.Log(ThisDeclaringType, m_levelDebug, new SystemStringFormat(CultureInfo.InvariantCulture, format, new[] { arg0, arg1, arg2 }), null);
Review Comment:
same
##########
src/log4net/Core/LogImpl.cs:
##########
@@ -476,7 +470,7 @@ public virtual void InfoFormat(string format, object arg0, object arg1)
{
if (IsInfoEnabled)
{
- Logger.Log(ThisDeclaringType, m_levelInfo, new SystemStringFormat(CultureInfo.InvariantCulture, format, new object[] { arg0, arg1 }), null);
+ Logger.Log(ThisDeclaringType, m_levelInfo, new SystemStringFormat(CultureInfo.InvariantCulture, format, new[] { arg0, arg1 }), null);
Review Comment:
same
##########
src/log4net/Core/LogImpl.cs:
##########
@@ -904,7 +898,7 @@ public virtual void ErrorFormat(string format, object arg0, object arg1, object
{
if (IsErrorEnabled)
{
- Logger.Log(ThisDeclaringType, m_levelError, new SystemStringFormat(CultureInfo.InvariantCulture, format, new object[] { arg0, arg1, arg2 }), null);
+ Logger.Log(ThisDeclaringType, m_levelError, new SystemStringFormat(CultureInfo.InvariantCulture, format, new[] { arg0, arg1, arg2 }), null);
Review Comment:
same
##########
src/log4net/Core/LogImpl.cs:
##########
@@ -508,7 +502,7 @@ public virtual void InfoFormat(string format, object arg0, object arg1, object a
{
if (IsInfoEnabled)
{
- Logger.Log(ThisDeclaringType, m_levelInfo, new SystemStringFormat(CultureInfo.InvariantCulture, format, new object[] { arg0, arg1, arg2 }), null);
+ Logger.Log(ThisDeclaringType, m_levelInfo, new SystemStringFormat(CultureInfo.InvariantCulture, format, new[] { arg0, arg1, arg2 }), null);
Review Comment:
same
##########
src/log4net/Core/LogImpl.cs:
##########
@@ -706,7 +700,7 @@ public virtual void WarnFormat(string format, object arg0, object arg1, object a
{
if (IsWarnEnabled)
{
- Logger.Log(ThisDeclaringType, m_levelWarn, new SystemStringFormat(CultureInfo.InvariantCulture, format, new object[] { arg0, arg1, arg2 }), null);
+ Logger.Log(ThisDeclaringType, m_levelWarn, new SystemStringFormat(CultureInfo.InvariantCulture, format, new[] { arg0, arg1, arg2 }), null);
Review Comment:
same
##########
src/log4net/Core/LogImpl.cs:
##########
@@ -1039,7 +1033,7 @@ public virtual void FatalFormat(string format, object arg0)
{
if (IsFatalEnabled)
{
- Logger.Log(ThisDeclaringType, m_levelFatal, new SystemStringFormat(CultureInfo.InvariantCulture, format, new object[] { arg0 }), null);
+ Logger.Log(ThisDeclaringType, m_levelFatal, new SystemStringFormat(CultureInfo.InvariantCulture, format, new[] { arg0 }), null);
Review Comment:
same
##########
src/log4net/Core/LogImpl.cs:
##########
@@ -841,7 +835,7 @@ public virtual void ErrorFormat(string format, object arg0)
{
if (IsErrorEnabled)
{
- Logger.Log(ThisDeclaringType, m_levelError, new SystemStringFormat(CultureInfo.InvariantCulture, format, new object[] { arg0 }), null);
+ Logger.Log(ThisDeclaringType, m_levelError, new SystemStringFormat(CultureInfo.InvariantCulture, format, new[] { arg0 }), null);
Review Comment:
same
##########
src/log4net/Appender/SmtpAppender.cs:
##########
@@ -375,10 +282,10 @@ protected override void SendBuffer(LoggingEvent[] events)
// appender. This frees us from needing to synchronize again.
try
{
- using StringWriter writer = new StringWriter(System.Globalization.CultureInfo.InvariantCulture);
+ using var writer = new StringWriter(System.Globalization.CultureInfo.InvariantCulture);
- string t = Layout.Header;
- if (t != null)
+ string? t = Layout?.Header;
+ if (t is not null)
Review Comment:
```suggestion
if (Layout?.Header is string header)
```
##########
src/log4net/Core/LogImpl.cs:
##########
@@ -1070,7 +1064,7 @@ public virtual void FatalFormat(string format, object arg0, object arg1)
{
if (IsFatalEnabled)
{
- Logger.Log(ThisDeclaringType, m_levelFatal, new SystemStringFormat(CultureInfo.InvariantCulture, format, new object[] { arg0, arg1 }), null);
+ Logger.Log(ThisDeclaringType, m_levelFatal, new SystemStringFormat(CultureInfo.InvariantCulture, format, new[] { arg0, arg1 }), null);
Review Comment:
same
##########
src/log4net/Core/StackFrameItem.cs:
##########
@@ -24,40 +25,40 @@
namespace log4net.Core
{
/// <summary>
- /// provides stack frame information without actually referencing a System.Diagnostics.StackFrame
+ /// Provides stack frame information without actually referencing a System.Diagnostics.StackFrame
/// as that would require that the containing assembly is loaded.
/// </summary>
- ///
[Serializable]
public class StackFrameItem
{
- #region Public Instance Constructors
-
/// <summary>
- /// returns a stack frame item from a stack frame. This
+ /// Creates a stack frame item from a stack frame.
/// </summary>
/// <param name="frame"></param>
- /// <returns></returns>
public StackFrameItem(StackFrame frame)
{
// set default values
- m_lineNumber = NA;
- m_fileName = NA;
- m_method = new MethodItem();
- m_className = NA;
+ LineNumber = NA;
+ FileName = NA;
+ Method = new MethodItem();
+ ClassName = NA;
try
{
// get frame values
- m_lineNumber = frame.GetFileLineNumber().ToString(System.Globalization.NumberFormatInfo.InvariantInfo);
- m_fileName = frame.GetFileName();
+ LineNumber = frame.GetFileLineNumber().ToString(System.Globalization.NumberFormatInfo.InvariantInfo);
+ FileName = frame.GetFileName();
+
// get method values
- MethodBase method = frame.GetMethod();
- if (method != null)
+ MethodBase? method = frame.GetMethod();
+ if (method is not null)
Review Comment:
```suggestion
if (frame.GetMethod() is MethodBase method)
```
##########
src/log4net/Appender/SmtpAppender.cs:
##########
@@ -389,8 +296,8 @@ protected override void SendBuffer(LoggingEvent[] events)
RenderLoggingEvent(writer, events[i]);
}
- t = Layout.Footer;
- if (t != null)
+ t = Layout?.Footer;
+ if (t is not null)
Review Comment:
same here
##########
src/log4net/Core/LogImpl.cs:
##########
@@ -1102,7 +1096,7 @@ public virtual void FatalFormat(string format, object arg0, object arg1, object
{
if (IsFatalEnabled)
{
- Logger.Log(ThisDeclaringType, m_levelFatal, new SystemStringFormat(CultureInfo.InvariantCulture, format, new object[] { arg0, arg1, arg2 }), null);
+ Logger.Log(ThisDeclaringType, m_levelFatal, new SystemStringFormat(CultureInfo.InvariantCulture, format, new[] { arg0, arg1, arg2 }), null);
Review Comment:
same
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org