fuel / core

Fuel PHP Framework - The core of the Fuel v1 framework
http://fuelphp.com
813 stars 345 forks source link

The format class doesn't support XML namespaces. #1442

Closed lhoucine closed 11 years ago

lhoucine commented 11 years ago

am using the Format class to parse a gdata xml response from youtube Api, the response contain many prefixed xml elements like media:thumbnail or media:title ..., the issue is that elements are ignored and not returned when using Format::to_array() in the array result!!

WanWizard commented 11 years ago

See http://fuelphp.com/forums/discussion/12195/class-format-ignoring-xml-prefixed-elements for a use case.

hackoh commented 11 years ago

How about this?

https://github.com/hackoh/core/commit/8bfe8b8587e5cdb8abaa74ca81805f89bdafcdc6

// config/app/format.php
array(
    'xml' => array(
        'ignore_namespaces' => true,
    ),
)

// Get namespace attributes
$xml = '<?xml version="1.0" encoding="utf-8"?>
<xml xmlns="http://www.w3.org/2005/Atom" xmlns:media="http://search.yahoo.com/mrss/" xmlns:app="http://www.w3.org/2007/app"><article><title>test</title><app:title>app test</app:title></article></xml>';

$data = Format::forge($xml, 'xml')->to_array();

/*
array(
    '@attributes' => array(
        'xmlns' => 'http://www.w3.org/2005/Atom',
        'xmlns:media' => 'http://search.yahoo.com/mrss/',
        'xmlns:app' => 'http://www.w3.org/2007/app',
    ),
    'article' => array(
        'title' => 'test',
        'app:title' => 'app test',
    )
);
*/
WanWizard commented 11 years ago

this assumes that the namespace prefix is fixed "xmlns", which is not the case?

hackoh commented 11 years ago

are there defferent cases?

WanWizard commented 11 years ago

The example in the issue report mentions the namespace "media" for example.

hackoh commented 11 years ago

This fixed all namespaced-prefix for example "xmlns:media" and "media:thumbnail"

WanWizard commented 11 years ago

Can you verify with the output from http://gdata.youtube.com/feeds/api/videos/HIpQ4ah5XTY?v=2 ?

hackoh commented 11 years ago
/*

array(15) {
  ["@attributes"]=>
  array(5) {
    ["xmlns"]=>
    string(27) "http://www.w3.org/2005/Atom"
    ["xmlns:media"]=>
    string(29) "http://search.yahoo.com/mrss/"
    ["xmlns:gd"]=>
    string(32) "http://schemas.google.com/g/2005"
    ["xmlns:yt"]=>
    string(37) "http://gdata.youtube.com/schemas/2007"
    ["gd:etag"]=>
    string(28) "W/"A0ACSX47eCp7I2A9Wh5SGU4.""
  }
  ["id"]=>
  string(38) "tag:youtube.com,2008:video:HIpQ4ah5XTY"
  ["published"]=>
  string(24) "2013-10-11T11:53:07.000Z"
  ["updated"]=>
  string(24) "2013-10-16T16:09:28.000Z"
  ["category"]=>
  array(2) {
    [0]=>
    array(1) {
      ["@attributes"]=>
      array(2) {
        ["scheme"]=>
        string(37) "http://schemas.google.com/g/2005#kind"
        ["term"]=>
        string(43) "http://gdata.youtube.com/schemas/2007#video"
      }
    }
    [1]=>
    array(1) {
      ["@attributes"]=>
      array(3) {
        ["scheme"]=>
        string(52) "http://gdata.youtube.com/schemas/2007/categories.cat"
        ["term"]=>
        string(6) "Comedy"
        ["label"]=>
        string(6) "Comedy"
      }
    }
  }
  ["title"]=>
  string(27) "Worst exit a parking  EVER!"
  ["content"]=>
  array(1) {
    ["@attributes"]=>
    array(2) {
      ["type"]=>
      string(29) "application/x-shockwave-flash"
      ["src"]=>
      string(73) "http://www.youtube.com/v/HIpQ4ah5XTY?version=3&f=videos&app=youtube_gdata"
    }
  }
  ["link"]=>
  array(6) {
    [0]=>
    array(1) {
      ["@attributes"]=>
      array(3) {
        ["rel"]=>
        string(9) "alternate"
        ["type"]=>
        string(9) "text/html"
        ["href"]=>
        string(64) "http://www.youtube.com/watch?v=HIpQ4ah5XTY&feature=youtube_gdata"
      }
    }
    [1]=>
    array(1) {
      ["@attributes"]=>
      array(3) {
        ["rel"]=>
        string(53) "http://gdata.youtube.com/schemas/2007#video.responses"
        ["type"]=>
        string(20) "application/atom+xml"
        ["href"]=>
        string(67) "http://gdata.youtube.com/feeds/api/videos/HIpQ4ah5XTY/responses?v=2"
      }
    }
    [2]=>
    array(1) {
      ["@attributes"]=>
      array(3) {
        ["rel"]=>
        string(51) "http://gdata.youtube.com/schemas/2007#video.related"
        ["type"]=>
        string(20) "application/atom+xml"
        ["href"]=>
        string(65) "http://gdata.youtube.com/feeds/api/videos/HIpQ4ah5XTY/related?v=2"
      }
    }
    [3]=>
    array(1) {
      ["@attributes"]=>
      array(3) {
        ["rel"]=>
        string(44) "http://gdata.youtube.com/schemas/2007#mobile"
        ["type"]=>
        string(9) "text/html"
        ["href"]=>
        string(42) "http://m.youtube.com/details?v=HIpQ4ah5XTY"
      }
    }
    [4]=>
    array(1) {
      ["@attributes"]=>
      array(3) {
        ["rel"]=>
        string(46) "http://gdata.youtube.com/schemas/2007#uploader"
        ["type"]=>
        string(20) "application/atom+xml"
        ["href"]=>
        string(67) "http://gdata.youtube.com/feeds/api/users/cX6V5Wb-Ga_oyP6mWB0WRA?v=2"
      }
    }
    [5]=>
    array(1) {
      ["@attributes"]=>
      array(3) {
        ["rel"]=>
        string(4) "self"
        ["type"]=>
        string(20) "application/atom+xml"
        ["href"]=>
        string(57) "http://gdata.youtube.com/feeds/api/videos/HIpQ4ah5XTY?v=2"
      }
    }
  }
  ["author"]=>
  array(3) {
    ["name"]=>
    string(12) "Ben McKenzie"
    ["uri"]=>
    string(63) "http://gdata.youtube.com/feeds/api/users/cX6V5Wb-Ga_oyP6mWB0WRA"
    ["yt:userId"]=>
    string(22) "cX6V5Wb-Ga_oyP6mWB0WRA"
  }
  ["yt:accessControl"]=>
  array(8) {
    [0]=>
    array(1) {
      ["@attributes"]=>
      array(2) {
        ["action"]=>
        string(7) "comment"
        ["permission"]=>
        string(7) "allowed"
      }
    }
    [1]=>
    array(1) {
      ["@attributes"]=>
      array(2) {
        ["action"]=>
        string(11) "commentVote"
        ["permission"]=>
        string(7) "allowed"
      }
    }
    [2]=>
    array(1) {
      ["@attributes"]=>
      array(2) {
        ["action"]=>
        string(12) "videoRespond"
        ["permission"]=>
        string(9) "moderated"
      }
    }
    [3]=>
    array(1) {
      ["@attributes"]=>
      array(2) {
        ["action"]=>
        string(4) "rate"
        ["permission"]=>
        string(7) "allowed"
      }
    }
    [4]=>
    array(1) {
      ["@attributes"]=>
      array(2) {
        ["action"]=>
        string(5) "embed"
        ["permission"]=>
        string(7) "allowed"
      }
    }
    [5]=>
    array(1) {
      ["@attributes"]=>
      array(2) {
        ["action"]=>
        string(4) "list"
        ["permission"]=>
        string(7) "allowed"
      }
    }
    [6]=>
    array(1) {
      ["@attributes"]=>
      array(2) {
        ["action"]=>
        string(8) "autoPlay"
        ["permission"]=>
        string(7) "allowed"
      }
    }
    [7]=>
    array(1) {
      ["@attributes"]=>
      array(2) {
        ["action"]=>
        string(9) "syndicate"
        ["permission"]=>
        string(7) "allowed"
      }
    }
  }
  ["gd:comments"]=>
  array(1) {
    ["gd:feedLink"]=>
    array(1) {
      ["@attributes"]=>
      array(3) {
        ["rel"]=>
        string(46) "http://gdata.youtube.com/schemas/2007#comments"
        ["href"]=>
        string(66) "http://gdata.youtube.com/feeds/api/videos/HIpQ4ah5XTY/comments?v=2"
        ["countHint"]=>
        string(4) "3787"
      }
    }
  }
  ["media:group"]=>
  array(13) {
    ["media:category"]=>
    string(6) "Comedy"
    ["media:content"]=>
    array(3) {
      [0]=>
      array(1) {
        ["@attributes"]=>
        array(7) {
          ["url"]=>
          string(73) "http://www.youtube.com/v/HIpQ4ah5XTY?version=3&f=videos&app=youtube_gdata"
          ["type"]=>
          string(29) "application/x-shockwave-flash"
          ["medium"]=>
          string(5) "video"
          ["isDefault"]=>
          string(4) "true"
          ["expression"]=>
          string(4) "full"
          ["duration"]=>
          string(3) "243"
          ["yt:format"]=>
          string(1) "5"
        }
      }
      [1]=>
      array(1) {
        ["@attributes"]=>
        array(6) {
          ["url"]=>
          string(102) "rtsp://r8---sn-a5m7zu7e.c.youtube.com/CiILENy73wIaGQk2XXmo4VCKHBMYDSANFEgGUgZ2aWRlb3MM/0/0/0/video.3gp"
          ["type"]=>
          string(10) "video/3gpp"
          ["medium"]=>
          string(5) "video"
          ["expression"]=>
          string(4) "full"
          ["duration"]=>
          string(3) "243"
          ["yt:format"]=>
          string(1) "1"
        }
      }
      [2]=>
      array(1) {
        ["@attributes"]=>
        array(6) {
          ["url"]=>
          string(102) "rtsp://r8---sn-a5m7zu7e.c.youtube.com/CiILENy73wIaGQk2XXmo4VCKHBMYESARFEgGUgZ2aWRlb3MM/0/0/0/video.3gp"
          ["type"]=>
          string(10) "video/3gpp"
          ["medium"]=>
          string(5) "video"
          ["expression"]=>
          string(4) "full"
          ["duration"]=>
          string(3) "243"
          ["yt:format"]=>
          string(1) "6"
        }
      }
    }
    ["media:credit"]=>
    string(22) "cX6V5Wb-Ga_oyP6mWB0WRA"
    ["media:description"]=>
    array(1) {
      ["@attributes"]=>
      array(1) {
        ["type"]=>
        string(5) "plain"
      }
    }
    ["media:keywords"]=>
    array(0) {
    }
    ["media:license"]=>
    string(7) "youtube"
    ["media:player"]=>
    array(1) {
      ["@attributes"]=>
      array(1) {
        ["url"]=>
        string(71) "http://www.youtube.com/watch?v=HIpQ4ah5XTY&feature=youtube_gdata_player"
      }
    }
    ["media:thumbnail"]=>
    array(7) {
      [0]=>
      array(1) {
        ["@attributes"]=>
        array(5) {
          ["url"]=>
          string(46) "http://i1.ytimg.com/vi/HIpQ4ah5XTY/default.jpg"
          ["height"]=>
          string(2) "90"
          ["width"]=>
          string(3) "120"
          ["time"]=>
          string(12) "00:02:01.500"
          ["yt:name"]=>
          string(7) "default"
        }
      }
      [1]=>
      array(1) {
        ["@attributes"]=>
        array(4) {
          ["url"]=>
          string(48) "http://i1.ytimg.com/vi/HIpQ4ah5XTY/mqdefault.jpg"
          ["height"]=>
          string(3) "180"
          ["width"]=>
          string(3) "320"
          ["yt:name"]=>
          string(9) "mqdefault"
        }
      }
      [2]=>
      array(1) {
        ["@attributes"]=>
        array(4) {
          ["url"]=>
          string(48) "http://i1.ytimg.com/vi/HIpQ4ah5XTY/hqdefault.jpg"
          ["height"]=>
          string(3) "360"
          ["width"]=>
          string(3) "480"
          ["yt:name"]=>
          string(9) "hqdefault"
        }
      }
      [3]=>
      array(1) {
        ["@attributes"]=>
        array(4) {
          ["url"]=>
          string(48) "http://i1.ytimg.com/vi/HIpQ4ah5XTY/sddefault.jpg"
          ["height"]=>
          string(3) "480"
          ["width"]=>
          string(3) "640"
          ["yt:name"]=>
          string(9) "sddefault"
        }
      }
      [4]=>
      array(1) {
        ["@attributes"]=>
        array(5) {
          ["url"]=>
          string(40) "http://i1.ytimg.com/vi/HIpQ4ah5XTY/1.jpg"
          ["height"]=>
          string(2) "90"
          ["width"]=>
          string(3) "120"
          ["time"]=>
          string(12) "00:01:00.750"
          ["yt:name"]=>
          string(5) "start"
        }
      }
      [5]=>
      array(1) {
        ["@attributes"]=>
        array(5) {
          ["url"]=>
          string(40) "http://i1.ytimg.com/vi/HIpQ4ah5XTY/2.jpg"
          ["height"]=>
          string(2) "90"
          ["width"]=>
          string(3) "120"
          ["time"]=>
          string(12) "00:02:01.500"
          ["yt:name"]=>
          string(6) "middle"
        }
      }
      [6]=>
      array(1) {
        ["@attributes"]=>
        array(5) {
          ["url"]=>
          string(40) "http://i1.ytimg.com/vi/HIpQ4ah5XTY/3.jpg"
          ["height"]=>
          string(2) "90"
          ["width"]=>
          string(3) "120"
          ["time"]=>
          string(12) "00:03:02.250"
          ["yt:name"]=>
          string(3) "end"
        }
      }
    }
    ["media:title"]=>
    string(27) "Worst exit a parking  EVER!"
    ["yt:duration"]=>
    array(1) {
      ["@attributes"]=>
      array(1) {
        ["seconds"]=>
        string(3) "243"
      }
    }
    ["yt:uploaded"]=>
    string(24) "2013-10-11T11:53:07.000Z"
    ["yt:uploaderId"]=>
    string(24) "UCcX6V5Wb-Ga_oyP6mWB0WRA"
    ["yt:videoid"]=>
    string(11) "HIpQ4ah5XTY"
  }
  ["gd:rating"]=>
  array(1) {
    ["@attributes"]=>
    array(5) {
      ["average"]=>
      string(9) "4.4633794"
      ["max"]=>
      string(1) "5"
      ["min"]=>
      string(1) "1"
      ["numRaters"]=>
      string(4) "6895"
      ["rel"]=>
      string(40) "http://schemas.google.com/g/2005#overall"
    }
  }
  ["yt:statistics"]=>
  array(1) {
    ["@attributes"]=>
    array(2) {
      ["favoriteCount"]=>
      string(1) "0"
      ["viewCount"]=>
      string(7) "3276948"
    }
  }
  ["yt:rating"]=>
  array(1) {
    ["@attributes"]=>
    array(2) {
      ["numDislikes"]=>
      string(3) "925"
      ["numLikes"]=>
      string(4) "5970"
    }
  }
}

*/
WanWizard commented 11 years ago

Looks good! Can you send the PR?

hackoh commented 11 years ago

I was thinking about that this setting should be "method argument", not "config setting". So how about this?

 $xml = '<?xml version="1.0" encoding="utf-8"?>
<xml xmlns="http://www.w3.org/2005/Atom" xmlns:media="http://search.yahoo.com/mrss/" xmlns:app="http://www.w3.org/2007/app"><article><title>test</title><app:title>app test</app:title></article></xml>';

$data = Format::forge($xml, 'xml:ns')->to_array();
WanWizard commented 11 years ago

The second parameter is used to directly map to the conversion methods, so the constructor would need something like

if ($from_type == 'xml:ns')
{
    $this->ignore_namespace = false;
    $from_type = 'xml';
}

before the method_exists() call. "ignore_namespace" then needs to be a protected class property instead of a config key.

hackoh commented 11 years ago

@WanWizard

Okay, I do so.

WanWizard commented 11 years ago

merged: https://github.com/fuel/core/pull/1549