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

[TASK] Adjust mount point indexing #4195

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dkd-friedrich
Copy link
Member

Mount point indexing and corresponding tests have been adjusted for
TYPO3 13. Mount points are supported in general and the mounted pages
will be indexed like standard pages.

But there are some points to consider:

  • Cross-site mounts require that config.index_enable = 1 is set
    in the pagetree of the mounted page, as TYPO3 loads the
    TypoScript of the mounted page.
    This is probably a bug in the TYPO3 core and may change, due to
    changes in RootlineUtility->generateRootlineCache() the mount page
    pid isn't used any more.
  • Pages in a pagetree without a site configuration cannot be
    indexed, in fact TYPO3 currently can't mount a page from a page
    tree without a site configuration and an exeception occurs.

Test procedure in PageIndexerTest has been improved and now uses
the PageUriBuilder to build the url to index, instead of building
the url in the test itself.

Resolves: #4160

Fixtures of ConnectionManagerTest were not yet updated
and included basic configuration that can be provided
automatically, this commit cleans the fixtures and
enables the automatic provision.
@dkd-friedrich
Copy link
Member Author

dkd-friedrich commented Oct 16, 2024

Test ConnectionManagerTest::canFindSolrConnectionForMountedPageIfMountPointIsGiven is still disabled and fails.

Reason is probably a bug in RootlineUtility->generateRootlineCache(), as the pid of the mounted page is used
for the root line instead of the mount page pid like in former versions. We should investigate and discuss this
behavior.

Mount point indexing and corresponding tests have been adjusted for
TYPO3 13. Mount points are supported in general and the mounted pages
will be indexed like standard pages.

But there are some points to consider:

*   Cross-site mounts require that `config.index_enable = 1` is set
    in the pagetree of the mounted page, as TYPO3 loads the
    TypoScript of the mounted page.
    This is probably a bug in the TYPO3 core and may change, due to
    changes in RootlineUtility->generateRootlineCache() the mount page
    pid isn't used any more.
*   Pages in a pagetree without a site configuration cannot be
    indexed, in fact TYPO3 currently can't mount a page from a page
    tree without a site configuration and an exeception occurs.

Test procedure in PageIndexerTest has been improved and now uses
the PageUriBuilder to build the url to index, instead of building
the url in the test itself.

Resolves: TYPO3-Solr#4160
@dkd-friedrich dkd-friedrich force-pushed the bugfix/main/4160-fix_mountpage_indexing_and_tests branch from ea65d7e to 08e5ff0 Compare October 16, 2024 13:56
Copy link
Collaborator

@dkd-kaehm dkd-kaehm left a comment

Choose a reason for hiding this comment

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

Is there a issue in forge for trouble in RootlineUtility->generateRootlineCache()?

Comment on lines +36 to +45
Adjust mount point indexing
~~~~~~~~~~~~~~~~~~~~~~~~~~~

Mount point indexing and corresponding tests have been adjusted for TYPO3 13. Mount points are supported in general and the mounted pages will be indexed like standard pages.

But there are some points to consider:

* Cross-site mounts require that `config.index_enable = 1` is set in the pagetree of the mounted page, as TYPO3 loads the TypoScript of the mounted page
* Mounted pages from a pagetree without a site configuration cannot be indexed, in fact TYPO3 currently can't mount a page from a page tree without a site configuration and an exeception occurs

Copy link
Collaborator

Choose a reason for hiding this comment

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

Double pasted the copy?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, will remove the duplicate

* |—[14] Mount Point (to [24] to show contents from)
*/
#[Test]
public function canIndexCrossSiteMountsWithoutSiteConfiguration(): void
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is that scenario possible in TYPO3 Backend at all?
As far as I know, TYPO3 creates a site-config.yaml file as soon the page is marked as root.
Can that test-case be removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the original test using a sysfolder with pages that should be mounted (no site root), but currently TYPO3 cannot mount such pages and an exception occurs:

 (1/1) #1521716622 TYPO3\CMS\Core\Exception\SiteNotFoundException 
No site found in root line of page 86

This issue already exists in TYPO3 12, I am currently not sure why this test worked in TYPO3 12, test results in v12 might be misleading.

As it's not a valid and supported scenario in the TYPO3 core, we could IMHO delete the test.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK,
I guess in TYPO3 12 it is/was still possible to use old data(created in Versions prior 12 or 11 LTS).

@dkd-friedrich
Copy link
Member Author

Is there a issue in forge for trouble in RootlineUtility->generateRootlineCache()?

Yes, I've created an issue and prepared a patch, see https://forge.typo3.org/issues/105376

@dkd-friedrich
Copy link
Member Author

I am postponing work on this problem until we receive feedback in https://forge.typo3.org/issues/105376.

Our further action depends on whether the patch is adopted.

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.

[TASK:T13] fix mount-pages issues
2 participants