fix(federation): Allow Oracles empty strings

[nickvergessen]

• Resolves: Cannot assign null to property NCU\\Security\\Signature\\Model\\Signatory::$account of type string in file '/home/runner/actions-runner/_work/spreed/spreed/host/lib/public/AppFramework/Db/Entity.php' line 149
• Before https://github.com/nextcloud/spreed/actions/runs/12251789635/job/34177104152?pr=13967
• After https://github.com/nextcloud/spreed/actions/runs/12254262324/job/34184792486?pr=13967

Summary

• Since this is a general problem I wonder if we should fix it in a better way. Basically we need $this->addType('account', 'string'); to specify if they are "nullable" and if not "heal oracle" by converting null to empty string?

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)

[ArtificialOwl]

• Since this is a general problem I wonder if we should fix it in a better way. Basically we need $this->addType('account', 'string'); to specify if they are "nullable" and if not "heal oracle" by converting null to empty string?

can we fix this in the QueryBuilder directly and not just at the QBMapper level ?

[nickvergessen]

can we fix this in the QueryBuilder directly and not just at the QBMapper level ?

It's not the query builder but the entity. The problem is when explicit null is allowed, we shouldn't map it to an empty string.
But that is basically the other way a problem. If a field is nullable, we should not allow other databases to return/use empty strings…

[miaulalala]

So IIRC Oracle doesn't have null values in the DB, is that right? If so, I think the entity class in general should compensate for that. The returns should be identical across all supported Database types, and the Entity should abstract that for us.

[nickvergessen]

So IIRC Oracle doesn't have null values in the DB, is that right?

Other way around, empty strings '' are stored as null

[nickvergessen]

Let's merge this for now, so Talk tests are green again and then we can have a look at the general Entity patch later.

[nickvergessen]

@ArtificialOwl after some further checking, you marked the column nullable / notnull => false in the migration:

server/core/Migrations/Version31000Date20240101084401.php

Lines 89 to 93 in 91a83b6

	$table->addColumn('account', Types::STRING, [
	'notnull' => false,
	'length' => 127,
	'default' => ''
	]);

So in this case the entity also needs to be able to understand null and the change in this PR is actually required, regardless of any other entity fiddling for empty strings.
If it would be notnull => true the migration service would throw an exception

server/lib/private/DB/MigrationService.php

Lines 570 to 573 in 91a83b6

	if ($thing->getNotnull() && $thing->getDefault() === ''
	&& $sourceTable instanceof Table && !$sourceTable->hasColumn($thing->getName())) {
	throw new \InvalidArgumentException('Column "' . $table->getName() . '"."' . $thing->getName() . '" is NotNull, but has empty string or null as default.');
	}

I guess it did and that is when you changed changed the schema, but didn't adjust the entity to also allow null.