Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make strict enforcement of DTOs optional #22

Open
cboulanger opened this issue Feb 23, 2018 · 9 comments
Open

Make strict enforcement of DTOs optional #22

cboulanger opened this issue Feb 23, 2018 · 9 comments

Comments

@cboulanger
Copy link

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
Copy link
Owner

cranetm commented Feb 23, 2018

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
Copy link
Author

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
Copy link
Author

cboulanger commented Feb 27, 2018

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
Copy link
Author

cboulanger commented Feb 27, 2018

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
Copy link
Owner

cranetm commented Feb 27, 2018

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

@cboulanger
Copy link
Author

cboulanger commented Feb 27, 2018

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
Copy link
Owner

cranetm commented Feb 27, 2018

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
Copy link
Author

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
Copy link
Owner

cranetm commented Feb 27, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants