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

API: XML is now generated without templates #2682

Merged
merged 13 commits into from Aug 1, 2016

Conversation

annando
Copy link
Collaborator

@annando annando commented Jul 19, 2016

There is now a generic functionality to generate XML output for API calls. Additionally some old code was dropped (support for a deprecated Activity Streams output). Also there are some changes to the sql queries.

There is some work left at the queries. But I wanted to do the pull request now so that the changes are split in several pull requests.

"friendica" => "http://friendi.ca/schema/api/1/",
"georss" => "http://www.georss.org/georss");

/// @todo Auto detection of needed namespaces
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's not needed. Just always put in namespace declarations and make the parser happy.. The parser don't complain on unused declared ns, but will stop on used undeclared ones...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are XML that never will return something with different namespaces (like the IDs of friends). I find it a little bit ugly to return namespaces there. Of course this is only some beautification.

Do you think this is a show stopper?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think this is a show stopper?

absolutely not

@fabrixxm
Copy link
Collaborator

Have you tried this with some clients?
I want to try with Friendly on Jolla, that uses xml

@annando
Copy link
Collaborator Author

annando commented Jul 19, 2016

I don't know clients that use the XML, but I compared every API call with an older Friendica server version and with a GNU Social server.

@fabrixxm
Copy link
Collaborator

Ok, just give me some time to test it with Friendly :-)

@annando
Copy link
Collaborator Author

annando commented Jul 19, 2016

I will be so friendly :-)

@annando
Copy link
Collaborator Author

annando commented Jul 19, 2016

BTW: There had been massive differences between XML and JSON (missing fields and/or fields that aren't part of the JSON) and there were many functions that never had worked with XML output because of missing templates.

@rabuzarus
Copy link
Collaborator

Short note from my side (like always): Please add the description of the new db row to the db docu

@annando
Copy link
Collaborator Author

annando commented Jul 19, 2016

There is no new database row, just a new index.

@rabuzarus
Copy link
Collaborator

ahh ok. sorry

@annando
Copy link
Collaborator Author

annando commented Jul 19, 2016

@fabrixxm could you give me a hint how to install a sailfish emulator and this app? Then I could do some testing (for future changes) on my own. I want to do some more changes to the api in the future.

@tobiasd
Copy link
Collaborator

tobiasd commented Jul 19, 2016

you can find the SailfishOS SDK stuff here and the Friendly source is at Fabios projects repository. It's a customized QtCreator.

You need the i486 kit an run it then in debug mode,. Open the friendly.pro file and on the button below the help choose "Debug" -> "deploy as RPM package". QtCreator should tell you that the VM is not running and the emulator and ask it it should start them for you.

@annando
Copy link
Collaborator Author

annando commented Jul 22, 2016

@fabrixxm Have you done your testings?

@fabrixxm
Copy link
Collaborator

No, sorry, I'm quite busy right now. I'll test it this sunday.

@fabrixxm
Copy link
Collaborator

I was trying this PR but I have some problems because I'm on latest PHP (7.0.9) which raises a strange error:

Warning: Parameter 1 to api_statuses_home_timeline() expected to be a reference, value given in include/api.php on line 277

Which is:

$r = call_user_func($info['func'], $a, $type);

Looks like it want $a passed by reference because the function in $info['func'] is defined as api_statuses_home_timeline(&$a, $type).

I get this warning also in current develop, looks like a php regression.

I never get any warning so far here. The only way I found to make it work is using

$r = call_user_func_array($info['func'], array(&$a, $type));

That's made the code work in develop.

If I change that line in this patch, I get a new series of warning:

Warning: It is not yet possible to assign complex types to properties in include/xml.php on line 55

@annando
Copy link
Collaborator Author

annando commented Jul 24, 2016

So we just should drop passing the "a" variable to the functions. I could include this in the current pull request if you like.

@fabrixxm
Copy link
Collaborator

Update:
I've made a test.
Looks like old PHP simply not complains and pass $a as value instead of reference when using call_user_func().
The correct way is using call_user_func_array() with $a passed as reference to the array

I tried this
https://gist.github.com/fabrixxm/02dab6eadbbc6a93871b7d9bfcbb79bb
on php 5.6 and 7.0.2
Note that the output value should increase after every call to my_function() but doesn't after call_user_func(), without warning in old php and with warning in last php.

I would simply change that line to use

$r = call_user_func_array($info['func'], array(&$a, $type));

After this, there is the new warning in include/xml.php :-D

@fabrixxm
Copy link
Collaborator

Ok, the warning is raised when there is an activity of an item. The array is:

{
  "id" : ...,
  [...]
  "friendica_activities": {
      "like": [
        {
          "name": [
            "Mario Rossi"
          ],
          "url": [
            "http://friendica.local/profile/mrossi"
          ]
        }
      ],
      "dislike": [],
      "attendyes": [],
      "attendno": [],
      "attendmaybe": []
    },

So, every activity is an array of [user=>", url=>""] for every user that like or disliked or whatever the item.

The code at include/xml.php:53-57 assume that if the key is a number, the value is a scalar, which in this case is false, because the value is an array.

As this is a friendica-only extension to the api output, we can decide how the xml should be, but I'm not sure on how to make the code work.

Using templates I was thinking something like:

<friendica:activities>
  <like>
    <contact>
        <user>Mario Rossi</user>
        <url>http://friendica.local/profile/mrossi</url>
    </contact>
  </like>
 ...
</activities>

(Note that the in the current develop branch, the XML still returns only the count of every sinlge activity, while the json is updated)

@annando
Copy link
Collaborator Author

annando commented Jul 24, 2016

The warning because of $a should now have disappeared.

Concerning the activities we should do it like Twitter once did (when they hadn't deprecated the XML output like they did now): there you have the plural and singular versions of the elements. Means: "statuses" and "status" or "users" and "user". We could do the same with "our" fields.

@tobiasd tobiasd merged commit 75be187 into friendica:develop Aug 1, 2016

if (is_integer($key)) {
if (isset($element))
$element[0] = $value;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for this I get the error PHP Warning: It is not yet possible to assign complex types to properties in /.../include/xml.php on line 55 with PHP 7.0.9.

Posting new entries works (but raises the error) fetching the home time line does not (using Friendly).

fabrixxm added a commit to fabrixxm/friendica that referenced this pull request Aug 2, 2016
annando added a commit that referenced this pull request Aug 2, 2016
@annando annando deleted the 1607-api-generic-xml branch August 2, 2016 10:52
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

Successfully merging this pull request may close these issues.

None yet

4 participants