Skip to content

Make provider fully overridable#34

Merged
ksavosteev merged 28 commits intodevfrom
feat/protected-provider
Nov 26, 2025
Merged

Make provider fully overridable#34
ksavosteev merged 28 commits intodevfrom
feat/protected-provider

Conversation

@mihey8800
Copy link
Copy Markdown
Contributor

@mihey8800 mihey8800 commented Oct 23, 2025

Description

  • provider fully overridable. protected methods added to all public methods
  • implement ISupportSuggestions

References

QA-test:

Jira-link:

Artifact URL:

https://vc3prerelease.blob.core.windows.net/packages/VirtoCommerce.ElasticSearch8_3.823.0-pr-34-a375.zip

@vc-ci
Copy link
Copy Markdown
Contributor

vc-ci commented Oct 23, 2025

Review task created: https://virtocommerce.atlassian.net/browse/VCST-4180

@cursor
Copy link
Copy Markdown

cursor Bot commented Oct 23, 2025

Bug: Missing Virtual Keyword in Method

The InternalAddActiveAlias method is missing the virtual keyword. This makes it non-overridable, which is inconsistent with other Internal methods and prevents derived classes from fully extending the provider as intended.

Fix in Cursor Fix in Web

@cursor
Copy link
Copy Markdown

cursor Bot commented Oct 23, 2025

Bug: Missing Virtual Keyword in Internal Method

The InternalAddActiveAlias method is missing the virtual keyword. This makes it inconsistent with other Internal* methods and prevents overriding in subclasses, which goes against the goal of a fully overridable provider.

Fix in Cursor Fix in Web

@OlegoO OlegoO requested a review from alexeyshibanov October 23, 2025 10:39
Copy link
Copy Markdown
Contributor

@vc-ci vc-ci left a comment

Choose a reason for hiding this comment

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

Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 7.455
Timestamp: 23-10-2025T10:35:14

Copy link
Copy Markdown
Contributor

@vc-ci vc-ci left a comment

Choose a reason for hiding this comment

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

Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 7.323
Timestamp: 23-10-2025T10:44:51

Comment thread src/VirtoCommerce.ElasticSearch8.Data/Services/ElasticSearch8Provider.cs Outdated
Comment thread src/VirtoCommerce.ElasticSearch8.Data/Services/ElasticSearch8Provider.cs Outdated
Comment thread src/VirtoCommerce.ElasticSearch8.Data/Services/ElasticSearch8Provider.cs Outdated
Comment thread src/VirtoCommerce.ElasticSearch8.Data/Services/ElasticSearch8Provider.cs Outdated
Comment thread src/VirtoCommerce.ElasticSearch8.Data/Services/ElasticSearch8Provider.cs Outdated
Comment thread src/VirtoCommerce.ElasticSearch8.Data/Services/ElasticSearch8Provider.cs Outdated
Comment thread src/VirtoCommerce.ElasticSearch8.Data/Services/ElasticSearch8Provider.cs Outdated
Comment thread src/VirtoCommerce.ElasticSearch8.Data/Services/ElasticSearch8Provider.cs Outdated
Comment thread src/VirtoCommerce.ElasticSearch8.Data/Services/ElasticSearch8Provider.cs Outdated
Copy link
Copy Markdown
Contributor

@vc-ci vc-ci left a comment

Choose a reason for hiding this comment

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

Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 8.156
Timestamp: 23-10-2025T11:51:24

Copy link
Copy Markdown
Contributor

@vc-ci vc-ci left a comment

Choose a reason for hiding this comment

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

Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 7.961
Timestamp: 23-10-2025T12:04:41

Copy link
Copy Markdown
Contributor

@vc-ci vc-ci left a comment

Choose a reason for hiding this comment

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

Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 7.727
Timestamp: 23-10-2025T12:40:53

@mihey8800 mihey8800 requested a review from OlegoO October 29, 2025 11:56
Copy link
Copy Markdown
Contributor

@vc-ci vc-ci left a comment

Choose a reason for hiding this comment

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

Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 7.663
Timestamp: 29-10-2025T11:58:28

Copy link
Copy Markdown
Contributor

@vc-ci vc-ci left a comment

Choose a reason for hiding this comment

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

Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 7.592
Timestamp: 29-10-2025T15:15:00

Copy link
Copy Markdown
Contributor

@vc-ci vc-ci left a comment

Choose a reason for hiding this comment

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

Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 7.636
Timestamp: 03-11-2025T11:06:22

@alexeyshibanov alexeyshibanov marked this pull request as draft November 5, 2025 17:47
Copy link
Copy Markdown
Contributor

@vc-ci vc-ci left a comment

Choose a reason for hiding this comment

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

Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 7.07
Timestamp: 06-11-2025T06:54:04

Copy link
Copy Markdown
Contributor

@vc-ci vc-ci left a comment

Choose a reason for hiding this comment

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

Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 7.737
Timestamp: 06-11-2025T14:05:47

Copy link
Copy Markdown
Contributor

@vc-ci vc-ci left a comment

Choose a reason for hiding this comment

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

Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 7.614
Timestamp: 08-11-2025T14:53:21

Copy link
Copy Markdown
Contributor

@vc-ci vc-ci left a comment

Choose a reason for hiding this comment

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

Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 7.534
Timestamp: 09-11-2025T04:41:56

Copy link
Copy Markdown
Contributor

@vc-ci vc-ci left a comment

Choose a reason for hiding this comment

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

Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 7.679
Timestamp: 09-11-2025T05:32:31

Copy link
Copy Markdown
Contributor

@vc-ci vc-ci left a comment

Choose a reason for hiding this comment

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

Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 7.791
Timestamp: 09-11-2025T06:50:34

@alexeyshibanov alexeyshibanov marked this pull request as ready for review November 9, 2025 10:13
Copy link
Copy Markdown
Contributor

@vc-ci vc-ci left a comment

Choose a reason for hiding this comment

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

Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 8.184
Timestamp: 13-11-2025T10:38:12

Copy link
Copy Markdown
Contributor

@vc-ci vc-ci left a comment

Choose a reason for hiding this comment

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

Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 7.811
Timestamp: 14-11-2025T09:55:36

Copy link
Copy Markdown
Contributor

@vc-ci vc-ci left a comment

Choose a reason for hiding this comment

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

Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 7.624
Timestamp: 17-11-2025T18:11:54

appBuilder.UseSearchProvider<ElasticSearch8Provider>(ModuleConstants.ProviderName, (provider, documentTypes) =>
{
provider.AddActiveAlias(documentTypes);
_ = provider.AddActiveAlias(documentTypes);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: Async method called without awaiting in PostInitialize

The AddActiveAlias method returns a Task but is called with a discard operator without awaiting. This fire-and-forget pattern means the application initialization continues before the aliases are created, potentially causing race conditions where search operations fail because aliases haven't been set up yet. The PostInitialize method is synchronous, so either it should be made async to properly await the call, or the call should be wrapped in a synchronous wait if blocking is acceptable during initialization.

Fix in Cursor Fix in Web

Copy link
Copy Markdown
Contributor

@vc-ci vc-ci left a comment

Choose a reason for hiding this comment

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

Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 7.786
Timestamp: 19-11-2025T14:28:05

Copy link
Copy Markdown
Contributor

@vc-ci vc-ci left a comment

Choose a reason for hiding this comment

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

Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 7.196
Timestamp: 21-11-2025T08:41:32

Copy link
Copy Markdown
Contributor

@vc-ci vc-ci left a comment

Choose a reason for hiding this comment

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

Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 6.883
Timestamp: 21-11-2025T09:02:42

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Nov 26, 2025

Copy link
Copy Markdown
Contributor

@vc-ci vc-ci left a comment

Choose a reason for hiding this comment

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

Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 7.759
Timestamp: 26-11-2025T12:57:45

Copy link
Copy Markdown
Contributor

@vc-ci vc-ci left a comment

Choose a reason for hiding this comment

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

Test Suite: Test Suites/Modules/module_Assets
Tests: 13
Failures: 0
Errors: 0
Time: 7.87
Timestamp: 26-11-2025T13:07:56

@ksavosteev ksavosteev merged commit 1d67412 into dev Nov 26, 2025
9 checks passed
@artem-dudarev artem-dudarev deleted the feat/protected-provider branch November 27, 2025 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants