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

Updated Group constructor to allow URLs #43

Merged
merged 2 commits into from
Jul 5, 2015

Conversation

paul-blundell
Copy link
Contributor

As mentioned in issue #38 the group constructor could not handle URLs. This was creating a problem for multiple file uploads which returns a URL.

I have assumed with this fix that the Group ID will be the last part of the URL. Please correct me if I am wrong.

@dmitry-mukhin
Copy link
Member

What will be returned by basename with trailing slash (which is default)?
I think that the better approach is using regexp, like in File's constructor.

@paul-blundell
Copy link
Contributor Author

Trailing slashes are ignored.

I would strongly recommend native PHP functions over regex, especially when the URL is as simple as this. Of course the choice is yours, I just need something that works and fixes this problem :)

@dmitry-mukhin
Copy link
Member

Is it a good idea to let pass "file in group" URL to instantiate a group that contains the file?
I.e. passing ucarecdn.com/:Group-UUID/nth/x/-/:postfixes/filename will get Group with :Group-UUID

@dmitry-mukhin
Copy link
Member

@homm, any comments on this?

@homm
Copy link
Contributor

homm commented Jun 9, 2015

@paul-blundell with regexps we can check exactly format of UUID and throw on wrong links. Moreover, PATHINFO_BASENAME not compliant our CDN rules:

php > print pathinfo("https://ucarecdn.com/c4fed349-2317-4eb3-9e4c-a390081a1e5b~1/meh", PATHINFO_BASENAME);
meh
php > print pathinfo("https://ucarecdn.com/c4fed349-2317-4eb3-9e4c-a390081a1e5b~1/nth/0/", PATHINFO_BASENAME);
0

I would like to see something like this rather than pathinfo().

@paul-blundell
Copy link
Contributor Author

@homm, makes sense. Like I said I assumed the UUID was the last part, my fault for not checking the documentation fully :)
I shall update this tomorrow with regex.

@homm
Copy link
Contributor

homm commented Jun 9, 2015

I think it should be like this:
(?P<uuid>[a-z0-9]{8}-(?:[a-z0-9]{4}-){3}[a-z0-9]{12}~(?P<files_qty>\d+))

Where files_qty is part of uuid and also separate integer field (intval($files_qty) is required).

@dmitry-mukhin
Copy link
Member

thanks!

dmitry-mukhin added a commit that referenced this pull request Jul 5, 2015
Updated Group constructor to allow URLs
@dmitry-mukhin dmitry-mukhin merged commit a7ba1f7 into uploadcare:master Jul 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants