fix migration generator to ensure distinct version numbers

[atarihomestar]

The addition of the add_blocker_id_to_friendships functionality meant that the migration generator now generates 2 migrations. The method to generate each template is fast enough that the 2 calls might happen within the same second, causing their version numbers to be the same. I thought of adding a sleep(1) in between the calls, but that seems kind of lame. Instead the code I changed ensures that each call to migration_template generates a name that has a unique version number. It works even if more migrations are added later.

[sungwoncho]

isn't the first part the same as next_num = current_num + 1? Any reason you used +=?

[atarihomestar]

Here’s the deal. This method is so fast, that even 10 calls to migration_template could all happen within the same second. So next_num will stay the same (since it’s the datetime stamp) but current_num keeps incrementing with every call. Therefore next_num shouldn’t always be current_num + 1. You have to get the difference between the two, add one to that, and then INCREMENT next_num by that value.

Once you wrap your head around how it all works, it’s pretty simple and works well. Like I said before, you could add a sleep(1) between calls, or inside the call to next_migration_number but it seems lame to deliberately make your gem install go slower. You could also add the milliseconds on to the end of the version number, but then the version number format isn’t consistent with how most other migration version numbers are formatted.

[sungwoncho]

Thanks for the clarification. How about this:

next_num = current_num + 1 if current_num == next_num

Our solution to avoid overlapping migration timestamp is to bump the next migration timestamp by 1 if it is equal to the current_num. current_num will never be greater than next_num. I feel that the above line will do.

If you really think about it, x += (y - x + 1) <-> x = x + (y - x + 1) <-> x = y + 1. Please advise.

[atarihomestar]

How you propose it to be, is one of the ways I had it at first, but then I realized that current_num CAN be greater than next_num. Remember, next_num is basically the timestamp when you call the method, and that method can be called many times per second. So over 10 calls, they can all have the same next_num. But current_num will increment with every call. So current_num can be greater than next_num. And because of that, you have to do it how I propose - figuring the difference between next and current (or something like it.)

And as far as what Pensarfeo said, the sleep(1) solution was the first one I thought of, but it just seems wrong to make your gem install slower, and also, the only way there would be a problem is if another gem install or migration generator was run a second or two after this one finishes, which doesn't seem likely. And really, then you just go and manually change the numbers. Which really could be said for this gem install - leave it alone and let people manually change the numbers if it doesn't work. But I almost didn't use this gem because of the error. I'm glad I ended up looking into it, because I'm using it and it's fantastic.

So I still say that this pull request is the way to go. Thanks!

[atarihomestar]

By the way, when I first encountered this problem, I tried the
next_num = current_num + 1 if current_num == next_num
solution. It worked when there were only 2 migrations. It fails when there are more than 2, for the reasons I outlined above.

[Pensarfeo]

To be honest I would prefer the sleep(1) solution. This can create conflicting migrations if other generators are used later on. Besides if a problem like this exists keeping a consistent migration number it is not something we should we take care of... After that I am out :)

[sungwoncho]

Thanks for pointing out and solving this issue. @Pensarfeo, thanks for your input. Adding sleep(1) is more readable, but as @atarihomestar said, making a code deliberately slow seems not the best way.

I will publish this fix because the issue might be preventing people from using the gem. But we can certainly discuss further to find a better solution.