Documentation packages refactoring #19
Conversation
…ights Based on discussion in #7 we discovered that GitHub name and actual package name are two distinct concepts and should be modeled distinctively. Updated feature reflects this change.
Rename Release\Package to Release\Repository
|
|
||
| $meta = json_decode(file_get_contents($download->getFilePath('composer.json')), true); | ||
|
|
||
| return new ComposerPackage($meta['name']); |
There was a problem hiding this comment.
This does not handle the case insensitivity properly (it is needed either here on in ComposerPackage)
There was a problem hiding this comment.
Added strtolower() call to ComposerPackage
|
I see an issue currently: the package name validation in the domain and the package name requirement in routes are not consistent (the routing is stricter by allowing only some chars in the names). And the routing requirements are not compatible with Composer package names (which are allowing dots as well) |
I think we can actually make route requirements much more liberating now. Lets just make sure there are no security tricks allowed, other than that - trying to get wrong documentation should just result us in being unable to find that documentation. |
|
@stof "more liberating" = |
|
IMO, this will create other issues. If someone names its package |
|
@stof such a package will not get into the system because of the name constraints in |
|
@everzet this is a valid composer package name |
|
oh wait. No it will. |
|
that's a very-very good point. Should we mount extensions documentation under different prefix ( |
|
anyway, let's discuss the URL patterns separately. They are not part of this domain refactoring. |
|
@stof opened separate issue for that. Ok back to this refactoring. What else? |
| */ | ||
| final class DocumentationPackage implements Package | ||
| { | ||
| const PACKAGE_NAME_REGEX = '#^[A-Za-z0-9][A-Za-z0-9_.-]*/[A-Za-z0-9][A-Za-z0-9_.-]*$#u'; |
There was a problem hiding this comment.
What about moving this regex to the interface rather than duplicating it in both implementation ? Valid package names are expected to be the same in all cases. They are part of the domain.
|
@stof anything else? |
|
👍 |
…es-and-versions Documentation packages refactoring
|
@stof next is support for |
|
I have an idea for a small improvement making it easier to handle consistent routing requirements. I will send it before this refactoring. |
This PR is a direct answer to the discussion in #7.
It separates notions of package and repository into separate domain concepts and basically builds thing discussed in #7 (comment) on top of it.
This should essentially fix issues 1, 4 and 5 of #7 and enable easy resolution of 2 and 3, which will come with a separate PR.