Skip to content
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

Update feed only if nextUpdateTime has been reached #2999

Merged
merged 6 commits into from
Dec 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ You can also check [on GitHub](https://github.com/nextcloud/news/releases), the
## [25.x.x]
### Changed
- API add new field to Feed that indicates when the next update will be done "nextUpdateTime" (#2993)
- Change logic to update feed only if the nextUpdateTime has been reached (#2999)
- Add setting to disable the usage of nextUpdateTime (#2999)

### Fixed
- `TypeError: this.$refs.actions.$refs.menuButton is undefined` when tabbing through feeds and folders
Expand Down
2 changes: 1 addition & 1 deletion appinfo/info.xml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ Create a [feature request](https://github.com/nextcloud/news/discussions/new)

Report a [feed issue](https://github.com/nextcloud/news/discussions/new)
]]></description>
<version>25.1.2</version>
<version>25.2.0</version>
<licence>agpl</licence>
<author>Benjamin Brahmer</author>
<author>Sean Molenaar</author>
Expand Down
9 changes: 2 additions & 7 deletions docs/admin.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,6 @@ The update interval is used to determine when the next update of all feeds shoul
By default, the value is set to 3600 seconds (1 hour) You can configure this interval as an administrator.
The new value is only applied after the next run of the updater.

#### What is a good update interval?
Since News 25.2.0 News no longer will update all feeds instead it will make a individual decision based on when an update for the feed will make sense. Therefore this interval is now only to be understood as the check if an update should be done.

That depends on your individual needs.
Please keep in mind that the lower you set your update interval, the more traffic is generated.

#### Can I set individual update intervals per feed/user?

No, that is not possible.
This behavior can be disabled in the Settings although we generally do not recommend that.
14 changes: 14 additions & 0 deletions docs/user.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# User

Welcome to the User documentation of News.

# TODO
This documentation is work in progress.

# User interface

explain the different options in the UI especially options for feeds and settings

# Using News with Clients

explain sync and link to clients page
1 change: 1 addition & 0 deletions lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ class Application extends App implements IBootstrap
'useCronUpdates' => true,
'exploreUrl' => '',
'updateInterval' => 3600,
'useNextUpdateTime' => true,
];

public function __construct(array $urlParams = [])
Expand Down
4 changes: 3 additions & 1 deletion lib/Db/Feed.php
Original file line number Diff line number Diff line change
Expand Up @@ -663,10 +663,12 @@ public function setUserId(string $userId): Feed
/**
* @param int $nextUpdateTime
*/
public function setNextUpdateTime(?int $nextUpdateTime): void
public function setNextUpdateTime(?int $nextUpdateTime): Feed
{
$this->nextUpdateTime = $nextUpdateTime;
$this->markFieldUpdated('nextUpdateTime');

return $this;
}

public function toAPI(): array
Expand Down
10 changes: 9 additions & 1 deletion lib/Fetcher/FeedFetcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,13 @@ public function fetch(
);

$feed->setNextUpdateTime($resource->getNextUpdate()?->getTimestamp());
$this->logger->debug(
'Feed {url} was parsed and nextUpdateTime is {nextUpdateTime}',
[
'url' => $url,
'nextUpdateTime' => $feed->getNextUpdateTime()
]
);

if (!is_null($resource->getResponse()->getLastModified())) {
$feed->setHttpLastModified($resource->getResponse()->getLastModified()->format(DateTime::RSS));
Expand All @@ -158,10 +165,11 @@ public function fetch(
$feedName = $parsedFeed->getTitle();
$feedAuthor = $parsedFeed->getAuthor();
$this->logger->debug(
'Feed {url} was modified since last fetch. #{count} items',
'Feed {url} was modified since last fetch. #{count} items, nextUpdateTime is {nextUpdateTime}',
[
'url' => $url,
'count' => count($parsedFeed),
'nextUpdateTime' => $feed->getNextUpdateTime(),
]
);

Expand Down
46 changes: 44 additions & 2 deletions lib/Service/FeedServiceV2.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@

use OCA\News\Db\FeedMapperV2;
use OCA\News\Fetcher\FeedFetcher;
use OCA\News\AppInfo\Application;
use OCA\News\Service\Exceptions\ServiceConflictException;
use OCA\News\Service\Exceptions\ServiceNotFoundException;
use OCP\AppFramework\Db\Entity;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\IAppConfig;

use OCA\News\Db\Feed;
use OCA\News\Db\Item;
Expand Down Expand Up @@ -61,6 +63,10 @@ class FeedServiceV2 extends Service
* @var Explorer
*/
protected $explorer;
/**
* @var IAppConfig
*/
protected $config;

/**
* FeedService constructor.
Expand All @@ -71,21 +77,24 @@ class FeedServiceV2 extends Service
* @param Explorer $explorer Feed Explorer
* @param HTMLPurifier $purifier HTML Purifier
* @param LoggerInterface $logger Logger
* @param IAppConfig $config App config
*/
public function __construct(
FeedMapperV2 $mapper,
FeedFetcher $feedFetcher,
ItemServiceV2 $itemService,
Explorer $explorer,
HTMLPurifier $purifier,
LoggerInterface $logger
LoggerInterface $logger,
IAppConfig $config
) {
parent::__construct($mapper, $logger);

$this->feedFetcher = $feedFetcher;
$this->itemService = $itemService;
$this->explorer = $explorer;
$this->purifier = $purifier;
$this->config = $config;
}

/**
Expand Down Expand Up @@ -244,12 +253,13 @@ public function create(
$feed->setFolderId($folderId)
->setUserId($userId)
->setHttpLastModified(null)
->setNextUpdateTime(null)
->setArticlesPerUpdate(count($items));

if ($title !== null) {
$feed->setTitle($title);
}

if ($feed->getTitle() === null) {
$feed->setTitle(parse_url($feedUrl)['host']);
}
Expand All @@ -276,6 +286,28 @@ public function fetch(Entity $feed): Entity
return $feed;
}

// Check if the nextUpdateTime check should be used
$useNextUpdateTime = $this->config->getValueBool(
Application::NAME,
'useNextUpdateTime',
Application::DEFAULT_SETTINGS['useNextUpdateTime']
);

if ($useNextUpdateTime) {
$nextUpdateTime = $feed->getNextUpdateTime();
$currentTime = time();
$tolerance = 10 * 60; // 10 minutes tolerance

if ($nextUpdateTime !== null && ($currentTime + $tolerance) < $nextUpdateTime) {
$this->logger->info('Feed update skipped. Next update time not reached.', [
'feedUrl' => $feed->getUrl(),
'nextUpdateTime' => $nextUpdateTime,
'currentTime' => $currentTime,
]);
return $feed;
}
}

// for backwards compatibility it can be that the location is not set
// yet, if so use the url
$location = $feed->getLocation() ?? $feed->getUrl();
Expand Down Expand Up @@ -326,6 +358,16 @@ public function fetch(Entity $feed): Entity
$feed->setHttpLastModified($fetchedFeed->getHttpLastModified())
->setLocation($fetchedFeed->getLocation());

// check if useNextUpdateTime is set by the admin
// if so update value with the timestamp
// otherwise set it to null to indicate to clients
// that they can not use that field for any info.
if ($useNextUpdateTime) {
$feed->setNextUpdateTime($fetchedFeed->getNextUpdateTime());
} else {
$feed->setNextUpdateTime(null);
}

foreach (array_reverse($items) as &$item) {
$item->setFeedId($feed->getId())
->setBody($this->purifier->purify($item->getBody()));
Expand Down
14 changes: 13 additions & 1 deletion src/components/AdminSettings.vue
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,17 @@ SPDX-Licence-Identifier: AGPL-3.0-or-later
<p class="settings-hint">
{{ t("news", "Interval in seconds in which the feeds will be updated.") }}
</p>

<div class="field">
<NcCheckboxRadioSwitch type="switch"
:checked.sync="useNextUpdateTime"
@update:checked="update('useNextUpdateTime', useNextUpdateTime)">
{{ t("news", "Use next update time for feed updates") }}
</NcCheckboxRadioSwitch>
</div>
<p class="settings-hint">
{{ t("news", "Enable this to use the calculated next update time for feed updates. Disable to update feeds based solely on the update interval.") }}
</p>
</NcSettingsSection>
</template>

Expand Down Expand Up @@ -140,6 +151,7 @@ export default {
feedFetcherTimeout: loadState('news', 'feedFetcherTimeout'),
exploreUrl: loadState('news', 'exploreUrl'),
updateInterval: loadState('news', 'updateInterval'),
useNextUpdateTime: loadState('news', 'useNextUpdateTime') === '1',
relativeTime: moment(lastCron * 1000).fromNow(),
lastCron,
}
Expand All @@ -159,7 +171,7 @@ export default {
key,
},
)
if (key === 'useCronUpdates' || key === 'purgeUnread') {
if (key === 'useCronUpdates' || key === 'purgeUnread' || key === 'useNextUpdateTime') {
value = value ? '1' : '0'
}
try {
Expand Down
15 changes: 13 additions & 2 deletions tests/Unit/Service/FeedServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
use OCA\News\Service\ItemServiceV2;
use OCA\News\Utility\Time;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\IAppConfig;

use OCA\News\Db\Feed;
use OCA\News\Db\Item;
Expand Down Expand Up @@ -77,6 +78,11 @@ class FeedServiceTest extends TestCase
*/
private $explorer;

/**
* @var \PHPUnit\Framework\MockObject\MockObject|IAppConfig
*/
private $config;

private $response;

protected function setUp(): void
Expand Down Expand Up @@ -112,14 +118,19 @@ protected function setUp(): void
->getMockBuilder(\HTMLPurifier::class)
->disableOriginalConstructor()
->getMock();

$this->config = $this
->getMockBuilder(IAppConfig::class)
->disableOriginalConstructor()
->getMock();

$this->class = new FeedServiceV2(
$this->mapper,
$this->fetcher,
$this->itemService,
$this->explorer,
$this->purifier,
$this->logger
$this->logger,
$this->config
);
$this->uid = 'jack';
}
Expand Down
5 changes: 3 additions & 2 deletions tests/api/feeds.bats
Original file line number Diff line number Diff line change
Expand Up @@ -147,5 +147,6 @@ teardown() {
# run is not working here.
output=$(http --ignore-stdin -b -a ${user}:${APP_PASSWORD} POST ${BASE_URLv1}/feeds url=$NC_FEED | jq '.feeds | .[0].nextUpdateTime')

assert_output '2071387335'
}
# the field nextUpdateTime should be null here since the feed just got created in the DB an the items have not yet been fetched
assert_output 'null'
}
2 changes: 1 addition & 1 deletion tests/javascript/unit/components/AdminSettings.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('AdminSettings.vue', () => {
})

it('should initialize and fetch settings from state', () => {
expect(loadState).toBeCalledTimes(8)
expect(loadState).toBeCalledTimes(9)
})

it('should send post with updated settings', async () => {
Expand Down
2 changes: 1 addition & 1 deletion tests/test_helper/php-feed-generator
Loading
Loading