Change interface return for getSize in FileInfo to string

[mrow4a]

Fixes issue #28275

[PVince81]

Hmm, doesn't strval convert the int value to a string ? The problem is that some other parts of the code might expect an int for additions. Unless PHP does some magic to convert them back to an int...

We'll see what CI says

[mrow4a]

Hmm, it passed I will try to identify places where getSize() is used and check if that is correct there

[PVince81]

My problem here is that the size is supposed to be an int and not a string.

The reason size is sometimes a string is only due to PHP on 32-bit. So I'd expect the PHPDoc to say "int".

Not sure how to deal with this on interface level, we could add "int|string" but it's ugly.

@DeepDiver1975 can you help make a cut here ?

[mrow4a]

To clarify, interface returns numeric string @PVince81, checked by is_numeric(), check the interface description.

[mrow4a]

@DeepDiver1975 any cut here and in #28275. This interface anyways previously was returning sometimes int and simetimes string, so as unit tests and integration tests passed it seems it can work with returning numeric strings only..

[PVince81]

@jvillafanez @individual-it if you guys all agree that this change will not break any apps due to the way PHP handles these values then I'm fine getting this in OC 10.0.3

[jvillafanez]

From my point of view, this needs a full regression testing, including apps. In addition, we're changing the interface to something a bit worse and kind of unexpected (for a good reason, of course), so this will very likely require changes in 3rdparty apps.

Unless really really critical, I'd delay this for the next big version

[PVince81]

I don't think it's critical to have return values matching the interface documentation at this point.

So a possible plan would be to:

• revert the old PR on stable10 / 10.0.3 to bring back former functionality
• in future OC 10.1 / 11 bring in this PR with its interface data type consistency (do it early) and do more regression testing and also inform the community about this breaking change

[phil-davis]

Something like this shows the differences:

error_reporting(E_ALL ^ E_NOTICE);
function getSizeintval($data) {
	return isset($data['size']) ? intval($data['size']) : 0;
}

function getSizeisnumeric($data) {
	return is_numeric($data['size']) ? strval($data['size']) : "0";
}

function showSize($data) {
	echo (isset($data['size']) ? $data['size'] : 'notset') . "\n";
	$x = getSizeintval($data);
	var_dump($x);
	$x = getSizeisnumeric($data);
	var_dump($x);
	echo "\n";
}

$testData = [
	"a",
	0,
	"0",
	1,
	"1",
	"2b",
	"0xdead",
	123456789,
	12345678901234567890,
	"12345678901234567890",
	123456789012345678901234567890,
	"123456789012345678901234567890"];

foreach ($testData as $size) {
	$data['size'] = $size;
	showSize($data);
}

output:

a
int(0)
string(1) "0"

0
int(0)
string(1) "0"

0
int(0)
string(1) "0"

1
int(1)
string(1) "1"

1
int(1)
string(1) "1"

2b
int(2)
string(1) "0"

0xdead
int(0)
string(1) "0"

123456789
int(123456789)
string(9) "123456789"

1.2345678901235E+19
int(-6101065172474984448)
string(19) "1.2345678901235E+19"

12345678901234567890
int(9223372036854775807)
string(20) "12345678901234567890"

1.2345678901235E+29
int(-4362897323387256832)
string(19) "1.2345678901235E+29"

123456789012345678901234567890
int(9223372036854775807)
string(30) "123456789012345678901234567890"

An "interesting" case from backward compatibility is that if you passed it "2b" then you used to get back 2 and now you get back "0".

The other cases are either functionally equivalent return values, or the new one is better - e.g. for the big numbers that overflow maxint.

[mrow4a]

I mean, if it feels better to have returned both strings/int (reverting), do it, but that way array will look like that most of the time:

Not sure which value goes where though.

[PVince81]

@mrow4a the way I understood it so far is that the size can only be a string if running on PHP on 32-bit system and the value is bigger than 32-bit max int.

But the screenshot above seems to show that sometimes it's a string even for small values ?!

If that is really the case then we need to find the code paths that create these string values.

For the short term I suggest reverting your old PR to get back the old way.

[PVince81]

revert PR here for the short term: #28649

[mrow4a]

Open if you think this is relevant (64-bit support only)

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.