-
Notifications
You must be signed in to change notification settings - Fork 345
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
use proper types in API #948
Conversation
controllers/Sources.php
Outdated
@@ -209,7 +209,7 @@ public function write() { | |||
|
|||
$return = [ | |||
'success' => true, | |||
'id' => $id, | |||
'id' => intval($id), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be (int) $id
cast for consistency (will be enforced once refactoring PR is merged).
daos/mysql/Statements.php
Outdated
@@ -22,7 +22,7 @@ public static function insert($query, array $params) { | |||
\F3::get('db')->exec($query, $params); | |||
$res = \F3::get('db')->exec('SELECT LAST_INSERT_ID() as lastid'); | |||
|
|||
return $res[0]['lastid']; | |||
return intval($res[0]['lastid']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
daos/sqlite/Statements.php
Outdated
@@ -22,7 +22,7 @@ public static function insert($query, array $params) { | |||
\F3::get('db')->exec($query, $params); | |||
$res = \F3::get('db')->exec('SELECT last_insert_rowid() as lastid'); | |||
|
|||
return $res[0]['lastid']; | |||
return intval($res[0]['lastid']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto.
Are there still changes requested on this? |
Sorry, this is stalling on my end. I will finish the API schema today and then we can move forward. |
Unfortunately the OpenAPI 2 is too limited so I was not able to finish it. This will have to wait until #993 is done. |
7d198ff
to
0a80317
Compare
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. |
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() |
Okay this is only showing when |
$ 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)' |
$ 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() |
The effect on the public API seems to be minor: api.patch |
Another issue when saving a source, tags should also be sent as an array:
|
Another one in OPML export:
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 { |
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
No description provided.