DYN-6055 Lucene Search Category Based. #14663

RobertGlobant20 · 2023-11-30T18:38:42Z

Purpose

Implemeting Lucene Search Category Based.
I've updated the Lucene Search in a way that if the user type the "." character then the search will be "category based" (e.g. the search criteria "list.r" will find all the nodes which belong to the list category and the node name starts with r).
For this implementation I've indexed two new fields: NameSplitted and CategorySplitted. For NameSplitted when the node name contains the Category (like List.Shop) then we will be using the last part (after the "." character), the same case for CategorySplitted, we will be using the last part after the "." character.

Declarations

Check these if you believe they are true

The codebase is in a better state after this PR
Is documented according to the standards
The level of testing this PR includes is appropriate
User facing strings, if any, are extracted into *.resx files
All tests pass using the self-service CI.
Snapshot of UI changes, if any.
Changes to the API follow Semantic Versioning and are documented in the API Changes document.
This PR modifies some build requirements and the readme is updated
This PR contains no files larger than 50 MB

Release Notes

Implemeting Lucene Search Category Based.

Reviewers

@QilongTang

FYIs

@Amoursol

I've updated the Lucene Search in a way that if the user type the "." character then the search will be "category based" (e.g. the search criteria "list.r" will find all the nodes which belong to the list category and the node name starts with r). For this implementation I've indexed two new fields: NameSplitted and CategorySplitted. For NameSplitted when the node name contains the Category (like List.Shop) then we will be using the last part (after the "." character), the same case for CategorySplitted, we will be using the last part after the "." character.

RobertGlobant20 · 2023-11-30T18:39:22Z

This the GIF showing the added search behavior.

QilongTang · 2023-12-05T13:22:04Z

src/DynamoCore/Utilities/LuceneSearchUtility.cs

+                        f != nameof(LuceneConfig.NodeFieldsEnum.CategorySplitted))
+                        continue;
+
+                    var categorySearchBased = searchTerm.Split('.');


Would there be an edge case where we have, A.B.C? So the category would be A, and node name is B.C?

@QilongTang for this implementation I'm just considering the base "category.node" search criteria, if we want for support more complex categories we will need think in another solution.
For this case I can add some code to prevent a search of the type "A.B.C" if that's the case we won't return results, what do you think?

Hi @RobertGlobant20 Not necessarily, I am trying to confirm that when the user search for A.B.C, with this current code, we are searching for B.C node under the category A right?

I've added the validation for supporting the criteria A.B.C, not sure if this is what you expected (if not please put a more detailed example), see the GIF attached with the behavior.
commit: a3f7bd6

QilongTang · 2023-12-06T13:21:20Z

src/DynamoCore/Utilities/LuceneSearchUtility.cs

@@ -317,19 +355,31 @@ private WildcardQuery CalculateFieldWeight(string fieldName, string searchTerm,
        {
            WildcardQuery query;

+            var termText = fieldName == nameof(LuceneConfig.NodeFieldsEnum.NameSplitted) ? searchTerm + "*" : "*" + searchTerm + "*";


Would you add some comments on this line?

I've added a comment in the next commit: a3f7bd6

QilongTang · 2023-12-06T13:28:41Z

src/DynamoCore/Utilities/LuceneSearchUtility.cs

+
+            var categoryParts = node.FullCategoryName.Split('.');
+            string categoryParsed = categoryParts.Length > 1 ? categoryParts[categoryParts.Length - 1] : node.FullCategoryName;
+            SetDocumentFieldValue(doc, nameof(LuceneConfig.NodeFieldsEnum.CategorySplitted), categoryParsed);


Would you add some comments?

I've added a comment in the next commit: a3f7bd6

QilongTang

Overall looks good, need some more comments for book keeping and could use a unit test if doable

I've added more comments I've changed the validation for always taking the last two sections (the NameSplitted can be empty due that later there is a validation) so if the search criteria use a large category like "Core.File.FileSystem.A". it will take only the last two sections.

RobertGlobant20 · 2023-12-06T20:37:02Z

Overall looks good, need some more comments for book keeping and could use a unit test if doable

@QilongTang does it mean that I need to add a unit test for this behavior?

github-actions · 2023-12-06T20:55:07Z

UI Smoke Tests

Test: success. 2 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests - net8.0

github-actions · 2023-12-06T20:59:22Z

UI Smoke Tests

Test: success. 2 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests - net6.0

QilongTang · 2023-12-07T07:16:33Z

Overall looks good, need some more comments for book keeping and could use a unit test if doable

@QilongTang does it mean that I need to add a unit test for this behavior?

Please do

Adding a unit test that will validate category search based.

RobertGlobant20 · 2023-12-07T16:01:00Z

Overall looks good, need some more comments for book keeping and could use a unit test if doable

@QilongTang does it mean that I need to add a unit test for this behavior?

Please do

@QilongTang a new unit test for validate this functionality was added, please check the next commit: d38e79a

github-actions · 2023-12-07T16:29:54Z

UI Smoke Tests

Test: success. 2 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests - net6.0

github-actions · 2023-12-07T16:32:55Z

UI Smoke Tests

Test: success. 2 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests - net8.0

QilongTang

LGTM

* DYN-6055 Lucene Search Category Based. I've updated the Lucene Search in a way that if the user type the "." character then the search will be "category based" (e.g. the search criteria "list.r" will find all the nodes which belong to the list category and the node name starts with r). For this implementation I've indexed two new fields: NameSplitted and CategorySplitted. For NameSplitted when the node name contains the Category (like List.Shop) then we will be using the last part (after the "." character), the same case for CategorySplitted, we will be using the last part after the "." character. * DYN-6055 Lucene Search Category Based Code Review I've added more comments I've changed the validation for always taking the last two sections (the NameSplitted can be empty due that later there is a validation) so if the search criteria use a large category like "Core.File.FileSystem.A". it will take only the last two sections. * DYN-6055 Lucene Search Category Based Code Review Adding a unit test that will validate category search based.

* DYN-6524 Packages Tour Regression (#14721) I did several updates for the Packages guide: I've splitted the functionality of collapseExpandPackage so now we will have one method for collapse and another one for expand, so the function collapseExpandPackage was renamed to expandPackageDiv. Some Steps in the dynamo_guides.json were modified to stick to the functionality of expanding the package when passing from Step8 to Step9. Finally I added a validation in the function that highlight a div so if the div is already highlighted when we don't add the functionality ( this will prevent showing the orange rectangle when closing the guide ). * DYN-6055 Lucene Search Category Based. (#14663) * DYN-6055 Lucene Search Category Based. I've updated the Lucene Search in a way that if the user type the "." character then the search will be "category based" (e.g. the search criteria "list.r" will find all the nodes which belong to the list category and the node name starts with r). For this implementation I've indexed two new fields: NameSplitted and CategorySplitted. For NameSplitted when the node name contains the Category (like List.Shop) then we will be using the last part (after the "." character), the same case for CategorySplitted, we will be using the last part after the "." character. * DYN-6055 Lucene Search Category Based Code Review I've added more comments I've changed the validation for always taking the last two sections (the NameSplitted can be empty due that later there is a validation) so if the search criteria use a large category like "Core.File.FileSystem.A". it will take only the last two sections. * DYN-6055 Lucene Search Category Based Code Review Adding a unit test that will validate category search based. * Fixing Search Regressions (#14738) For the test SearchingForACategoryReturnsAllItsChildren now with my changes related to category based search the term "Category.Child" will be searching the nodes with Category = Category and Name=Child so that's why was not returning results adding an extra "." in the search term fixed it. The same case for the test LuceneSearchNodesByCategoryValidation, was expecting all the nodes under the category "Core.Input" so we have to add an extra "." in the search term. For the test LuceneSearchNodesOrderingValidation I have to change a node due that with my changes the order changed and now is one position below (for the search term "list.join" we only guarante that the items at the top will be Category = list and the name starts with "join"). * mark test failure * Add selection handler after binding. (#14744) --------- Co-authored-by: Roberto T <[email protected]> Co-authored-by: Trygve Wastvedt <[email protected]>

RobertGlobant20 requested review from QilongTang and reddyashish November 30, 2023 18:38

QilongTang added this to the 3.0 milestone Dec 5, 2023

QilongTang reviewed Dec 5, 2023

View reviewed changes

QilongTang reviewed Dec 6, 2023

View reviewed changes

DYN-6055 Lucene Search Category Based Code Review

d38e79a

Adding a unit test that will validate category search based.

QilongTang approved these changes Dec 11, 2023

View reviewed changes

QilongTang merged commit eaff9b3 into DynamoDS:master Dec 11, 2023
22 checks passed

QilongTang mentioned this pull request Dec 11, 2023

Cherry-picks #14733

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DYN-6055 Lucene Search Category Based. #14663

DYN-6055 Lucene Search Category Based. #14663

RobertGlobant20 commented Nov 30, 2023 •

edited

Loading

RobertGlobant20 commented Nov 30, 2023

QilongTang Dec 5, 2023

RobertGlobant20 Dec 5, 2023

QilongTang Dec 6, 2023 •

edited

Loading

RobertGlobant20 Dec 6, 2023

QilongTang Dec 6, 2023

RobertGlobant20 Dec 6, 2023

QilongTang Dec 6, 2023

RobertGlobant20 Dec 6, 2023

QilongTang left a comment

RobertGlobant20 commented Dec 6, 2023

github-actions bot commented Dec 6, 2023

github-actions bot commented Dec 6, 2023

QilongTang commented Dec 7, 2023

RobertGlobant20 commented Dec 7, 2023

github-actions bot commented Dec 7, 2023

github-actions bot commented Dec 7, 2023

QilongTang left a comment

DYN-6055 Lucene Search Category Based. #14663

DYN-6055 Lucene Search Category Based. #14663

Conversation

RobertGlobant20 commented Nov 30, 2023 • edited Loading

Purpose

Declarations

Release Notes

Reviewers

FYIs

RobertGlobant20 commented Nov 30, 2023

QilongTang Dec 5, 2023

Choose a reason for hiding this comment

RobertGlobant20 Dec 5, 2023

Choose a reason for hiding this comment

QilongTang Dec 6, 2023 • edited Loading

Choose a reason for hiding this comment

RobertGlobant20 Dec 6, 2023

Choose a reason for hiding this comment

QilongTang Dec 6, 2023

Choose a reason for hiding this comment

RobertGlobant20 Dec 6, 2023

Choose a reason for hiding this comment

QilongTang Dec 6, 2023

Choose a reason for hiding this comment

RobertGlobant20 Dec 6, 2023

Choose a reason for hiding this comment

QilongTang left a comment

Choose a reason for hiding this comment

RobertGlobant20 commented Dec 6, 2023

github-actions bot commented Dec 6, 2023

UI Smoke Tests

github-actions bot commented Dec 6, 2023

UI Smoke Tests

QilongTang commented Dec 7, 2023

RobertGlobant20 commented Dec 7, 2023

github-actions bot commented Dec 7, 2023

UI Smoke Tests

github-actions bot commented Dec 7, 2023

UI Smoke Tests

QilongTang left a comment

Choose a reason for hiding this comment

RobertGlobant20 commented Nov 30, 2023 •

edited

Loading

QilongTang Dec 6, 2023 •

edited

Loading