ptupitsyn commented on a change in pull request #8746:
URL: https://github.com/apache/ignite/pull/8746#discussion_r570871183
##########
File path:
modules/platforms/dotnet/Apache.Ignite.Core/Impl/Plugin/PluginProcessor.cs
##########
@@ -145,18 +149,40 @@ public ExceptionFactory GetExceptionMapping(string
className)
ExceptionFactory res;
- return _exceptionMappings.TryGetValue(className, out res) ? res :
null;
+ if (_exactExceptionMappings.TryGetValue(className, out res))
+ {
+ return res;
+ }
+
+ foreach (var mapping in _patternExceptionMappings)
+ {
+ if (className.StartsWith(mapping.Key))
Review comment:
`className.StartsWith(mapping.Key, StringComparison.Ordinal)`- use
`Ordinal` comparison explicitly, otherwise `CurrentCulture` is used, which can
have unexpected behavior with some cultures.
##########
File path:
modules/platforms/dotnet/Apache.Ignite.Core.Tests/Cache/Query/CacheQueriesTest.cs
##########
@@ -823,7 +823,8 @@ public void TestSqlQueryTimeout()
// ReSharper disable once ReturnValueOfPureMethodIsNotUsed
var ex = Assert.Throws<CacheException>(() =>
cache.Query(sqlQry).ToArray());
- Assert.IsTrue(ex.ToString().Contains("QueryCancelledException: The
query was cancelled while executing."));
+ Assert.IsTrue(ex.GetBaseException().StackTrace.Contains(
Review comment:
The original idea behind keeping the Java stack trace in
`JavaException.Message` was to make it `ToString()`-friendly. `ToString()`
representation is what users will see (in the log, in console window, etc) and
what they'll use to ask for help on the user list, stackoverflow, etc. So it is
important that this string representation includes all the details, including
the full Java stack trace.
The changes to this test indicate that the old behavior is now broken. Let's
either revert StackTrace-related changes, or go further and override
`JavaException.ToString`.
##########
File path:
modules/platforms/dotnet/Apache.Ignite.Core/Impl/Plugin/PluginProcessor.cs
##########
@@ -145,18 +149,40 @@ public ExceptionFactory GetExceptionMapping(string
className)
ExceptionFactory res;
- return _exceptionMappings.TryGetValue(className, out res) ? res :
null;
+ if (_exactExceptionMappings.TryGetValue(className, out res))
+ {
+ return res;
+ }
+
+ foreach (var mapping in _patternExceptionMappings)
+ {
+ if (className.StartsWith(mapping.Key))
+ {
+ return mapping.Value;
+ }
+ }
+
+ return null;
}
/// <summary>
/// Registers the exception mapping.
/// </summary>
+ /// <param name="className">Full class name of java exception or class
name pattern (if ends with '*').</param>
+ /// <param name="factory">Exception factory for matched java class
name.</param>
public void RegisterExceptionMapping(string className,
ExceptionFactory factory)
{
Debug.Assert(className != null);
Debug.Assert(factory != null);
- _exceptionMappings.GetOrAdd(className, _ => factory);
+ if (className.EndsWith("*"))
Review comment:
`className.EndsWith("*", StringComparison.Ordinal)` - see above.
##########
File path:
modules/platforms/dotnet/Apache.Ignite.Core/Impl/Plugin/PluginProcessor.cs
##########
@@ -145,18 +149,40 @@ public ExceptionFactory GetExceptionMapping(string
className)
ExceptionFactory res;
- return _exceptionMappings.TryGetValue(className, out res) ? res :
null;
+ if (_exactExceptionMappings.TryGetValue(className, out res))
+ {
+ return res;
+ }
+
+ foreach (var mapping in _patternExceptionMappings)
+ {
+ if (className.StartsWith(mapping.Key))
+ {
+ return mapping.Value;
+ }
+ }
+
+ return null;
}
/// <summary>
/// Registers the exception mapping.
/// </summary>
+ /// <param name="className">Full class name of java exception or class
name pattern (if ends with '*').</param>
+ /// <param name="factory">Exception factory for matched java class
name.</param>
public void RegisterExceptionMapping(string className,
ExceptionFactory factory)
{
Debug.Assert(className != null);
Debug.Assert(factory != null);
- _exceptionMappings.GetOrAdd(className, _ => factory);
+ if (className.EndsWith("*"))
+ {
+ _patternExceptionMappings.GetOrAdd(className.TrimEnd('*'), _
=> factory);
Review comment:
`className.Substring(0, className.Length - 1)` to avoid dealing with
cultures.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
