nathanheffley / laravel-watermelon

Easily set up a sync endpoint to support Watermelon DB in your Laravel projects.
MIT License
44 stars 13 forks source link

More compatibilty to older Laravel and PHP version #7

Closed rafwell closed 9 months ago

rafwell commented 9 months ago

Hi,

Here I'm working in Laravel 6.x and PHP 7.2 and I need do some modifies to improve compatibility. I think those changes wont break in php 8 or new versions, basically was typing changes.

Another change is why here I'm in GMT-3, my created_at, updated_at, deleted_at was saved in this timezone, in a datetime field on my database mysql. When the sync push changes they send the latest pull timestamp, who are in gmt-3, and this library whas changing this to GMT-0. It cause not pull my recent changes.

If this changes can break the project I can improve some modifies, jus let me know.

Thank you for your work.

nathanheffley commented 9 months ago

Sorry, but Laravel 6 hasn't received security fixes in over a year, and PHP 7.2 hasn't received security fixes in almost 3 years, I am not going to support them even if it is "simple." In my professional opinion, you need to upgrade the project ASAP as it's a massive security vulnerability. I will not go out of my way to assist in keeping Laravel 6/PHP 7.2 projects running beyond their EOL window. I'm sorry that you're in this situation. That being said, it's an open source project so if you wish to fork it and implement these changes, I can't stop you.

I'd be more open to addressing the $lastPulledAt issue as I can appreciate working with a preexisting database. Although I must admit I'm not exactly sure what the issue is, as WatermelonDB uses timestamps which aren't tied to a timezone. Do the dates in your database not store the timezone they are in?

rafwell commented 9 months ago

I agree with you, but we fight with the arms who we have, so... I think this change wont bring some vulnerability to your package, it was only sintaxe changes. But I understand you. No changes.

About the lastPulledAt, I really don't understand exactly what are doing. Like you see, my column is a timestamp, but when I do a select it return like a datetime (it is a mysql behavior) , in gmt-3 what is my timezone:

Captura de Tela 2023-11-29 às 14 48 12 Captura de Tela 2023-11-29 às 14 48 33

If I do a insert in laravel, and do a select after, the datetime is right with my current timezone.

So, when the push occurrs in watermelon sync, the datetime who was parsed from your package give me 3 hours after now, when the query do 'created_at>=$latestpull' the lastest pull is 3 hours after now, so it not return rows.

I expect it was clear now, but if not sounds good for you, maybe it was an problem of my old project. However i think not.

nathanheffley commented 9 months ago

Okay, I think I am understanding. The timestamp that is returned to the client in that pull endpoint is generated with now()->timestamp which will respect your server's timezone configuration, but when that same endpoint parses it, it will parse it into UTC but the timestamp column doesn't seem to have timezones built into it.

I would love to see this fixed. Can you try out whether instead of adding that new line with setTimezone if you change line 41 from $lastPulledAt = Carbon::createFromTimestampUTC($lastPulledAt); to $lastPulledAt = Carbon::createFromTimestamp($lastPulledAt); and see if that also fixes your problem?

rafwell commented 9 months ago

Yes. I can confirm, if I use createFromTimestamp instead of createFromTimestampUTC fix the problem without set timezone.

nathanheffley commented 9 months ago

@rafwell I have made release v1 with a fix for the timezone issue. Thank you for finding that discrepancy! If you need to back port this to older versions of Laravel/PHP still, I recommend forking the newest version of the repo. Hopefully you can get the project up to date at some point in the future and move back to the official repo.