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

Exception on duplicate code insertion #9

Open
mxschll opened this issue Apr 27, 2021 · 9 comments
Open

Exception on duplicate code insertion #9

mxschll opened this issue Apr 27, 2021 · 9 comments
Assignees

Comments

@mxschll
Copy link

mxschll commented Apr 27, 2021

Hello,
If a code is generated which already exists in the database, and it is then inserted into the database, an error is thrown.

SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '1234' for key 'art_urls_code_unique'  

I would suggest checking if the code already exists in the database in the creating() mtehod in URL.php. If so, a new code should be created until it does not already exist.

Perhaps one could also dynamically change the length of the code if too many collisions occur.

@mxschll mxschll changed the title Duplicate code exception Exception on duplicate code insertion Apr 27, 2021
@arietimmerman arietimmerman self-assigned this Apr 27, 2021
@arietimmerman
Copy link
Owner

Excellent idea. Will try to work on it. Or alternatively, I would be happy if you can create a pull request.

@mxschll
Copy link
Author

mxschll commented Apr 28, 2021

I'll try to implement it this week.

@mxschll
Copy link
Author

mxschll commented Apr 30, 2021

I created a pull request, see #10

@diereysaa
Copy link

Following with this idea, why not to check if the URL already exists on the table?
I've ended having several rows with the same URL, and different codes:

image

I don't see any downside to this, even the click tracking will work, since it's the same URL.

Right now I'm doing it this way:
$shortUrl = URL::where('url', 'https://url-goes-here.com')->first() ?? URLShortener::shorten('https://url-goes-here.com');

But I guess there would be a better/more effective way to do this if we include it inside the shorten method itself

Thanks! :)

@mxschll
Copy link
Author

mxschll commented May 9, 2021

@arietimmerman What dou you think about this?

@arietimmerman
Copy link
Owner

Thanks for the pull request @mxschll
I think something like this could be a solution. But I'm not sure if I like the "brute force" mechanism to check if a code is used and after 2 attempts increase the code length.

Wouldn't it be an idea to use some kind of encryption algorithm, and encrypt an incrementer? So this would look like this.

1. Check the number of shortened URLs
2. Encrypt the number found

This should guarantee that the number of database lookups is very limited (only one). And the code runs deterministically.

@diereysaa Depending on the application this could be very useful. In some applications you probably still want unique shortened url's, even though the same url is shorted. bitly always generates a unique url for example.

@arietimmerman
Copy link
Owner

Perhaps something like this could also work as well #11
Or maybe it should be made configurable. That one could choose to either use a random code - with checks like created by @mxschll - or to generate an always unique code (which looks random, but is not random).

@mxschll
Copy link
Author

mxschll commented May 12, 2021

You are right, the brute force approach is not ideal, especially if the database has lots of entries. However, an extremely large number of entries would be required before performance problems should arise. So I believe that this approach is manageable.

I like the approach in #11 and that would be sufficient for me. Depending on the application, however, it could be problematic if the codes are not generated randomly, but rather follow a pattern.

One could possibly adapt #10 in such a way that, depending on the number of entries in the database and the associated probability of a collision, the code length is increased in advance. Thus, one could reduce the database queries in advance and the code would still be random.

@JensenWD
Copy link

JensenWD commented Sep 28, 2021

Was the fix ever merged? Still getting that error when it tries to create a code with no characters ''

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants