You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucenenet.apache.org by "Shazwazza (via GitHub)" <gi...@apache.org> on 2025/11/27 23:31:15 UTC

[I] Lucene.Net.Replicator.SessionToken ToString() overload doesn't output values correctly [lucenenet]

Shazwazza opened a new issue, #1226:
URL: https://github.com/apache/lucenenet/issues/1226

   ### Is there an existing issue for this?
   
   - [x] I have searched the existing issues
   
   ### Describe the bug
   
   ReplicationClient logs Lucene.Net.Replicator.SessionToken as a string and its ToString() overload results in a logging output that can't be used for debugging. For example, on this line:
   
   https://github.com/apache/lucenenet/blob/36f587de8481a39a2695305c06728df2a3dedccc/src/Lucene.Net.Replicator/ReplicationClient.cs#L176
   
   ```cs
   WriteToInfoStream(string.Format("DoUpdate(): handlerVersion={0} session={1}", version, session));
   ```
   
   It will end up logging like:
   
   ```
   ReplicationThread - DoUpdate(): handlerVersion= session=id=1 version=2:1 files=System.Collections.Generic.Dictionary`2[System.String,System.Collections.Generic.IList`1[Lucene.Net.Replicator.RevisionFile]]
   ```
   
   Which is using the ToString() override in SessionToken, but this is just outputting the generic ToString() of a generic list instead of outputting what is in that list.
   
   ### Expected Behavior
   
   Ideally it would output all of this information:
   
   <img width="473" height="324" alt="Image" src="https://github.com/user-attachments/assets/05387c3e-ed6d-42ad-8a54-c86d0b830c2d" />
   
   However, this will incur some overhead, so then all places where a SessionToken is output as a string within the ReplicationClient would need to be put behind a check like 
   
   ```cs```
   if (infoStream.IsEnabled(INFO_STREAM_COMPONENT))
   ```
   
   Before the ToString() overload would be called. In fact, this should be done anyways since it is concatenating and allocating a ton of strings even if the above check was in place because the string allocations happen before the call to WriteToInfoStream which does this check.
   
   ### Steps To Reproduce
   
   _No response_
   
   ### Exceptions (if any)
   
   _No response_
   
   ### Lucene.NET Version
   
   4.8.latest
   
   ### .NET Version
   
   8
   
   ### Operating System
   
   windows
   
   ### Anything else?
   
   _No response_


-- 
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: dev-unsubscribe@lucenenet.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Lucene.Net.Replicator.SessionToken ToString() overload doesn't output values correctly [lucenenet]

Posted by "NightOwl888 (via GitHub)" <gi...@apache.org>.
NightOwl888 commented on issue #1226:
URL: https://github.com/apache/lucenenet/issues/1226#issuecomment-3588505803

   Thanks for the report.
   
   None of the BCL collection types are structurally formattable and `Dictionary<TKey, TValue>` wasn't replaced everywhere with `JCG.Dictionary<TKey, TValue>` because we used to have a wrapped implementation that was slower than the BCL type to handle `null` keys. However, since J2N 2.1.0, we have a low-level dictionary that will perform about the same on .NET Core and probably faster on .NET Framework than the BCL types.
   
   ### Short Term
   
   There are 2 potential fixes for this issue:
   
   1. Replace `Dictionary<TKey, TValue>` with `JCG.Dictionary<TKey, TValue>` (preferred)
   2. Use `J2N.Text.StringFormatter` as the `IFormatProvider` in `string.Format()`
   
   ### Long Term
   
   Longer term, we should do a review on the whole solution to catch issues such as this.
   
   #### Option A
   
   Create a Roslyn analyzer to catch scenarios where we are formatting collection types (or collection interfaces) to ensure the underlying implementation is structurally formattable (as well as any nested collections).
   
   - The analyzer should check for string concatenation, `string.Format()` and string interpolation
   - The code fix should be:
      - If the collection is only created inside of the current type and there is a structurally formattable J2N replacement, the J2N replacement should be used. Note that `ConcurrentDictionary<TKey, TValue>` has no replacement in J2N.
      - If the collection is passed into the current type (thus we cannot control the collection type), the code fix should be to use `J2N.Text.StringFormatter`.
   
   This would be preferable because it would check for regressions on this issue in future versions of Lucene.NET.
   
   #### Option B
   
   A quick fix for this Lucene.NET release would be to simply replace all `Dictionary<TKey, TValue>` instances with `JCG.Dictionary<TKey, TValue>` throughout the solution. This will catch any potential issues where a `null` key was allowed in Lucene but they didn't create any tests to confirm `null` keys would work.
   
   Of course, as previously mentioned, there may be cases where the collection is passed in through the constructor or set through a field or property using an interface and we don't have control over the collection type. In those cases, we should use `J2N.Text.StringFormatter`.


-- 
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: dev-unsubscribe@lucenenet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Lucene.Net.Replicator.SessionToken ToString() overload doesn't output values correctly [lucenenet]

Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin closed issue #1226: Lucene.Net.Replicator.SessionToken ToString() overload doesn't output values correctly
URL: https://github.com/apache/lucenenet/issues/1226


-- 
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: dev-unsubscribe@lucenenet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org