Skip to content

Comments

Hotfix 1#10

Merged
tsutomi merged 1 commit intomainfrom
hotfix-security
Aug 19, 2025
Merged

Hotfix 1#10
tsutomi merged 1 commit intomainfrom
hotfix-security

Conversation

@tsutomi
Copy link
Member

@tsutomi tsutomi commented Aug 19, 2025

This pull request refactors and enhances logging across the messaging connector components, streamlining logger extension classes and improving the clarity and consistency of authentication-related logs. It also tightens validation when registering channel connectors and makes a minor fix to connector display name logic.

Logging Refactoring and Improvements

  • Renamed logger extension classes to a unified LoggerExtensions name in both Deveel.Messaging.Connector.Abstractions and Deveel.Messaging.Connector.Firebase namespaces for consistency and maintainability. (src/Deveel.Messaging.Connector.Abstractions/Messaging/LoggerExtensions.cs, src/Deveel.Messaging.Connector.Firebase/Messaging/LoggerExtensions.cs) [1] [2]
  • Changed logger extension methods for authentication events to use the strongly-typed AuthenticationType enum instead of raw strings, improving type safety and log clarity. (src/Deveel.Messaging.Connector.Abstractions/Messaging/LoggerExtensions.cs, usages in ChannelConnectorBase.cs) [1] [2] [3]
  • Updated authentication failure and refresh logging to remove error message parameters, focusing logs on the authentication type or event instead. (src/Deveel.Messaging.Connector.Abstractions/Messaging/LoggerExtensions.cs, usages in ChannelConnectorBase.cs) [1] [2]

Firebase Connector Logging Enhancements

  • Added dedicated logger extension methods for Firebase service account authentication events, such as credential preparation, validation, and error handling, and refactored FirebaseServiceAccountAuthenticationProvider to use these methods for clearer, more structured logs. (src/Deveel.Messaging.Connector.Firebase/Messaging/LoggerExtensions.cs, usages in FirebaseServiceAccountAuthenticationProvider.cs) [1] [2] [3] [4] [5]

Channel Connector Registration and Descriptor Improvements

  • Improved the connector registration process in ChannelRegistryBuilder by enforcing that registered connector types must be decorated with the ChannelSchemaAttribute, ensuring proper schema association. (src/Deveel.Messaging.Connectors/Messaging/ChannelRegistryBuilder.cs)
  • Simplified the connector registration logic to delegate to the generic method and ensure consistent handling of connector factories. (src/Deveel.Messaging.Connectors/Messaging/ChannelRegistryBuilder.cs)
  • Fixed the logic for determining connector display names to properly fall back to the type name if the schema display name is null or whitespace. (src/Deveel.Messaging.Connectors/Messaging/ConnectorDescriptor.cs)

@tsutomi tsutomi self-assigned this Aug 19, 2025
Copilot AI review requested due to automatic review settings August 19, 2025 18:18
@tsutomi tsutomi added the enhancement New feature or request label Aug 19, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request adds comprehensive test coverage for the messaging connector components, specifically targeting edge cases, error handling scenarios, and improving code coverage for connector registration, descriptor functionality, and channel registry operations.

  • Adds extensive unit tests for ServiceCollectionExtensions, ConnectorDescriptor, ChannelRegistry, and ChannelRegistryBuilder classes
  • Introduces comprehensive error handling and edge case testing scenarios including disposal, concurrent operations, and validation
  • Includes tests for authentication provider functionality and various connector initialization scenarios

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/Deveel.Messaging.Connectors.XUnit/Messaging/ServiceCollectionExtensionsTests.cs New comprehensive test file covering service collection extension methods with edge cases and error validation
test/Deveel.Messaging.Connectors.XUnit/Messaging/ConnectorDescriptorExtendedTests.cs New test file providing extensive coverage of ConnectorDescriptor functionality including equality, validation, and property handling
test/Deveel.Messaging.Connectors.XUnit/Messaging/ChannelRegistryErrorHandlingTests.cs New test file focused on error handling scenarios in ChannelRegistry operations
test/Deveel.Messaging.Connectors.XUnit/Messaging/ChannelRegistryBuilderExtendedTests.cs New test file covering ChannelRegistryBuilder edge cases and hosted service registration
test/Deveel.Messaging.Connectors.XUnit/Messaging/ChannelRegistryAdvancedTests.cs New test file for advanced scenarios including disposal patterns and concurrent operations
test/Deveel.Messaging.Connectors.XUnit/Messaging/ChannelDescriptorExtendedTests.cs New test file covering ChannelDescriptor edge cases and comprehensive validation scenarios
src/Deveel.Messaging.Connectors/Messaging/ConnectorDescriptor.cs Fix for display name logic to handle whitespace properly
src/Deveel.Messaging.Connectors/Messaging/ChannelRegistryBuilder.cs Enhancement to validate schema attributes during registration and consolidate registration logic
src/Deveel.Messaging.Connector.Firebase/Messaging/LoggerExtensions.cs Renamed class and added Firebase-specific authentication logging methods
src/Deveel.Messaging.Connector.Firebase/Messaging/FirebaseServiceAccountAuthenticationProvider.cs Updated to use new structured logging methods
src/Deveel.Messaging.Connector.Abstractions/Messaging/LoggerExtensions.cs Renamed class and improved authentication logging with strongly-typed enums
src/Deveel.Messaging.Connector.Abstractions/Messaging/ChannelConnectorBase.cs Updated to use improved logging methods with typed authentication parameters

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

}

[Fact]
public void AddChannelRegistry_RegistersChannelRegistryAssingleton()
Copy link

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method name contains a typo: 'Assingleton' should be 'AsSingleton'.

Suggested change
public void AddChannelRegistry_RegistersChannelRegistryAssingleton()
public void AddChannelRegistry_RegistersChannelRegistryAsSingleton()

Copilot uses AI. Check for mistakes.
protected override async Task ShutdownConnectorAsync(CancellationToken cancellationToken)
{
// Simulate slow shutdown but still complete within reasonable time for sync disposal
await Task.Delay(3000, cancellationToken);
Copy link

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 3-second delay in ShutdownConnectorAsync could cause test performance issues. Consider reducing this delay or using a configurable timeout for testing.

Suggested change
await Task.Delay(3000, cancellationToken);
// Simulate slow shutdown but allow configurable delay for testing
await Task.Delay(_shutdownDelayMs, cancellationToken);

Copilot uses AI. Check for mistakes.
@tsutomi tsutomi merged commit 8a28005 into main Aug 19, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant