Muhannad/native adapters changes - #125
Merged
Merged
Conversation
…NativeAdapterDiscoveryService to scoped
📝 WalkthroughWalkthroughThis PR refactors the native adapter discovery and initialization system from cache-based reflection scanning to dependency-injected adapters with explicit startup-value initialization. Changes include introducing new adapter interfaces, a centralized NativePrefix constant replacing hardcoded strings, HTTP client pooling via DynamicHttpProxy, and updating service consumers to use new discovery methods. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client Request
participant Service as Service<br/>(ReceivingService,<br/>XchangeService, etc.)
participant Discovery as NativeAdapterDiscoveryService
participant Handlers as Registered Native Handlers
participant HttpProxy as DynamicHttpProxy
participant HttpFactory as IHttpClientFactory
Client->>Service: Request with adapterId
Service->>Service: Check if adapterId.StartsWith(NativePrefix)
Service->>Discovery: GetExpectedStartupValues(adapterId)
Discovery->>Handlers: Find matching adapter
Discovery->>Discovery: Inspect StartupValuesType properties
Discovery-->>Service: Return {propertyName: value/default}
Service->>Discovery: GetNativeHandler(adapterId, settings)
Discovery->>Handlers: Locate handler by name
Discovery->>Handlers: Call InitializeStartupValues(settings)
Handlers->>Handlers: ConvertTo<HandlerInput>(settings)
Handlers->>HttpProxy: GetClient(url)
HttpProxy->>HttpFactory: GetHttpClient if not cached
HttpFactory-->>HttpProxy: HttpClient instance
HttpProxy->>HttpProxy: Update LRU, enqueue for scavenging
HttpProxy-->>Handlers: Return pooled HttpClient
Discovery-->>Service: Return initialized handler
Service->>Service: Execute handler with request
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
SW.Bitween.Api/Services/ReceivingService.cs (1)
82-103:⚠️ Potential issue | 🟠 MajorMissing null check for
receiver.
nativeAdapterDiscovery.GetNativeReceiverreturnsnullif the adapter is not found (per the relevant code snippet showingFirstOrDefault). Line 87 callsreceiver.Initialize()without a null check, which will throwNullReferenceExceptionif the adapter ID is invalid or not registered.🛡️ Proposed fix with null check
if (serverlessId.StartsWith(NativeAdapterDiscoveryService.NativePrefix, StringComparison.OrdinalIgnoreCase)) { var nativeAdapterDiscovery = serviceProvider.GetRequiredService<NativeAdapterDiscoveryService>(); var receiver = nativeAdapterDiscovery.GetNativeReceiver(serverlessId, startupParameters); + + if (receiver == null) + { + logger.LogError($"Native receiver '{serverlessId}' not found"); + throw new InvalidOperationException($"Native receiver '{serverlessId}' is not registered"); + } await receiver.Initialize(); var fileList = (await receiver.ListFiles()).ToList();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@SW.Bitween.Api/Services/ReceivingService.cs` around lines 82 - 103, GetNativeReceiver can return null so add a null-check after calling nativeAdapterDiscovery.GetNativeReceiver(serverlessId, startupParameters);: if receiver is null, log an error or warning including the serverlessId/subId and skip processing (return or continue as appropriate) instead of calling receiver.Initialize(); otherwise proceed with Initialize/ListFiles/GetFile/SubmitSubscriptionXchange/DeleteFile/Finalize. Ensure you still resolve XchangeService via serviceProvider.GetService<XchangeService>() only when receiver is non-null and avoid calling any receiver methods when null.SW.Bitween.NativeAdapters/HttpHandler/NativeHttpHandler.cs (1)
33-45:⚠️ Potential issue | 🔴 CriticalAuth headers set on shared
HttpClient— seeDynamicHttpProxycomment.This code modifies
client.DefaultRequestHeaderson a shared/cachedHttpClientinstance. As noted in theDynamicHttpProxy.csreview, this causes thread-safety issues when multiple requests share the same origin. Move authentication headers toHttpRequestMessage.Headersinstead.🐛 Refactor to use request-level headers
public async Task<XchangeFile> Handle(XchangeFile xchangeFile) { HttpClient client = httpProxy.GetClient(_options.Url); + + // Build request first, set auth headers on request not client + // ... (build uri and content first, then set headers on request.Headers) + - if (_options.AuthType == "ApiKey") - client.DefaultRequestHeaders.Add("ApiKey", _options.ApiKey); - else if (_options.AuthType == "Bearer") - client.DefaultRequestHeaders.Authorization = - new AuthenticationHeaderValue("Bearer", _options.LoginPassword); // ... rest of auth logic should use request.Headers insteadFor OAuth2 and Login flows that need to make preliminary requests, you may need to create separate request messages or use a different client.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@SW.Bitween.NativeAdapters/HttpHandler/NativeHttpHandler.cs` around lines 33 - 45, The code is mutating the shared HttpClient's DefaultRequestHeaders (see HttpClient client, client.DefaultRequestHeaders and _options.AuthType) which is not thread-safe; instead create an HttpRequestMessage for each call and set per-request headers on request.Headers (add "ApiKey" for ApiKey auth or set request.Headers.Authorization with new AuthenticationHeaderValue for Bearer/Basic using _options.LoginPassword/_options.LoginUsername) before sending via the cached client; ensure no changes to DefaultRequestHeaders in NativeHttpHandler methods and handle any multi-step OAuth/login flows with separate request messages as noted in DynamicHttpProxy.
🧹 Nitpick comments (3)
SW.Bitween.Api/Services/XchangeService.cs (1)
213-263: Consider removing commented-out code rather than leaving it in.The
InstantiateNativeAdapter<T>method is commented out. If the new discovery service approach is confirmed to work, this dead code should be removed to improve maintainability. Leaving large blocks of commented-out code clutters the codebase.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@SW.Bitween.Api/Services/XchangeService.cs` around lines 213 - 263, The commented-out InstantiateNativeAdapter<T> block is dead code and should be removed to reduce clutter: delete the entire commented method including references to _nativeAdapterDiscovery, adapterInfo, inputType/inputInstance mapping, and the Activator.CreateInstance adapter instantiation; if you need to preserve the logic for historical reasons, move it to a separate backup file or a git branch instead of leaving it commented in XchangeService.cs.SW.Bitween.Api/Services/NativeAdapterDiscoveryService.cs (1)
125-130: Redundant nullable type check.The condition
Nullable.GetUnderlyingType(type) != nullalready covers the case where the type isNullable<T>, making the subsequenttype.IsGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>)check redundant.♻️ Simplified nullable check
private bool IsNullableType(Type type) { - return !type.IsValueType || - Nullable.GetUnderlyingType(type) != null || - (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>)); + return !type.IsValueType || Nullable.GetUnderlyingType(type) != null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@SW.Bitween.Api/Services/NativeAdapterDiscoveryService.cs` around lines 125 - 130, The IsNullableType method contains a redundant check: Nullable.GetUnderlyingType(type) already returns non-null for Nullable<T>, so remove the extra generic-type check (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>)) and simplify IsNullableType to return !type.IsValueType || Nullable.GetUnderlyingType(type) != null; update the method body in NativeAdapterDiscoveryService.IsNullableType accordingly.SW.Bitween.NativeAdapters/ServiceCollectionExtensions.cs (1)
24-25: Consider extensibility for multiple native adapters.Currently, only
NativeHttpHandleris registered for bothINativeInfolinkHandlerandINativeAdapter. If additional native adapters are introduced in the future, you'll need to register each one explicitly or implement a convention-based registration approach. This is acceptable for now but worth noting for future scalability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@SW.Bitween.NativeAdapters/ServiceCollectionExtensions.cs` around lines 24 - 25, Current registration binds both INativeInfolinkHandler and INativeAdapter to a single implementation (NativeHttpHandler) which prevents adding additional adapters; update ServiceCollectionExtensions to support multiple adapters by registering each implementation as its own service (e.g., AddScoped<INativeAdapter, SomeOtherNativeHandler> or register all implementations so constructor injection can receive IEnumerable<INativeAdapter>), or implement a convention-based scan/factory in the registration method that discovers and registers all types implementing INativeAdapter/INativeInfolinkHandler (refer to serviceCollection.AddScoped, INativeInfolinkHandler, INativeAdapter, and NativeHttpHandler in the file).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@SW.Bitween.Api/Resources/Adapters/GetProperties.cs`:
- Around line 28-30: The prefix match using
NativeAdapterDiscoveryService.NativePrefix is too permissive (NativePrefix is
"native") and will match unintended keys like "nativelyBuilt"; update the check
so it only matches the delimiter-separated prefix — either change
NativeAdapterDiscoveryService.NativePrefix to include the dot (e.g., "native.")
or modify the condition in GetProperties to require the separator (e.g.,
StartsWith(NativeAdapterDiscoveryService.NativePrefix + ".") or an Equals on the
prefix segment of decodedKey) before calling
_nativeAdapterDiscovery.GetExpectedStartupValues(decodedKey).
In `@SW.Bitween.Api/Services/NativeAdapterDiscoveryService.cs`:
- Around line 48-82: GetNativeHandler matches by the handler's Name property
while GetNativeReceiver and GetNativeValidator match by GetType().Name, causing
inconsistent lookups; change GetNativeReceiver and GetNativeValidator to use the
same matching logic as GetNativeHandler (match adapterId against the component's
Name using StringComparison.OrdinalIgnoreCase) and leave the
InitializeStartupValues(settings) call as-is so all three methods consistently
resolve adapters by their Name property.
In `@SW.Bitween.Api/Services/XchangeService.cs`:
- Around line 139-143: GetNativeHandler can return null so before calling
handler.Handle(xchangeFile) add a null check for the handler returned from
_nativeAdapterDiscovery.GetNativeHandler(xchange.MapperId, mapperProperties)
(when xchange.MapperId starts with NativeAdapterDiscoveryService.NativePrefix);
if handler is null, handle it explicitly—e.g., log a descriptive error including
xchange.MapperId and mapperProperties, and either throw a specific exception or
return/continue with a safe fallback—so you never call handler.Handle on a null
reference.
- Around line 457-460: GetNativeHandler may return null in the code paths inside
XchangeService (the block checking
notifier.HandlerId.StartsWith(NativeAdapterDiscoveryService.NativePrefix...)),
so add a null check after calling
_nativeAdapterDiscovery.GetNativeHandler(notifier.HandlerId, handlerProperties)
and avoid calling handler.Handle() when handler is null — instead log an error
(or throw a descriptive exception) including notifier.HandlerId and
xchangeResult.Id; apply the same null-check and error-handling change in the
NotifyResult code path where GetNativeHandler is used.
- Around line 195-199: In XchangeService (when checking
NativeAdapterDiscoveryService.NativePrefix), guard against a null return from
_nativeAdapterDiscovery.GetNativeHandler by checking the result of
GetNativeHandler(handlerId, handlerProperties) before calling
handler.Handle(xchangeFile); do the same in RunHandler: if the returned handler
is null, either log an error with the xchange.HandlerId and handlerProperties
and skip/return, or throw a clear InvalidOperationException indicating the
native handler was not found—ensure you reference
_nativeAdapterDiscovery.GetNativeHandler, the local handler variable, and
handler.Handle(xchangeFile) when adding the null check and error handling.
- Around line 168-172: The call to
_nativeAdapterDiscovery.GetNativeValidator(validatorId, properties) can return
null, so before invoking validator.Validate(xchangeFile) add a null check for
the validator returned by GetNativeValidator (when validatorId starts with
NativeAdapterDiscoveryService.NativePrefix); if null, handle it consistently
(e.g., set result to an error/failed ValidationResult and/or throw a descriptive
exception) rather than calling Validate on a null reference, and ensure the
behavior aligns with how missing handlers are handled elsewhere in
XchangeService.
In `@SW.Bitween.NativeAdapters/HttpHandler/DynamicHttpProxy.cs`:
- Around line 20-36: The cached HttpClient returned by GetClient is shared and
its mutable DefaultRequestHeaders are being modified per-request in
NativeHttpHandler.Handle, causing race conditions; fix by updating
NativeHttpHandler.Handle to stop mutating client.DefaultRequestHeaders and
instead set per-request headers on the HttpRequestMessage (create headers on the
request before sending), or alternatively change callers to obtain a fresh
unconfigured HttpClient from a factory when per-request DefaultRequestHeaders
are required; locate GetClient, _cache, httpClientFactory,
NativeHttpHandler.Handle and replace usages of DefaultRequestHeaders with
request.Headers on the HttpRequestMessage so shared clients remain immutable.
- Around line 48-61: UpdateLru can throw a NullReferenceException when pruning
because _lruList.Last may be null; guard the prune loop by checking that
_lruList.Count (or _lruList.Last) is > 0 before accessing Last/RemoveLast and
only call _cache.TryRemove when you successfully retrieved an oldest value; in
other words, inside UpdateLru ensure you check _lruList.Count > 0 (or
_lruList.Last != null) before using _lruList.Last.Value and calling
_lruList.RemoveLast(), and break the while loop if the list is empty so
_cache.TryRemove never receives a null key.
In `@SW.Bitween.NativeAdapters/HttpHandler/NativeHttpHandler.cs`:
- Line 157: The lookup for adapters is inconsistent: NativeHttpHandler exposes
Name ("native.httpHandler") but GetExpectedStartupValues uses GetType().Name
("NativeHttpHandler"), causing misses; update the discovery methods
GetExpectedStartupValues, GetNativeReceiver, and GetNativeValidator to match the
same strategy used by GetNativeHandler (use a.Name.Equals(adapterId,
StringComparison.OrdinalIgnoreCase) or the existing StringComparison used in
GetNativeHandler) so they compare the adapter's Name property instead of
GetType().Name, ensuring all lookups (including GetNativeHandler) use the
adapter Name consistently.
In `@SW.Bitween.NativeAdapters/ReflectionExtensions.cs`:
- Around line 5-35: ConvertTo<T> can produce a null/cast issue and currently
swallows all exceptions; constrain the generic and tighten error handling: add a
where T : new() constraint and instantiate via new T() instead of
Activator.CreateInstance(typeof(T)) to guarantee a non-null inputInstance;
replace the catch { } around Convert.ChangeType in ConvertTo<T> with specific
catches (e.g., FormatException, InvalidCastException, OverflowException) and
handle them explicitly—preserve the existing fallback of setting string
properties when prop.PropertyType == typeof(string), but for other failures
rethrow or surface the exception (include the caught Exception variable) rather
than silently swallowing it so callers can observe conversion errors.
---
Outside diff comments:
In `@SW.Bitween.Api/Services/ReceivingService.cs`:
- Around line 82-103: GetNativeReceiver can return null so add a null-check
after calling nativeAdapterDiscovery.GetNativeReceiver(serverlessId,
startupParameters);: if receiver is null, log an error or warning including the
serverlessId/subId and skip processing (return or continue as appropriate)
instead of calling receiver.Initialize(); otherwise proceed with
Initialize/ListFiles/GetFile/SubmitSubscriptionXchange/DeleteFile/Finalize.
Ensure you still resolve XchangeService via
serviceProvider.GetService<XchangeService>() only when receiver is non-null and
avoid calling any receiver methods when null.
In `@SW.Bitween.NativeAdapters/HttpHandler/NativeHttpHandler.cs`:
- Around line 33-45: The code is mutating the shared HttpClient's
DefaultRequestHeaders (see HttpClient client, client.DefaultRequestHeaders and
_options.AuthType) which is not thread-safe; instead create an
HttpRequestMessage for each call and set per-request headers on request.Headers
(add "ApiKey" for ApiKey auth or set request.Headers.Authorization with new
AuthenticationHeaderValue for Bearer/Basic using
_options.LoginPassword/_options.LoginUsername) before sending via the cached
client; ensure no changes to DefaultRequestHeaders in NativeHttpHandler methods
and handle any multi-step OAuth/login flows with separate request messages as
noted in DynamicHttpProxy.
---
Nitpick comments:
In `@SW.Bitween.Api/Services/NativeAdapterDiscoveryService.cs`:
- Around line 125-130: The IsNullableType method contains a redundant check:
Nullable.GetUnderlyingType(type) already returns non-null for Nullable<T>, so
remove the extra generic-type check (type.IsGenericType &&
type.GetGenericTypeDefinition() == typeof(Nullable<>)) and simplify
IsNullableType to return !type.IsValueType || Nullable.GetUnderlyingType(type)
!= null; update the method body in NativeAdapterDiscoveryService.IsNullableType
accordingly.
In `@SW.Bitween.Api/Services/XchangeService.cs`:
- Around line 213-263: The commented-out InstantiateNativeAdapter<T> block is
dead code and should be removed to reduce clutter: delete the entire commented
method including references to _nativeAdapterDiscovery, adapterInfo,
inputType/inputInstance mapping, and the Activator.CreateInstance adapter
instantiation; if you need to preserve the logic for historical reasons, move it
to a separate backup file or a git branch instead of leaving it commented in
XchangeService.cs.
In `@SW.Bitween.NativeAdapters/ServiceCollectionExtensions.cs`:
- Around line 24-25: Current registration binds both INativeInfolinkHandler and
INativeAdapter to a single implementation (NativeHttpHandler) which prevents
adding additional adapters; update ServiceCollectionExtensions to support
multiple adapters by registering each implementation as its own service (e.g.,
AddScoped<INativeAdapter, SomeOtherNativeHandler> or register all
implementations so constructor injection can receive
IEnumerable<INativeAdapter>), or implement a convention-based scan/factory in
the registration method that discovers and registers all types implementing
INativeAdapter/INativeInfolinkHandler (refer to serviceCollection.AddScoped,
INativeInfolinkHandler, INativeAdapter, and NativeHttpHandler in the file).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
SW.Bitween.Api/Resources/Adapters/GetProperties.csSW.Bitween.Api/Resources/Subscriptions/Update.csSW.Bitween.Api/Services/NativeAdapterDiscoveryService.csSW.Bitween.Api/Services/ReceivingService.csSW.Bitween.Api/Services/XchangeService.csSW.Bitween.NativeAdapters/HttpHandler/DynamicHttpProxy.csSW.Bitween.NativeAdapters/HttpHandler/NativeHttpHandler.csSW.Bitween.NativeAdapters/Interfaces.csSW.Bitween.NativeAdapters/ReflectionExtensions.csSW.Bitween.NativeAdapters/SW.Bitween.NativeAdapters.csprojSW.Bitween.NativeAdapters/ServiceCollectionExtensions.csSW.Bitween.Web/Startup.cs
Comment on lines
+28
to
+30
| if (decodedKey.StartsWith(NativeAdapterDiscoveryService.NativePrefix, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| return _nativeAdapterDiscovery.GetNativeAdapterProperties(decodedKey); | ||
| return _nativeAdapterDiscovery.GetExpectedStartupValues(decodedKey); |
There was a problem hiding this comment.
Prefix matching may be too permissive.
NativeAdapterDiscoveryService.NativePrefix is "native" (without a trailing dot). Using StartsWith("native", ...) will match any string beginning with "native", including potentially unintended matches like "nativelyBuilt". The previous hardcoded prefix was likely "native." with a dot to ensure proper delimiter.
Consider updating NativePrefix to include the dot separator or adjusting the check:
🛡️ Proposed fix
In NativeAdapterDiscoveryService.cs:
-public const string NativePrefix = "native";
+public const string NativePrefix = "native.";Or adjust the check here:
-if (decodedKey.StartsWith(NativeAdapterDiscoveryService.NativePrefix, StringComparison.OrdinalIgnoreCase))
+if (decodedKey.StartsWith($"{NativeAdapterDiscoveryService.NativePrefix}.", StringComparison.OrdinalIgnoreCase))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@SW.Bitween.Api/Resources/Adapters/GetProperties.cs` around lines 28 - 30, The
prefix match using NativeAdapterDiscoveryService.NativePrefix is too permissive
(NativePrefix is "native") and will match unintended keys like "nativelyBuilt";
update the check so it only matches the delimiter-separated prefix — either
change NativeAdapterDiscoveryService.NativePrefix to include the dot (e.g.,
"native.") or modify the condition in GetProperties to require the separator
(e.g., StartsWith(NativeAdapterDiscoveryService.NativePrefix + ".") or an Equals
on the prefix segment of decodedKey) before calling
_nativeAdapterDiscovery.GetExpectedStartupValues(decodedKey).
Comment on lines
+48
to
82
| public INativeInfolinkHandler GetNativeHandler(string adapterId, Dictionary<string, string> settings) | ||
| { | ||
| if (!_adaptersCache.ContainsKey(category)) | ||
| var result = | ||
| nativeHandlers.FirstOrDefault(a => a.Name.Equals(adapterId, StringComparison.OrdinalIgnoreCase)); | ||
| if (result != null) | ||
| { | ||
| _adaptersCache[category] = new List<NativeAdapterInfo>(); | ||
| result.InitializeStartupValues(settings); | ||
| } | ||
|
|
||
| var adapterName = type.Name;//.Replace("Handler", "").Replace("Mapper", "") | ||
| //.Replace("Validator", "").Replace("Receiver", "").ToLower(); | ||
|
|
||
| _adaptersCache[category].Add(new NativeAdapterInfo | ||
| { | ||
| Key = $"native.{adapterName}", | ||
| Name = type.Name, | ||
| Type = type, | ||
| Category = category | ||
| }); | ||
| return result; | ||
| } | ||
|
|
||
| public IEnumerable<string> GetNativeAdapters(string prefix) | ||
| public INativeInfolinkReceiver GetNativeReceiver(string adapterId, IDictionary<string, string> settings) | ||
| { | ||
| if (string.IsNullOrEmpty(prefix)) | ||
| var result = | ||
| nativeReceivers.FirstOrDefault(a => a.GetType().Name.Equals(adapterId, StringComparison.OrdinalIgnoreCase)); | ||
| if (result != null) | ||
| { | ||
| return _adaptersCache.Values.SelectMany(v => v).Select(a => a.Key); | ||
| result.InitializeStartupValues(settings); | ||
| } | ||
|
|
||
| var category = prefix.ToLower().TrimStart('.'); | ||
|
|
||
| if (_adaptersCache.TryGetValue(category, out var adapters)) | ||
| { | ||
| return adapters.Select(a => a.Key); | ||
| } | ||
|
|
||
| return Enumerable.Empty<string>(); | ||
| } | ||
|
|
||
| public NativeAdapterInfo GetNativeAdapterInfo(string adapterId) | ||
| { | ||
| return _adaptersCache.Values | ||
| .SelectMany(v => v) | ||
| .FirstOrDefault(a => a.Key.Equals(adapterId, StringComparison.OrdinalIgnoreCase)); | ||
| return result; | ||
| } | ||
|
|
||
| public Dictionary<string, string> GetNativeAdapterProperties(string adapterId) | ||
| public INativeInfolinkValidator GetNativeValidator(string adapterId, IDictionary<string, string> settings) | ||
| { | ||
| var adapterInfo = GetNativeAdapterInfo(adapterId); | ||
| if (adapterInfo == null) | ||
| return new Dictionary<string, string>(); | ||
|
|
||
| var result = new Dictionary<string, string>(); | ||
|
|
||
| // Get constructor parameters | ||
| var constructor = adapterInfo.Type.GetConstructors() | ||
| .FirstOrDefault(c => c.GetParameters().Length > 0); | ||
|
|
||
| if (constructor == null) | ||
| return result; | ||
|
|
||
| // Get the first parameter type (input model) | ||
| var inputParameter = constructor.GetParameters().FirstOrDefault(); | ||
| if (inputParameter == null) | ||
| return result; | ||
|
|
||
| var inputType = inputParameter.ParameterType; | ||
|
|
||
| // Get all properties from the input model | ||
| var properties = inputType.GetProperties(BindingFlags.Public | BindingFlags.Instance); | ||
|
|
||
| foreach (var prop in properties) | ||
| var result = | ||
| nativeValidators.FirstOrDefault(a => a.GetType().Name.Equals(adapterId, StringComparison.OrdinalIgnoreCase)); | ||
| if (result != null) | ||
| { | ||
| var defaultValue = GetDefaultValue(prop); | ||
| var hasRequiredAttribute = prop.GetCustomAttribute<System.ComponentModel.DataAnnotations.RequiredAttribute>() != null; | ||
| var isRequired = hasRequiredAttribute || (!IsNullableType(prop.PropertyType) && defaultValue == null); | ||
|
|
||
| if (isRequired) | ||
| { | ||
| result[prop.Name] = $"{prop.Name} *"; | ||
| } | ||
| else | ||
| { | ||
| result[prop.Name] = $"{prop.Name} ({defaultValue ?? "null"})"; | ||
| } | ||
| result.InitializeStartupValues(settings); | ||
| } | ||
|
|
||
| return result; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's find the NativeAdapterDiscoveryService.cs file and examine it
fd "NativeAdapterDiscoveryService.cs" --type fRepository: simplify9/Bitween-api
Length of output: 121
🏁 Script executed:
#!/bin/bash
# Search for Name property implementations in native adapters
rg -n "public string Name\s*[={]" --type cs -A 2Repository: simplify9/Bitween-api
Length of output: 7083
🏁 Script executed:
#!/bin/bash
# Look for INativeInfolinkHandler, INativeInfolinkReceiver, INativeInfolinkValidator implementations
rg -n "class.*implements.*INativeInfolink(Handler|Receiver|Validator)" --type cs -A 3Repository: simplify9/Bitween-api
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Search more broadly for native adapter implementations
rg -n "class.*Native.*Handler|class.*Native.*Receiver|class.*Native.*Validator" --type cs -B 1 -A 5Repository: simplify9/Bitween-api
Length of output: 694
🏁 Script executed:
#!/bin/bash
# Read the NativeAdapterDiscoveryService to see the complete matching logic
cat -n SW.Bitween.Api/Services/NativeAdapterDiscoveryService.cs | head -120Repository: simplify9/Bitween-api
Length of output: 5489
🏁 Script executed:
#!/bin/bash
# Look for receiver and validator implementations
fd ".*Receiver.*\.cs|.*Validator.*\.cs" --type f | grep -i nativeRepository: simplify9/Bitween-api
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Check the Interfaces.cs to see the interface definitions
cat -n SW.Bitween.NativeAdapters/Interfaces.csRepository: simplify9/Bitween-api
Length of output: 651
🏁 Script executed:
#!/bin/bash
# Search for any INativeInfolinkReceiver and INativeInfolinkValidator implementations
rg -n "INativeInfolink(Receiver|Validator)" --type cs -B 2 -A 8Repository: simplify9/Bitween-api
Length of output: 3975
🏁 Script executed:
#!/bin/bash
# Let's look for actual receiver and validator implementations
fd ".*\.cs" -x grep -l "INativeInfolinkReceiver\|INativeInfolinkValidator" {} \;Repository: simplify9/Bitween-api
Length of output: 165
🏁 Script executed:
#!/bin/bash
# Check for receiver and validator implementations with their Name properties
rg -n "class.*:.*INativeInfolink(Receiver|Validator)" --type cs -A 10 | head -50Repository: simplify9/Bitween-api
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Search for any implementations of these interfaces more broadly
rg "INativeInfolinkReceiver|INativeInfolinkValidator" --type cs -B 5 -A 5Repository: simplify9/Bitween-api
Length of output: 3861
🏁 Script executed:
#!/bin/bash
# Check what adapters actually exist in the repository
find . -name "*Adapter*.cs" -o -name "*Handler*.cs" -o -name "*Receiver*.cs" -o -name "*Validator*.cs" | grep -v node_modulesRepository: simplify9/Bitween-api
Length of output: 673
🏁 Script executed:
#!/bin/bash
# Let's check if there are any other implementations by looking at the inheritance
rg "class.*:" --type cs | grep -i "inative"Repository: simplify9/Bitween-api
Length of output: 208
🏁 Script executed:
#!/bin/bash
# Check the sample implementations
cat -n SW.Bitween.SampleValidator/Handler.csRepository: simplify9/Bitween-api
Length of output: 1037
🏁 Script executed:
#!/bin/bash
# Check the sample handler
cat -n SW.Bitween.SampleHandler/Handler.csRepository: simplify9/Bitween-api
Length of output: 739
🏁 Script executed:
#!/bin/bash
# Check the sample mapper
cat -n SW.Bitween.SampleMapper/Handler.csRepository: simplify9/Bitween-api
Length of output: 747
🏁 Script executed:
#!/bin/bash
# Let's search for where GetNativeHandler, GetNativeReceiver, and GetNativeValidator are called
rg "GetNativeHandler|GetNativeReceiver|GetNativeValidator" --type cs -B 2 -A 2Repository: simplify9/Bitween-api
Length of output: 4069
🏁 Script executed:
#!/bin/bash
# Search for any class implementing INativeInfolinkReceiver or INativeInfolinkValidator
rg "implements.*INativeInfolink(Receiver|Validator)|:\s*(INativeInfolink(Receiver|Validator))" --type csRepository: simplify9/Bitween-api
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Get the full context of how adapters are passed to the service
cat -n SW.Bitween.Api/Services/XchangeService.cs | grep -A 5 -B 5 "GetNativeHandler\|GetNativeReceiver\|GetNativeValidator" | head -60Repository: simplify9/Bitween-api
Length of output: 2291
🏁 Script executed:
#!/bin/bash
# Check the NativeHttpHandler to understand its Name property value
cat -n SW.Bitween.NativeAdapters/HttpHandler/NativeHttpHandler.cs | head -170 | tail -20Repository: simplify9/Bitween-api
Length of output: 1087
🏁 Script executed:
#!/bin/bash
# Let's look for where adapterId/serverlessId/validatorId strings come from
rg "GetNativeReceiver\|GetNativeValidator" --type cs -B 10 | head -80Repository: simplify9/Bitween-api
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Check ReceivingService more carefully to see what value is passed to GetNativeReceiver
cat -n SW.Bitween.Api/Services/ReceivingService.cs | grep -B 20 "GetNativeReceiver"Repository: simplify9/Bitween-api
Length of output: 1193
🏁 Script executed:
#!/bin/bash
# Search for any other implementations or test files that might show expected adapter IDs
find . -name "*.cs" -type f | xargs grep -l "native\." | head -20Repository: simplify9/Bitween-api
Length of output: 125
🏁 Script executed:
#!/bin/bash
# Let's verify if there are any tests or integration files that show expected adapter IDs
find . -name "*Test*" -o -name "*test*" -o -name "*spec*" | grep -i adapterRepository: simplify9/Bitween-api
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Check for any database or configuration files that might define adapter IDs
fd ".*\.(json|xml|yaml|yml|config)" | xargs grep -l "native\." 2>/dev/null | head -10Repository: simplify9/Bitween-api
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Let's check the complete NativeHttpHandler to see if it implements multiple interfaces
cat -n SW.Bitween.NativeAdapters/HttpHandler/NativeHttpHandler.cs | head -20Repository: simplify9/Bitween-api
Length of output: 728
🏁 Script executed:
#!/bin/bash
# Let's check if there are any other native adapter implementations we missed
find . -path ./node_modules -prune -o -type f -name "*.cs" -exec grep -l "INativeInfolink" {} \;Repository: simplify9/Bitween-api
Length of output: 285
🏁 Script executed:
#!/antml:thinking
Let me reconsider what I've found:
1. I found NativeHttpHandler which implements INativeInfolinkHandler
2. NativeHttpHandler.Name = "native.httpHandler"
3. GetNativeHandler uses a.Name for matching
4. GetNativeReceiver and GetNativeValidator use a.GetType().Name for matching
5. I could not find any implementations of INativeInfolinkReceiver or INativeInfolinkValidator
The review states that GetNativeHandler uses `a.Name` while GetNativeReceiver/GetNativeValidator use `a.GetType().Name`. This is indeed what the code shows.
The inconsistency is real. However, since there are no actual implementations of Receiver or Validator, the impact is unknown. But the inconsistency is still a valid concern for code maintainability and potential future implementations.
The review comment appears to be correct - there IS an inconsistency in the matching logic. Whether this causes actual problems depends on how the adapters are named when they are implemented, but the inconsistency itself is valid to flag.
</thinking>
bash
#!/bin/bash
# Let's see the full list of files in NativeAdapters directory
ls -la SW.Bitween.NativeAdapters/Repository: simplify9/Bitween-api
Length of output: 660
🏁 Script executed:
#!/bin/bash
# Check ServiceCollectionExtensions to see how adapters are registered
cat -n SW.Bitween.NativeAdapters/ServiceCollectionExtensions.csRepository: simplify9/Bitween-api
Length of output: 1248
🏁 Script executed:
#!/bin/bash
# Check the Services directory
ls -la SW.Bitween.NativeAdapters/Services/Repository: simplify9/Bitween-api
Length of output: 247
🏁 Script executed:
#!/bin/bash
# Check ReflectionExtensions
cat -n SW.Bitween.NativeAdapters/ReflectionExtensions.csRepository: simplify9/Bitween-api
Length of output: 1577
🏁 Script executed:
#!/bin/bash
# Let me verify what IDs are actually being passed to these methods
# by checking if there are test files or database initialization scripts
find . -name "*Seed*" -o -name "*Migration*" -o -name "*Test*" | head -20Repository: simplify9/Bitween-api
Length of output: 831
🏁 Script executed:
#!/an/bash
# Search for where validatorId, MapperId, HandlerId are set
rg "validatorId|MapperId|HandlerId" --type cs -B 2 -A 2 | head -100Repository: simplify9/Bitween-api
Length of output: 9290
🏁 Script executed:
#!/bin/bash
# One more check - verify the complete method signatures to ensure we have the right picture
cat -n SW.Bitween.Api/Services/NativeAdapterDiscoveryService.cs | sed -n '48,82p'Repository: simplify9/Bitween-api
Length of output: 1587
Inconsistent matching logic between handler, receiver, and validator lookups.
GetNativeHandler matches by the Name property (line 51, e.g., "native.httpHandler"), while GetNativeReceiver and GetNativeValidator match by GetType().Name (lines 63, 75, e.g., "NativeHttpHandler"). This inconsistency could cause lookup failures when implementing receiver or validator adapters if their naming conventions differ between the Name property and type name.
Consider using consistent matching logic across all three methods, or document the expected naming convention clearly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@SW.Bitween.Api/Services/NativeAdapterDiscoveryService.cs` around lines 48 -
82, GetNativeHandler matches by the handler's Name property while
GetNativeReceiver and GetNativeValidator match by GetType().Name, causing
inconsistent lookups; change GetNativeReceiver and GetNativeValidator to use the
same matching logic as GetNativeHandler (match adapterId against the component's
Name using StringComparison.OrdinalIgnoreCase) and leave the
InitializeStartupValues(settings) call as-is so all three methods consistently
resolve adapters by their Name property.
Comment on lines
+139
to
143
| if (xchange.MapperId.StartsWith(NativeAdapterDiscoveryService.NativePrefix, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| var handler = InstantiateNativeAdapter<IInfolinkHandler>(xchange.MapperId, mapperProperties); | ||
| var handler = _nativeAdapterDiscovery.GetNativeHandler(xchange.MapperId, mapperProperties); | ||
| xchangeFile = await handler.Handle(xchangeFile); | ||
| } |
There was a problem hiding this comment.
Potential NullReferenceException: GetNativeHandler may return null.
GetNativeHandler returns null if no adapter matches the adapterId. The code proceeds to call handler.Handle(xchangeFile) without a null check, which will throw a NullReferenceException at runtime if the adapter is not found.
🛡️ Proposed fix to add null check
if (xchange.MapperId.StartsWith(NativeAdapterDiscoveryService.NativePrefix, StringComparison.OrdinalIgnoreCase))
{
var handler = _nativeAdapterDiscovery.GetNativeHandler(xchange.MapperId, mapperProperties);
+ if (handler == null)
+ throw new BitweenException($"Native adapter not found: {xchange.MapperId}");
xchangeFile = await handler.Handle(xchangeFile);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@SW.Bitween.Api/Services/XchangeService.cs` around lines 139 - 143,
GetNativeHandler can return null so before calling handler.Handle(xchangeFile)
add a null check for the handler returned from
_nativeAdapterDiscovery.GetNativeHandler(xchange.MapperId, mapperProperties)
(when xchange.MapperId starts with NativeAdapterDiscoveryService.NativePrefix);
if handler is null, handle it explicitly—e.g., log a descriptive error including
xchange.MapperId and mapperProperties, and either throw a specific exception or
return/continue with a safe fallback—so you never call handler.Handle on a null
reference.
Comment on lines
+168
to
172
| if (validatorId.StartsWith(NativeAdapterDiscoveryService.NativePrefix, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| var validator = InstantiateNativeAdapter<IInfolinkValidator>(validatorId, properties); | ||
| var validator = _nativeAdapterDiscovery.GetNativeValidator(validatorId, properties); | ||
|
|
||
| result = await validator.Validate(xchangeFile); |
There was a problem hiding this comment.
Potential NullReferenceException: GetNativeValidator may return null.
Same issue as with the handler - GetNativeValidator returns null if the validator is not found. Line 172 calls validator.Validate() without null check.
🛡️ Proposed fix to add null check
if (validatorId.StartsWith(NativeAdapterDiscoveryService.NativePrefix, StringComparison.OrdinalIgnoreCase))
{
var validator = _nativeAdapterDiscovery.GetNativeValidator(validatorId, properties);
-
+ if (validator == null)
+ throw new BitweenException($"Native validator not found: {validatorId}");
result = await validator.Validate(xchangeFile);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (validatorId.StartsWith(NativeAdapterDiscoveryService.NativePrefix, StringComparison.OrdinalIgnoreCase)) | |
| { | |
| var validator = InstantiateNativeAdapter<IInfolinkValidator>(validatorId, properties); | |
| var validator = _nativeAdapterDiscovery.GetNativeValidator(validatorId, properties); | |
| result = await validator.Validate(xchangeFile); | |
| if (validatorId.StartsWith(NativeAdapterDiscoveryService.NativePrefix, StringComparison.OrdinalIgnoreCase)) | |
| { | |
| var validator = _nativeAdapterDiscovery.GetNativeValidator(validatorId, properties); | |
| if (validator == null) | |
| throw new BitweenException($"Native validator not found: {validatorId}"); | |
| result = await validator.Validate(xchangeFile); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@SW.Bitween.Api/Services/XchangeService.cs` around lines 168 - 172, The call
to _nativeAdapterDiscovery.GetNativeValidator(validatorId, properties) can
return null, so before invoking validator.Validate(xchangeFile) add a null check
for the validator returned by GetNativeValidator (when validatorId starts with
NativeAdapterDiscoveryService.NativePrefix); if null, handle it consistently
(e.g., set result to an error/failed ValidationResult and/or throw a descriptive
exception) rather than calling Validate on a null reference, and ensure the
behavior aligns with how missing handlers are handled elsewhere in
XchangeService.
Comment on lines
+195
to
199
| if (xchange.HandlerId.StartsWith(NativeAdapterDiscoveryService.NativePrefix, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| var handler = InstantiateNativeAdapter<IInfolinkHandler>(xchange.HandlerId, handlerProperties); | ||
| var handler = _nativeAdapterDiscovery.GetNativeHandler(xchange.HandlerId, handlerProperties); | ||
| xchangeFile = await handler.Handle(xchangeFile); | ||
| } |
There was a problem hiding this comment.
Potential NullReferenceException: GetNativeHandler may return null.
Same issue in RunHandler - no null check before calling handler.Handle().
🛡️ Proposed fix to add null check
if (xchange.HandlerId.StartsWith(NativeAdapterDiscoveryService.NativePrefix, StringComparison.OrdinalIgnoreCase))
{
var handler = _nativeAdapterDiscovery.GetNativeHandler(xchange.HandlerId, handlerProperties);
+ if (handler == null)
+ throw new BitweenException($"Native adapter not found: {xchange.HandlerId}");
xchangeFile = await handler.Handle(xchangeFile);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (xchange.HandlerId.StartsWith(NativeAdapterDiscoveryService.NativePrefix, StringComparison.OrdinalIgnoreCase)) | |
| { | |
| var handler = InstantiateNativeAdapter<IInfolinkHandler>(xchange.HandlerId, handlerProperties); | |
| var handler = _nativeAdapterDiscovery.GetNativeHandler(xchange.HandlerId, handlerProperties); | |
| xchangeFile = await handler.Handle(xchangeFile); | |
| } | |
| if (xchange.HandlerId.StartsWith(NativeAdapterDiscoveryService.NativePrefix, StringComparison.OrdinalIgnoreCase)) | |
| { | |
| var handler = _nativeAdapterDiscovery.GetNativeHandler(xchange.HandlerId, handlerProperties); | |
| if (handler == null) | |
| throw new BitweenException($"Native adapter not found: {xchange.HandlerId}"); | |
| xchangeFile = await handler.Handle(xchangeFile); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@SW.Bitween.Api/Services/XchangeService.cs` around lines 195 - 199, In
XchangeService (when checking NativeAdapterDiscoveryService.NativePrefix), guard
against a null return from _nativeAdapterDiscovery.GetNativeHandler by checking
the result of GetNativeHandler(handlerId, handlerProperties) before calling
handler.Handle(xchangeFile); do the same in RunHandler: if the returned handler
is null, either log an error with the xchange.HandlerId and handlerProperties
and skip/return, or throw a clear InvalidOperationException indicating the
native handler was not found—ensure you reference
_nativeAdapterDiscovery.GetNativeHandler, the local handler variable, and
handler.Handle(xchangeFile) when adding the null check and error handling.
Comment on lines
+457
to
460
| if (notifier.HandlerId.StartsWith(NativeAdapterDiscoveryService.NativePrefix, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| var handler = InstantiateNativeAdapter<IInfolinkHandler>(notifier.HandlerId, handlerProperties); | ||
| var handler = _nativeAdapterDiscovery.GetNativeHandler(notifier.HandlerId, handlerProperties); | ||
| await handler.Handle(new XchangeFile(JsonConvert.SerializeObject(notificationData), xchangeResult.Id)); |
There was a problem hiding this comment.
Potential NullReferenceException: GetNativeHandler may return null.
Same issue in NotifyResult - no null check before calling handler.Handle().
🛡️ Proposed fix to add null check
if (notifier.HandlerId.StartsWith(NativeAdapterDiscoveryService.NativePrefix, StringComparison.OrdinalIgnoreCase))
{
- var handler = _nativeAdapterDiscovery.GetNativeHandler(notifier.HandlerId, handlerProperties);
+ var handler = _nativeAdapterDiscovery.GetNativeHandler(notifier.HandlerId, handlerProperties);
+ if (handler == null)
+ throw new BitweenException($"Native adapter not found: {notifier.HandlerId}");
await handler.Handle(new XchangeFile(JsonConvert.SerializeObject(notificationData), xchangeResult.Id));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (notifier.HandlerId.StartsWith(NativeAdapterDiscoveryService.NativePrefix, StringComparison.OrdinalIgnoreCase)) | |
| { | |
| var handler = InstantiateNativeAdapter<IInfolinkHandler>(notifier.HandlerId, handlerProperties); | |
| var handler = _nativeAdapterDiscovery.GetNativeHandler(notifier.HandlerId, handlerProperties); | |
| await handler.Handle(new XchangeFile(JsonConvert.SerializeObject(notificationData), xchangeResult.Id)); | |
| if (notifier.HandlerId.StartsWith(NativeAdapterDiscoveryService.NativePrefix, StringComparison.OrdinalIgnoreCase)) | |
| { | |
| var handler = _nativeAdapterDiscovery.GetNativeHandler(notifier.HandlerId, handlerProperties); | |
| if (handler == null) | |
| throw new BitweenException($"Native adapter not found: {notifier.HandlerId}"); | |
| await handler.Handle(new XchangeFile(JsonConvert.SerializeObject(notificationData), xchangeResult.Id)); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@SW.Bitween.Api/Services/XchangeService.cs` around lines 457 - 460,
GetNativeHandler may return null in the code paths inside XchangeService (the
block checking
notifier.HandlerId.StartsWith(NativeAdapterDiscoveryService.NativePrefix...)),
so add a null check after calling
_nativeAdapterDiscovery.GetNativeHandler(notifier.HandlerId, handlerProperties)
and avoid calling handler.Handle() when handler is null — instead log an error
(or throw a descriptive exception) including notifier.HandlerId and
xchangeResult.Id; apply the same null-check and error-handling change in the
NotifyResult code path where GetNativeHandler is used.
Comment on lines
+20
to
+36
| public HttpClient GetClient(string fullUrl) | ||
| { | ||
| var uri = new Uri(fullUrl); | ||
| string origin = $"{uri.Scheme}://{uri.Authority}"; | ||
|
|
||
| // Fast path: No lock, thread-safe read | ||
| var client = _cache.GetOrAdd(origin, key => { | ||
| var newClient = httpClientFactory.CreateClient(key); | ||
| newClient.BaseAddress = new Uri(key); | ||
| return newClient; | ||
| }); | ||
|
|
||
| // Notify background worker of usage (non-blocking) | ||
| _usageChannel.Writer.TryWrite(origin); | ||
|
|
||
| return client; | ||
| } |
There was a problem hiding this comment.
Thread-safety issue: Shared HttpClient with mutable DefaultRequestHeaders.
The HttpClient instances are cached by origin and shared across concurrent requests. However, consumers like NativeHttpHandler (lines 34-77 in that file) mutate client.DefaultRequestHeaders for authentication per-request. This creates a race condition where concurrent requests to the same origin will corrupt each other's headers.
🐛 Suggested approaches to fix
Option 1 (Recommended): Return a new HttpClient wrapper or use HttpRequestMessage headers instead of DefaultRequestHeaders in consumers:
In NativeHttpHandler.Handle, set headers on the HttpRequestMessage instead of HttpClient.DefaultRequestHeaders:
-if (_options.AuthType == "ApiKey")
- client.DefaultRequestHeaders.Add("ApiKey", _options.ApiKey);
+// Set auth headers on the request, not the shared client
+if (_options.AuthType == "ApiKey")
+ request.Headers.Add("ApiKey", _options.ApiKey);Option 2: Don't cache clients that need per-request configuration, or provide a factory method that returns unconfigured clients.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@SW.Bitween.NativeAdapters/HttpHandler/DynamicHttpProxy.cs` around lines 20 -
36, The cached HttpClient returned by GetClient is shared and its mutable
DefaultRequestHeaders are being modified per-request in
NativeHttpHandler.Handle, causing race conditions; fix by updating
NativeHttpHandler.Handle to stop mutating client.DefaultRequestHeaders and
instead set per-request headers on the HttpRequestMessage (create headers on the
request before sending), or alternatively change callers to obtain a fresh
unconfigured HttpClient from a factory when per-request DefaultRequestHeaders
are required; locate GetClient, _cache, httpClientFactory,
NativeHttpHandler.Handle and replace usages of DefaultRequestHeaders with
request.Headers on the HttpRequestMessage so shared clients remain immutable.
Comment on lines
+48
to
+61
| private void UpdateLru(string origin) | ||
| { | ||
| // Reorder list | ||
| _lruList.Remove(origin); | ||
| _lruList.AddFirst(origin); | ||
|
|
||
| // Prune if we went over capacity | ||
| while (_cache.Count > MaxCapacity) | ||
| { | ||
| var oldest = _lruList.Last.Value; | ||
| _lruList.RemoveLast(); | ||
| _cache.TryRemove(oldest, out _); | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential NullReferenceException if LRU list is empty.
If _cache.Count > MaxCapacity but _lruList is empty (possible during rapid concurrent additions before the background worker processes the channel), _lruList.Last will be null, causing a NullReferenceException at line 57.
🛡️ Proposed defensive fix
private void UpdateLru(string origin)
{
// Reorder list
_lruList.Remove(origin);
_lruList.AddFirst(origin);
// Prune if we went over capacity
- while (_cache.Count > MaxCapacity)
+ while (_cache.Count > MaxCapacity && _lruList.Last != null)
{
var oldest = _lruList.Last.Value;
_lruList.RemoveLast();
_cache.TryRemove(oldest, out _);
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@SW.Bitween.NativeAdapters/HttpHandler/DynamicHttpProxy.cs` around lines 48 -
61, UpdateLru can throw a NullReferenceException when pruning because
_lruList.Last may be null; guard the prune loop by checking that _lruList.Count
(or _lruList.Last) is > 0 before accessing Last/RemoveLast and only call
_cache.TryRemove when you successfully retrieved an oldest value; in other
words, inside UpdateLru ensure you check _lruList.Count > 0 (or _lruList.Last !=
null) before using _lruList.Last.Value and calling _lruList.RemoveLast(), and
break the while loop if the list is empty so _cache.TryRemove never receives a
null key.
| } | ||
|
|
||
|
|
||
| public string Name => "native.httpHandler"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the lookup inconsistency in NativeAdapterDiscoveryService
echo "=== Checking GetNativeHandler lookup (uses Name) ==="
rg -n "a\.Name\.Equals" --type cs -A 2 -B 2
echo ""
echo "=== Checking GetExpectedStartupValues lookup (uses Type.Name) ==="
rg -n "GetType\(\)\.Name\.Equals" --type cs -A 2 -B 2
echo ""
echo "=== Checking Name property implementations ==="
ast-grep --pattern 'public string Name => $_'Repository: simplify9/Bitween-api
Length of output: 2316
🏁 Script executed:
#!/bin/bash
# Find NativeAdapterDiscoveryService and examine both methods
echo "=== Finding NativeAdapterDiscoveryService ==="
fd "NativeAdapterDiscoveryService" --type f
echo ""
echo "=== Examining the file content ==="
cat -n SW.Bitween.NativeAdapters/Discovery/NativeAdapterDiscoveryService.cs | head -100Repository: simplify9/Bitween-api
Length of output: 321
🏁 Script executed:
#!/bin/bash
# Read the full NativeAdapterDiscoveryService file
cat -n SW.Bitween.Api/Services/NativeAdapterDiscoveryService.csRepository: simplify9/Bitween-api
Length of output: 6212
🏁 Script executed:
#!/bin/bash
# Find all calls to GetExpectedStartupValues to see what adapterId is passed
echo "=== Calls to GetExpectedStartupValues ==="
rg "GetExpectedStartupValues" --type cs -B 3 -A 3
echo ""
echo "=== Calls to GetNativeHandler ==="
rg "GetNativeHandler" --type cs -B 3 -A 3Repository: simplify9/Bitween-api
Length of output: 9845
🏁 Script executed:
#!/bin/bash
# Check if there are any other adapters with inconsistent names
echo "=== Finding all Name property implementations ==="
rg 'public string Name => ' --type cs -B 2 -A 1
echo ""
echo "=== Finding all adapter class names ==="
fd "Native.*\.cs" --type f | xargs grep -l "public string Name =>"Repository: simplify9/Bitween-api
Length of output: 1660
Inconsistent adapter lookup between handlers and startup values discovery.
GetNativeHandler (line 51 in NativeAdapterDiscoveryService) uses a.Name.Equals(adapterId, ...) to match adapters, while GetExpectedStartupValues (line 21) uses a.GetType().Name.Equals(adapterId, ...). Since NativeHttpHandler.Name returns "native.httpHandler" but its type name is "NativeHttpHandler", calls like GetExpectedStartupValues("native.httpHandler") will fail to find the adapter. The same mismatch affects GetNativeReceiver and GetNativeValidator (lines 63, 75), creating inconsistency across all adapter lookup methods.
Standardize all methods to use the same lookup strategy—either all use Name or all use GetType().Name.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@SW.Bitween.NativeAdapters/HttpHandler/NativeHttpHandler.cs` at line 157, The
lookup for adapters is inconsistent: NativeHttpHandler exposes Name
("native.httpHandler") but GetExpectedStartupValues uses GetType().Name
("NativeHttpHandler"), causing misses; update the discovery methods
GetExpectedStartupValues, GetNativeReceiver, and GetNativeValidator to match the
same strategy used by GetNativeHandler (use a.Name.Equals(adapterId,
StringComparison.OrdinalIgnoreCase) or the existing StringComparison used in
GetNativeHandler) so they compare the adapter's Name property instead of
GetType().Name, ensuring all lookups (including GetNativeHandler) use the
adapter Name consistently.
Comment on lines
+5
to
+35
| public static T ConvertTo<T>(this IDictionary<string,string> settings) | ||
| { | ||
| var inputInstance = Activator.CreateInstance(typeof(T)); | ||
|
|
||
| // Map dictionary properties to the input model | ||
| foreach (var prop in typeof(T).GetProperties()) | ||
| { | ||
| // Case-insensitive property lookup | ||
| var propEntry = settings.FirstOrDefault(p => | ||
| string.Equals(p.Key, prop.Name, StringComparison.OrdinalIgnoreCase)); | ||
|
|
||
| if (!string.IsNullOrEmpty(propEntry.Key)) | ||
| { | ||
| var value = propEntry.Value; | ||
| try | ||
| { | ||
| var convertedValue = Convert.ChangeType(value, | ||
| Nullable.GetUnderlyingType(prop.PropertyType) ?? prop.PropertyType); | ||
| prop.SetValue(inputInstance, convertedValue); | ||
| } | ||
| catch | ||
| { | ||
| // If conversion fails, set string value directly | ||
| if (prop.PropertyType == typeof(string)) | ||
| prop.SetValue(inputInstance, value); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return (T)inputInstance; | ||
| } |
There was a problem hiding this comment.
Potential NullReferenceException and swallowed exceptions.
Activator.CreateInstance(typeof(T))returnsobject?which can be null. The cast at line 34 will throw ifTis a value type with null input.- The catch-all at line 25 silently swallows exceptions, which could hide legitimate conversion failures beyond type mismatches.
🛡️ Proposed fix with null check and constrained generic
-public static T ConvertTo<T>(this IDictionary<string,string> settings)
+public static T ConvertTo<T>(this IDictionary<string,string> settings) where T : new()
{
- var inputInstance = Activator.CreateInstance(typeof(T));
+ var inputInstance = new T();
// Map dictionary properties to the input model
foreach (var prop in typeof(T).GetProperties())
{
// Case-insensitive property lookup
var propEntry = settings.FirstOrDefault(p =>
string.Equals(p.Key, prop.Name, StringComparison.OrdinalIgnoreCase));
if (!string.IsNullOrEmpty(propEntry.Key))
{
var value = propEntry.Value;
try
{
var convertedValue = Convert.ChangeType(value,
Nullable.GetUnderlyingType(prop.PropertyType) ?? prop.PropertyType);
prop.SetValue(inputInstance, convertedValue);
}
- catch
+ catch (Exception) when (prop.PropertyType == typeof(string))
{
// If conversion fails, set string value directly
- if (prop.PropertyType == typeof(string))
- prop.SetValue(inputInstance, value);
+ prop.SetValue(inputInstance, value);
}
}
}
- return (T)inputInstance;
+ return inputInstance;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public static T ConvertTo<T>(this IDictionary<string,string> settings) | |
| { | |
| var inputInstance = Activator.CreateInstance(typeof(T)); | |
| // Map dictionary properties to the input model | |
| foreach (var prop in typeof(T).GetProperties()) | |
| { | |
| // Case-insensitive property lookup | |
| var propEntry = settings.FirstOrDefault(p => | |
| string.Equals(p.Key, prop.Name, StringComparison.OrdinalIgnoreCase)); | |
| if (!string.IsNullOrEmpty(propEntry.Key)) | |
| { | |
| var value = propEntry.Value; | |
| try | |
| { | |
| var convertedValue = Convert.ChangeType(value, | |
| Nullable.GetUnderlyingType(prop.PropertyType) ?? prop.PropertyType); | |
| prop.SetValue(inputInstance, convertedValue); | |
| } | |
| catch | |
| { | |
| // If conversion fails, set string value directly | |
| if (prop.PropertyType == typeof(string)) | |
| prop.SetValue(inputInstance, value); | |
| } | |
| } | |
| } | |
| return (T)inputInstance; | |
| } | |
| public static T ConvertTo<T>(this IDictionary<string,string> settings) where T : new() | |
| { | |
| var inputInstance = new T(); | |
| // Map dictionary properties to the input model | |
| foreach (var prop in typeof(T).GetProperties()) | |
| { | |
| // Case-insensitive property lookup | |
| var propEntry = settings.FirstOrDefault(p => | |
| string.Equals(p.Key, prop.Name, StringComparison.OrdinalIgnoreCase)); | |
| if (!string.IsNullOrEmpty(propEntry.Key)) | |
| { | |
| var value = propEntry.Value; | |
| try | |
| { | |
| var convertedValue = Convert.ChangeType(value, | |
| Nullable.GetUnderlyingType(prop.PropertyType) ?? prop.PropertyType); | |
| prop.SetValue(inputInstance, convertedValue); | |
| } | |
| catch (Exception) when (prop.PropertyType == typeof(string)) | |
| { | |
| // If conversion fails, set string value directly | |
| prop.SetValue(inputInstance, value); | |
| } | |
| } | |
| } | |
| return inputInstance; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@SW.Bitween.NativeAdapters/ReflectionExtensions.cs` around lines 5 - 35,
ConvertTo<T> can produce a null/cast issue and currently swallows all
exceptions; constrain the generic and tighten error handling: add a where T :
new() constraint and instantiate via new T() instead of
Activator.CreateInstance(typeof(T)) to guarantee a non-null inputInstance;
replace the catch { } around Convert.ChangeType in ConvertTo<T> with specific
catches (e.g., FormatException, InvalidCastException, OverflowException) and
handle them explicitly—preserve the existing fallback of setting string
properties when prop.PropertyType == typeof(string), but for other failures
rethrow or surface the exception (include the caught Exception variable) rather
than silently swallowing it so callers can observe conversion errors.
mmalkhatib
approved these changes
Mar 1, 2026
This was referenced Mar 25, 2026
This was referenced Apr 5, 2026
Merged
This was referenced Jul 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
New Features
Refactor