[Corona] Change return type for raw data

Review Request #885 — Created Nov. 15, 2022 and submitted

b.stoop
Lunr
fix/nobodypost
e36edaa...
lunr

Corona: Change return type for raw data

In a attempt to test a new POST api I forgot to send a body and got this response:

{
    "data": {},
    "status": {
        "code": 500,
        "message": "Return value of Lunr\\Corona\\Request::get_raw_data() must be of the type string, null returned"
    }
}

The probablity that this happens in with real requests is low but I still think this must be handled better

unit tests

  • 0
  • 0
  • 4
  • 1
  • 5
Description From Last Updated
smillernl
  1. 
      
  2. These use file_get_contents under the hood though, which is defined as string|false. How were you testing this?

  3. 
      
smillernl
  1. 
      
  2. src/Lunr/Corona/Request.php (Diff revision 1)
     
     
     
     

    I think it would be better to set the default value to an empty string

  3. src/Lunr/Corona/Request.php (Diff revision 1)
     
     

    Maybe we should check only for false here.

  4. 
      
b.stoop
smillernl
  1. 
      
  2. src/Lunr/Corona/Request.php (Diff revision 2)
     
     

    Let's just do !== FALSE since that's what we want to avoid.

    1. This would also avoid a empty string, why assign an empty string if it is already an empty string?

    2. Two problems with that:
      1. You don't check it's already an empty string, all you know is what the new value is.
      2. You're depending on PHP type magic here. boolval('0') is also false. You're not checking for the case you want to avoid, you're checking if it can become the case you want to avoid.

  3. 
      
b.stoop
b.stoop
smillernl
  1. Ship It!
  2. 
      
pprkut
  1. Ship It!
  2. 
      
pprkut
  1. 
      
  2. Unit tests fail:

    1) Lunr\Corona\Tests\RequestGetDataTest::testGetRawDataReturnsCachedRawRequestData
    Failed asserting that two strings are equal.
    --- Expected
    +++ Actual
    @@ @@
    -'raw'
    +''
    
  3. 
      
b.stoop
b.stoop
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to master (4d94200)
Loading...