Skip to content

Add DatabaseObjectBuilder with TagBuilder implementation#6689

Draft
BurntimeX wants to merge 1 commit into
6.3from
63-dbo-builder
Draft

Add DatabaseObjectBuilder with TagBuilder implementation#6689
BurntimeX wants to merge 1 commit into
6.3from
63-dbo-builder

Conversation

@BurntimeX

Copy link
Copy Markdown
Member

Introduces an abstract builder for creating, updating and deleting database objects with a fluent setter API, batched transactional deletes and an INSERT IGNORE-style helper. TagBuilder is the first concrete implementation.

Introduces an abstract builder for creating, updating and deleting database objects with a fluent setter API, batched transactional deletes and an `INSERT IGNORE`-style helper. `TagBuilder` is the first concrete implementation.
*/
final class TagBuilder extends DatabaseObjectBuilder
{
public function setTagID(int $tagID): static

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This must not be available inside the forUpdate() flow. Maybe introduce a generic method DatabaseObjectBuilder::setID() that does that. After all, this is only ever required for (a) sessions and (b) data imports.

*
* @return TDatabaseObject
*/
public function save(): DatabaseObject

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This and all other methods declared by DatabaseObjectBuilder should be declared as final to allow us to make adjustments to this crucial component.

{
$keys = $values = '';
$statementParameters = [];
foreach (array_merge($this->properties, $this->customProperties) as $key => $value) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should be \array_merge(), not array_merge().

Comment on lines +89 to +97
if (static::getBaseClass()::getDatabaseTableIndexIsIdentity()) {
$id = WCF::getDB()->getInsertID(static::getBaseClass()::getDatabaseTableName(), static::getBaseClass()::getDatabaseTableIndexName());
} elseif (isset($this->properties[static::getBaseClass()::getDatabaseTableIndexName()])) {
$id = $this->properties[static::getBaseClass()::getDatabaseTableIndexName()];
} else {
throw new \BadMethodCallException("Missing value for '" . static::getBaseClass()::getDatabaseTableIndexName() . "'");
}

return $id;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This can be untangled into two conditional returns and an unconditional exception. The return $id just obfuscates the flow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants