Fix solr query types

[bix0r]

This fixes issue #37.

But this is a BC break. Before this ie. setRows() only accepted strings in strict mode.

But the question is should we instead use Z_PARAM_STR_OR_LONG and change the input type to string|int? That would allow current code (where devs have changed the type of their arguments (like setRows('2')) to continue to work, but still accept the correct value of an int.

We could even trigger a deprecated notice about sending a string when int is wanted.

[bix0r]

OK, now there is no BC break. One can run $query->setStart('somestring'); and "somestring" will be sent to the server.

[bix0r]

@omars1111 Sorry, but I managed to miss that my commit around fixing the BC issue did not come here (had a separate branch that I had not merged into this one).

In there I changed almost everything I had touched :)

I felt the need to use more zend_* types because of the new ZPP api, but I am not that comfortable (yet?) with jumping between types in C.

[omars44]

@bix0r it is best to remain consistent, as a mix of types creates more problems, so please have a look at our macros and types.

a change of approach should rather be discussed to be implemented globally throughout the extension if it is deemed beneficial.

Cheers, and thanks for your efforts :)

[bix0r]

@omars1111 I agree that we should try to keep the types consistent. I am just not that good at C and I could not see any macros that could handle both int and string. So, while checking out how others do this, I found out about the "new ZPP" api. I guess it would be best to change all the types to the new ZPP at the same time, but that is quite a lot of work :)

These few method arguments were the only ones that found that have a "wrong" type signature. It would be nice to get just these fixed so that people can use strict types properly with Solr.

If you point me to an example or a macro that can allow both types I can try to change this PR.

[Ingimarsson]

@bix0r maybe it is better to parse the input parameter as a zval instead of zend_string / zend_long and then use the built-in function to convert a long zval to a string zval. This seems to work:

PHP_METHOD(SolrQuery, setStart)
{
	solr_char_t *param_name = (solr_char_t *) "start";
	COMPAT_ARG_SIZE_T param_name_len = sizeof("start")-1;
	zval* param;

	ZEND_PARSE_PARAMETERS_START(1, 1)
		Z_PARAM_ZVAL(param)
	ZEND_PARSE_PARAMETERS_END();

	if (Z_TYPE_P(param) == IS_LONG) {
		convert_to_string(param);
	}

	if (Z_TYPE_P(param) != IS_STRING) {
		php_error_docref(NULL, E_WARNING, "Invalid parameters");

		RETURN_NULL();
	}

	if (solr_set_normal_param(getThis(), param_name, param_name_len, Z_STRVAL_P(param), Z_STRLEN_P(param)) == FAILURE) {
		RETURN_NULL();
	}

	solr_return_solr_params_object();
}

[bix0r]

@Ingimarsson Thank you for the comment, but I've found multiple other issues with this way. I will create a new PR for this fix. I would love some review on that one if you can :) (hopefully this week)