Enable Episode upload as array #30

JonOfUs · 2021-10-04T16:43:27Z

This enables gpoddersync to work with EpisodeActions from an multidimensional array as wished here.
SInce it is described in the gpodder api docs this way, this would be preferrable. This would make it easier for other podcast apps to implement gpoddersync as sync service as well.

With this, gpoddersync checks whether the data is an array or a string and parses it with the according functions.
In order to maintain the logic, the array has to come in the format given at the bottom, 3-dimensional. The "data"-dimension could fall away, but then the Controller had to be rewritten drastically.

So it's discussable if the current variant is preferrable or a version where an input without this 3rd dimension. The last variant would be even closer to the gpodder api.

I also wrote two tests, for an input without guid and an input of multiple EpisodeActions - and phpunit doesn't complain.

Format example:

{"data": [
 {
  "podcast": "https://www.ndr.de/nachrichten/info/podcast4684.xml", 
  "episode": "https://mediandr-a.akamaihd.net/download/podcasts/podcast4684/AU-20210914-1706-2200.mp31", 
  "guid": "AU-20210914-1706-2200-A1", 
  "action": "PLAY", 
  "timestamp": "Sun Oct 03 14:03:17 GMT+02:00 2021", 
  "started": 5600, 
  "position": 5600, 
  "total": 5600
 },
 {
  "podcast": "https://www.ndr.de/nachrichten/info/podcast4684.xml", 
  "episode": "https://mediandr-a.akamaihd.net/download/podcasts/podcast4684/AU-20210914-1706-2201.mp31", 
  "guid": "AU-20210914-1706-2201-A1", 
  "action": "PLAY", 
  "timestamp": "Sun Oct 03 14:03:17 GMT+02:00 2021", 
  "started": 5600, 
  "position": 5600, 
  "total": 5600
 }
]}

thrillfall · 2021-10-05T11:41:11Z

Thanks for fixing this.
I am currently changing the database timestamp handling to also make them work with mysql.
Once that is fixed and phpunit github actions go green your PR can be rebased and merged.

In the mean time you can drop the original string parsing part and keep only your new json parsing.
I always wondered why AntennaPod would create the request as this weird string but it was a copy&paste mistake on my side.

JonOfUs · 2021-10-05T13:01:44Z

Great! Did this right away. But now, this PR isn't compatible with the current AntennaPod versions - we should keep this in mind.

I also adjusted the format to the gpodder api documentation, so now a valid input would be:

[
  {
   "podcast": "https://www.ndr.de/nachrichten/info/podcast4684.xml", 
   "episode": "https://mediandr-a.akamaihd.net/download/podcasts/podcast4684/AU-20210914-1706-2200.mp31", 
   "guid": "AU-20210914-1706-2200-A1", 
   "action": "PLAY", 
   "timestamp": "Sun Oct 03 14:03:17 GMT+02:00 2021", 
   "started": 5600, 
   "position": 5600, 
   "total": 5600
  },
  {
   "podcast": "https://www.ndr.de/nachrichten/info/podcast4684.xml", 
   "episode": "https://mediandr-a.akamaihd.net/download/podcasts/podcast4684/AU-20210914-1706-2201.mp31", 
   "guid": "AU-20210914-1706-2201-A1", 
   "action": "PLAY", 
   "timestamp": "Sun Oct 03 14:03:17 GMT+02:00 2021", 
   "started": 5600, 
   "position": 5600, 
   "total": 5600
  }
]

JonOfUs · 2021-10-05T13:05:32Z

lib/Controller/EpisodeActionController.php

-		return $this->episodeActionSaver->saveEpisodeActions($data, $this->userId);
+	public function create(): JSONResponse {
+
+		$episodeActionsArray = $this->filterEpisodesFromRequestParams($this->request->getParams());


I'm not sure if this is done the right way, because the array filtering could have been done right here as well. I decided to do it this way to make the code better understandable.
Filtering out non-numeric indexes is necessary, because in the request params there is always one element "_route":"gpoddersync.episode_action.create", that has to be filtered out.

Don't we get the json body as first argument in the controller action? Why do you retrieve the data from class property?

We had {"data": [{"podcast": ... }, {"podcast": ... }]}, we only got the json body as first argument because it is stored in the field "data" (before, the EpisodeAction string was stored in this field).
With my changes we expect [{"podcast": ... }, {"podcast": ... }]. This is not only very close to the gpodder api, Bytehamster even changed the payload for EpisodeAction upload to the same as gpodder.net in AntennaPod.

JonOfUs · 2021-10-05T14:30:23Z

I updated the README to contain a better api documentation and some examples, e.g. to point out the differences to gpodder. I hope this is okay.

And the episode action upload now expects a UTC ISO 8601 DateTime - the one gpodder uses. But this could possibly lead to merge problems since you work on timestamp related things too.

thrillfall · 2021-10-06T12:30:43Z

@JonOfUs thanks for your patience.
Can you please rebase you branch. I already changed (inspired by you) the timestamp output format.

lib/Core/EpisodeAction/EpisodeActionSaver.php

Parsing EpisodeActions from String can lead to problems if the String is changed by only a little bits. Additionally, the gpodder api describes the upload as Array. Uploading EpisodeActions as String is still possible.

Remove 3rd dimension of POST data by directly accessing request data - a simple EpisodeActions array has to be posted now. This way the api is closer to gpodder. Additionally, removed EpisodeAction upload as String, so it's now incompatible with older versions

'started', 'position' and 'total' are optional and 'timestamp' is in 'Y-m-d\TH:i:s' format

Background: action is lowercase in gpodder api description - gpoddersync always worked with uppercase action.

JonOfUs · 2021-10-06T15:32:03Z

Now it works with latest AntennaPod debug (e0140a8). This version uses the same POST format as for gpodder.net. I am currently testing it in practice, but it seems to work fine - while using the new epoch_timestamp.

thrillfall · 2021-10-06T15:47:14Z

The tests still fail.

JonOfUs · 2021-10-06T16:20:05Z

I've got a problem with the tests:
all of them run EpisodeActionSaver->saveEpisodeActions() and use the return value for the tests. Since 80fbe44 in this PR this function returns void.
There is no alternative function the tests can call in order to get the expected return values. Should I now add a return value again to this function, just for the integration tests?

JonOfUs · 2021-10-06T16:22:01Z

With this function returning the old values, all tests pass, but nevertheless phpunit returns a warning about skipped tests:

$ phpunit8 --bootstrap tests/bootstrap.php -c tests/phpunit.xml
PHPUnit 8.5.20 by Sebastian Bergmann and contributors.

.............S                                                    14 / 14 (100%)

Time: 309 ms, Memory: 10.00 MB

OK, but incomplete, skipped, or risky tests!
Tests: 14, Assertions: 62, Skipped: 1.

Do you know what this could mean?

thrillfall · 2021-10-06T17:03:53Z

With this function returning the old values, all tests pass, but nevertheless phpunit returns a warning about skipped tests:
$ phpunit8 --bootstrap tests/bootstrap.php -c tests/phpunit.xml
PHPUnit 8.5.20 by Sebastian Bergmann and contributors.

.............S                                                    14 / 14 (100%)

Time: 309 ms, Memory: 10.00 MB

OK, but incomplete, skipped, or risky tests!
Tests: 14, Assertions: 62, Skipped: 1.
Do you know what this could mean?

That would then be that test:

nextcloud-gpodder/tests/Integration/Migration/TimestampMigrationTest.php

Line 57 in 896943a

self::markTestSkipped("This test only works on postgres");

That shouldn't be a problem.

Push your changes and let's see

lib/Core/EpisodeAction/EpisodeActionSaver.php

thrillfall · 2021-10-06T17:46:22Z

I pushed all your commits to main. All tests pass. Thanks a million!!!
Now we even can be sure that this stuff works for 4 databases and the latest 2 nextcloud releases. These automated tests are awesome!

thrillfall · 2021-10-06T17:49:15Z

@JonOfUs would you do the honor and create a release commit for this? I'd go with version 3.0.0 since we break (yet again) the api ;-)

The actual release to the appstore will be done by creating a tag

JonOfUs · 2021-10-06T19:04:27Z

Too kind of you ;)
~~Do I just have to create a new release? And will the gpoddersync.tar.gz file be automatically added or do I have to generate one?~~

JonOfUs · 2021-10-06T19:38:43Z

Nope, something's missing here. The AppStore just got a new v2.0.0. Where do I have to add "3.0.0" and the changelog? In appinfo/info.xml?

JonOfUs · 2021-10-06T19:39:05Z

I think you have to take over again ^^

thrillfall · 2021-10-06T19:51:08Z

Nope, something's missing here. The AppStore just got a new v2.0.0. Where do I have to add "3.0.0" and the changelog? In appinfo/info.xml?

Exactly. You prepare a PR with updated info.xml and CHANGELOG.MD.
Then we merge and tag

JonOfUs commented Oct 5, 2021

View reviewed changes

mattsches mentioned this pull request Oct 5, 2021

Fix "undefined index" notice if no matches found #31

Closed

thrillfall reviewed Oct 6, 2021

View reviewed changes

lib/Core/EpisodeAction/EpisodeActionSaver.php Outdated Show resolved Hide resolved

JonOfUs added 5 commits October 6, 2021 15:52

Allow EpisodeAction upload as array

445cf92

Parsing EpisodeActions from String can lead to problems if the String is changed by only a little bits. Additionally, the gpodder api describes the upload as Array. Uploading EpisodeActions as String is still possible.

Test array parsing without guid

901112d

Adjust expected DateTime format to gpodder - UTC ISO 8601

657996e

Update API documentation - add examples & differences between gpodder

11de3ea

JonOfUs force-pushed the episode-upload-as-array branch from 462431c to 11de3ea Compare October 6, 2021 13:56

JonOfUs added 2 commits October 6, 2021 16:41

Align episode upload format to gpodder-api

4b630c7

'started', 'position' and 'total' are optional and 'timestamp' is in 'Y-m-d\TH:i:s' format

Ensure uppercase action in EpisodeAction upload

e3afdd8

Background: action is lowercase in gpodder api description - gpoddersync always worked with uppercase action.

Update API documentation

4365b2c

Update tests to new upload format

78f0161

JonOfUs commented Oct 6, 2021

View reviewed changes

lib/Core/EpisodeAction/EpisodeActionSaver.php Outdated Show resolved Hide resolved

thrillfall closed this Oct 6, 2021

JonOfUs deleted the episode-upload-as-array branch June 14, 2022 20:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable Episode upload as array #30

Enable Episode upload as array #30

JonOfUs commented Oct 4, 2021

thrillfall commented Oct 5, 2021 •

edited

JonOfUs commented Oct 5, 2021

JonOfUs Oct 5, 2021

thrillfall Oct 6, 2021

JonOfUs Oct 6, 2021 •

edited

JonOfUs commented Oct 5, 2021 •

edited

thrillfall commented Oct 6, 2021

JonOfUs commented Oct 6, 2021 •

edited

thrillfall commented Oct 6, 2021

JonOfUs commented Oct 6, 2021

JonOfUs commented Oct 6, 2021

thrillfall commented Oct 6, 2021 •

edited

thrillfall commented Oct 6, 2021

thrillfall commented Oct 6, 2021 •

edited

JonOfUs commented Oct 6, 2021 •

edited

JonOfUs commented Oct 6, 2021

JonOfUs commented Oct 6, 2021

thrillfall commented Oct 6, 2021

Enable Episode upload as array #30

Enable Episode upload as array #30

Conversation

JonOfUs commented Oct 4, 2021

thrillfall commented Oct 5, 2021 • edited

JonOfUs commented Oct 5, 2021

JonOfUs Oct 5, 2021

Choose a reason for hiding this comment

thrillfall Oct 6, 2021

Choose a reason for hiding this comment

JonOfUs Oct 6, 2021 • edited

Choose a reason for hiding this comment

JonOfUs commented Oct 5, 2021 • edited

thrillfall commented Oct 6, 2021

JonOfUs commented Oct 6, 2021 • edited

thrillfall commented Oct 6, 2021

JonOfUs commented Oct 6, 2021

JonOfUs commented Oct 6, 2021

thrillfall commented Oct 6, 2021 • edited

thrillfall commented Oct 6, 2021

thrillfall commented Oct 6, 2021 • edited

JonOfUs commented Oct 6, 2021 • edited

JonOfUs commented Oct 6, 2021

JonOfUs commented Oct 6, 2021

thrillfall commented Oct 6, 2021

thrillfall commented Oct 5, 2021 •

edited

JonOfUs Oct 6, 2021 •

edited

JonOfUs commented Oct 5, 2021 •

edited

JonOfUs commented Oct 6, 2021 •

edited

thrillfall commented Oct 6, 2021 •

edited

thrillfall commented Oct 6, 2021 •

edited

JonOfUs commented Oct 6, 2021 •

edited