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

Support checking the registry for table prefixed queries #1161

Merged
merged 4 commits into from
Nov 9, 2017
Merged

Support checking the registry for table prefixed queries #1161

merged 4 commits into from
Nov 9, 2017

Conversation

Toflar
Copy link
Member

@Toflar Toflar commented Oct 26, 2017

No description provided.

Copy link
Contributor

@discordier discordier left a comment

Choose a reason for hiding this comment

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

Works in my use case.

$varKey = is_array($arrOptions['value']) ? $arrOptions['value'][0] : $arrOptions['value'];
$objModel = \Model\Registry::getInstance()->fetch(static::$strTable, $varKey, $arrColumn[0]);
// Support table prefixes
$arrColumn[0] = $result = preg_replace('/^' . preg_quote(static::getTable(), '/') . '\./', '', $arrColumn[0]);
Copy link
Member

Choose a reason for hiding this comment

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

Is the $result = part supposed to be there?

@leofeyer leofeyer added the bug label Oct 26, 2017
@leofeyer leofeyer added this to the 4.4.8 milestone Oct 26, 2017
@@ -175,6 +175,14 @@ public static function findByUuid($strUuid, array $arrOptions=array())
$strUuid = \StringUtil::uuidToBin($strUuid);
}

// Check in model registry (does not work by default due to UNHEX())
$objModel = \Model\Registry::getInstance()->fetch(static::$strTable, $strUuid, 'uuid');
Copy link
Member

Choose a reason for hiding this comment

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

Similar to

public static function findPublishedFallbackByHostname($strHost, array $arrOptions=array())
{
// Try to load from the registry (see #8544)
if (empty($arrOptions))
{
$objModel = \Model\Registry::getInstance()->fetch(static::$strTable, $strHost, 'contao.dns-fallback');
if ($objModel !== null)
{
return $objModel;
}
}
$t = static::$strTable;
$arrColumns = array("$t.dns=? AND $t.fallback='1'");
if (isset($arrOptions['ignoreFePreview']) || !BE_USER_LOGGED_IN)
{
$time = \Date::floorToMinute();
$arrColumns[] = "($t.start='' OR $t.start<='$time') AND ($t.stop='' OR $t.stop>'" . ($time + 60) . "') AND $t.published='1'";
}
return static::findOneBy($arrColumns, $strHost, $arrOptions);
}
we should add a empty($arrOptions) check here, shouldn't we?

@Toflar
Copy link
Member Author

Toflar commented Oct 26, 2017

Updated :)

@leofeyer leofeyer changed the base branch from master to hotfix/4.4.8 November 9, 2017 09:06
@leofeyer leofeyer merged commit 6309d04 into contao:hotfix/4.4.8 Nov 9, 2017
@Toflar Toflar deleted the improve-perf-filesmodel branch November 9, 2017 10:01
@discordier
Copy link
Contributor

I wonder if we should backport this to 3.5 as the non usage of the registry could be considered a bug.

@leofeyer
Copy link
Member

@Toflar What do you think?

@Toflar
Copy link
Member Author

Toflar commented Nov 17, 2017 via email

leofeyer added a commit to contao/core that referenced this pull request Feb 23, 2018
@leofeyer
Copy link
Member

leofeyer commented Feb 23, 2018

Backported in contao/core@36bb1a7.

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Mar 6, 2018
Version 3.5.34 (2018-03-06)
---------------------------

### Fixed
Check the registry for table prefixed queries (see contao/core-bundle#1161).

### Fixed
Improve the folder hashing performance (see #8856).

### Fixed
Reset the autologin hash if the username or password changes (see #8843).

### Fixed
Correctly encode the sitemap URLs (see #8849).
@leofeyer leofeyer modified the milestones: 4.4.8, 4.4 May 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants