use proper types in API by niol · Pull Request #948 · fossar/selfoss

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

No description provided.

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

        $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

        $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

        $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

jtojnar added this to the 2.19 milestone Mar 9, 2018

use proper types in API (#879)

6cd9feb

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

Uh oh!

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