WordPress.org

Making WordPress.org

Changeset 5497


Ignore:
Timestamp:
05/17/2017 08:30:29 PM (4 years ago)
Author:
iandunn
Message:

Events: Remove get_city_from_coordinates() to improve performance

The function was not performant, and it's not easy to make it that way. It's only needed for a few edge cases, and we can get the location from the events themselves (similar to r5491). So, removing this is a good trade-off.

Because we are able to rebuild the location, no changes are needed in Core, and a new version of this endpoint is not needed.

See #2823
Props dd32

Location:
sites/trunk/api.wordpress.org/public_html/events/1.0
Files:
2 edited

Legend:

Unmodified
Added
Removed
  • sites/trunk/api.wordpress.org/public_html/events/1.0/index.php

    r5495 r5497  
    125125        $events = get_events( $event_args );
    126126
    127         if ( isset( $location['internal'] ) && $location['internal'] ) {
     127        if ( empty( $location['description'] ) || ( isset( $location['internal'] ) && $location['internal'] ) ) {
    128128            $location = rebuild_location_from_event_source( $events );
    129129        }
     
    475475        )
    476476    ) {
    477         $city = get_city_from_coordinates( $args['latitude'], $args['longitude'] );
    478 
    479477        $location = array(
    480             'description' => $city ? $city : "{$args['latitude']}, {$args['longitude']}",
     478            'description' => false,
    481479            'latitude'    => $args['latitude'],
    482480            'longitude'   => $args['longitude']
     
    674672
    675673    return $country;
    676 }
    677 
    678 /**
    679  * Get the name of the city that's closest to the given coordinates
    680  *
    681  * @todo - This can probably be optimized by SELECT'ing from a derived table of the closest rows, instead of the
    682  *         entire table, similar to the technique described at
    683  *         http://www.techfounder.net/2009/02/02/selecting-closest-values-in-mysql/
    684  *         There's only 140k rows in the table, though, so this is performant for now.
    685  *
    686  * NOTE: If this causes any performance issues, it's possible that it could be removed entirely. The Core client
    687  *       saves the location locally, so it could display that instead of using this. However, there were some
    688  *       edge cases early in development that caused us to add this. I don't remember what they were, though, and
    689  *       didn't properly document them in r5128. So, if we ever want to attempt removing this, we'll need to test
    690  *       for unintended side effects. The Core client would need to be updated to display the saved location, so
    691  *       removing this would probably require creating a new version of the endpoint, and leaving this version for
    692  *       older installs.
    693  *
    694  * @param float $latitude
    695  * @param float $longitude
    696  *
    697  * @return false|string
    698  */
    699 function get_city_from_coordinates( $latitude, $longitude ) {
    700     global $wpdb;
    701 
    702     $results = $wpdb->get_col( $wpdb->prepare( "
    703         SELECT
    704             name,
    705             ABS( %f - latitude  ) AS latitude_distance,
    706             ABS( %f - longitude ) AS longitude_distance
    707         FROM geoname
    708         HAVING
    709             latitude_distance  < 0.3 AND    -- 0.3 degrees is about 30 miles
    710             longitude_distance < 0.3
    711         ORDER by latitude_distance ASC, longitude_distance ASC
    712         LIMIT 1",
    713         $latitude,
    714         $longitude
    715     ) );
    716 
    717     return isset( $results[0] ) ? $results[0] : false;
    718674}
    719675
  • sites/trunk/api.wordpress.org/public_html/events/1.0/tests/test-index.php

    r5496 r5497  
    1919    $tests_failed = 0;
    2020    $tests_failed += test_get_location();
    21     $tests_failed += test_get_city_from_coordinates();
    2221    $query_count  = count( $wpdb->queries );
    2322    $query_time   = array_sum( array_column( $wpdb->queries, 1 ) );
     
    698697            ),
    699698            'expected' => array(
    700                 'description' => 'seattle',
     699                'description' => false,
    701700                'latitude'    => '47.606',
    702701                'longitude'   => '-122.332',
     
    713712            ),
    714713            'expected' => array(
    715                 'description' => 'otavi',
     714                'description' => false,
    716715                'latitude'    => '-19.634',
    717716                'longitude'   => '17.332',
     
    792791}
    793792
    794 /**
    795  * Test `get_city_from_coordinates()`
    796  *
    797  * @todo This can probably be refactored along with test_get_location() into a more abstract/DRY general-purpose
    798  *       test runner.
    799  *
    800  * @return bool The number of failures
    801  */
    802 function test_get_city_from_coordinates() {
    803     $failed = 0;
    804     $cases  = get_city_from_coordinates_test_cases();
    805 
    806     printf( "\n\nRunning %d city from coordinate tests\n", count( $cases ) );
    807 
    808     foreach ( $cases as $case_id => $case ) {
    809         $case['input'] = add_cachebusting_parameter( $case['input'] );
    810         $actual_result = get_city_from_coordinates( $case['input']['latitude'], $case['input']['longitude'] );
    811         $passed        = $case['expected'] === $actual_result;
    812 
    813         output_results( $case_id, $passed, $case['expected'], $actual_result );
    814 
    815         if ( ! $passed ) {
    816             $failed++;
    817         }
    818     }
    819 
    820     return $failed;
    821 }
    822 
    823 /**
    824  * Get the cases for testing `get_city_from_coordinates()`
    825  *
    826  * @return array
    827  */
    828 function get_city_from_coordinates_test_cases() {
    829      $cases = array(
    830         'lower-latitude-higher-longitude' => array(
    831             'input' => array(
    832                 'latitude'  => '60.199',
    833                 'longitude' => '24.660'
    834             ),
    835             'expected' => 'Espoo',
    836         ),
    837 
    838         'higher-latitude-lower-longitude' => array(
    839             'input' => array(
    840                 'latitude'  => '22.000',
    841                 'longitude' => '95.900'
    842             ),
    843             'expected' => 'Mandalay',
    844         ),
    845 
    846         'middle-of-no-and-where' => array(
    847             'input' => array(
    848                 'latitude'  => '-23.121',
    849                 'longitude' => '125.071'
    850             ),
    851             'expected' => false,
    852         ),
    853     );
    854 
    855     return $cases;
    856 }
    857 
    858793run_tests();
Note: See TracChangeset for help on using the changeset viewer.