use proper types in API#948

niol · 2017-06-16T08:21:04Z

No description provided.

jtojnar · 2017-06-16T12:08:55Z

controllers/Sources.php

        $return = [
            'success' => true,
-            'id' => $id,
+            'id' => intval($id),


This should be (int) $id cast for consistency (will be enforced once refactoring PR is merged).

jtojnar · 2017-06-16T12:10:15Z

daos/mysql/Statements.php

        $res = \F3::get('db')->exec('SELECT LAST_INSERT_ID() as lastid');

-        return $res[0]['lastid'];
+        return intval($res[0]['lastid']);


jtojnar · 2017-06-16T12:10:29Z

daos/sqlite/Statements.php

        $res = \F3::get('db')->exec('SELECT last_insert_rowid() as lastid');

-        return $res[0]['lastid'];
+        return intval($res[0]['lastid']);


niol · 2017-08-02T09:03:16Z

Are there still changes requested on this?

jtojnar · 2017-08-02T13:04:07Z

Sorry, this is stalling on my end. I will finish the API schema today and then we can move forward.

jtojnar · 2017-11-02T14:02:06Z

Unfortunately the OpenAPI 2 is too limited so I was not able to finish it. This will have to wait until #993 is done.

jtojnar · 2018-01-11T22:53:32Z

Okay, I finally finished the OpenAPI schema, could you please take a look if you see anything too bad? Unfortunately the tooling still did not catch up with OpenAPI 3 so we cannot test it automatically yet but it should still be more accurate than the wiki.

jtojnar · 2018-03-09T02:56:02Z

I am getting an error with this branch rebased onto master

$ curl 'http://localhost/selfoss/?ajax=true'
error occured: explode() expects parameter 2 to be string, array given
[selfoss/vendor/bcosca/fatfree-core/base.php:2190] Base->error()
[selfoss/daos/Items.php:83] explode()
[selfoss/controllers/Index.php:218] daos\Items->get()
[selfoss/controllers/Index.php:52] controllers\Index->loadItems()
[selfoss/vendor/bcosca/fatfree-core/base.php:1806] controllers\Index->home()
[selfoss/vendor/bcosca/fatfree-core/base.php:1627] Base->call()
[selfoss/index.php:80] Base->run()

niol · 2018-03-09T08:47:09Z

Okay this is only showing when public=1.

jtojnar · 2018-03-09T12:44:31Z

/sources/list still contains unsplit tags.

$ curl -X POST "http://localhost/selfoss/login?username=test&password=pass" -b jar -c jar
{"success":true}⏎
$ curl 'http://localhost/selfoss/sources/list' -b jar -c jar | jq 'map(.tags)'

jtojnar · 2018-03-09T13:23:23Z

POST /source does not seem to accept arrays:

$ curl -X POST "http://localhost/selfoss/source" -H  "Content-Type: application/json" -d '{"title": "Test", "spout": "spouts\\rss\\feed", "tags": ["IOT", "test"], "url": "https://github.com/SSilence/selfoss/commits/master.atom"}' -b jar -c jar
nastala chyba: htmlspecialchars() expects parameter 1 to be string, array given
[selfoss/vendor/bcosca/fatfree-core/base.php:2190] Base->error()
[selfoss/controllers/Sources.php:156] htmlspecialchars()
[selfoss/vendor/bcosca/fatfree-core/base.php:1806] controllers\Sources->write()
[selfoss/vendor/bcosca/fatfree-core/base.php:1627] Base->call()
[selfoss/index.php:80] Base->run()

jtojnar · 2018-03-09T13:44:44Z

The effect on the public API seems to be minor: api.patch

jtojnar · 2018-04-21T12:56:58Z

The client side will need to be updated as well:

jtojnar · 2018-04-23T10:59:50Z

Thanks, @niol. I will test it a little more but it should be good to go. Initially, I wanted to have tests ready first but as they say, perfect is the enemy of good – we can just fix anything broken later.

jtojnar · 2018-04-23T12:45:19Z

Another issue when saving a source, tags should also be sent as an array:

an error occured: implode(): Invalid arguments passed
[vendor/bcosca/fatfree-core/base.php:2190] Base->error()
[controllers/Sources.php:156] implode()
[vendor/bcosca/fatfree-core/base.php:1806] controllers\Sources->write()
[vendor/bcosca/fatfree-core/base.php:1627] Base->call()
[index.php:80] Base->run()

jtojnar · 2018-04-23T13:49:03Z

Another one in OPML export:

an error occured: explode() expects parameter 2 to be string, array given
[vendor/bcosca/fatfree-core/base.php:2190] Base->error()
[controllers/Opml.php:304] explode()
[vendor/bcosca/fatfree-core/base.php:1806] controllers\Opml->export()
[vendor/bcosca/fatfree-core/base.php:1627] Base->call()
[index.php:80] Base->run()

I was also thinking about changing the datatype in pgsql using something like

--- a/daos/pgsql/Database.php
+++ b/daos/pgsql/Database.php
@@ -227,6 +227,12 @@
                         'INSERT INTO version (version) VALUES (12)'
                     ]);
                 }
+                if (strnatcmp($version, '13') < 0) {
+                    \F3::get('db')->exec([
+                        "ALTER TABLE sources ALTER COLUMN tags SET DATA TYPE TEXT[] USING regexp_split_to_array(tags, E', ?');",
+                        'INSERT INTO version (version) VALUES (13)'
+                    ]);
+                }
             }
 
             // just initialize once
--- a/daos/pgsql/Statements.php
+++ b/daos/pgsql/Statements.php
@@ -84,7 +84,7 @@
      * @return string full statement
      */
     public static function csvRowMatches($column, $value) {
-        return "$value=ANY(string_to_array($column, ','))";
+        return "$value=ANY($column)";
     }
 
     /**
@@ -98,23 +98,7 @@
      *         expected types
      */
     public function ensureRowTypes(array $rows, array $expectedRowTypes) {
-        foreach ($rows as $rowIndex => $row) {
-            foreach ($expectedRowTypes as $columnIndex => $type) {
-                if (array_key_exists($columnIndex, $row)) {
-                    switch ($type) {
-                        // pgsql returns correct PHP types for INT and BOOL
-                        case \daos\PARAM_CSV:
-                            $value = explode(',', $row[$columnIndex]);
-                            break;
-                        default:
-                            $value = null;
-                    }
-                    if ($value !== null) {
-                        $rows[$rowIndex][$columnIndex] = $value;
-                    }
-                }
-            }
-        }
+        // pgsql uses correct PHP types
 
         return $rows;
     }

but the inconsistency between databases probably is not worth it.

We could probably clean the internal piping a bit, though:

--- a/controllers/Sources.php
+++ b/controllers/Sources.php
@@ -153,7 +153,7 @@
 
         // clean up title and tag data to prevent XSS
         $title = htmlspecialchars($data['title']);
-        $tags = htmlspecialchars(implode(',', $data['tags']));
+        $tags = array_map('htmlspecialchars', $data['tags']);
         $spout = $data['spout'];
         $filter = $data['filter'];
         $isAjax = isset($data['ajax']);
@@ -199,7 +199,6 @@
 
         // autocolor tags
         $tagsDao = new \daos\Tags();
-        $tags = explode(',', $tags);
         foreach ($tags as $tag) {
             $tagsDao->autocolorTag(trim($tag));
         }
--- a/daos/mysql/Sources.php
+++ b/daos/mysql/Sources.php
@@ -14,7 +14,7 @@
      * add new source
      *
      * @param string $title
-     * @param string $tags
+     * @param string[] $tags
      * @param string $spout the source type
      * @param mixed $params depends from spout
      *
@@ -38,7 +38,7 @@
      *
      * @param int $id the source id
      * @param string $title new title
-     * @param string $tags new tags
+     * @param string[] $tags new tags
      * @param string $spout new spout
      * @param mixed $params the new params
      *
--- a/controllers/Opml.php
+++ b/controllers/Opml.php
@@ -206,16 +206,16 @@
         $hash = md5($title . $spout . json_encode($data));
         if (array_key_exists($hash, $this->imported)) {
             $this->imported[$hash]['tags'] = array_unique(array_merge($this->imported[$hash]['tags'], $tags));
-            $tags = implode(',', $this->imported[$hash]['tags']);
+            $tags = $this->imported[$hash]['tags'];
             $this->sourcesDao->edit($this->imported[$hash]['id'], $title, $tags, '', $spout, $data);
             \F3::get('logger')->debug("OPML import: updated tags for '$title'");
         } elseif ($id = $this->sourcesDao->checkIfExists($title, $spout, $data)) {
             $tags = array_unique(array_merge($this->sourcesDao->getTags($id), $tags));
-            $this->sourcesDao->edit($id, $title, implode(',', $tags), '', $spout, $data);
+            $this->sourcesDao->edit($id, $title, $tags, '', $spout, $data);
             $this->imported[$hash] = ['id' => $id, 'tags' => $tags];
             \F3::get('logger')->debug("OPML import: updated tags for '$title'");
         } else {
-            $id = $this->sourcesDao->add($title, implode(',', $tags), '', $spout, $data);
+            $id = $this->sourcesDao->add($title, $tags, '', $spout, $data);
             $this->imported[$hash] = ['id' => $id, 'tags' => $tags];
             \F3::get('logger')->debug("OPML import: successfully imported '$title'");
         }
@@ -301,7 +301,7 @@
         $sources = ['tagged' => [], 'untagged' => []];
         foreach ($this->sourcesDao->get() as $source) {
             if ($source['tags']) {
-                foreach (explode(',', $source['tags']) as $tag) {
+                foreach ($source['tags'] as $tag) {
                     $sources['tagged'][$tag][] = $source;
                 }
             } else {

niol · 2018-04-23T14:56:13Z

Yes we could use pgsql datatypes or use an association table, I tried this some time ago but could not come up with better performance, so I did not go further.

Feed was forgotten during the API clean up (#948) resuting in the following error when opening `/feed`: ``` error occurred: explode() expects parameter 2 to be string, array given [selfoss/vendor/bcosca/fatfree-core/base.php:2190] Base->error() [selfoss/controllers/Rss.php:76] explode() [selfoss/vendor/bcosca/fatfree-core/base.php:1806] controllers\Rss->rss() [selfoss/vendor/bcosca/fatfree-core/base.php:1627] Base->call() [selfoss/index.php:80] Base->run() ```

It was forgotten during the API clean-ups: #948

jtojnar requested changes Jun 16, 2017

View reviewed changes

jtojnar added api bc break labels Nov 2, 2017

niol force-pushed the ppr/apitypes branch 2 times, most recently from 7d198ff to 0a80317 Compare January 11, 2018 10:10

jtojnar added this to the 2.19 milestone Mar 9, 2018

niol force-pushed the ppr/apitypes branch from 0a80317 to 1741808 Compare March 9, 2018 08:53

jtojnar force-pushed the ppr/apitypes branch from 0966c2b to 1553ffc Compare April 21, 2018 12:10

jtojnar force-pushed the ppr/apitypes branch from 1553ffc to 668106a Compare April 21, 2018 13:49

niol force-pushed the ppr/apitypes branch from 668106a to 66b1fc7 Compare April 22, 2018 18:51

jtojnar force-pushed the ppr/apitypes branch from 66b1fc7 to 42d12bd Compare April 23, 2018 10:19

niol force-pushed the ppr/apitypes branch from 42d12bd to 412c3bd Compare April 23, 2018 13:23

jtojnar force-pushed the ppr/apitypes branch from 412c3bd to 57efa9c Compare April 23, 2018 13:37

niol force-pushed the ppr/apitypes branch from 57efa9c to f873289 Compare April 23, 2018 14:52

use proper types in API (#879)

6cd9feb

jtojnar force-pushed the ppr/apitypes branch from 1b3fcbc to 6cd9feb Compare April 23, 2018 15:11

jtojnar merged commit 6cd9feb into fossar:master Apr 23, 2018

jtojnar added a commit that referenced this pull request Oct 21, 2018

import: fix OMPL importer

ba24e91

It was forgotten during the API clean-ups: #948

jtojnar mentioned this pull request May 3, 2020

Use proper types in API #879

Closed

Comments

Conversation

niol commented Jun 16, 2017

Uh oh!

jtojnar Jun 16, 2017

Choose a reason for hiding this comment

Uh oh!

jtojnar Jun 16, 2017

Choose a reason for hiding this comment

Uh oh!

jtojnar Jun 16, 2017

Choose a reason for hiding this comment

Uh oh!

niol commented Aug 2, 2017

Uh oh!

jtojnar commented Aug 2, 2017

Uh oh!

jtojnar commented Nov 2, 2017

Uh oh!

jtojnar commented Jan 11, 2018

Uh oh!

jtojnar commented Mar 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

niol commented Mar 9, 2018

Uh oh!

jtojnar commented Mar 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jtojnar commented Mar 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jtojnar commented Mar 9, 2018

Uh oh!

jtojnar commented Apr 21, 2018

Uh oh!

jtojnar commented Apr 23, 2018

Uh oh!

jtojnar commented Apr 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jtojnar commented Apr 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

niol commented Apr 23, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jtojnar commented Mar 9, 2018 •

edited

Loading

jtojnar commented Mar 9, 2018 •

edited

Loading

jtojnar commented Mar 9, 2018 •

edited

Loading

jtojnar commented Apr 23, 2018 •

edited

Loading

jtojnar commented Apr 23, 2018 •

edited

Loading