Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#3308 - Add fulltext search to redirect manager #3309

Merged
merged 7 commits into from Apr 15, 2024

Conversation

davidjgonzalez
Copy link
Contributor

@davidjgonzalez davidjgonzalez commented Apr 1, 2024

This performs a fulltext search over:

  • note
  • source
  • target
  • cq:tags. ... tho this is on tag IDs so isnt that useful
  • cacheControlHeader
  • createdBy
  • lastModifiedBy

2024-04-01 at 10 00 AM

2024-04-01 at 10 00 AM

For this to work the checkbox needs to be toggled on; and you need a supporting Oak index deployed (example below). If this is not checked, then it falls back to orginal search. When search is enabled, you cannot re-arrange rows.

2024-04-03 at 10 23 AM

I also made some cosmetic changes to the size of the table, and change the sling resource include for each row, to use the HTL template w/ RequestAttribute in SlingModel which i think speeds up the page rendering.

Oak index required

{
    "compatVersion": 2,
    "async": "async",
    "queryPaths": [
        "/conf"
    ],
    "includedPaths": [
        "/conf"
    ],
    "jcr:primaryType": "oak:QueryIndexDefinition",
    "evaluatePathRestrictions": true,
    "type": "lucene",
    "indexRules": {
        "jcr:primaryType": "nt:unstructured",
        "nt:unstructured": {
            "jcr:primaryType": "nt:unstructured",
            "properties": {
                "jcr:primaryType": "nt:unstructured",
                "cacheControlHeader": {
                    "name": "cacheControlHeader",
                    "analyzed": true,
                    "jcr:primaryType": "nt:unstructured"
                },
                "note": {
                    "name": "note",
                    "analyzed": true,
                    "jcr:primaryType": "nt:unstructured"
                },
                "createdBy": {
                    "name": "jcr:createdBy",
                    "analyzed": true,
                    "jcr:primaryType": "nt:unstructured"
                },
               "statusCode": {
                    "name": "statusCode",
                    "analyzed": true,
                    "jcr:primaryType": "nt:unstructured"
                }
                "tags": {
                    "name": "cq:tags",
                    "analyzed": true,
                    "jcr:primaryType": "nt:unstructured"
                },
                "source": {
                    "name": "source",
                    "analyzed": true,
                    "jcr:primaryType": "nt:unstructured"
                },
                "resourceType": {
                    "name": "sling:resourceType",
                    "propertyIndex": true,
                    "jcr:primaryType": "nt:unstructured"
                },
                "target": {
                    "name": "target",
                    "analyzed": true,
                    "jcr:primaryType": "nt:unstructured"
                },
                "statusCode": {
                    "name": "statusCode",
                    "analyzed": true,
                    "jcr:primaryType": "nt:unstructured"
                }
            }
        }
    }
}

Copy link

codecov bot commented Apr 1, 2024

Codecov Report

Attention: Patch coverage is 28.75000% with 57 lines in your changes are missing coverage. Please review.

Project coverage is 55.28%. Comparing base (7676ed5) to head (b271cea).
Report is 3 commits behind head on master.

❗ Current head b271cea differs from pull request most recent head 96531f5. Consider uploading reports for the commit 96531f5 to get more accurate results

Files Patch % Lines
.../adobe/acs/commons/redirects/models/Redirects.java 0.00% 53 Missing ⚠️
...obe/acs/commons/redirects/models/RedirectRule.java 85.18% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3309      +/-   ##
============================================
- Coverage     55.34%   55.28%   -0.06%     
- Complexity     5543     5545       +2     
============================================
  Files           728      728              
  Lines         29705    29769      +64     
  Branches       3875     3880       +5     
============================================
+ Hits          16440    16458      +18     
- Misses        11722    11767      +45     
- Partials       1543     1544       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@YegorKozlov
Copy link
Contributor

@davidjgonzalez Thanks! It looks very cool.

The original limit of 1000 is an overkill, we can surely drop it down to 100.

I think the last touch would be to warn users if the Oak index is missing. I think we can catch the exception and show an alert. Can't we?

@davidjgonzalez
Copy link
Contributor Author

@YegorKozlov I just made the page sizes (browse and search) configurable under Options; just incase someone likes the big pages.

Wrt to the missing - no exception is thrown -- i believe a WARN is just logged (atleast if there are not MAX_TRAVERSE_RESULTS are not there) ... I think the only ways to check this is:

  1. Run an Explain on the query before it runs, and determine if its traversal form that.
  2. Use a service user can check if a well-named index exists.

I updated the Options field for the checkbox, it display inline text stating an index is required.

Open to other ideas .. just not sure there are any great ones.

@davidjgonzalez
Copy link
Contributor Author

@YegorKozlov also - i originally was doing a jcr:contains fulltext search over the redirect row resource itself; however i could not figure out how to exclude the node name/path from the aggregate (yes - i tried excludeNodeName :)) .. .which is why it search on specific properties (which might be better anyhow?)

@davidjgonzalez davidjgonzalez merged commit 3432936 into Adobe-Consulting-Services:master Apr 15, 2024
9 of 10 checks passed
@davidjgonzalez davidjgonzalez deleted the 3308 branch April 15, 2024 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants