Summary | Splitting long content into multiple messages |
Queue | Synchronization |
Queue Version | Git master |
Type | Enhancement |
State | Resolved |
Priority | 1. Low |
Owners | jan (at) horde (dot) org |
Requester | thomas (at) trethan (dot) net |
Created | 03/12/2012 (4860 days ago) |
Due | |
Updated | 01/28/2016 (3443 days ago) |
Assigned | |
Resolved | 01/28/2016 (3443 days ago) |
Milestone | |
Patch | No |
State ⇒ Resolved
commit bcb483d7d3ece91796f8aade0822a7f3c260c315
Author: Jan Schneider <jan@horde.org>
Date: Thu Jan 28 16:01:41 2016 +0100
[jan] Split large objects into multiple messages
(thomas@trethan.net,
Request #11071).framework/SyncMl/lib/Horde/SyncMl.php | 11 ++-
.../SyncMl/lib/Horde/SyncMl/Command/Alert.php | 23 +--
.../SyncMl/lib/Horde/SyncMl/Command/SyncHdr.php | 18 ++-
framework/SyncMl/lib/Horde/SyncMl/State.php | 9 +
framework/SyncMl/lib/Horde/SyncMl/Sync.php | 164
+++++++++++++++++---
framework/SyncMl/lib/Horde/SyncMl/XmlOutput.php | 16 ++-
framework/SyncMl/package.xml | 2 +
7 files changed, 203 insertions(+), 40 deletions(-)
http://github.com/horde/horde/commit/bcb483d7d3ece91796f8aade0822a7f3c260c315
me, runs stable since posted in march 2012... would be glad to see
this finally go live someday...
New Attachment: SyncSplit2.patch
Sync.php:
- code to get next chunk was moved to seperate function
_getServerLargeObjChunk()
- code to determine split position and ensure that no whitespaces are
on edges optimized by single preg_match function instead of loop (see
_getServerLargeObjChunk())
- checks whether there is space left in sync messages adapted: only
split objects, that are too large for a single message; objects that
fit into a single message won't be split anymore, but sent in the next
message; this prevents skipping of smaller objects when client device
doesn't support splitting
I think handling of client responses should be treated in another
ticket, since it is a general issue and not related to splitting
objects only.
(Command/Status.php)? We simply ignore responses like
RESPONSE_SIZE_MISMATCH or RESPONSE_COMMAND_FAILED? Also
when sending items to the client, $_server_add/replace_count are
simply incremented without checking the client's response. Is this the
desired behavior? Wouldn't it be better to handle responses in
Horde_SyncMl_Command::handleCommand() and react on problems? This way
we could also log client side errors.
added RESPONSE_SIZE_REQUIRED because it reflects the correct state.
But true, it is not used yet. I guess it should be handled in
Command/Alert.php by creating a log entry that there was an error. I
will have a look at this and adapt it.
2) I think you're talking about finding the right split position. It
is not really about stripping whitespaces rather than ensuring that
there are no whitespaces on the edges. But I agree, that the while
loop is not very efficient. I will rewrite this.
3) I will look at this again. For sure the add/replace parts can be
moved to a seperate function because they are quite the same. Handling
other chunks differs a bit, but I guess it should be possible to move
at least the code for splitting into this seperate function too.
4) It is the latter; maxObjSize defines the maximum size of a single
object the client supports, whereas maxMsgSize defines the size of a
single sync message (which can hold a split object). The if ensures,
that objects that are too large for the client won't be sent at all.
As far as I understood, clientContent holds only one object at a time
(within the add/replace loop). clientContent is filled by
"list($clientContent, $clientContentType, $clientEncodingType) =
$device->convertServer2Client($c, $contentType, $syncDB);" and $c
refers to "$c = $backend->retrieveEntry($syncDB, $suid, $ct,
$fields[$ct]);" which should by just a single sync object. Therefore I
think the comparison should be correct, but please correct if I'm wrong.
As written above I will implement the fixes and post another patch
within the next days.
State ⇒ Feedback
- Why do you add RESPONSE_SIZE_REQUIRED and replace
RESPONSE_INVALID_CREDENTIALS with it? It's not even used.
- You could optimize the whitespace stripping. You don't need a loop
there, a preg_match('/(\s*)$/') should be sufficient.
- Since you do the size-exceeding twice and the chunk splitting even
thrice, you should implement this in a single helper method. At least
the larger if-then-else blocks.
- I'm trying to understand why the "if ($state->maxObjSize <
$clientContentLength)" cases exists. Isn't the whole purpose to split
up objects (not only messages) that exceed the client's maximum size?
Or is this the maximum size of the individual object on the client
side storage (opposed to the maximum size allowed during the
transfer)? If the latter, isn't the comparison with
$clientContentLength incorrect because it contains more than the
individual object?
New Attachment: SyncSplit.patch
via SyncML by extending the Horde sync functionalities. It was not
necessary to refactor the whole code, I just adapted and added a view
things and it works fine (see attached SyncSplit.patch).
Tested it with Horde 4.0.14 and a Samsung Galaxy S II running the
Synthesis Std client for Android. Surely more tests on other devices
will be necessary, unfortunately I don't have that much possibilites...
Priority ⇒ 1. Low
Type ⇒ Enhancement
Summary ⇒ Splitting long content into multiple messages
Queue ⇒ Synchronization
Milestone ⇒
Patch ⇒ No
State ⇒ New
multiple messages? Newer smartphones are able to use larger images for
contact photos. Unfortunately Horde skips entries, when data exceeds
MaxMsgSize, so most contacts won't get synced.