? Pending

User tests: Successful: Unsuccessful:

avatar laoneo
laoneo
7 Jun 2018

Summary of Changes

While working on a new component which integrates external systems, where category ids are strings and not integers. I ran into an issue that the lookup in the router failed because category ids and item ids are assumed to be integers. This pr removes that restriction.

Ping @Hackwar for review and feedback.

Testing Instructions

Just browse around the articles in the front end with modern routing and id removed.

Expected result

All should work as before.

Actual result

All ok.

avatar laoneo laoneo - open - 7 Jun 2018
avatar laoneo laoneo - change - 7 Jun 2018
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 7 Jun 2018
Category Libraries
avatar laoneo laoneo - change - 7 Jun 2018
The description was changed
avatar laoneo laoneo - edited - 7 Jun 2018
avatar brianteeman
brianteeman - comment - 7 Jun 2018

why would an id that is supposed to be an integer ever not be an integer?
this sounds like removing a valid check in the code to allow for some sort of error in some other code.

avatar laoneo
laoneo - comment - 7 Jun 2018

Why is an id supposed to be an integer? Id's are not always integers. Mostly they are integers when they are rows from a database. But for example when you want to reference an event from a CalDAV server then they can look like DC6C50A017428C5216A2F1CD@example.com.

avatar dgrammatiko
dgrammatiko - comment - 7 Jun 2018

why would an id that is supposed to be an integer ever not be an integer

The id in the db column can be an integer or not (eg it can be UUID: http://kccoder.com/mysql/uuid-vs-int-insert-performance/). I'm not sure if Joomla supports only integers, but if it does so then this is kinda stupid limitation

avatar brianteeman
brianteeman - comment - 7 Jun 2018

every ID field in joomla is an integer

avatar ciar4n ciar4n - test_item - 7 Jun 2018 - Tested successfully
avatar ciar4n
ciar4n - comment - 7 Jun 2018

I have tested this item successfully on d405dd7


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20683.

avatar dgrammatiko
dgrammatiko - comment - 7 Jun 2018

every ID field in joomla is an integer

True, but that was done (historically) for performance reasons. A reminder here that at some point the default admin id wasn't random and it needed some code in the installation to make it so. And this might be a very legit case where a dev might not want to use the auto increment integers in the id (sensitive data that cannot follow a predicted pattern).

All and all I think allowing only integers is an artificial limitation and should be removed as there are a lot of cases that wouldn't necessarily follow that pattern.

@laoneo in your example: I wouldn't ever put id's as plain text, use GUID or UUID

avatar Hackwar
Hackwar - comment - 7 Jun 2018

If you remove the casting, you break the current system. The lookup table uses the data from #__menus and that table only contains the integer ID for categories and articles. The URL that is processed by JRoute however contains the alias, too, and thus the lookup would fail.

Yes, an identifier could be a UID, a string or an integer, but the RouterView system can only handle integer identifier. That group of classes is not meant to solve all routing demands that anybody would ever have. It is meant to provide good quality routers for the 98% of simple components out there that are similar to com_content. Components that are more complex than this (and for example require non-integer identifiers) still should role their own router.

avatar laoneo
laoneo - comment - 7 Jun 2018

Ok, if this is the design of the router system, then I'm closing it.

avatar laoneo laoneo - change - 7 Jun 2018
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2018-06-07 11:07:59
Closed_By laoneo
Labels Added: ?
avatar laoneo laoneo - close - 7 Jun 2018

Add a Comment

Login with GitHub to post a comment