Limit request size to api in MiteSynchronizer::TimeEntries remote_records

[das-peter]

We had the case that over 1000 id's had to be synced which failed permanently.
After a long debugging session I figured out that the request size is exceeded if around 700 ids are used for Mite::TimeEntry.find().
If the request limit is exceeded you'll get an error like this Net::HTTPBadResponse: wrong status line: - but to see it you've to add a logger to MiteController::save_account_data()

I tried to workaround this issue by adding a loop - but frankly speaking I've no clue of ruby.
From an architecture point of view the fix also should be located in Mite::TimeEntry or even Mite and not in MiteSynchronizer::TimeEntries.
But to solve it the right way I've simply not enough ruby knowledge ;)

However it would be nice if the change or an enhanced version of it would get pulled into the official branch.

Thank you very much & best regards
Peter

[yolk]

Hello Peter,

first of all: Thanks for your afford to fix the problem directly in redmine2mite by your self! And sorry to hear about your lost data.

But your fix suffers from one issue: It sets the variable @remote_records only to the first 500 records and because of the "||=" never performs the additional calls to the mite.api. Here is a fast attempt to fix this by building up an array with inject and setting @remote_records to it:

@remote_records ||= local_records.map(&:mite_time_entry_id).each_slice(500).inject([]) do |records, ids|
    records += Mite::TimeEntry.find(:all, :params => {:ids => ids.join(",")})
end

I share your point about fixing this directly in mite.rb. But at the moment I am not sure how to detect and do it properly: I could change the GET-Request into a POST to prevent the "To much Data"-Error or as you did split it up into several requests. I'll give it a thought and let you and @thomasklein know about the result.

Thanks again!
Sebastian.

[thomasklein]

Hi Peter, hi Sebastian,

sorry for my late response. Thanks for the bug report Peter and for code chunk to fix it! Also thanks Sebastian for the improved version of the bugfix. Could you, Peter, please confirm that everything works now as expected?

Best,
Thomas

[das-peter]

Thank you both for the response!
Happy to see this was fixed so fast :)
@yolk A change to use POST instead GET sounds reasonable - even if it's a bit odd because the REST request is to GET data and not to POST them ;)

[yolk]

Not very RESTful, indeed ;) The better – but slightly more complex solution – would be to POST the parameters, store them serverside with an ID and using this ID to refer to the stored parameters when sending the actual GET-Request.

We'll see.

[yolk]

@thomasklein You pulled in the code witch only receives the first 500 entries, as described above. Any plans to apply the fixes? No need to use my quick code above, but the described problems are still present.

Or are you waiting for an fix for mite.rb? Sorry but this will take some time, because it will involve a change in the mite.api.

Thx!

[thomasklein]

Currently I'm working on an update applying some other changes to the plugin as well. Publishing it soon, including your fix.