cranetm / yii2-json-rpc-2.0

Other
26 stars 18 forks source link

Make strict enforcement of DTOs optional #22

Open cboulanger opened 6 years ago

cboulanger commented 6 years ago

The deprecation of "array" as a parameter and return value is very impractical for development and testing (and has led to a number of forks specifically to remove this deprecation). It would be great if you could make it optional or enforce it only when YII_ENV_PROD = true.

cranetm commented 6 years ago

Why you cannot use Dto[]? If you don't need strict validation - just do not use it. Leave phpdoc comment empty for your parameter or for return.

cboulanger commented 6 years ago

Thank you for you quick feedback.

It works for no return tag but I have consistently been getting "invalid parameter" errors when leaving out the phpdoc and sending non-string data. But I'll try to construct a test case to see if it's just my mistake ...

Does "Dto[]" work as a replacement for "array" - if yes, that would take care of my problem.

cboulanger commented 6 years ago

Here's my method:

  /**
   * Remove references. If a folder id is given, remove from that folder
   * @param string|bool $first
   *    If boolean, the response to the confirmation dialog. Otherwise, the datasource name
   * @param string|int $second
   *    Optional. If string, the shelve id. Otherwise, the id of the folder from which to remove
   *    the reference
   * @param $third
   *    Optional. Dummy parameter required because of generic signature of the (move|remove|copy)Reference
   *    methods.
   * @param $ids
   *    If given, the ids of the references to remove
   */
  public function actionRemove($first, $second = null, $third = null, $ids = null)

Here is what the JsonRpc client sends:

["datasource1", 1, null, [5]] 

Here is what I receive in the action method:

[
    'datasource1',
    1,
    null,
    null,
]

I tried int[], Dto[], removig the tag altogether -- nothing works.

cboulanger commented 6 years ago

The problem seems to be that Dto[] doesn't accept something like [ "foo", "bar, [1,2,3]], i.e. nested arrays. I understand that I am supposed to be able to disable strict type checking (for example, by not giving any type declaration in the PHPDOC tag), but it seems to me that this doesn't work for nested array structures. I wonder what I am doing wrong.

cranetm commented 6 years ago

Nested arrays like ["foo", "bar", 1, [1,2,3]] not a strict type at all. Use objects! ;)

cboulanger commented 6 years ago

Correct me if I am wrong, but the JSONRPC spec requires only a "structured value" in JSON form for the parameters, and does not prohibit arrays. I think you should let the users of your library decided whatever type they want, as long as they are allowed by the JSON spec (at least in development). Otherwise, you'll just end up with more forks and less contributions to the code, which would be a shame. If people want to shoot themselves in the foot by using badly defined and non-strict method signatures, let them :-) and make the more strict stuff optional.

cranetm commented 6 years ago

JSONRPC just protocol, you can pass through any structure you want. This library includes strict validation. It supports array, but array of one of simple type or Object. If you would like to have anything in your value - don't use strict validation. What profit from optional disabling? To have phpdoc with @param array $body?

cboulanger commented 6 years ago

I am only insisting because I tried without strict validation (no type hint, no phpdoc at all) and it would always replace the nested array with null. But anyways, I solved it by converting the data into a json string and json_decode'ing it in the method. Kind of a hack, but it works.

cranetm commented 6 years ago

It should not convert to null if there is no validation. I'll take a look.