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

[feature/configurable-poll-interval] Configurable poll interval #1021

Merged
merged 7 commits into from
Sep 16, 2021

Conversation

felix-schwarz
Copy link
Contributor

@felix-schwarz felix-schwarz commented Aug 22, 2021

Description

Add support for configurable poll interval via capabilities.php and MDM:

  • through capabilities: mirroring changes from Poll interval from capabilities client#8777
  • through MDM via core.scan-for-changes-interval
  • time in milliseconds
    • defaulting to 10 seconds
    • enforcing (and logging a warning) for intervals below a minimum of 5 seconds
    • logging a warning for intervals greater than 60 seconds

Related Issue

owncloud/client#8777

Testing advice

Changing the return value of OCCapabilities.m:165, has the same effect as changing the poll interval number on the server. F.ex. for 5000 milliseconds:

- (NSNumber *)pollInterval
{
	return (@(5000));
	// return (OCTypedCast(_capabilities[@"core"][@"pollinterval"], NSNumber));
}

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

@CLAassistant
Copy link

CLAassistant commented Aug 22, 2021

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ hosy
❌ felix-schwarz
You have signed the CLA already but the status is still pending? Let us recheck it.

@felix-schwarz felix-schwarz added this to the 11.8.0-Current milestone Aug 22, 2021
@jesmrec
Copy link
Contributor

jesmrec commented Aug 25, 2021

QA

Tested with different intervals using the testing advice mentioned above

  • 3 secs:

i saw the log:

2021-08-25 11:21:46.935000+0200 ownCloud[631:063674] 🛑 | [CORE, PollForChanges, …] Poll interval in capabilities (3000) not server legacy default (60 (sec)), and - as milliseconds - less than minimum allowed poll interval (5.00 sec). Ignoring value. [… ItemList] [OCCore+ItemList.m:1488|FULL]

and the polling time is 10secs, by default.

here comes my question: if user sets 3secs, shouldn't be better to give him 5secs (minimum) instead of 10secs (default), so 5 is closer to the 3 he wanted?? @felix-schwarz @michaelstingl

  • 5 secs: ✅

  • 10secs ✅

  • 30secs ✅

  • 60secs ✅

  • 90secs ✅ , correctly logged:

2021-08-25 11:38:36.339000+0200 ownCloud[665:069779] ⚠️ | [CORE, PollForChanges, …] Poll interval (capabilities) of 90.00 sec > 60.00 sec [… ItemList] [OCCore+ItemList.m:1521|FULL]
2021-08-25 11:38:36.339000+0200 ownCloud[665:069779] 🔵 | [CORE, PollForChanges, …] Using poll interval of 90.00 sec (capabilities) [… ItemList] [OCCore+ItemList.m:1527|FULL]

@jesmrec
Copy link
Contributor

jesmrec commented Aug 25, 2021

OTOH, i realised that every PROPFIND is sent twice. This is not something from the current branch (also happens in master), but took my attention:

Screenshot 2021-08-25 at 12 59 17

both requests ask for the same properties

<?xml version="1.0" encoding="UTF-8"?>
<D:propfind xmlns:D="DAV:" xmlns:oc="http://owncloud.org/ns">
  <D:prop>
    <D:resourcetype/>
    <D:getlastmodified/>
    <D:getcontentlength/>
    <D:getcontenttype/>
    <D:getetag/>
    <oc:id/>
    <oc:size/>
    <oc:permissions/>
    <oc:favorite/>
    <oc:share-types/>
    <oc:owner-id/>
    <oc:owner-display-name/>
    <D:quota-available-bytes/>
    <D:quota-used-bytes/>
    <oc:checksums/>
  </D:prop>
</D:propfind>

@felix-schwarz
Copy link
Contributor Author

QA

Tested with different intervals using the testing advice mentioned above

  • 3 secs:

i saw the log:

2021-08-25 11:21:46.935000+0200 ownCloud[631:063674] 🛑 | [CORE, PollForChanges, …] Poll interval in capabilities (3000) not server legacy default (60 (sec)), and - as milliseconds - less than minimum allowed poll interval (5.00 sec). Ignoring value. [… ItemList] [OCCore+ItemList.m:1488|FULL]

and the polling time is 10secs, by default.

here comes my question: if user sets 3secs, shouldn't be better to give him 5secs (minimum) instead of 10secs (default), so 5 is closer to the 3 he wanted?? @felix-schwarz @michaelstingl

Since the unit changed from seconds to milliseconds, the idea here is that the unit of any value below 5000 can't be determined with certainty (seconds? milliseconds?) - and should therefore be deemed invalid and ignored.

The iOS implementation mirrors the desktop implementation in that regard.

OTOH, i realised that every PROPFIND is sent twice. This is not something from the current branch (also happens in master), but took my attention:

If you have client-side logging turned on, too: any chance this is sent - independently - by File Provider and app?

@jesmrec
Copy link
Contributor

jesmrec commented Aug 27, 2021

Since the unit changed from seconds to milliseconds, the idea here is that the unit of any value below 5000 can't be determined with certainty (seconds? milliseconds?) - and should therefore be deemed invalid and ignored.
The iOS implementation mirrors the desktop implementation in that regard.

👍 ok, thanks for the explanation. Will go with the approach.

If you have client-side logging turned on, too: any chance this is sent - independently - by File Provider and app?

sounds like an option, i will check. This is not a blocker here, so, i will do separately

Approved.

@jesmrec jesmrec added the Approved by QA Approved by QA label Aug 27, 2021
@hosy hosy changed the base branch from milestone/11.8 to milestone/11.7.1 September 15, 2021 12:36
@hosy hosy modified the milestones: 11.8.0-Current, 11.7.1-Fix Sep 15, 2021
# Conflicts:
#	ios-sdk
#	ownCloud.xcodeproj/xcshareddata/xcschemes/ownCloud.xcscheme
@hosy hosy merged commit 45063cb into milestone/11.7.1 Sep 16, 2021
@delete-merged-branch delete-merged-branch bot deleted the feature/configurable-poll-interval branch September 16, 2021 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved by QA Approved by QA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants