Uploaded image for project: 'eZ Publish / Platform'
  1. eZ Publish / Platform
  2. EZP-25088

Content items not indexed by legacySearchEngine

    Details

      Description

      Steps to reproduce:

      1. Install a master version without legacy;

      2. Import the attached bundle and register it on EzPublishKernel.php and on routing.yml;

      3. With the bundle working, from the ezpublish root, run the following command:

      php ezpublish/console test:testSearch footer
      

      You should get:

      Found: footer
      Execution terminated.
      

      4. Now, via platformUI, create a folder "folder" and on its short name, for instance write "footer" as well;

      5. From the ezpublish root, run the following command again:

      php ezpublish/console test:testSearch footer
      

      You still get only the default footer:

      Found: footer
      Execution terminated.
      

      even if "footer" is contained in a field of "folder";

      6. Run the following command:

      php ezpublish/console test:testSearch folder
      

      You'll get empty results:

      Execution terminated.
      

      doesn't find either by its name.

        Issue Links

          Activity

          Hide
          Damien Pobel (Inactive) added a comment -

          Actually, it goes way beyond content created in PlatformUI. Any content created in Platform (with REST or with the Public API seems to not be indexed in the "legacy search engine".

          Show
          Damien Pobel (Inactive) added a comment - Actually, it goes way beyond content created in PlatformUI. Any content created in Platform (with REST or with the Public API seems to not be indexed in the "legacy search engine".
          Hide
          André Rømcke added a comment -

          Initial empty implementation for anyone to get started on this
          https://github.com/ezsystems/ezpublish-kernel/pull/1533

          Show
          André Rømcke added a comment - Initial empty implementation for anyone to get started on this https://github.com/ezsystems/ezpublish-kernel/pull/1533
          Hide
          Andrzej Longosz added a comment -

          My PR: https://github.com/ezsystems/ezpublish-kernel/pull/1660

          I think code still needs some improvements (as I've mentioned in PR comments), I'll appreciate any feedback.

          Show
          Andrzej Longosz added a comment - My PR: https://github.com/ezsystems/ezpublish-kernel/pull/1660 I think code still needs some improvements (as I've mentioned in PR comments), I'll appreciate any feedback.
          Hide
          Andrzej Longosz added a comment - - edited

          Due to dependencies on Solr, which was changed as well, following PR's were created:

          Show
          Andrzej Longosz added a comment - - edited Due to dependencies on Solr, which was changed as well, following PR's were created: Solr PR #49 - merged to satisfy dependencies during Kernel PR #1660 Kernel PR #1707 CI tests; Solr PR #48 - to be reviewed and merged after review and merge of Kernel PR #1660.
          Hide
          Andrzej Longosz added a comment - - edited

          During a review we discovered that this bugfix requires several changes including implementing missing features on ezpublish-kernel. Therefore it cannot be backported to previous versions (5.0, 5.4).
          Also, an Epic was created ("Full Text Search SQL") to link all related issues.

          This issue is fixed by the WordIndexer Kernel PR #1707 (reopened #1660 against the `master` branch).
          Ready for a review.

          Before this could be completed, separate PRs needed to be merged solving required, separate tasks:

          1. Move FieldValueMapper namespace in each engine to the common one `eZ\Publish\Core\Search\Common\FieldValueMapper`:
            • Merged - Kernel PR #1697 - an improvement refactoring FieldValueMapper namespace to be common for all search engine bundles.
            • Merged - Solr PR #50 - removed FieldValueMapper namespace in favor of the one introduced by Kernel PR #1697|https://github.com/ezsystems/ezpublish-kernel/pull/1697]
          2. Decouple Signal Slots for each search engine to exclude not needed search Slots (e.g. for legacy search engine we don't need Slots related to Locations as Locations are not indexable by this engine):

          Note: To work properly when multiple search engine bundles are enabled in `AppKernel`, Signal Slots names are different in each search engine. If the names remained the same, each bundle would override configuration. This use case occurs when siteacceses in a web application use different repositories which can use different search engines.

          Mentioned common/generic indexing command is a separate story in the "Full Text Search SQL" Epic.

          Show
          Andrzej Longosz added a comment - - edited During a review we discovered that this bugfix requires several changes including implementing missing features on ezpublish-kernel. Therefore it cannot be backported to previous versions (5.0, 5.4). Also, an Epic was created ("Full Text Search SQL") to link all related issues. This issue is fixed by the WordIndexer Kernel PR #1707 (reopened #1660 against the `master` branch). Ready for a review. Before this could be completed, separate PRs needed to be merged solving required, separate tasks: Move FieldValueMapper namespace in each engine to the common one `eZ\Publish\Core\Search\Common\FieldValueMapper`: Merged - Kernel PR #1697 - an improvement refactoring FieldValueMapper namespace to be common for all search engine bundles. Merged - Solr PR #50 - removed FieldValueMapper namespace in favor of the one introduced by Kernel PR #1697|https://github.com/ezsystems/ezpublish-kernel/pull/1697] Decouple Signal Slots for each search engine to exclude not needed search Slots (e.g. for legacy search engine we don't need Slots related to Locations as Locations are not indexable by this engine): Merged - Kernel PR #1704 - introduces SignalDispatcherFactory creating SignalDispatcher with a proper SignalSlot map. Ready for a review - Solr PR #54 - aligns with changes done in the Kernel PR #1704 Merged Solr PR #51 - moves search engine Signal Slots configuration to Solr-specific yaml file. Note: To work properly when multiple search engine bundles are enabled in `AppKernel`, Signal Slots names are different in each search engine. If the names remained the same, each bundle would override configuration. This use case occurs when siteacceses in a web application use different repositories which can use different search engines. Mentioned common/generic indexing command is a separate story in the "Full Text Search SQL" Epic.
          Show
          Andrzej Longosz added a comment - Merged Kernel PR #1707 solving this issue: https://github.com/ezsystems/ezpublish-kernel/commit/e391f2bfb9b39ec77393e7a7cc5714074bf2bc24 Merged Solr PR #54 aligning with Kernel PR #1707: https://github.com/ezsystems/ezplatform-solr-search-engine/commit/d48cd690679939c6a4c54d631dbf16cbafda7c55
          Hide
          Paulo Nunes (Inactive) added a comment -

          Andrzej Longosz: can you please update "Fix Version/s" tag?
          Thank you.

          Show
          Paulo Nunes (Inactive) added a comment - Andrzej Longosz : can you please update "Fix Version/s" tag? Thank you.
          Hide
          Rui Silva (Inactive) added a comment -

          Andrzej Longosz, what exactly is the testing scope of these fixes, and which of the six of them exactly are necessary to test the legacy indexing issue?
          QA is having difficulty understanding what exactly is necessary to test this and if and why fixes for Solr are needed for a fix on Legacy search engine.
          Thanks.

          Show
          Rui Silva (Inactive) added a comment - Andrzej Longosz , what exactly is the testing scope of these fixes, and which of the six of them exactly are necessary to test the legacy indexing issue? QA is having difficulty understanding what exactly is necessary to test this and if and why fixes for Solr are needed for a fix on Legacy search engine. Thanks.
          Hide
          Rui Silva (Inactive) added a comment -

          Tested and approved by QA for master.

          Show
          Rui Silva (Inactive) added a comment - Tested and approved by QA for master.
          Hide
          Andrzej Longosz added a comment -

          Paulo Nunes: done

          [~rui.silva@ez.no]: To test this you need to have eZ Platform web app with ezpublish-kernel taken from master branch (every Kernel PR is needed for this to work). eZ Platform needs to be configured for legacy search engine (for simple setup it can be set in app/config/parameters.yml). Attached test case and steps to reproduce should be enough to test this. After adding some content object (e.g. a Folder named "folder") attached test command should display it as found.

          Fixes for Solr are needed only if Solr bundle is enabled in app/AppKernel.php along with Legacy Search Bundle. Moreover most of these fixes are due to performance issues not the actual fix of a bug (as I understand it at least ).

          This is my first bug fix that covers so extensive changes, so I'm not sure if my explanation makes sense for QA. Maybe André Rømcke could comment on this?

          Show
          Andrzej Longosz added a comment - Paulo Nunes : done [~rui.silva@ez.no] : To test this you need to have eZ Platform web app with ezpublish-kernel taken from master branch (every Kernel PR is needed for this to work). eZ Platform needs to be configured for legacy search engine (for simple setup it can be set in app/config/parameters.yml ). Attached test case and steps to reproduce should be enough to test this. After adding some content object (e.g. a Folder named "folder") attached test command should display it as found. Fixes for Solr are needed only if Solr bundle is enabled in app/AppKernel.php along with Legacy Search Bundle. Moreover most of these fixes are due to performance issues not the actual fix of a bug (as I understand it at least ). This is my first bug fix that covers so extensive changes, so I'm not sure if my explanation makes sense for QA. Maybe André Rømcke could comment on this?
          Hide
          Rui Silva (Inactive) added a comment -

          Thanks Andrzej Longosz, as you can see, QA was afterwards able to certify this. We'd had just a little trouble understanding what this was meant to fix, exactly, because we were having consistency issues reproducing the issue I opened, and then run the test case again with the fix to see if it was fixed: we were not seeing it as fixed though, but then QA realized it must be due to the not-yet-implemented second jira of the epic that envelops this, since I was taking advantage of the previously existent contents created on the first run.
          I reinstalled it and did it all from scratch the second time, and it went ok.
          However, any further explanation or detailed information on this, or on how it is supposed to work as a whole, is welcome, since as for now, only this first part was tested and QA'ed, and it may be useful for later developments following this.

          Show
          Rui Silva (Inactive) added a comment - Thanks Andrzej Longosz , as you can see, QA was afterwards able to certify this. We'd had just a little trouble understanding what this was meant to fix, exactly, because we were having consistency issues reproducing the issue I opened, and then run the test case again with the fix to see if it was fixed: we were not seeing it as fixed though, but then QA realized it must be due to the not-yet-implemented second jira of the epic that envelops this, since I was taking advantage of the previously existent contents created on the first run. I reinstalled it and did it all from scratch the second time, and it went ok. However, any further explanation or detailed information on this, or on how it is supposed to work as a whole, is welcome, since as for now, only this first part was tested and QA'ed, and it may be useful for later developments following this.

            People

            • Assignee:
              Unassigned
              Reporter:
              Rui Silva (Inactive)
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 0 minutes
                0m
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 5 weeks, 2 days, 2 hours, 15 minutes
                5w 2d 2h 15m