Skip to content

Promote TTRSS_API extension#1526

Closed
Alkarex wants to merge 18 commits intoFreshRSS:edgefrom
Alkarex:ttrss-api
Closed

Promote TTRSS_API extension#1526
Alkarex wants to merge 18 commits intoFreshRSS:edgefrom
Alkarex:ttrss-api

Conversation

@Alkarex
Copy link
Copy Markdown
Member

@Alkarex Alkarex commented May 8, 2017

@Alkarex Alkarex added this to the 1.8.0 milestone May 8, 2017
@Alkarex Alkarex added the Work in progress 🚧 Wait before merging label May 8, 2017
@Alkarex
Copy link
Copy Markdown
Member Author

Alkarex commented May 8, 2017

Will also address FreshRSS/Extensions#15

@Alkarex
Copy link
Copy Markdown
Member Author

Alkarex commented May 8, 2017

@Alkarex
Copy link
Copy Markdown
Member Author

Alkarex commented May 8, 2017

@Alwaysin Could you give a try to 511f729 ? (not tested at all)
(The TTRSS API documentation says to return an integer, but the example shows a string...)

@Alwaysin
Copy link
Copy Markdown
Contributor

Alwaysin commented May 8, 2017

Looks like it is not working. I know get a "404 not found" in the app even though http://freshrss/api/ttrss.php is working (I have removed the extension).

@Alkarex
Copy link
Copy Markdown
Member Author

Alkarex commented May 8, 2017

What do you get in your HTTP logs?

@Alwaysin
Copy link
Copy Markdown
Contributor

Alwaysin commented May 8, 2017

Strange things for Newsjet:

"POST /api/ttrss.php/api/ HTTP/1.1" 200 866 "-" "NewsJet/5.1.1 (newsjet.mobi)"

and TTRSS:

POST /api/ttrss.php/api/ HTTP/1.1" 200 297848 "-" "Dalvik/2.1.0 (Linux; U; Android 6.0; LG-H815 Build/MRA58K)"

I don't know why there is an added "/api/'...

Nothing in error.log and nothing in FreshRSS logs apparently.

Alkarex added 2 commits May 10, 2017 19:55
Fix MySQL bug, optimize some queries, fix type bug.
@Alkarex
Copy link
Copy Markdown
Member Author

Alkarex commented May 10, 2017

@Alwaysin The app NewsJet seems to work for me with the commit I have just done.
Example of HTTP request:

rss.example.net:443 2a05::1 - - [10/May/2017:20:37:53 +0200] "POST /api/ttrss.php/api/ HTTP/1.1" 200 52700 "-" "NewsJet/5.1.1 (newsjet.mobi)"

screenshot_2017-05-10-21-21-47

@Alkarex
Copy link
Copy Markdown
Member Author

Alkarex commented May 10, 2017

P.S.: You might have to log out, clear the app, and start NewsJet again.

@Alkarex Alkarex modified the milestones: 1.7.0, 1.8.0 May 10, 2017
@Alkarex
Copy link
Copy Markdown
Member Author

Alkarex commented May 10, 2017

P.S. It looks like a few more APIs calls will be needed to get NewsJet to work fully.

@Alwaysin
Copy link
Copy Markdown
Contributor

Damn, it still won't work. I'll try to see why.

@Alkarex
Copy link
Copy Markdown
Member Author

Alkarex commented May 12, 2017

Some TT-RSS clients only support 32-bit article IDs, and FreshRSS is using 64-bit article IDs. We might have to do some changes if we want to support those clients.

@Alkarex
Copy link
Copy Markdown
Member Author

Alkarex commented May 13, 2017

What do you think @marienfressinaud , @aledeg ?

  1. We can kill the support for TT-RSS API (it does not look like any client is working fine in its current state);
  2. We can chose to support only TT-RSS clients allowing 64-bit article IDs (which?);
  3. We can work to support TT-RSS clients with 32-bit article IDS;
    3a. One option is to add a new column to the entry table with an autoincrement 32-bit integer;
    3b. Other options?

@marienfressinaud
Copy link
Copy Markdown
Member

Is TTRSS officially support 64-bit IDs? If not, I would prefer option 3 but adding a new column just for an API is frustrating… If it's too much work, option 1 is not so bad since the plugin has always been in a work-in-progress state.

@Alkarex
Copy link
Copy Markdown
Member Author

Alkarex commented May 18, 2017

I do not think it would be so much work with the new column, and it would enable compatibility with a good number of clients. To compensate for the "cost" of adding a column, we could try to take advantage of the shorter article ID in other places.
I can first try to investigate a bit more client compatibilities.

@Alkarex Alkarex modified the milestones: 1.8.0, 1.7.0 May 21, 2017
@Alkarex
Copy link
Copy Markdown
Member Author

Alkarex commented May 26, 2017

Do not use blind `call_user_func`,  validate username, limit POST input
size
And use FreshRSS_Context like in other similar places
@Alkarex
Copy link
Copy Markdown
Member Author

Alkarex commented Dec 30, 2019

I have just updated this branch so that it is functional again (tested with NewsJet), and in line with the latest changes in the rest of the FreshRSS code.
It would not be a big work to add the remaining functions

Adding a 32-bit column would not be a big deal either.

It would be motivating to list a few (good) clients that are supporting the TT-RSS API but not the Google Reader API (for self-hosted)

@Frenzie
Copy link
Copy Markdown
Member

Frenzie commented Dec 30, 2019

I assume this one doesn't support GReader. I haven't tried it but it seems to have decent ratings on Google Play.

https://f-droid.org/en/packages/org.ttrssreader/

@aledeg
Copy link
Copy Markdown
Member

aledeg commented Feb 22, 2021

It's weird to see that PR. Instead of including extension in the core, I would go the other way around and extract things as extension. This way, FreshRSS can be lightweight for when you don't need the bells and whistles.

For instance, I'd like to have views (normal, global, reading) as extensions to only install the one I am using. But I would love to enable different views (to be created). Same goes with the APIs since I am not using them.
I think those could still be in the core as core extensions though. This way, it's easy to maintain them, to turn them on and off.

But for sure, to be able to do that we need few things:

  • a proper documentation.
  • a better extension system (support for API extensions, view extensions, ...).
  • a better way to include extensions.
  • a better way to promote extensions.

My 2¢

@Frenzie
Copy link
Copy Markdown
Member

Frenzie commented Feb 23, 2021

I agree that for something like this a core extension seems like it could make sense, but lightweight strikes me as an argument that's unlikely to be valid. Maybe if you don't have opcache and it'd have to be parsed again and again? But I hardly think we should optimize for such a scenario. ;-) The number of APIs is and will remain extremely limited, and it's not like they're any easier or harder to implement one way or the other.

@aledeg
Copy link
Copy Markdown
Member

aledeg commented Feb 23, 2021

My point was more to have a modular architecture which makes more sense when the range of use is really wide. We cannot address all of them so maybe we can have more extension contributors.
When I mentioned lightweight, I was thinking about the code-base not the server use :)

@Alkarex
Copy link
Copy Markdown
Member Author

Alkarex commented Feb 26, 2021

This feature requires some changes in the core, if we were to implement it fully. Afterwards, it can be a standalone file like the other APIs.

@PetraOleum
Copy link
Copy Markdown
Contributor

The app I'd like to be able to use this api with is newsboat, which as it stands I can't get to work with freshrss - of course, that's not a phone app!

Base automatically changed from master to edge March 14, 2021 18:46
Alkarex added a commit to FreshRSS/Extensions that referenced this pull request Dec 31, 2023
It was not working, and has not been maintained in a long time.
A follow-up was started on FreshRSS/FreshRSS#1526 as it would require changes in the core for proper support.
But if anybody was using this extension for anything, reach out :-)
@math-GH math-GH added the BLOCKED ❌ merge conflict or failing tests that block the PR label Jun 14, 2024
Alkarex added a commit to FreshRSS/Extensions that referenced this pull request Oct 27, 2024
It was not working, and has not been maintained in a long time.
A follow-up was started on FreshRSS/FreshRSS#1526 as it would require changes in the core for proper support.
But if anybody was using this extension for anything, reach out :-)
@Alkarex Alkarex closed this Oct 25, 2025
@Alkarex Alkarex removed this from the Backlog milestone Nov 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API 🤝 API for other clients BLOCKED ❌ merge conflict or failing tests that block the PR Update action 💾 Work in progress 🚧 Wait before merging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants