Conversation
|
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
📝 WalkthroughWalkthroughThis PR adds hardware unit tracking across configuration, persistence, services, RabbitMQ messaging, HTTP APIs, security middleware, administrative UI, and queued location processing. It also introduces idempotent location writes, capability-path redaction, device and credential lifecycle management, and unit-location retry/dead-letter handling. ChangesUnit tracking contracts and storage
Tracking services and messaging
Web surfaces and processing
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| [ProducesResponseType(StatusCodes.Status400BadRequest)] | ||
| [ProducesResponseType(StatusCodes.Status403Forbidden)] | ||
| [Authorize(Policy = ResgridResources.Unit_Update)] | ||
| public async Task<ActionResult<UnitTrackingDeviceData>> CreateTracker( |
| [ProducesResponseType(StatusCodes.Status403Forbidden)] | ||
| [ProducesResponseType(StatusCodes.Status404NotFound)] | ||
| [Authorize(Policy = ResgridResources.Unit_Update)] | ||
| public async Task<ActionResult<UnitTrackingCredentialProvisionData>> CreateCredential( |
| [ProducesResponseType(StatusCodes.Status403Forbidden)] | ||
| [ProducesResponseType(StatusCodes.Status404NotFound)] | ||
| [Authorize(Policy = ResgridResources.Unit_Update)] | ||
| public async Task<ActionResult<UnitTrackingCredentialProvisionData>> RotateCredential( |
| [ProducesResponseType(StatusCodes.Status403Forbidden)] | ||
| [ProducesResponseType(StatusCodes.Status404NotFound)] | ||
| [Authorize(Policy = ResgridResources.Unit_Update)] | ||
| public async Task<ActionResult<UnitTrackingCredentialData>> RevokeCredential( |
| [ProducesResponseType(StatusCodes.Status403Forbidden)] | ||
| [ProducesResponseType(StatusCodes.Status404NotFound)] | ||
| [Authorize(Policy = ResgridResources.Unit_Update)] | ||
| public async Task<ActionResult<UnitTrackingDeviceData>> DisableTracker( |
| [ProducesResponseType(StatusCodes.Status403Forbidden)] | ||
| [ProducesResponseType(StatusCodes.Status404NotFound)] | ||
| [Authorize(Policy = ResgridResources.Unit_Update)] | ||
| public async Task<ActionResult<UnitTrackingDeviceData>> RebindTracker( |
| [ProducesResponseType(typeof(UnitTrackingIngressErrorResponse), StatusCodes.Status422UnprocessableEntity)] | ||
| [ProducesResponseType(StatusCodes.Status429TooManyRequests)] | ||
| [ProducesResponseType(StatusCodes.Status503ServiceUnavailable)] | ||
| public async Task<IActionResult> PostPositions(string unitTrackingDeviceId) |
| [ProducesResponseType(typeof(UnitTrackingIngressErrorResponse), StatusCodes.Status422UnprocessableEntity)] | ||
| [ProducesResponseType(StatusCodes.Status429TooManyRequests)] | ||
| [ProducesResponseType(StatusCodes.Status503ServiceUnavailable)] | ||
| public async Task<IActionResult> PostCapability(string capabilityToken) |
| !ClaimsAuthorizationHelper.IsUserDepartmentAdmin()) | ||
| return NotFound(); | ||
|
|
||
| if (model == null || string.IsNullOrWhiteSpace(model.UnitTrackingDeviceId)) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Core/Resgrid.Services/DepartmentSettingsService.cs (1)
48-69: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winCache invalidated before the write lands — reintroduces stale-cache window.
InvalidateSettingCacheAsyncruns beforeSaveOrUpdateAsyncpersists the change (Line 51 vs. Lines 60/65). A reader hitting the cache miss in that window re-populates it with the pre-update value via the fallback function, and since these settings useLongCacheLength(14 days), the stale value can survive for up to 14 days after the update. Invalidate after the write succeeds instead.🛠️ Proposed fix: invalidate after the write succeeds
public async Task<DepartmentSetting> SaveOrUpdateSettingAsync(int departmentId, string setting, DepartmentSettingTypes type, CancellationToken cancellationToken = default(CancellationToken)) { var savedSetting = await GetSettingByDepartmentIdType(departmentId, type); - await InvalidateSettingCacheAsync(departmentId, type); + DepartmentSetting result; if (savedSetting == null) { DepartmentSetting newSetting = new DepartmentSetting(); newSetting.DepartmentId = departmentId; newSetting.Setting = setting; newSetting.SettingType = (int)type; - return await _departmentSettingsRepository.SaveOrUpdateAsync(newSetting, cancellationToken); + result = await _departmentSettingsRepository.SaveOrUpdateAsync(newSetting, cancellationToken); } else { savedSetting.Setting = setting; - return await _departmentSettingsRepository.SaveOrUpdateAsync(savedSetting, cancellationToken); + result = await _departmentSettingsRepository.SaveOrUpdateAsync(savedSetting, cancellationToken); } - return null; + await InvalidateSettingCacheAsync(departmentId, type); + return result; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Core/Resgrid.Services/DepartmentSettingsService.cs` around lines 48 - 69, Update SaveOrUpdateSettingAsync so InvalidateSettingCacheAsync runs only after SaveOrUpdateAsync completes successfully for both the new-setting and existing-setting branches. Preserve the saved result, invalidate using departmentId and type after persistence, then return the result; remove the current pre-write invalidation.
🧹 Nitpick comments (21)
Web/Resgrid.Web/Areas/User/Models/UnitTracking/UnitTrackingViewModels.cs (1)
79-80: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
[EnumDataType]over hardcoded[Range(1,4)]for enum-backed field.Hardcoding the valid range ties validation to the current
UnitTrackingAuthModemember count/numbering; it will silently misvalidate if the enum changes.♻️ Suggested fix
- [Range(1, 4)] + [EnumDataType(typeof(UnitTrackingAuthMode))] public int AuthMode { get; set; } = (int)UnitTrackingAuthMode.Bearer;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Web/Resgrid.Web/Areas/User/Models/UnitTracking/UnitTrackingViewModels.cs` around lines 79 - 80, Replace the hardcoded Range validation on AuthMode with EnumDataType targeting UnitTrackingAuthMode, preserving the existing Bearer default and enum-backed validation behavior.Web/Resgrid.Web/Areas/User/Views/UnitTracking/Details.cshtml (1)
169-169: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded adapter key literal in view.
"resgrid-json-v1"is duplicated as a magic string here rather than referenced from a shared constant (e.g., onUnitTrackingCatalogProfileor a dedicated constants class), risking silent drift if the key changes elsewhere.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Web/Resgrid.Web/Areas/User/Views/UnitTracking/Details.cshtml` at line 169, Replace the hardcoded "resgrid-json-v1" comparison in the Details view with the shared constant exposed by UnitTrackingCatalogProfile or the established adapter-key constants class. Preserve the existing case-insensitive comparison and preview condition while ensuring the value comes from the single canonical definition.Web/Resgrid.Web/Areas/User/Views/UnitTracking/_Editor.cshtml (1)
19-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
data-identifier-requiredattribute is seeded but unused client-side.The profile
<option>carriesdata-identifier-required, butupdateProfileHelp()never reads it, so the UI never surfaces the identifier requirement before submit even though the server enforces it (ValidateProfilein the controller). Consider wiring this up to toggle a "required" hint/marker on the Device Identifier field when the selected profile requires one, or remove the unused attribute.Also applies to: 79-92
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Web/Resgrid.Web/Areas/User/Views/UnitTracking/_Editor.cshtml` around lines 19 - 30, The profile option’s data-identifier-required attribute is unused by updateProfileHelp(), so the UI does not reflect the server-side requirement. Update updateProfileHelp() and the Device Identifier field markup to read the selected option’s flag and show or hide an appropriate required hint/marker; preserve the existing behavior when the profile does not require an identifier.Web/Resgrid.Web/Areas/User/Controllers/UnitTrackingController.cs (2)
489-551: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftPreview validation duplicates (and may diverge from) the production JSON parser.
ValidatePreviewJsonre-implements structural checks (size/depth limits, duplicate-key handling, positions array shape, per-position field validation) independently of the ingestion-side parser (UnitTrackingJsonPayloadParser, per the PR stack outline). If the two diverge, this preview tool could report success for payloads the real ingestion endpoint would reject, or vice versa, misleading admins testing device setups.Consider extracting/reusing the shared parsing logic (e.g., expose a
TryParse/Validatemethod on the ingress parser) so both paths stay in sync.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Web/Resgrid.Web/Areas/User/Controllers/UnitTrackingController.cs` around lines 489 - 551, The ValidatePreviewJson method duplicates ingestion validation; expose and reuse a shared TryParse or Validate entry point from UnitTrackingJsonPayloadParser for preview requests. Move or centralize size, depth, duplicate-key, positions-shape, and coordinate checks there, then have ValidatePreviewJson consume its result and preserve the existing error key and position-count behavior.
609-647: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated field-mapping between
BuildDeviceandApplyUpdate.Both methods map the same set of profile/model fields onto a
UnitTrackingDevice; consider a single mapper that both call (withIsEnabledsupplied by the caller) to avoid drift when new fields are added.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Web/Resgrid.Web/Areas/User/Controllers/UnitTrackingController.cs` around lines 609 - 647, Consolidate the duplicated field assignments in BuildDevice and ApplyUpdate into one shared UnitTrackingDevice mapper, accepting IsEnabled from each caller so creation remains enabled while updates use model.IsEnabled. Have both methods reuse this mapper and preserve all existing model/profile field mappings.Web/Resgrid.Web.Services/Models/v4/UnitTracking/UnitTrackingAdminModels.cs (1)
132-133: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
[Range(1, 4)]duplicates enum knowledge.
UnitTrackingDevicesController.CreateCredentialalready validates withEnum.IsDefinedplus the profile'sSupportedAuthModes. Hardcoding the upper bound means the nextUnitTrackingAuthModevalue is rejected with a 400 that no one will trace back to this attribute.♻️ Rely on the enum-based validation already in the controller
- [Range(1, 4)] public int AuthMode { get; set; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Web/Resgrid.Web.Services/Models/v4/UnitTracking/UnitTrackingAdminModels.cs` around lines 132 - 133, Remove the hardcoded [Range(1, 4)] validation from the AuthMode property in the unit-tracking admin model, leaving validation to UnitTrackingDevicesController.CreateCredential’s Enum.IsDefined and SupportedAuthModes checks so future enum values are accepted when supported.Web/Resgrid.Web.Services/ApplicationCore/UnitTracking/UnitTrackingHttpAuthenticationService.cs (1)
84-102: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUnsupported
Authorizationscheme short-circuits custom-header credentials.If any
Authorizationheader is present with a scheme other than Bearer/Basic (some trackers or intermediary proxies inject one), Line 101 returnsnulland the CustomHeader credential path is never evaluated, so an otherwise valid custom-header device is rejected. Consider falling through to the custom-header lookup instead of returning early.♻️ Fall through to custom-header lookup
if (authorization.StartsWith("Basic ", StringComparison.OrdinalIgnoreCase)) return ParseBasic(authorization.Substring("Basic ".Length).Trim()); - - return null; }Note: with this change the
MaximumAuthorizationHeaderLengthearly return should also fall through rather than reject outright, or be kept as-is if strict rejection of oversized headers is intended.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Web/Resgrid.Web.Services/ApplicationCore/UnitTracking/UnitTrackingHttpAuthenticationService.cs` around lines 84 - 102, Update the Authorization parsing branch in the authentication method so unsupported schemes fall through to the existing custom-header credential lookup instead of returning null. Apply the same fall-through behavior to the MaximumAuthorizationHeaderLength check unless strict rejection of oversized headers is explicitly required; preserve the Bearer, Basic, and empty-token handling.Web/Resgrid.Web.Services/Controllers/v4/UnitTrackingIngressController.cs (1)
30-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the repository’s required dependency-resolution pattern.
This new controller constructor injects four services directly. Use
Bootstrapper.GetKernel().Resolve<T>()explicitly instead, per the repository rule.As per coding guidelines, “Use
Service Locatorpattern viaBootstrapper.GetKernel().Resolve<T>()to resolve dependencies explicitly in constructors, rather than constructor injection.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Web/Resgrid.Web.Services/Controllers/v4/UnitTrackingIngressController.cs` around lines 30 - 40, Update the UnitTrackingIngressController constructor to remove the four service parameters and resolve UnitTrackingHttpAuthenticationService, UnitTrackingJsonPayloadParser, UnitTrackingRateLimiter, and IUnitTrackingIngressService through Bootstrapper.GetKernel().Resolve<T>(), assigning each result to the corresponding fields.Source: Coding guidelines
Core/Resgrid.Model/Services/IUnitTrackingCatalogService.cs (1)
10-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDo not expose mutable singleton catalog entries.
UnitTrackingCatalogServicereturns its staticProfilesinstances directly, whileUnitTrackingCatalogProfileis initialized through mutable properties. A consumer can alter shared profile state, including selection/auth configuration, for subsequent requests. Return immutable profiles or defensive copies.As per coding guidelines, “Prefer functional patterns and immutable data where appropriate in C#.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Core/Resgrid.Model/Services/IUnitTrackingCatalogService.cs` around lines 10 - 15, Update UnitTrackingCatalogService methods GetProfilesAsync and GetProfileAsync so they never return the shared static Profiles instances directly. Return immutable profile values or defensive copies with independently owned mutable properties, preserving the existing profile data and lookup behavior while preventing consumers from modifying catalog state across requests.Source: Coding guidelines
Core/Resgrid.Services/UnitLocationSourceResolver.cs (1)
10-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffNaming deviates from the service convention for this project.
IUnitLocationSourceResolver/UnitLocationSourceResolverlives inCore/Resgrid.Servicesand is registered like other services, but does not use theI{Name}Service/{Name}Servicepair (e.g.IUnitLocationSourceService). Low priority given the DI/registration churn a rename implies.As per coding guidelines, "Service interface names must follow the pattern
I{Name}Serviceand implementations must be named{Name}Service".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Core/Resgrid.Services/UnitLocationSourceResolver.cs` around lines 10 - 17, Rename the resolver symbols to follow the service convention: use IUnitLocationSourceService and UnitLocationSourceService instead of IUnitLocationSourceResolver and UnitLocationSourceResolver. Update the constructor, implemented interface, dependency-injection registration, and all references consistently while preserving behavior.Source: Coding guidelines
Core/Resgrid.Services/UnitTrackingAuthenticationService.cs (2)
231-247: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCollapse the duplicated validity predicate.
The credential window check in the
UnitTrackingAuthenticationResultoverload is identical to theUnitTrackingCredentialoverload; delegate to avoid the two drifting.♻️ Proposed refactor
private static bool IsActive(UnitTrackingAuthenticationResult result, DateTime utcNow) { if (result?.Credential == null || !IsEnabled(result.Device)) return false; - return result.Credential.ValidFrom <= utcNow && - !result.Credential.RevokedOn.HasValue && - (!result.Credential.ExpiresOn.HasValue || result.Credential.ExpiresOn > utcNow); + return IsActive(result.Credential, utcNow); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Core/Resgrid.Services/UnitTrackingAuthenticationService.cs` around lines 231 - 247, Update the UnitTrackingAuthenticationResult overload of IsActive to retain its null-result and device-enabled checks, then delegate credential validity evaluation to the existing IsActive(UnitTrackingCredential, DateTime) overload instead of duplicating the validity predicate.
252-272: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated credential-sanitization/key-normalization helpers across both tracking services.
SanitizeCredentialandNormalizeKeyare copied verbatim into two files; the shared root cause is the absence of one shared helper, so any futureUnitTrackingCredentialfield addition must be mirrored in both copies orSecretHashleaks / data is dropped from whichever copy is missed.
Core/Resgrid.Services/UnitTrackingAuthenticationService.cs#L252-L272: extractSanitizeCredentialandNormalizeKeyinto a single shared internal static helper inCore/Resgrid.Servicesand call it here.Core/Resgrid.Services/UnitTrackingService.cs#L560-L577: delete the localSanitizeCredential(andNormalizeKeyat L707-L708) and call the shared helper instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Core/Resgrid.Services/UnitTrackingAuthenticationService.cs` around lines 252 - 272, Extract the duplicated SanitizeCredential and NormalizeKey logic into one shared internal static helper in Core/Resgrid.Services, then update Core/Resgrid.Services/UnitTrackingAuthenticationService.cs lines 252-272 to call it. Remove the local SanitizeCredential and NormalizeKey implementations from Core/Resgrid.Services/UnitTrackingService.cs lines 560-577 and 707-708, replacing their usages with the shared helper while preserving the existing sanitization and normalization behavior.Core/Resgrid.Model/Services/IUnitTrackingAuthenticationService.cs (1)
24-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a
bypassCacheparameter and consistentCancellationTokensurface.
GetEnabledDeviceByEndpointIdAsync,GetEnabledDeviceByProtocolIdentifierAsync, andGetActiveCredentialsForDeviceAsyncare cache-backed inUnitTrackingAuthenticationService, but callers can only get fresh data by calling theInvalidate*methods. AddingbypassCache = falsematches the service convention used elsewhere. Also,InvalidateCredentialAsync/InvalidateDeviceAsyncare the only async members without aCancellationToken.As per coding guidelines, "Service methods should include a
bypassCacheparameter (default:false) to allow callers to skip cache retrieval when fresh data is required".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Core/Resgrid.Model/Services/IUnitTrackingAuthenticationService.cs` around lines 24 - 29, Update the cache-backed methods GetEnabledDeviceByEndpointIdAsync, GetEnabledDeviceByProtocolIdentifierAsync, and GetActiveCredentialsForDeviceAsync to accept an optional bypassCache parameter defaulting to false, and propagate it through their implementations and cache lookup logic. Add optional CancellationToken parameters defaulting to default to InvalidateCredentialAsync and InvalidateDeviceAsync, updating implementations and call sites to preserve cancellation support.Source: Coding guidelines
Providers/Resgrid.Providers.Bus/EventAggregator.cs (1)
26-32: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAsync dispatch keys on the static type, unlike the sync path.
typeof(TMessage)resolves at compile time, so a caller holding the event in a base-class/interface/object-typed variable silently matches no listeners and the message is dropped without any signal._hub.Publish(sync path) dispatches on the runtime type, so the two APIs behave differently for the same call shape. Keying onmessage?.GetType()(with a null guard) removes the trap.♻️ Proposed change
public async Task SendMessageAsync<TMessage>(TMessage message) { - if (!_asyncListeners.TryGetValue(typeof(TMessage), out var listeners)) + if (message == null) + throw new ArgumentNullException(nameof(message)); + + if (!_asyncListeners.TryGetValue(message.GetType(), out var listeners)) return; await Task.WhenAll(listeners.Values.Select(listener => listener(message))); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Providers/Resgrid.Providers.Bus/EventAggregator.cs` around lines 26 - 32, Update SendMessageAsync<TMessage> to resolve the listener key from message.GetType() at runtime, with an appropriate null guard, instead of typeof(TMessage). Preserve the existing no-listener return and asynchronous listener dispatch behavior so async dispatch matches the sync path for base-class, interface, and object-typed references.Providers/Resgrid.Providers.Bus.Rabbit/RabbitOutboundQueueProvider.cs (1)
252-268: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBatch-wide publish timeout can leave the batch half-published.
publishTimeoutis created once andCancelAfterapplies to the whole loop, so with a large collection the laterBasicPublishAsynccalls can be cancelled after earlier ones were already confirmed. The method then returnsfalse, and the caller has no way to know how many messages landed — a retry re-publishes the entire batch. A per-message timeout (or returning the count/failure index) makes the contract honest.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Providers/Resgrid.Providers.Bus.Rabbit/RabbitOutboundQueueProvider.cs` around lines 252 - 268, Change the publish loop in the outbound batch method so each message gets its own timeout token and timeout window when calling BasicPublishAsync, preventing one shared publishTimeout from expiring across the entire batch. Preserve the existing success return and cancellation behavior while ensuring a timeout applies only to the current message.Providers/Resgrid.Providers.Bus.Rabbit/RabbitInboundQueueProvider.cs (3)
727-748: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate header-parsing switch.
This is the same numeric-header parse already inlined in
RetryQueueItem(Lines 759-778), now with clamping added — the two copies have already diverged. Extracting a singleTryGetIntHeader(headers, name)helper and using it from both keeps the semantics aligned.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Providers/Resgrid.Providers.Bus.Rabbit/RabbitInboundQueueProvider.cs` around lines 727 - 748, Replace the duplicated numeric header parsing in GetRetryCount and RetryQueueItem with a shared TryGetIntHeader(headers, name) helper. Move the existing type handling and non-negative/int-range clamping into that helper, then have both callers use it while preserving a zero result for missing or unparseable headers.
663-671: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPermanent and transient failures are handled identically. The worker signals unrecoverable payload problems (missing
UnitId/DepartmentId, absent coordinates) with the same plain exceptions used for transient store/broker failures, and the consumer's catch cannot tell them apart — so a message that can never succeed still consumes the fullUnitLocationMaxRetryAttemptsbudget, and requeues in a tight loop whenever the republish itself fails.
Providers/Resgrid.Providers.Bus.Rabbit/RabbitInboundQueueProvider.cs#L663-L671: catch a dedicated non-retryable exception type separately andBasicNackAsync(..., requeue: false)(or publish straight to the dead-letter queue) instead of the retry path.Workers/Resgrid.Workers.Framework/Logic/UnitLocationQueueLogic.cs#L15-L28: throw a distinct non-retryable exception type from the validation guards so the consumer can classify them.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Providers/Resgrid.Providers.Bus.Rabbit/RabbitInboundQueueProvider.cs` around lines 663 - 671, The unit-location consumer must distinguish unrecoverable validation failures from retryable exceptions. In Providers/Resgrid.Providers.Bus.Rabbit/RabbitInboundQueueProvider.cs lines 663-671, catch the dedicated non-retryable exception separately and acknowledge it with BasicNackAsync using requeue false, while preserving RetryOrDeadLetterUnitLocationAsync for other exceptions; in Workers/Resgrid.Workers.Framework/Logic/UnitLocationQueueLogic.cs lines 15-28, update the UnitId, DepartmentId, and coordinate validation guards to throw that distinct non-retryable exception type.
688-716: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReuse a confirm channel for retry publisher
RetryOrDeadLetterUnitLocationAsynccreates a new channel for every failed delivery, andRabbitConnection.CreateConnectioncurrently creates a new shared connection on failed initialization paths. Under failure bursts this generates one channel open/close per message, increasing broker-handshake load and exhausting connection channel capacity. Keep the retry publisher as a single lazy/channel-recoverable channel instead of per-message churn.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Providers/Resgrid.Providers.Bus.Rabbit/RabbitInboundQueueProvider.cs` around lines 688 - 716, Update RetryOrDeadLetterUnitLocationAsync to reuse a single lazily initialized confirm channel for retry publishing instead of creating and disposing a channel per delivery. Back the channel with a shared connection, recover or recreate it when closed or publishing fails, and preserve the existing publish options, timeout, and retry behavior.Core/Resgrid.Services/UnitsService.cs (2)
545-565: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
cancellationTokenis now unused, and the Mongo new-vs-existing check is obscure.Neither
InsertAsync/UpdateAsyncnorSendMessageAsyncreceive the token, so callers passing one (e.g.UnitLocationQueueLogic) get no cancellation. Alsolocation.Id.Timestamp == 0is an indirect way of asking "is this ObjectId unset" —location.Id == ObjectId.Emptystates the intent directly.♻️ Proposed change
else { - if (location.Id.Timestamp == 0) - result = await _unitLocationsMongoRepository.Value.InsertAsync(location); + if (location.Id == ObjectId.Empty) + result = await _unitLocationsMongoRepository.Value.InsertAsync(location, cancellationToken); else - result = await _unitLocationsMongoRepository.Value.UpdateAsync(location); + result = await _unitLocationsMongoRepository.Value.UpdateAsync(location, cancellationToken); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Core/Resgrid.Services/UnitsService.cs` around lines 545 - 565, Update AddUnitLocationAsync to propagate cancellationToken through the relevant repository and SendMessageAsync calls so caller cancellation is honored, including the UnitLocationQueueLogic path. Replace the Mongo insert/update condition based on location.Id.Timestamp with a direct comparison against ObjectId.Empty, preserving the existing insert behavior for unset IDs and update behavior for existing IDs.
25-25: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsolidate the overlapping unit-location read/write repositories
SaveUnitLocationAsyncnow uses_unitLocationsMongoRepositoryfor writes, butGetLatestUnitLocationAsyncandGetLatestUnitLocationsAsyncstill rely on_unitLocationRepositoryfor MongoDB reads. Move the read paths under_unitLocationsMongoRepositoryor expose equivalent read methods on that abstraction so the same data source is not split between two abstractions.This also increases
UnitsServiceto 17 constructor parameters; per the C# guidelines, prefer resolving dependencies explicitly viaBootstrapper.GetKernel().Resolve<T>()rather than constructor injection.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Core/Resgrid.Services/UnitsService.cs` at line 25, Consolidate unit-location access in UnitsService by updating GetLatestUnitLocationAsync and GetLatestUnitLocationsAsync to use _unitLocationsMongoRepository, adding equivalent read methods there if needed, while preserving their current behavior. Remove the redundant _unitLocationRepository dependency and resolve any required repository explicitly through Bootstrapper.GetKernel().Resolve<T>() instead of increasing constructor injection.Source: Coding guidelines
Providers/Resgrid.Providers.Bus/OutboundEventProvider.cs (1)
61-61: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winThrowing here fails the caller's persistence path, not just the broadcast.
EventAggregator.SendMessageAsyncpropagates listener exceptions, and the only caller isUnitsService.AddUnitLocationAsync(Line 569), which invokes it after the location row is already written. So a transient topic-publish hiccup makesAddUnitLocationAsyncthrow, which makesUnitLocationQueueLogic.ProcessUnitLocationQueueItemthrow, which drives the whole message back through the retry/dead-letter pipeline and re-persists the location.If that at-least-once behavior is intentional it's workable given idempotent writes, but consider surfacing it distinctly (e.g. log + a dedicated broadcast-retry) so a realtime failure doesn't re-drive persistence and doesn't consume the location message's retry budget.
Also applies to: 613-620
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Providers/Resgrid.Providers.Bus/OutboundEventProvider.cs` at line 61, Update the outbound broadcast handling around the event listeners at the referenced locations so EventAggregator.SendMessageAsync failures are caught and handled separately from the persistence call. Log the broadcast failure and route it through a dedicated broadcast retry mechanism, ensuring UnitsService.AddUnitLocationAsync and UnitLocationQueueLogic.ProcessUnitLocationQueueItem do not rethrow realtime delivery errors or consume the location message retry budget.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Core/Resgrid.Config/ServiceBusConfig.cs`:
- Around line 53-55: Update the UnitLocationQueueV2Name,
UnitLocationRetryQueueV2Name, and UnitLocationDeadQueueV2Name definitions in
ServiceBusConfig to follow the existing `#if` DEBUG queue-name split, applying the
established test suffix or equivalent DEBUG-only isolation while preserving the
current release names.
In `@Core/Resgrid.Framework/SentryTransactionFilter.cs`:
- Around line 64-70: Update the transaction filtering logic around
RedactCapabilityPath so transaction.Name is also passed through the same
redaction before the status check. Preserve the existing Request.Url redaction
and ensure both fields are scrubbed, including 404 transactions whose names
derive from the raw request path.
In `@Core/Resgrid.Services/UnitTrackingAuthenticationService.cs`:
- Around line 39-45: Update the token generation logic around
RandomNumberGenerator and keyPrefix so the stored/displayed prefix is generated
from separate random bytes rather than derived from encodedSecret. Encode the
independent prefix with the same base64url-safe normalization, then prepend it
to the token while preserving the existing rgtrk token format and full secret
entropy.
- Around line 195-205: Update the cached result handling in the credential
retrieval flow before the `cached.Where(...)` call to coalesce a null provider
result to an empty credential collection. Preserve the existing filtering and
list conversion for non-null results.
In `@Core/Resgrid.Services/UnitTrackingService.cs`:
- Around line 135-136: Update the edit flow containing the existing.IsEnabled
and device.IsEnabled check so requested field changes are applied before
handling the disable transition; do not return early to DisableDeviceAsync
before updating DisplayName, keys, identifiers, SourcePriority,
AllowedSourceCidrs, and FirmwareVersion. Preserve the disable behavior after the
updates, and expose the credential-revocation consequence through the existing
caller/UI notification or response mechanism.
- Line 102: Validate device.AllowedSourceCidrs before persisting it in the
device create/update flow, rather than only applying TrimToNull. Parse each
nonblank CIDR entry with TryParse, enforce valid prefix bounds, and require
canonical entries; reject malformed values before saving while preserving the
network policy’s blank-value behavior.
- Around line 681-697: Update IsHttpTokenCharacter to accept only ASCII letters
and digits, while retaining the existing RFC 9110 tchar punctuation set; replace
the Unicode-permissive char.IsLetterOrDigit check so CustomHeader validation in
the surrounding authentication configuration rejects characters such as é and
non-ASCII digits.
- Around line 633-660: Update CopyForRebind to preserve the source device’s
enabled state instead of hardcoding IsEnabled to true, and derive LastStatus
from that state using the same behavior as CreateDeviceAsync. Keep all other
copied properties unchanged.
In `@Providers/Resgrid.Providers.Bus.Rabbit/RabbitConnection.cs`:
- Around line 142-186: Isolate the UnitLocation V2 queue declaration and binding
block from the main setup flow by moving it after the unrelated queue
declarations and/or extracting it into a dedicated helper such as
DeclareUnitLocationV2QueuesAsync. Catch and log failures within that isolated
flow so a PRECONDITION_FAILED from the TTL arguments does not abort declarations
for PersonnelLoactionQueueName, EventingTopic, SecurityRefreshQueueName,
WorkflowQueueName, or ChatbotProcessingQueueName.
In `@Providers/Resgrid.Providers.Bus.Rabbit/RabbitOutboundQueueProvider.cs`:
- Around line 188-194: Replace ASCII encoding with UTF-8 in both unit-location
publish paths: SendMessage at
Providers/Resgrid.Providers.Bus.Rabbit/RabbitOutboundQueueProvider.cs lines
188-194 and SendMessagesWithConfirmation at lines 259-265. Update each message
body encoding while preserving the existing publish behavior.
In `@Providers/Resgrid.Providers.Migrations/Migrations/M0101_AddUnitTracking.cs`:
- Around line 42-48: Replace the separate department and unit foreign keys in
M0101_AddUnitTracking.cs and M0101_AddUnitTrackingPg.cs at lines 42-48 with a
composite foreign key linking UnitTrackingDevices.(DepartmentId, UnitId) to the
matching department-owned unit key (DepartmentId, UnitId), preserving the
existing table and constraint intent in both migrations.
In
`@Repositories/Resgrid.Repositories.DataRepository/UnitTrackingCredentialsRepository.cs`:
- Around line 22-32: Replace constructor-injected collaborator parameters with
explicit Bootstrapper.GetKernel().Resolve<T>() calls in
UnitTrackingCredentialsRepository.cs (lines 22-32) and
UnitTrackingDevicesRepository.cs (lines 22-32), assigning each resolved
dependency to the existing fields. In DocumentDbRepository.cs (lines 13-18),
resolve the Mongo repository through the same service-locator pattern while
preserving its deferred initialization behavior.
In
`@Repositories/Resgrid.Repositories.NoSqlRepository/UnitLocationsMongoRepository.cs`:
- Around line 57-62: Update EnsureIndexesAsync and the cached _ensureIndexesTask
flow so a failed CreateIndexesAsync operation clears the cached task after its
await fails, allowing the next call to retry index initialization. Preserve the
existing lock and successful-task caching behavior.
In
`@Web/Resgrid.Web.Services/ApplicationCore/UnitTracking/UnitTrackingJsonPayloadParser.cs`:
- Around line 232-238: Update ReadBoundedBodyAsync so the chunk allocation no
longer computes maximumBytes + 1; size the chunk using the bounded maximum
directly while retaining the existing total-byte limit enforcement.
In `@Web/Resgrid.Web.Services/Controllers/v4/UnitLocationController.cs`:
- Around line 103-104: Update the enqueue flow in EnqueueUnitLocationEventAsync
handling within the controller action so exceptions from queue submission are
caught and mapped to 503 ServiceUnavailable, matching the existing false-result
path. Ensure this handling occurs before the outer catch that returns 400, while
preserving the current response behavior for other failures.
In `@Web/Resgrid.Web.Services/Models/v4/UnitTracking/UnitTrackingAdminModels.cs`:
- Around line 98-99: Validate AllowedSourceCidrs at the API model boundary in
the relevant model properties, rejecting malformed CIDR entries such as invalid
prefixes or incomplete addresses instead of relying on
UnitTrackingNetworkPolicy.Contains. Preserve valid CIDR-list input and return a
clear validation error for any invalid entry, while retaining the existing
MaxLength constraint.
In `@Web/Resgrid.Web/Areas/User/Views/UnitTracking/Details.cshtml`:
- Around line 137-154: Associate the Create Credential labels with their
controls by updating the labels in the authentication mode, custom header, and
basic username groups to use the corresponding asp-for bindings or matching for
attributes. Ensure they target CredentialInput.AuthMode,
CredentialInput.HeaderName, and CredentialInput.BasicUsername, while preserving
the existing label styling and text.
In `@Workers/Resgrid.Workers.Framework/Logic/UnitLocationQueueLogic.cs`:
- Around line 24-25: Update the UnitLocationQueueLogic handling so an unknown
IsValidFix value remains null rather than being coerced to true when persisted
to UnitsLocation. Preserve the existing invalid-fix early return, but add the
appropriate operational logging or metric before returning success so dropped
invalid fixes are observable.
- Around line 15-28: Restore try-catch handling around the validation logic in
the relevant method, retain the existing exception propagation for rejected
events, and call Resgrid.Framework.Logging.LogException() in the catch before
rethrowing. Re-add the Resgrid.Framework import required for the static logging
API, while preserving the current validation and retry/nack behavior.
---
Outside diff comments:
In `@Core/Resgrid.Services/DepartmentSettingsService.cs`:
- Around line 48-69: Update SaveOrUpdateSettingAsync so
InvalidateSettingCacheAsync runs only after SaveOrUpdateAsync completes
successfully for both the new-setting and existing-setting branches. Preserve
the saved result, invalidate using departmentId and type after persistence, then
return the result; remove the current pre-write invalidation.
---
Nitpick comments:
In `@Core/Resgrid.Model/Services/IUnitTrackingAuthenticationService.cs`:
- Around line 24-29: Update the cache-backed methods
GetEnabledDeviceByEndpointIdAsync, GetEnabledDeviceByProtocolIdentifierAsync,
and GetActiveCredentialsForDeviceAsync to accept an optional bypassCache
parameter defaulting to false, and propagate it through their implementations
and cache lookup logic. Add optional CancellationToken parameters defaulting to
default to InvalidateCredentialAsync and InvalidateDeviceAsync, updating
implementations and call sites to preserve cancellation support.
In `@Core/Resgrid.Model/Services/IUnitTrackingCatalogService.cs`:
- Around line 10-15: Update UnitTrackingCatalogService methods GetProfilesAsync
and GetProfileAsync so they never return the shared static Profiles instances
directly. Return immutable profile values or defensive copies with independently
owned mutable properties, preserving the existing profile data and lookup
behavior while preventing consumers from modifying catalog state across
requests.
In `@Core/Resgrid.Services/UnitLocationSourceResolver.cs`:
- Around line 10-17: Rename the resolver symbols to follow the service
convention: use IUnitLocationSourceService and UnitLocationSourceService instead
of IUnitLocationSourceResolver and UnitLocationSourceResolver. Update the
constructor, implemented interface, dependency-injection registration, and all
references consistently while preserving behavior.
In `@Core/Resgrid.Services/UnitsService.cs`:
- Around line 545-565: Update AddUnitLocationAsync to propagate
cancellationToken through the relevant repository and SendMessageAsync calls so
caller cancellation is honored, including the UnitLocationQueueLogic path.
Replace the Mongo insert/update condition based on location.Id.Timestamp with a
direct comparison against ObjectId.Empty, preserving the existing insert
behavior for unset IDs and update behavior for existing IDs.
- Line 25: Consolidate unit-location access in UnitsService by updating
GetLatestUnitLocationAsync and GetLatestUnitLocationsAsync to use
_unitLocationsMongoRepository, adding equivalent read methods there if needed,
while preserving their current behavior. Remove the redundant
_unitLocationRepository dependency and resolve any required repository
explicitly through Bootstrapper.GetKernel().Resolve<T>() instead of increasing
constructor injection.
In `@Core/Resgrid.Services/UnitTrackingAuthenticationService.cs`:
- Around line 231-247: Update the UnitTrackingAuthenticationResult overload of
IsActive to retain its null-result and device-enabled checks, then delegate
credential validity evaluation to the existing IsActive(UnitTrackingCredential,
DateTime) overload instead of duplicating the validity predicate.
- Around line 252-272: Extract the duplicated SanitizeCredential and
NormalizeKey logic into one shared internal static helper in
Core/Resgrid.Services, then update
Core/Resgrid.Services/UnitTrackingAuthenticationService.cs lines 252-272 to call
it. Remove the local SanitizeCredential and NormalizeKey implementations from
Core/Resgrid.Services/UnitTrackingService.cs lines 560-577 and 707-708,
replacing their usages with the shared helper while preserving the existing
sanitization and normalization behavior.
In `@Providers/Resgrid.Providers.Bus.Rabbit/RabbitInboundQueueProvider.cs`:
- Around line 727-748: Replace the duplicated numeric header parsing in
GetRetryCount and RetryQueueItem with a shared TryGetIntHeader(headers, name)
helper. Move the existing type handling and non-negative/int-range clamping into
that helper, then have both callers use it while preserving a zero result for
missing or unparseable headers.
- Around line 663-671: The unit-location consumer must distinguish unrecoverable
validation failures from retryable exceptions. In
Providers/Resgrid.Providers.Bus.Rabbit/RabbitInboundQueueProvider.cs lines
663-671, catch the dedicated non-retryable exception separately and acknowledge
it with BasicNackAsync using requeue false, while preserving
RetryOrDeadLetterUnitLocationAsync for other exceptions; in
Workers/Resgrid.Workers.Framework/Logic/UnitLocationQueueLogic.cs lines 15-28,
update the UnitId, DepartmentId, and coordinate validation guards to throw that
distinct non-retryable exception type.
- Around line 688-716: Update RetryOrDeadLetterUnitLocationAsync to reuse a
single lazily initialized confirm channel for retry publishing instead of
creating and disposing a channel per delivery. Back the channel with a shared
connection, recover or recreate it when closed or publishing fails, and preserve
the existing publish options, timeout, and retry behavior.
In `@Providers/Resgrid.Providers.Bus.Rabbit/RabbitOutboundQueueProvider.cs`:
- Around line 252-268: Change the publish loop in the outbound batch method so
each message gets its own timeout token and timeout window when calling
BasicPublishAsync, preventing one shared publishTimeout from expiring across the
entire batch. Preserve the existing success return and cancellation behavior
while ensuring a timeout applies only to the current message.
In `@Providers/Resgrid.Providers.Bus/EventAggregator.cs`:
- Around line 26-32: Update SendMessageAsync<TMessage> to resolve the listener
key from message.GetType() at runtime, with an appropriate null guard, instead
of typeof(TMessage). Preserve the existing no-listener return and asynchronous
listener dispatch behavior so async dispatch matches the sync path for
base-class, interface, and object-typed references.
In `@Providers/Resgrid.Providers.Bus/OutboundEventProvider.cs`:
- Line 61: Update the outbound broadcast handling around the event listeners at
the referenced locations so EventAggregator.SendMessageAsync failures are caught
and handled separately from the persistence call. Log the broadcast failure and
route it through a dedicated broadcast retry mechanism, ensuring
UnitsService.AddUnitLocationAsync and
UnitLocationQueueLogic.ProcessUnitLocationQueueItem do not rethrow realtime
delivery errors or consume the location message retry budget.
In
`@Web/Resgrid.Web.Services/ApplicationCore/UnitTracking/UnitTrackingHttpAuthenticationService.cs`:
- Around line 84-102: Update the Authorization parsing branch in the
authentication method so unsupported schemes fall through to the existing
custom-header credential lookup instead of returning null. Apply the same
fall-through behavior to the MaximumAuthorizationHeaderLength check unless
strict rejection of oversized headers is explicitly required; preserve the
Bearer, Basic, and empty-token handling.
In `@Web/Resgrid.Web.Services/Controllers/v4/UnitTrackingIngressController.cs`:
- Around line 30-40: Update the UnitTrackingIngressController constructor to
remove the four service parameters and resolve
UnitTrackingHttpAuthenticationService, UnitTrackingJsonPayloadParser,
UnitTrackingRateLimiter, and IUnitTrackingIngressService through
Bootstrapper.GetKernel().Resolve<T>(), assigning each result to the
corresponding fields.
In `@Web/Resgrid.Web.Services/Models/v4/UnitTracking/UnitTrackingAdminModels.cs`:
- Around line 132-133: Remove the hardcoded [Range(1, 4)] validation from the
AuthMode property in the unit-tracking admin model, leaving validation to
UnitTrackingDevicesController.CreateCredential’s Enum.IsDefined and
SupportedAuthModes checks so future enum values are accepted when supported.
In `@Web/Resgrid.Web/Areas/User/Controllers/UnitTrackingController.cs`:
- Around line 489-551: The ValidatePreviewJson method duplicates ingestion
validation; expose and reuse a shared TryParse or Validate entry point from
UnitTrackingJsonPayloadParser for preview requests. Move or centralize size,
depth, duplicate-key, positions-shape, and coordinate checks there, then have
ValidatePreviewJson consume its result and preserve the existing error key and
position-count behavior.
- Around line 609-647: Consolidate the duplicated field assignments in
BuildDevice and ApplyUpdate into one shared UnitTrackingDevice mapper, accepting
IsEnabled from each caller so creation remains enabled while updates use
model.IsEnabled. Have both methods reuse this mapper and preserve all existing
model/profile field mappings.
In `@Web/Resgrid.Web/Areas/User/Models/UnitTracking/UnitTrackingViewModels.cs`:
- Around line 79-80: Replace the hardcoded Range validation on AuthMode with
EnumDataType targeting UnitTrackingAuthMode, preserving the existing Bearer
default and enum-backed validation behavior.
In `@Web/Resgrid.Web/Areas/User/Views/UnitTracking/_Editor.cshtml`:
- Around line 19-30: The profile option’s data-identifier-required attribute is
unused by updateProfileHelp(), so the UI does not reflect the server-side
requirement. Update updateProfileHelp() and the Device Identifier field markup
to read the selected option’s flag and show or hide an appropriate required
hint/marker; preserve the existing behavior when the profile does not require an
identifier.
In `@Web/Resgrid.Web/Areas/User/Views/UnitTracking/Details.cshtml`:
- Line 169: Replace the hardcoded "resgrid-json-v1" comparison in the Details
view with the shared constant exposed by UnitTrackingCatalogProfile or the
established adapter-key constants class. Preserve the existing case-insensitive
comparison and preview condition while ensuring the value comes from the single
canonical definition.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8d6e3773-5a91-4618-9a51-1c93ee77db66
⛔ Files ignored due to path filters (26)
Core/Resgrid.Localization/Areas/User/Units/Units.resxis excluded by!**/*.resxTests/Resgrid.Tests/Framework/SentryTransactionFilterTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Framework/UnitLocationEventSerializationTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Providers/EventAggregatorTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Providers/UnitLocationEventProviderTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Repositories/DocumentDbRepositoryTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Resgrid.Tests.csprojis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/DepartmentSettingsServiceUnitTrackingTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/DocumentDatabaseProviderSelectionTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/UnitLocationSourceResolverTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/UnitTrackingAuthenticationServiceTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/UnitTrackingCatalogServiceTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/UnitTrackingEventIdServiceTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/UnitTrackingIdentifierServiceTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/UnitTrackingIngressServiceTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/UnitTrackingServiceTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/UnitTrackingStatusServiceTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Web/Services/CapabilityPathRedactionMiddlewareTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Web/Services/UnitLocationControllerTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Web/Services/UnitTrackingDevicesControllerTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Web/Services/UnitTrackingHttpAuthenticationServiceTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Web/Services/UnitTrackingIngressControllerTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Web/Services/UnitTrackingJsonPayloadParserTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Web/Services/UnitTrackingNetworkPolicyTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Web/Services/UnitTrackingRateLimiterTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Web/User/UnitTrackingControllerTests.csis excluded by!**/Tests/**
📒 Files selected for processing (94)
.coderabbit.yamlCore/Resgrid.Config/ServiceBusConfig.csCore/Resgrid.Config/UnitTrackingConfig.csCore/Resgrid.Framework/SentryTransactionFilter.csCore/Resgrid.Model/AuditLogTypes.csCore/Resgrid.Model/DepartmentSettingTypes.csCore/Resgrid.Model/Events/UnitLocationEvent.csCore/Resgrid.Model/Providers/IEventAggregator.csCore/Resgrid.Model/Providers/IRabbitOutboundQueueProvider.csCore/Resgrid.Model/Providers/IUnitLocationEventProvider.csCore/Resgrid.Model/Repositories/IDocumentDbRepository.csCore/Resgrid.Model/Repositories/IUnitLocationsDocRepository.csCore/Resgrid.Model/Repositories/IUnitLocationsMongoRepository.csCore/Resgrid.Model/Repositories/IUnitTrackingCredentialsRepository.csCore/Resgrid.Model/Repositories/IUnitTrackingDevicesRepository.csCore/Resgrid.Model/Services/IDepartmentSettingsService.csCore/Resgrid.Model/Services/IUnitLocationSourceResolver.csCore/Resgrid.Model/Services/IUnitTrackingAuthenticationService.csCore/Resgrid.Model/Services/IUnitTrackingCatalogService.csCore/Resgrid.Model/Services/IUnitTrackingEventIdService.csCore/Resgrid.Model/Services/IUnitTrackingIdentifierService.csCore/Resgrid.Model/Services/IUnitTrackingIngressService.csCore/Resgrid.Model/Services/IUnitTrackingService.csCore/Resgrid.Model/Services/IUnitTrackingStatusService.csCore/Resgrid.Model/Services/IUnitsService.csCore/Resgrid.Model/Tracking/AuthenticatedTrackingSource.csCore/Resgrid.Model/Tracking/CanonicalTrackingPosition.csCore/Resgrid.Model/Tracking/TrackingIngressResult.csCore/Resgrid.Model/Tracking/UnitTrackingCatalogProfile.csCore/Resgrid.Model/UnitLocationWriteResult.csCore/Resgrid.Model/UnitTrackingCredential.csCore/Resgrid.Model/UnitTrackingDevice.csCore/Resgrid.Model/UnitTrackingEnums.csCore/Resgrid.Model/UnitTrackingResults.csCore/Resgrid.Model/UnitsLocation.csCore/Resgrid.Services/AuditService.csCore/Resgrid.Services/DepartmentSettingsService.csCore/Resgrid.Services/ServicesModule.csCore/Resgrid.Services/UnitLocationSourceResolver.csCore/Resgrid.Services/UnitTrackingAuthenticationService.csCore/Resgrid.Services/UnitTrackingCacheKeys.csCore/Resgrid.Services/UnitTrackingCatalogService.csCore/Resgrid.Services/UnitTrackingEventIdService.csCore/Resgrid.Services/UnitTrackingIdentifierService.csCore/Resgrid.Services/UnitTrackingIngressService.csCore/Resgrid.Services/UnitTrackingService.csCore/Resgrid.Services/UnitTrackingStatusService.csCore/Resgrid.Services/UnitsService.csProviders/Resgrid.Providers.Bus.Rabbit/RabbitConnection.csProviders/Resgrid.Providers.Bus.Rabbit/RabbitInboundQueueProvider.csProviders/Resgrid.Providers.Bus.Rabbit/RabbitOutboundQueueProvider.csProviders/Resgrid.Providers.Bus.Rabbit/RabbitTopicProvider.csProviders/Resgrid.Providers.Bus/BusModule.csProviders/Resgrid.Providers.Bus/EventAggregator.csProviders/Resgrid.Providers.Bus/OutboundEventProvider.csProviders/Resgrid.Providers.Bus/UnitLocationEventProvider.csProviders/Resgrid.Providers.Migrations/Migrations/M0101_AddUnitTracking.csProviders/Resgrid.Providers.MigrationsPg/Migrations/M0101_AddUnitTrackingPg.csProviders/Resgrid.Providers.MigrationsPg/Sql/EF0003_PopulateDocDb.sqlRepositories/Resgrid.Repositories.DataRepository/Modules/DataModule.csRepositories/Resgrid.Repositories.DataRepository/Queries/UnitTracking/SelectUnitTrackingCredentialBySecretHashQuery.csRepositories/Resgrid.Repositories.DataRepository/Queries/UnitTracking/SelectUnitTrackingCredentialsByDeviceQuery.csRepositories/Resgrid.Repositories.DataRepository/Queries/UnitTracking/SelectUnitTrackingDeviceByProtocolIdentifierQuery.csRepositories/Resgrid.Repositories.DataRepository/Queries/UnitTracking/SelectUnitTrackingDevicesByUnitQuery.csRepositories/Resgrid.Repositories.DataRepository/UnitTrackingCredentialsRepository.csRepositories/Resgrid.Repositories.DataRepository/UnitTrackingDevicesRepository.csRepositories/Resgrid.Repositories.NoSqlRepository/DocumentDbRepository.csRepositories/Resgrid.Repositories.NoSqlRepository/NoSqlDataModule.csRepositories/Resgrid.Repositories.NoSqlRepository/UnitLocationsDocRepository.csRepositories/Resgrid.Repositories.NoSqlRepository/UnitLocationsMongoRepository.csWeb/Resgrid.Web.Services/ApplicationCore/UnitTracking/UnitTrackingHttpAuthenticationService.csWeb/Resgrid.Web.Services/ApplicationCore/UnitTracking/UnitTrackingJsonPayloadParser.csWeb/Resgrid.Web.Services/ApplicationCore/UnitTracking/UnitTrackingNetworkPolicy.csWeb/Resgrid.Web.Services/ApplicationCore/UnitTracking/UnitTrackingRateLimiter.csWeb/Resgrid.Web.Services/Controllers/v4/UnitLocationController.csWeb/Resgrid.Web.Services/Controllers/v4/UnitTrackingDevicesController.csWeb/Resgrid.Web.Services/Controllers/v4/UnitTrackingIngressController.csWeb/Resgrid.Web.Services/Middleware/CapabilityPathRedactionMiddleware.csWeb/Resgrid.Web.Services/Middleware/SentryEventProcessor.csWeb/Resgrid.Web.Services/Models/v4/UnitTracking/UnitTrackingAdminModels.csWeb/Resgrid.Web.Services/Models/v4/UnitTracking/UnitTrackingIngressResponse.csWeb/Resgrid.Web.Services/Models/v4/UnitTracking/UnitTrackingPositionInput.csWeb/Resgrid.Web.Services/Program.csWeb/Resgrid.Web.Services/Startup.csWeb/Resgrid.Web/Areas/User/Controllers/UnitTrackingController.csWeb/Resgrid.Web/Areas/User/Models/UnitTracking/UnitTrackingViewModels.csWeb/Resgrid.Web/Areas/User/Views/UnitTracking/Credential.cshtmlWeb/Resgrid.Web/Areas/User/Views/UnitTracking/Details.cshtmlWeb/Resgrid.Web/Areas/User/Views/UnitTracking/Edit.cshtmlWeb/Resgrid.Web/Areas/User/Views/UnitTracking/Index.cshtmlWeb/Resgrid.Web/Areas/User/Views/UnitTracking/New.cshtmlWeb/Resgrid.Web/Areas/User/Views/UnitTracking/_Editor.cshtmlWeb/Resgrid.Web/Areas/User/Views/Units/EditUnit.cshtmlWorkers/Resgrid.Workers.Framework/Logic/UnitLocationQueueLogic.cs
| public static string UnitLocationQueueV2Name = "unitlocation-v2"; | ||
| public static string UnitLocationRetryQueueV2Name = "unitlocation-v2.retry"; | ||
| public static string UnitLocationDeadQueueV2Name = "unitlocation-v2.dead"; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Isolate the v2 queues in DEBUG builds.
Lines 53-55 are outside the existing #if DEBUG split, so DEBUG and release builds use the same RabbitMQ queue names. Mirror the existing test suffix convention or verify that DEBUG always uses a completely isolated broker.
Proposed fix
+#if DEBUG
+ public static string UnitLocationQueueV2Name = "unitlocation-v2test";
+ public static string UnitLocationRetryQueueV2Name = "unitlocation-v2.retrytest";
+ public static string UnitLocationDeadQueueV2Name = "unitlocation-v2.deadtest";
+#else
public static string UnitLocationQueueV2Name = "unitlocation-v2";
public static string UnitLocationRetryQueueV2Name = "unitlocation-v2.retry";
public static string UnitLocationDeadQueueV2Name = "unitlocation-v2.dead";
+#endif📝 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.
| public static string UnitLocationQueueV2Name = "unitlocation-v2"; | |
| public static string UnitLocationRetryQueueV2Name = "unitlocation-v2.retry"; | |
| public static string UnitLocationDeadQueueV2Name = "unitlocation-v2.dead"; | |
| `#if` DEBUG | |
| public static string UnitLocationQueueV2Name = "unitlocation-v2test"; | |
| public static string UnitLocationRetryQueueV2Name = "unitlocation-v2.retrytest"; | |
| public static string UnitLocationDeadQueueV2Name = "unitlocation-v2.deadtest"; | |
| `#else` | |
| public static string UnitLocationQueueV2Name = "unitlocation-v2"; | |
| public static string UnitLocationRetryQueueV2Name = "unitlocation-v2.retry"; | |
| public static string UnitLocationDeadQueueV2Name = "unitlocation-v2.dead"; | |
| `#endif` |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Core/Resgrid.Config/ServiceBusConfig.cs` around lines 53 - 55, Update the
UnitLocationQueueV2Name, UnitLocationRetryQueueV2Name, and
UnitLocationDeadQueueV2Name definitions in ServiceBusConfig to follow the
existing `#if` DEBUG queue-name split, applying the established test suffix or
equivalent DEBUG-only isolation while preserving the current release names.
| if (transaction == null) | ||
| return null; | ||
|
|
||
| if (transaction.Request != null) | ||
| transaction.Request.Url = RedactCapabilityPath(transaction.Request.Url); | ||
|
|
||
| if (transaction.Status != SpanStatus.NotFound) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
transaction.Name is not redacted, so capability tokens can still reach Sentry.
Only Request.Url is scrubbed. Line 75 already assumes transaction.Name can carry the request target, and for requests that don't match a route template (the 404 case this filter exists for) Sentry derives the transaction name from the raw path — leaking the bearer-equivalent capability token to a third party.
🔒️ Redact the transaction name as well
if (transaction.Request != null)
transaction.Request.Url = RedactCapabilityPath(transaction.Request.Url);
+ transaction.Name = RedactCapabilityPath(transaction.Name);
+
if (transaction.Status != SpanStatus.NotFound)📝 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.
| if (transaction == null) | |
| return null; | |
| if (transaction.Request != null) | |
| transaction.Request.Url = RedactCapabilityPath(transaction.Request.Url); | |
| if (transaction.Status != SpanStatus.NotFound) | |
| if (transaction == null) | |
| return null; | |
| if (transaction.Request != null) | |
| transaction.Request.Url = RedactCapabilityPath(transaction.Request.Url); | |
| transaction.Name = RedactCapabilityPath(transaction.Name); | |
| if (transaction.Status != SpanStatus.NotFound) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Core/Resgrid.Framework/SentryTransactionFilter.cs` around lines 64 - 70,
Update the transaction filtering logic around RedactCapabilityPath so
transaction.Name is also passed through the same redaction before the status
check. Preserve the existing Request.Url redaction and ensure both fields are
scrubbed, including 404 transactions whose names derive from the raw request
path.
| var randomBytes = RandomNumberGenerator.GetBytes(32); | ||
| var encodedSecret = Convert.ToBase64String(randomBytes) | ||
| .TrimEnd('=') | ||
| .Replace('+', '-') | ||
| .Replace('/', '_'); | ||
| var keyPrefix = encodedSecret.Substring(0, 8); | ||
| var token = $"rgtrk_{keyPrefix}_{encodedSecret}"; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
KeyPrefix is the first 8 characters of the secret itself.
keyPrefix is stored in the database in cleartext and shown in admin UI/API listings, so anyone with read access to UnitTrackingCredentials learns the first 8 base64url chars (~48 bits) of the secret. Residual entropy is still large, but a non-secret identifier should not be a substring of the secret — derive the prefix from separate random bytes and prepend it to the token.
🔒 Proposed change
- var randomBytes = RandomNumberGenerator.GetBytes(32);
- var encodedSecret = Convert.ToBase64String(randomBytes)
- .TrimEnd('=')
- .Replace('+', '-')
- .Replace('/', '_');
- var keyPrefix = encodedSecret.Substring(0, 8);
- var token = $"rgtrk_{keyPrefix}_{encodedSecret}";
+ var encodedSecret = Convert.ToBase64String(RandomNumberGenerator.GetBytes(32))
+ .TrimEnd('=')
+ .Replace('+', '-')
+ .Replace('/', '_');
+ var keyPrefix = Convert.ToHexString(RandomNumberGenerator.GetBytes(4)).ToLowerInvariant();
+ var token = $"rgtrk_{keyPrefix}_{encodedSecret}";📝 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.
| var randomBytes = RandomNumberGenerator.GetBytes(32); | |
| var encodedSecret = Convert.ToBase64String(randomBytes) | |
| .TrimEnd('=') | |
| .Replace('+', '-') | |
| .Replace('/', '_'); | |
| var keyPrefix = encodedSecret.Substring(0, 8); | |
| var token = $"rgtrk_{keyPrefix}_{encodedSecret}"; | |
| var encodedSecret = Convert.ToBase64String(RandomNumberGenerator.GetBytes(32)) | |
| .TrimEnd('=') | |
| .Replace('+', '-') | |
| .Replace('/', '_'); | |
| var keyPrefix = Convert.ToHexString(RandomNumberGenerator.GetBytes(4)).ToLowerInvariant(); | |
| var token = $"rgtrk_{keyPrefix}_{encodedSecret}"; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Core/Resgrid.Services/UnitTrackingAuthenticationService.cs` around lines 39 -
45, Update the token generation logic around RandomNumberGenerator and keyPrefix
so the stored/displayed prefix is generated from separate random bytes rather
than derived from encodedSecret. Encode the independent prefix with the same
base64url-safe normalization, then prepend it to the token while preserving the
existing rgtrk token format and full secret entropy.
| var cached = SystemBehaviorConfig.CacheEnabled | ||
| ? await _cacheProvider.RetrieveAsync( | ||
| UnitTrackingCacheKeys.DeviceCredentials(deviceId), | ||
| load, | ||
| TimeSpan.FromSeconds(Math.Max(1, Math.Min(60, UnitTrackingConfig.CredentialCacheSeconds)))) | ||
| : await load(); | ||
| var now = utcNow ?? DateTime.UtcNow; | ||
|
|
||
| return cached | ||
| .Where(credential => IsActive(credential, now)) | ||
| .ToList(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Guard the cached list before enumerating.
If the provider round-trip yields null (empty/absent cached representation), line 203 dereferences it. load() itself never returns null, so a cheap coalesce removes the only null path.
🛡️ Proposed guard
var now = utcNow ?? DateTime.UtcNow;
- return cached
+ return (cached ?? new List<UnitTrackingCredential>())
.Where(credential => IsActive(credential, now))
.ToList();📝 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.
| var cached = SystemBehaviorConfig.CacheEnabled | |
| ? await _cacheProvider.RetrieveAsync( | |
| UnitTrackingCacheKeys.DeviceCredentials(deviceId), | |
| load, | |
| TimeSpan.FromSeconds(Math.Max(1, Math.Min(60, UnitTrackingConfig.CredentialCacheSeconds)))) | |
| : await load(); | |
| var now = utcNow ?? DateTime.UtcNow; | |
| return cached | |
| .Where(credential => IsActive(credential, now)) | |
| .ToList(); | |
| var cached = SystemBehaviorConfig.CacheEnabled | |
| ? await _cacheProvider.RetrieveAsync( | |
| UnitTrackingCacheKeys.DeviceCredentials(deviceId), | |
| load, | |
| TimeSpan.FromSeconds(Math.Max(1, Math.Min(60, UnitTrackingConfig.CredentialCacheSeconds)))) | |
| : await load(); | |
| var now = utcNow ?? DateTime.UtcNow; | |
| return (cached ?? new List<UnitTrackingCredential>()) | |
| .Where(credential => IsActive(credential, now)) | |
| .ToList(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Core/Resgrid.Services/UnitTrackingAuthenticationService.cs` around lines 195
- 205, Update the cached result handling in the credential retrieval flow before
the `cached.Where(...)` call to coalesce a null provider result to an empty
credential collection. Preserve the existing filtering and list conversion for
non-null results.
| IsEnabled = device.IsEnabled, | ||
| IsDeleted = false, | ||
| SourcePriority = device.SourcePriority, | ||
| AllowedSourceCidrs = TrimToNull(device.AllowedSourceCidrs), |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how AllowedSourceCidrs is parsed and whether unparsable entries fail open
fd -i 'UnitTrackingNetworkPolicy.cs' --exec cat -n
rg -nP --type=cs -C3 'AllowedSourceCidrs'Repository: Resgrid/Core
Length of output: 2822
🏁 Script executed:
#!/bin/bash
set -e
echo "== Locate service files =="
fd -i 'UnitTrackingService.cs' -x wc -l {} \;
fd -i 'UnitTrackingDevice.cs' -x wc -l {} \;
echo "== Relevant UnitTrackingService.cs section =="
file="$(fd -i 'UnitTrackingService.cs' | head -n 1)"
sed -n '70,140p' "$file" | cat -n -v | sed 's/^/\1/'
echo "== AllowedSourceCidrs references =="
rg -i -n --type=cs -C4 'AllowedSourceCidrs|UnitTrackingNetworkPolicy'Repository: Resgrid/Core
Length of output: 451
🏁 Script executed:
#!/bin/bash
set -u
echo "== Locate files =="
fd -i 'UnitTrackingService.cs'
printf '\n'
fd -i 'UnitTrackingDevice.cs'
echo "== Relevant UnitTrackingService.cs section =="
service=$(fd -i 'UnitTrackingService.cs' | head -n 1)
sed -n '70,140p' "$service"
echo "== UnitTrackingDevice section =="
device=$(fd -i 'UnitTrackingDevice.cs' | head -n 1)
sed -n '1,140p' "$device"
echo "== References =="
rg -i -n --type=cs -C4 'AllowedSourceCidrs|UnitTrackingNetworkPolicy|Contains\('Repository: Resgrid/Core
Length of output: 2705
Validate AllowedSourceCidrs before creating or updating devices.
This field is persisted after only trimming whitespace and can accept invalid source ACL entries. The network policy returns true when the field is blank/missing, so malformed configuration can be reached; catch bad entries with TryParse / prefix-bound checks and validate canonical entries before saving.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Core/Resgrid.Services/UnitTrackingService.cs` at line 102, Validate
device.AllowedSourceCidrs before persisting it in the device create/update flow,
rather than only applying TrimToNull. Parse each nonblank CIDR entry with
TryParse, enforce valid prefix bounds, and require canonical entries; reject
malformed values before saving while preserving the network policy’s blank-value
behavior.
| if (!await _unitLocationEventProvider.EnqueueUnitLocationEventAsync(location)) | ||
| return StatusCode(StatusCodes.Status503ServiceUnavailable); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Return 503 when enqueueing throws.
A faulted EnqueueUnitLocationEventAsync task reaches the outer catch and becomes 400 BadRequest, so clients will not treat a queue outage as retryable.
Suggested handling
- if (!await _unitLocationEventProvider.EnqueueUnitLocationEventAsync(location))
- return StatusCode(StatusCodes.Status503ServiceUnavailable);
+ try
+ {
+ if (!await _unitLocationEventProvider.EnqueueUnitLocationEventAsync(location))
+ return StatusCode(StatusCodes.Status503ServiceUnavailable);
+ }
+ catch (Exception ex)
+ {
+ Logging.LogException(ex);
+ return StatusCode(StatusCodes.Status503ServiceUnavailable);
+ }📝 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.
| if (!await _unitLocationEventProvider.EnqueueUnitLocationEventAsync(location)) | |
| return StatusCode(StatusCodes.Status503ServiceUnavailable); | |
| try | |
| { | |
| if (!await _unitLocationEventProvider.EnqueueUnitLocationEventAsync(location)) | |
| return StatusCode(StatusCodes.Status503ServiceUnavailable); | |
| } | |
| catch (Exception ex) | |
| { | |
| Logging.LogException(ex); | |
| return StatusCode(StatusCodes.Status503ServiceUnavailable); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Web/Resgrid.Web.Services/Controllers/v4/UnitLocationController.cs` around
lines 103 - 104, Update the enqueue flow in EnqueueUnitLocationEventAsync
handling within the controller action so exceptions from queue submission are
caught and mapped to 503 ServiceUnavailable, matching the existing false-result
path. Ensure this handling occurs before the outer catch that returns 400, while
preserving the current response behavior for other failures.
| [MaxLength(2048)] | ||
| public string AllowedSourceCidrs { get; set; } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Validate AllowedSourceCidrs entries on input.
Only length is constrained. UnitTrackingNetworkPolicy.Contains returns false for any entry it cannot parse, so a single typo (e.g. 10.0.0.0/33, 10.0.0/24) turns the allowlist into a silent deny-all for that device with no feedback to the operator. Reject malformed CIDR lists at the API boundary instead.
Also applies to: 123-124
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Web/Resgrid.Web.Services/Models/v4/UnitTracking/UnitTrackingAdminModels.cs`
around lines 98 - 99, Validate AllowedSourceCidrs at the API model boundary in
the relevant model properties, rejecting malformed CIDR entries such as invalid
prefixes or incomplete addresses instead of relying on
UnitTrackingNetworkPolicy.Contains. Preserve valid CIDR-list input and return a
clear validation error for any invalid entry, while retaining the existing
MaxLength constraint.
| <label class="col-sm-4 control-label">@localizer["AuthenticationMode"]</label> | ||
| <div class="col-sm-8"> | ||
| <select asp-for="CredentialInput.AuthMode" class="form-control" id="credential-auth-mode"> | ||
| @foreach (var authMode in Model.SupportedAuthModes) | ||
| { | ||
| <option value="@((int)authMode)">@authMode</option> | ||
| } | ||
| </select> | ||
| </div> | ||
| </div> | ||
| <div class="form-group" id="custom-header-group" style="display:none;"> | ||
| <label class="col-sm-4 control-label">@localizer["HeaderName"]</label> | ||
| <div class="col-sm-8"><input asp-for="CredentialInput.HeaderName" class="form-control" /></div> | ||
| </div> | ||
| <div class="form-group" id="basic-username-group" style="display:none;"> | ||
| <label class="col-sm-4 control-label">@localizer["BasicUsername"]</label> | ||
| <div class="col-sm-8"><input asp-for="CredentialInput.BasicUsername" class="form-control" /></div> | ||
| </div> |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Labels not associated with their form controls in Create Credential section.
Unlike _Editor.cshtml (which uses <label asp-for="..."> throughout), these labels are plain <label class="col-sm-4 control-label"> with no for/asp-for binding to credential-auth-mode, CredentialInput_HeaderName, or CredentialInput_BasicUsername. This breaks label-to-input association for screen readers.
♿ Suggested fix
- <label class="col-sm-4 control-label">`@localizer`["AuthenticationMode"]</label>
+ <label asp-for="CredentialInput.AuthMode" class="col-sm-4 control-label">`@localizer`["AuthenticationMode"]</label>
...
- <label class="col-sm-4 control-label">`@localizer`["HeaderName"]</label>
+ <label asp-for="CredentialInput.HeaderName" class="col-sm-4 control-label">`@localizer`["HeaderName"]</label>
...
- <label class="col-sm-4 control-label">`@localizer`["BasicUsername"]</label>
+ <label asp-for="CredentialInput.BasicUsername" class="col-sm-4 control-label">`@localizer`["BasicUsername"]</label>📝 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.
| <label class="col-sm-4 control-label">@localizer["AuthenticationMode"]</label> | |
| <div class="col-sm-8"> | |
| <select asp-for="CredentialInput.AuthMode" class="form-control" id="credential-auth-mode"> | |
| @foreach (var authMode in Model.SupportedAuthModes) | |
| { | |
| <option value="@((int)authMode)">@authMode</option> | |
| } | |
| </select> | |
| </div> | |
| </div> | |
| <div class="form-group" id="custom-header-group" style="display:none;"> | |
| <label class="col-sm-4 control-label">@localizer["HeaderName"]</label> | |
| <div class="col-sm-8"><input asp-for="CredentialInput.HeaderName" class="form-control" /></div> | |
| </div> | |
| <div class="form-group" id="basic-username-group" style="display:none;"> | |
| <label class="col-sm-4 control-label">@localizer["BasicUsername"]</label> | |
| <div class="col-sm-8"><input asp-for="CredentialInput.BasicUsername" class="form-control" /></div> | |
| </div> | |
| <label asp-for="CredentialInput.AuthMode" class="col-sm-4 control-label">`@localizer`["AuthenticationMode"]</label> | |
| <div class="col-sm-8"> | |
| <select asp-for="CredentialInput.AuthMode" class="form-control" id="credential-auth-mode"> | |
| `@foreach` (var authMode in Model.SupportedAuthModes) | |
| { | |
| <option value="@((int)authMode)">`@authMode`</option> | |
| } | |
| </select> | |
| </div> | |
| </div> | |
| <div class="form-group" id="custom-header-group" style="display:none;"> | |
| <label asp-for="CredentialInput.HeaderName" class="col-sm-4 control-label">`@localizer`["HeaderName"]</label> | |
| <div class="col-sm-8"><input asp-for="CredentialInput.HeaderName" class="form-control" /></div> | |
| </div> | |
| <div class="form-group" id="basic-username-group" style="display:none;"> | |
| <label asp-for="CredentialInput.BasicUsername" class="col-sm-4 control-label">`@localizer`["BasicUsername"]</label> | |
| <div class="col-sm-8"><input asp-for="CredentialInput.BasicUsername" class="form-control" /></div> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Web/Resgrid.Web/Areas/User/Views/UnitTracking/Details.cshtml` around lines
137 - 154, Associate the Create Credential labels with their controls by
updating the labels in the authentication mode, custom header, and basic
username groups to use the corresponding asp-for bindings or matching for
attributes. Ensure they target CredentialInput.AuthMode,
CredentialInput.HeaderName, and CredentialInput.BasicUsername, while preserving
the existing label styling and text.
| if (unitLocationEvent == null) | ||
| throw new ArgumentNullException(nameof(unitLocationEvent)); | ||
|
|
||
| if (unitLocationEvent != null) | ||
| if (unitLocationEvent.UnitId <= 0) | ||
| throw new InvalidOperationException("A Unit location queue event must identify a Unit."); | ||
|
|
||
| if (unitLocationEvent.DepartmentId <= 0) | ||
| throw new InvalidOperationException("A Unit location queue event must identify a Department."); | ||
|
|
||
| if (unitLocationEvent.IsValidFix == false) | ||
| return true; | ||
|
|
||
| if (!unitLocationEvent.Latitude.HasValue || !unitLocationEvent.Longitude.HasValue) | ||
| throw new InvalidOperationException("A valid Unit location queue event must contain latitude and longitude."); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Failures here are no longer logged in this layer.
The try/catch and the Resgrid.Framework import were both removed, so nothing in this logic records why an event was rejected — the only record is whatever the consumer's catch happens to log. Propagating the exception is needed for the new nack/retry pipeline, so keep the throw but log first.
As per coding guidelines, worker logic must "wrap logic in try-catch, logs exceptions via Logging.LogException()" and use Resgrid.Framework.Logging static methods for all logging.
🛡️ Log then rethrow to keep retry semantics
+using Resgrid.Framework;- if (unitLocationEvent == null)
- throw new ArgumentNullException(nameof(unitLocationEvent));
-
- if (unitLocationEvent.UnitId <= 0)
- throw new InvalidOperationException("A Unit location queue event must identify a Unit.");
-
- if (unitLocationEvent.DepartmentId <= 0)
- throw new InvalidOperationException("A Unit location queue event must identify a Department.");
+ try
+ {
+ return await ProcessInternal(unitLocationEvent, cancellationToken);
+ }
+ catch (Exception ex)
+ {
+ Logging.LogException(ex, $"UnitId: {unitLocationEvent?.UnitId}, EventId: {unitLocationEvent?.EventId}");
+ throw;
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Workers/Resgrid.Workers.Framework/Logic/UnitLocationQueueLogic.cs` around
lines 15 - 28, Restore try-catch handling around the validation logic in the
relevant method, retain the existing exception propagation for rejected events,
and call Resgrid.Framework.Logging.LogException() in the catch before
rethrowing. Re-add the Resgrid.Framework import required for the static logging
API, while preserving the current validation and retry/nack behavior.
Source: Coding guidelines
| if (unitLocationEvent.IsValidFix == false) | ||
| return true; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Unknown fix validity is silently promoted to valid.
The early return at Line 24 already handles IsValidFix == false, so Line 46 only ever sees true or null. Coercing null (source never reported validity) to true asserts a guarantee the device didn't make, and UnitsLocation.IsValidFix is bool? specifically so "unknown" is representable. Preserving the null keeps the persisted record faithful to the payload.
Separately, the Line 24 drop path returns success with no log or metric, so invalid-fix volume is invisible in operations.
🐛 Proposed fix
- IsValidFix = unitLocationEvent.IsValidFix ?? true,
+ IsValidFix = unitLocationEvent.IsValidFix,Also applies to: 46-46
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Workers/Resgrid.Workers.Framework/Logic/UnitLocationQueueLogic.cs` around
lines 24 - 25, Update the UnitLocationQueueLogic handling so an unknown
IsValidFix value remains null rather than being coerced to true when persisted
to UnitsLocation. Preserve the existing invalid-fix early return, but add the
appropriate operational logging or metric before returning success so dropped
invalid fixes are observable.
| public static bool Enabled = false; | ||
| public static bool HttpsIngressEnabled = false; | ||
| public static bool NativeGatewayEnabled = false; | ||
| public static string CredentialPepper = ""; |
There was a problem hiding this comment.
Mutable static fields, such as CredentialPepper, risk accidental runtime modification. Declare compile-time constant values using const or static readonly to ensure immutability and thread safety.
Kody rule violation: Use `readonly` or `const` for Immutable Data
Prompt for LLM
File Core/Resgrid.Config/UnitTrackingConfig.cs:
Line 8:
Mutable static fields, such as `CredentialPepper`, risk accidental runtime modification. Declare compile-time constant values using `const` or `static readonly` to ensure immutability and thread safety.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| public static string RedactCapabilityPath(string value) | ||
| { | ||
| const string prefix = "/api/v4/unit-trackers/c/"; |
There was a problem hiding this comment.
Decentralized string literals like route names violate reuse guidelines and complicate updates when paths change. Move this route-name constant to a centralized shared constants class, such as RouteConstants.UnitTrackerCapabilityPrefix, and reference it throughout the codebase.
Kody rule violation: Centralize string constants
Prompt for LLM
File Core/Resgrid.Framework/SentryTransactionFilter.cs:
Line 82:
Decentralized string literals like route names violate reuse guidelines and complicate updates when paths change. Move this route-name constant to a centralized shared constants class, such as `RouteConstants.UnitTrackerCapabilityPrefix`, and reference it throughout the codebase.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| /// <returns>Returns the current IEventSubscriptionManager to allow for easy fluent additions.</returns> | ||
| Guid AddListener<T>(Action<T> listener); | ||
|
|
||
| Guid AddAsyncListener<T>(Func<T, Task> listener); |
There was a problem hiding this comment.
Unobserved task rejections occur when async listeners provided to AddAsyncListener<T> fault without a defined error-handling path. Add an error handler parameter, such as Action<Exception> onError, to allow deterministic error handling alongside the subscription.
Kody rule violation: Provide error handlers to subscription/listener APIs
Prompt for LLM
File Core/Resgrid.Model/Providers/IEventAggregator.cs:
Line 29:
Unobserved task rejections occur when async listeners provided to `AddAsyncListener<T>` fault without a defined error-handling path. Add an error handler parameter, such as `Action<Exception> onError`, to allow deterministic error handling alongside the subscription.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| Task<TrackingIngressResult> AcceptAsync( | ||
| AuthenticatedTrackingSource source, | ||
| IReadOnlyCollection<CanonicalTrackingPosition> positions, | ||
| CancellationToken cancellationToken = default); |
There was a problem hiding this comment.
Ambiguous async contract identified on AcceptAsync due to missing XML documentation describing resolution values, rejection conditions, or cancellation semantics. Add XML doc comments to detail the success result shape, faulted states, and OperationCanceledException propagation behavior.
Kody rule violation: Document async/Promise behavior and errors
Prompt for LLM
File Core/Resgrid.Model/Services/IUnitTrackingIngressService.cs:
Line 10 to 13:
Ambiguous async contract identified on `AcceptAsync` due to missing XML documentation describing resolution values, rejection conditions, or cancellation semantics. Add XML doc comments to detail the success result shape, faulted states, and `OperationCanceledException` propagation behavior.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| Task<UnitTrackingDevice> GetDeviceByIdAsync(string deviceId, int departmentId); | ||
| Task<List<UnitTrackingDevice>> GetDevicesForDepartmentAsync(int departmentId); | ||
| Task<List<UnitTrackingDevice>> GetDevicesForUnitAsync(int departmentId, int unitId); | ||
| Task<List<UnitTrackingCredential>> GetCredentialsForDeviceAsync(string deviceId, int departmentId); |
There was a problem hiding this comment.
Mutable return types expose APIs to unintended side effects by allowing callers to modify the underlying collection. Change the GetCredentialsForDeviceAsync return type to Task<IReadOnlyList<UnitTrackingCredential>> to enforce immutability.
Kody rule violation: Use IReadOnlyList for immutable collections
Prompt for LLM
File Core/Resgrid.Model/Services/IUnitTrackingService.cs:
Line 13:
Mutable return types expose APIs to unintended side effects by allowing callers to modify the underlying collection. Change the `GetCredentialsForDeviceAsync` return type to `Task<IReadOnlyList<UnitTrackingCredential>>` to enforce immutability.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| /// <returns>Task<UnitLocation>.</returns> | ||
| Task<UnitsLocation> AddUnitLocationAsync(UnitsLocation location, int departmentId, | ||
| /// <returns>The idempotent document-store write result.</returns> | ||
| Task<UnitLocationWriteResult> AddUnitLocationAsync(UnitsLocation location, int departmentId, |
There was a problem hiding this comment.
Breaking contract change identified by modifying the AddUnitLocationAsync return type from Task<UnitsLocation> to Task<UnitLocationWriteResult>. Document this modification with an explicit 'BREAKING CHANGE' section detailing migration steps, affected consumers, and a rollout plan.
Kody rule violation: Call out breaking changes explicitly
Prompt for LLM
File Core/Resgrid.Model/Services/IUnitsService.cs:
Line 308:
Breaking contract change identified by modifying the `AddUnitLocationAsync` return type from `Task<UnitsLocation>` to `Task<UnitLocationWriteResult>`. Document this modification with an explicit 'BREAKING CHANGE' section detailing migration steps, affected consumers, and a rollout plan.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| public static UnitLocationWriteResult Duplicate(UnitsLocation location) | ||
| { | ||
| return new UnitLocationWriteResult | ||
| { | ||
| Status = UnitLocationWriteStatus.Duplicate, | ||
| Location = location | ||
| }; | ||
| } |
There was a problem hiding this comment.
Duplicate code sequences identified in the Duplicate and Inserted factory methods, which differ only by the Status enum value. Extract a private helper method accepting the status parameter to reduce maintenance burden and consolidate instantiation logic.
Kody rule violation: Extract duplicated logic into functions
Prompt for LLM
File Core/Resgrid.Model/UnitLocationWriteResult.cs:
Line 23 to 30:
Duplicate code sequences identified in the `Duplicate` and `Inserted` factory methods, which differ only by the `Status` enum value. Extract a private helper method accepting the status parameter to reduce maintenance burden and consolidate instantiation logic.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| LongCacheLength) | ||
| : await getSetting(); | ||
|
|
||
| return int.TryParse(value, out var seconds) ? Math.Max(1, seconds) : 180; |
There was a problem hiding this comment.
Magic numbers obscure intent and introduce maintenance risks when duplicated, such as 180 appearing as both an integer and a string literal. Extract named constants like DefaultStaleAfterSeconds and MinimumStaleAfterSeconds to clarify intent and centralize default values.
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File Core/Resgrid.Services/DepartmentSettingsService.cs:
Line 974:
Magic numbers obscure intent and introduce maintenance risks when duplicated, such as `180` appearing as both an integer and a string literal. Extract named constants like `DefaultStaleAfterSeconds` and `MinimumStaleAfterSeconds` to clarify intent and centralize default values.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| public async Task<int> GetHardwareTrackingStaleAfterSecondsAsync(int departmentId, bool bypassCache = false) | ||
| { | ||
| async Task<string> getSetting() |
There was a problem hiding this comment.
Naming convention violation identified on the local function getSetting. Rename it to GetSetting to comply with .NET PascalCase requirements for methods.
Kody rule violation: Use proper naming conventions
Prompt for LLM
File Core/Resgrid.Services/DepartmentSettingsService.cs:
Line 959:
Naming convention violation identified on the local function `getSetting`. Rename it to `GetSetting` to comply with .NET PascalCase requirements for methods.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var latestPerSource = locations | ||
| .Where(location => | ||
| location != null && | ||
| location.DepartmentId == departmentId && | ||
| location.IsValidFix != false) | ||
| .GroupBy(location => new | ||
| { | ||
| location.SourceType, | ||
| SourceId = location.SourceId ?? string.Empty | ||
| }) | ||
| .Select(group => group | ||
| .OrderByDescending(location => location.Timestamp) | ||
| .ThenByDescending(location => location.ReceivedOn ?? location.Timestamp) | ||
| .First()) | ||
| .ToList(); |
There was a problem hiding this comment.
Complex LINQ chains obscure hidden grouping and latest-selection logic, complicating readability, debugging, and testing. Break the 15-line expression into named intermediate steps to materialize the list through discrete filtering, grouping, and selection operations.
Kody rule violation: Limit Lengthy LINQ Chains
Prompt for LLM
File Core/Resgrid.Services/UnitLocationSourceResolver.cs:
Line 30 to 44:
Complex LINQ chains obscure hidden grouping and latest-selection logic, complicating readability, debugging, and testing. Break the 15-line expression into named intermediate steps to materialize the list through discrete filtering, grouping, and selection operations.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| return Invalid(receivedOn, "The tracking binding is not enabled."); | ||
|
|
||
| var device = source.Device; | ||
| var unit = await _unitsService.GetUnitByIdAsync(device.UnitId); |
There was a problem hiding this comment.
Unnecessary database queries occur because GetUnitByIdAsync executes before input validations complete. Move the positions.Count == 0 and batch-limit validations above the database call to allow invalid payloads to return early without a round-trip.
Kody rule violation: Order validations before database queries
Prompt for LLM
File Core/Resgrid.Services/UnitTrackingIngressService.cs:
Line 57:
Unnecessary database queries occur because `GetUnitByIdAsync` executes before input validations complete. Move the `positions.Count == 0` and batch-limit validations above the database call to allow invalid payloads to return early without a round-trip.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var oldCacheIdentity = CopyCacheIdentity(existing); | ||
| var now = DateTime.UtcNow; | ||
|
|
||
| _unitOfWork.CreateOrGetConnection(); |
There was a problem hiding this comment.
Thread starvation risk identified due to synchronous connection-creation calls blocking execution inside an async method. Replace _unitOfWork.CreateOrGetConnection() with its async equivalent, such as await _unitOfWork.CreateOrGetConnectionAsync(), to improve throughput.
Kody rule violation: Use Awaitable Methods in Async Code
Prompt for LLM
File Core/Resgrid.Services/UnitTrackingService.cs:
Line 202:
Thread starvation risk identified due to synchronous connection-creation calls blocking execution inside an async method. Replace `_unitOfWork.CreateOrGetConnection()` with its async equivalent, such as `await _unitOfWork.CreateOrGetConnectionAsync()`, to improve throughput.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| await _devicesRepository.InsertAsync(replacement, cancellationToken); | ||
| _unitOfWork.CommitChanges(); | ||
| } | ||
| catch |
There was a problem hiding this comment.
Silent transaction failures hinder production diagnostics because the catch block rolls back and rethrows without structured logging. Catch a typed exception, log it with operational context including deviceId and departmentId, and then rethrow.
Kody rule violation: Avoid empty catch blocks
Prompt for LLM
File Core/Resgrid.Services/UnitTrackingService.cs:
Line 218:
Silent transaction failures hinder production diagnostics because the catch block rolls back and rethrows without structured logging. Catch a typed exception, log it with operational context including `deviceId` and `departmentId`, and then rethrow.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| await _devicesRepository.InsertAsync(replacement, cancellationToken); | ||
| _unitOfWork.CommitChanges(); | ||
| } | ||
| catch |
There was a problem hiding this comment.
Indiscriminate exception handling masks transient database errors like timeouts and deadlocks, preventing retry policies from executing. Catch DbUpdateException to inspect for transient error codes and conditionally retry safe, idempotent operations.
Kody rule violation: Implement proper database error checking
Prompt for LLM
File Core/Resgrid.Services/UnitTrackingService.cs:
Line 218:
Indiscriminate exception handling masks transient database errors like timeouts and deadlocks, preventing retry policies from executing. Catch `DbUpdateException` to inspect for transient error codes and conditionally retry safe, idempotent operations.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| _eventAggregator.SendMessage(new AuditEvent | ||
| { | ||
| DepartmentId = departmentId, | ||
| UserId = userId, | ||
| Type = type, | ||
| Before = before == null ? null : JsonConvert.SerializeObject(before), | ||
| After = after == null ? null : JsonConvert.SerializeObject(after), | ||
| Successful = true, | ||
| ServerName = Environment.MachineName | ||
| }); |
There was a problem hiding this comment.
Incomplete audit trail fails security and compliance requirements due to missing tamper-evident fields. Populate timestamp, actor.role, resource.id, trace_id, ip, and user_agent in the AuditEvent to ensure immutable, verifiable logging.
Kody rule violation: Emit tamper-evident audit logs with required fields
Prompt for LLM
File Core/Resgrid.Services/UnitTrackingService.cs:
Line 495 to 504:
Incomplete audit trail fails security and compliance requirements due to missing tamper-evident fields. Populate `timestamp`, `actor.role`, `resource.id`, `trace_id`, `ip`, and `user_agent` in the `AuditEvent` to ensure immutable, verifiable logging.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| foreach (var credential in credentials.Where(item => !item.RevokedOn.HasValue)) | ||
| { | ||
| credential.RevokedOn = revokedOn; | ||
| await _credentialsRepository.UpdateAsync(credential, cancellationToken); |
There was a problem hiding this comment.
N+1 query performance degradation identified due to issuing database updates per credential inside a foreach loop. Batch the update into a single bulk operation or use ExecuteUpdateAsync to minimize database round-trips.
Kody rule violation: Detect N+1 style queries and suggest batching
Prompt for LLM
File Core/Resgrid.Services/UnitTrackingService.cs:
Line 472:
N+1 query performance degradation identified due to issuing database updates per credential inside a foreach loop. Batch the update into a single bulk operation or use `ExecuteUpdateAsync` to minimize database round-trips.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var unitLocation = ObjectSerialization.Deserialize<UnitLocationEvent>(message) | ||
| ?? throw new InvalidOperationException("Unit location queue message could not be deserialized."); | ||
|
|
||
| await UnitLocationEventQueueReceived.Invoke(unitLocation); |
There was a problem hiding this comment.
NullReferenceException vulnerability identified where the public UnitLocationEventQueueReceived delegate is invoked without a null check. Capture the delegate into a local variable and verify it is non-null before invoking it to prevent runtime crashes.
Kody rule violation: Add null checks to prevent NullReferenceException
Prompt for LLM
File Providers/Resgrid.Providers.Bus.Rabbit/RabbitInboundQueueProvider.cs:
Line 660:
NullReferenceException vulnerability identified where the public `UnitLocationEventQueueReceived` delegate is invoked without a null check. Capture the delegate into a local variable and verify it is non-null before invoking it to prevent runtime crashes.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var listeners = _asyncListeners.GetOrAdd( | ||
| typeof(T), | ||
| _ => new ConcurrentDictionary<Guid, Func<object, Task>>()); | ||
| listeners[token] = message => listener((T)message); |
There was a problem hiding this comment.
InvalidCastException risk identified due to a direct cast on an object parameter without type verification. Replace the direct cast with pattern matching or the as operator to ensure safe type evaluation before listener invocation.
Kody rule violation: Use safe type casting with as operator
Prompt for LLM
File Providers/Resgrid.Providers.Bus/EventAggregator.cs:
Line 48:
InvalidCastException risk identified due to a direct cast on an object parameter without type verification. Replace the direct cast with pattern matching or the `as` operator to ensure safe type evaluation before listener invocation.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (!await _rabbitTopicProvider.UnitLocationUpdatedChanged(message)) | ||
| throw new InvalidOperationException("Unable to publish the Unit location realtime update."); |
There was a problem hiding this comment.
Transient RabbitMQ realtime-publish failures in unitLocationUpdatedTopicHandler now propagate through UnitLocationQueueLogic.ProcessUnitLocationQueueItem into the queue consumer's retry path, requeuing already-persisted messages and causing duplicate inserts. Swallow or log the publish failure in the handler instead of throwing, or wrap the SendMessageAsync call in AddUnitLocationAsync with a try/catch to prevent realtime side-effects from failing the committed primary persistence operation.
if (!await _rabbitTopicProvider.UnitLocationUpdatedChanged(message))
Logging.LogError("Unable to publish the Unit location realtime update for department {DepartmentId}.", message.DepartmentId);Prompt for LLM
File Providers/Resgrid.Providers.Bus/OutboundEventProvider.cs:
Line 618 to 619:
Transient RabbitMQ realtime-publish failures in `unitLocationUpdatedTopicHandler` now propagate through `UnitLocationQueueLogic.ProcessUnitLocationQueueItem` into the queue consumer's retry path, requeuing already-persisted messages and causing duplicate inserts. Swallow or log the publish failure in the handler instead of throwing, or wrap the `SendMessageAsync` call in `AddUnitLocationAsync` with a try/catch to prevent realtime side-effects from failing the committed primary persistence operation.
Suggested Code:
if (!await _rabbitTopicProvider.UnitLocationUpdatedChanged(message))
Logging.LogError("Unable to publish the Unit location realtime update for department {DepartmentId}.", message.DepartmentId);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| .WithColumn("headername").AsCustom("citext").Nullable() | ||
| .WithColumn("basicusername").AsCustom("citext").Nullable() | ||
| .WithColumn("keyprefix").AsCustom("citext").NotNullable() | ||
| .WithColumn("secrethash").AsCustom("citext").NotNullable() |
There was a problem hiding this comment.
Case-insensitive text declaration on the secrethash column breaks cryptographic hash comparisons and authentication logic. Use AsCustom("text") instead of AsCustom("citext") to enforce exact byte-level hash equality.
Kody rule violation: Optimize string column types
Prompt for LLM
File Providers/Resgrid.Providers.MigrationsPg/Migrations/M0101_AddUnitTrackingPg.cs:
Line 90:
Case-insensitive text declaration on the `secrethash` column breaks cryptographic hash comparisons and authentication logic. Use `AsCustom("text")` instead of `AsCustom("citext")` to enforce exact byte-level hash equality.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| CREATE UNIQUE INDEX IF NOT EXISTS ux_unitlocations_eventid | ||
| ON public.unitlocations (eventid) | ||
| WHERE eventid IS NOT NULL; |
There was a problem hiding this comment.
Database locking risk identified because creating a unique index without CONCURRENTLY acquires an ACCESS EXCLUSIVE lock on the unitlocations table. Use CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS to prevent blocking reads and writes during index creation on large tables.
Kody rule violation: Block risky database migrations (locking ops, downtime risk)
Prompt for LLM
File Providers/Resgrid.Providers.MigrationsPg/Sql/EF0003_PopulateDocDb.sql:
Line 30 to 32:
Database locking risk identified because creating a unique index without `CONCURRENTLY` acquires an `ACCESS EXCLUSIVE` lock on the `unitlocations` table. Use `CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS` to prevent blocking reads and writes during index creation on large tables.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var query = _queryFactory.GetQuery<SelectUnitTrackingDevicesByUnitQuery>(); | ||
|
|
||
| return await WithConnectionAsync(connection => | ||
| connection.QueryAsync<UnitTrackingDevice>(query, parameters, _unitOfWork.Transaction)); |
There was a problem hiding this comment.
NullReferenceException vulnerability identified by accessing _unitOfWork.Transaction without a null guard inside a Dapper lambda. Capture the transaction into a local variable using _unitOfWork?.Transaction before passing it to the lambda to align with existing null-safe fallback paths.
Kody rule violation: Add null checks before accessing properties
Prompt for LLM
File Repositories/Resgrid.Repositories.DataRepository/UnitTrackingDevicesRepository.cs:
Line 44:
NullReferenceException vulnerability identified by accessing `_unitOfWork.Transaction` without a null guard inside a Dapper lambda. Capture the transaction into a local variable using `_unitOfWork?.Transaction` before passing it to the lambda to align with existing null-safe fallback paths.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <ProjectReference Include="..\..\Providers\Resgrid.Providers.Messaging\Resgrid.Providers.Messaging.csproj" /> | ||
| <ProjectReference Include="..\..\Repositories\Resgrid.Repositories.DataRepository\Resgrid.Repositories.DataRepository.csproj" /> | ||
| <ProjectReference Include="..\..\Repositories\Resgrid.Repositories.NoSqlRepository\Resgrid.Repositories.NoSqlRepository.csproj" /> | ||
| <ProjectReference Include="..\..\Web\Resgrid.Web\Resgrid.Web.csproj" /> |
There was a problem hiding this comment.
Architecture boundary violation identified by coupling the general test project directly to the presentation layer (Resgrid.Web). Move the tested logic into a service or domain object, or relocate web-layer integration tests to a dedicated assembly to maintain proper separation of concerns.
Kody rule violation: Enforce architecture boundaries and layering rules
Prompt for LLM
File Tests/Resgrid.Tests/Resgrid.Tests.csproj:
Line 58:
Architecture boundary violation identified by coupling the general test project directly to the presentation layer (`Resgrid.Web`). Move the tested logic into a service or domain object, or relocate web-layer integration tests to a dedicated assembly to maintain proper separation of concerns.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| // Assert | ||
| profiles.Select(profile => profile.Key) | ||
| .Should().BeEquivalentTo("generic-https", "traccar-forwarder"); |
There was a problem hiding this comment.
Inline magic strings representing a closed set of profile keys are brittle and resist refactoring. Define a static class or enum, such as UnitTrackingProfileKeys, to reference named members instead of raw literals.
Kody rule violation: Use enums instead of magic strings
Prompt for LLM
File Tests/Resgrid.Tests/Services/UnitTrackingCatalogServiceTests.cs:
Line 24:
Inline magic strings representing a closed set of profile keys are brittle and resist refactoring. Define a static class or enum, such as `UnitTrackingProfileKeys`, to reference named members instead of raw literals.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| [TestFixture] | ||
| public class UnitTrackingIdentifierServiceTests | ||
| { | ||
| private UnitTrackingIdentifierService _service; |
There was a problem hiding this comment.
Uninitialized fields like _service remain null outside the [SetUp] lifecycle, violating rules for sensible defaults. Initialize the field at declaration using null! or via a constructor to guarantee a non-null state.
Kody rule violation: Initialize properties with default values
Prompt for LLM
File Tests/Resgrid.Tests/Services/UnitTrackingIdentifierServiceTests.cs:
Line 11:
Uninitialized fields like `_service` remain null outside the `[SetUp]` lifecycle, violating rules for sensible defaults. Initialize the field at declaration using `null!` or via a constructor to guarantee a non-null state.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| Longitude = "-122.3321" | ||
| }); | ||
|
|
||
| response.Result.Should().BeOfType<StatusCodeResult>() |
There was a problem hiding this comment.
Deadlock vulnerability identified due to blocking async methods with .Result or .Wait(). Use await instead to ensure proper asynchronous execution and prevent thread starvation.
Kody rule violation: Avoid Blocking Calls to Async Methods
Prompt for LLM
File Tests/Resgrid.Tests/Web/Services/UnitLocationControllerTests.cs:
Line 74:
Deadlock vulnerability identified due to blocking async methods with `.Result` or `.Wait()`. Use `await` instead to ensure proper asynchronous execution and prevent thread starvation.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| Longitude = "-122.3321" | ||
| }); | ||
|
|
||
| response.Result.Should().BeOfType<StatusCodeResult>() |
There was a problem hiding this comment.
Synchronous blocking on async operations via .Result or .Wait() risks deadlocks and prevents efficient asynchronous execution. Await Tasks end-to-end and configure awaits appropriately to eliminate blocking calls.
Kody rule violation: Await async operations properly
Prompt for LLM
File Tests/Resgrid.Tests/Web/Services/UnitLocationControllerTests.cs:
Line 74:
Synchronous blocking on async operations via `.Result` or `.Wait()` risks deadlocks and prevents efficient asynchronous execution. Await Tasks end-to-end and configure awaits appropriately to eliminate blocking calls.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| try | ||
| { | ||
| var decodedBytes = Convert.FromBase64String(encoded); |
There was a problem hiding this comment.
Exception-driven control flow triggered by Convert.FromBase64String throwing FormatException on invalid HTTP header input. Replace the Parse-style API with Convert.TryFromBase64String to eliminate the try/catch block and handle IO failures safely.
Kody rule violation: Use TryParse for string conversions
Prompt for LLM
File Web/Resgrid.Web.Services/ApplicationCore/UnitTracking/UnitTrackingHttpAuthenticationService.cs:
Line 135:
Exception-driven control flow triggered by `Convert.FromBase64String` throwing `FormatException` on invalid HTTP header input. Replace the Parse-style API with `Convert.TryFromBase64String` to eliminate the try/catch block and handle IO failures safely.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| foreach (var device in devices) | ||
| mapped.Add(await MapDeviceAsync(device, canManage, false, cancellationToken)); |
There was a problem hiding this comment.
N+1 query pattern identified where MapDeviceAsync triggers individual database roundtrips for each device's statuses and credentials. Add batch service methods to query the entire device set at once and assemble the DTOs in memory to improve performance.
Kody rule violation: Optimize database queries with JOINs
Prompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/UnitTrackingDevicesController.cs:
Line 69 to 70:
N+1 query pattern identified where `MapDeviceAsync` triggers individual database roundtrips for each device's statuses and credentials. Add batch service methods to query the entire device set at once and assemble the DTOs in memory to improve performance.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| public async Task<ActionResult<UnitTrackingCredentialProvisionData>> CreateCredential( | ||
| string id, | ||
| [FromBody] CreateUnitTrackingCredentialInput input, | ||
| CancellationToken cancellationToken) | ||
| { | ||
| var authorization = await AuthorizeDeviceUpdateAsync(id); | ||
| if (authorization.Device == null) | ||
| return NotFound(); | ||
| if (!authorization.Allowed) | ||
| return Forbid(); | ||
| if (input == null || | ||
| !Enum.IsDefined(typeof(UnitTrackingAuthMode), input.AuthMode) || | ||
| input.AuthMode == (int)UnitTrackingAuthMode.Unknown) | ||
| return BadRequest(); |
There was a problem hiding this comment.
Insufficient authorization controls allow credential creation without step-up MFA, violating security rules for privileged operations. Assert a recent MFA claim where mfa_age <= 300s at the start of the action, return 403 if absent or stale, and stamp mfa_verified_at on the audit context.
Kody rule violation: Require step-up MFA for privileged operations
Prompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/UnitTrackingDevicesController.cs:
Line 184 to 197:
Insufficient authorization controls allow credential creation without step-up MFA, violating security rules for privileged operations. Assert a recent MFA claim where `mfa_age <= 300s` at the start of the action, return 403 if absent or stale, and stamp `mfa_verified_at` on the audit context.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (authentication.Status == UnitTrackingHttpAuthenticationStatus.NotFound) | ||
| return UnknownEndpointResponse(); | ||
| if (authentication.Status != UnitTrackingHttpAuthenticationStatus.Authenticated) | ||
| return Unauthorized(); | ||
|
|
||
| return await AcceptAsync(authentication.Source); |
There was a problem hiding this comment.
Unthrottled failed-authentication requests against known device endpoints allow attackers to drive unbounded DB queries via GetBySecretHashAsync and brute-force credentials, because PostPositions returns Unauthorized() at line 78 without consuming any rate-limit counter. Apply a rate-limit check, such as CheckUnknownEndpoint keyed on RemoteIpAddress or CheckRequest keyed on deviceId, before evaluating the authentication outcome to mirror the PostCapability path.
if (authentication.Status == UnitTrackingHttpAuthenticationStatus.NotFound)
return UnknownEndpointResponse();
if (authentication.Status != UnitTrackingHttpAuthenticationStatus.Authenticated)
{
var failedLimit = _rateLimiter.CheckUnknownEndpoint(
HttpContext.Connection.RemoteIpAddress?.ToString());
return failedLimit.Allowed ? Unauthorized() : RateLimited(failedLimit);
}
return await AcceptAsync(authentication.Source);Prompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/UnitTrackingIngressController.cs:
Line 75 to 80:
Unthrottled failed-authentication requests against known device endpoints allow attackers to drive unbounded DB queries via `GetBySecretHashAsync` and brute-force credentials, because `PostPositions` returns `Unauthorized()` at line 78 without consuming any rate-limit counter. Apply a rate-limit check, such as `CheckUnknownEndpoint` keyed on `RemoteIpAddress` or `CheckRequest` keyed on `deviceId`, before evaluating the authentication outcome to mirror the `PostCapability` path.
Suggested Code:
if (authentication.Status == UnitTrackingHttpAuthenticationStatus.NotFound)
return UnknownEndpointResponse();
if (authentication.Status != UnitTrackingHttpAuthenticationStatus.Authenticated)
{
var failedLimit = _rateLimiter.CheckUnknownEndpoint(
HttpContext.Connection.RemoteIpAddress?.ToString());
return failedLimit.Allowed ? Unauthorized() : RateLimited(failedLimit);
}
return await AcceptAsync(authentication.Source);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var parsed = await _payloadParser.ParseAsync( | ||
| Request, | ||
| receivedOn, | ||
| HttpContext.RequestAborted); |
There was a problem hiding this comment.
Unhandled I/O exceptions from _payloadParser.ParseAsync bubble up to ASP.NET's default handler, producing contextless 500 errors unlike other awaited calls in the controller. Wrap the call in a try/catch block, rethrowing OperationCanceledException and returning a contextual 503 response for general exceptions.
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/UnitTrackingIngressController.cs:
Line 140 to 143:
Unhandled I/O exceptions from `_payloadParser.ParseAsync` bubble up to ASP.NET's default handler, producing contextless 500 errors unlike other awaited calls in the controller. Wrap the call in a try/catch block, rethrowing `OperationCanceledException` and returning a contextual 503 response for general exceptions.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var parsed = await _payloadParser.ParseAsync( | ||
| Request, | ||
| receivedOn, | ||
| HttpContext.RequestAborted); |
There was a problem hiding this comment.
Unhandled exceptions from external I/O operations in ParseAsync yield raw 500 errors without operational context. Wrap the call in a try/catch block to map exceptions to appropriate HTTP status codes, such as 422 for parse failures and 503 for infrastructure errors, while including relevant diagnostic context.
Kody rule violation: Add try-catch blocks for external calls
Prompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/UnitTrackingIngressController.cs:
Line 140 to 143:
Unhandled exceptions from external I/O operations in `ParseAsync` yield raw 500 errors without operational context. Wrap the call in a try/catch block to map exceptions to appropriate HTTP status codes, such as 422 for parse failures and 503 for infrastructure errors, while including relevant diagnostic context.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| private IActionResult Unavailable(Exception exception, string message) | ||
| { | ||
| Logging.LogException(exception, message); |
There was a problem hiding this comment.
Unstructured error logs lacking operation names, device IDs, and trace IDs prevent effective correlation and diagnostics across services. Include relevant identifiers and context as structured parameters alongside the exception in Logging.LogException calls.
Kody rule violation: Include error context in structured logs
Prompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/UnitTrackingIngressController.cs:
Line 227:
Unstructured error logs lacking operation names, device IDs, and trace IDs prevent effective correlation and diagnostics across services. Include relevant identifiers and context as structured parameters alongside the exception in `Logging.LogException` calls.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| function updateProfileHelp() { | ||
| const option = select.options[select.selectedIndex]; | ||
| summary.textContent = option && option.dataset.summary | ||
| ? option.dataset.summary + " " + (option.dataset.retry || "") |
There was a problem hiding this comment.
Error-prone string concatenation utilized for building profile help text. Use template literals to improve readability and safely interpolate dataset properties.
Kody rule violation: Use Template Literals Instead of String Concatenation
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Views/UnitTracking/_Editor.cshtml:
Line 86:
Error-prone string concatenation utilized for building profile help text. Use template literals to improve readability and safely interpolate dataset properties.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
3 participants
Description
This PR implements comprehensive hardware GPS unit tracking support (RG-T127), enabling departments to bind physical hardware trackers or forwarding services to Units and receive live position data through authenticated HTTPS endpoints.
Key additions
Device & credential management
UnitTrackingDevicesandUnitTrackingCredentialsdata models with full database migrations (SQL Server and PostgreSQL)no-storecaching headers; stored secrets are never returned by APIsPosition ingestion pipeline
/api/v4/unit-trackers/{id}/positionsand capability-path variant) accepting single or batched JSON position payloadsLocation resolution and status
UnitLocationSourceResolverthat selects the freshest, highest-priority location across hardware and mobile-app sources, with configurable staleness thresholds and mobile fallback behaviorUnitTrackingStatusServicecomputing effective device status (Online, Stale, Error, Disabled, NeverSeen)UnitsLocationandUnitLocationEventdocuments with source metadata and rich telemetry (satellites, HDOP, battery, ignition, alarm codes, etc.)Web UI and API
User/UnitTracking) with views for listing, creating, editing, and managing tracker bindings, credentials, and a JSON preview tool (non-production only)UnitTrackingDevicesController) for programmatic lifecycle management with authorization scoped to Unit view/update permissionsSecurity and observability
Infrastructure improvements
Inserted/Duplicatestatus to suppress duplicate realtime events