Allow admins to select the desired UUID generation method

Yes, I support that.

IMO, it should be a option checkbox: "always use random based UUID generation" with a description saying the default is MySQL generated when using MySQL.

I also think a new argument should be added to the uuid_uuid() function, to by-pass this setting when needed, like uuid_uuid($random = FALSE) with a check if ($random || variable_get('uuid_random', FALSE)).

I'm looking forward to commit your patch.

Thanks!

Hi arski. While it looks more consistent having only one way to generate the UUIDs, the ones generated by MySQL are supposed to be better, version 2 UUIDs.

UUIDs are good because they are generated for a single cpu clock for a given domain (machine), it means it will not clash in any way.

In the other hand, random UUIDs CAN clash, and for me if something CAN happen, it WILL happen.

It's hard for PHP to generate version 2 UUIDs since this don't have access to the low level information, e.g. C has. The algorithms for generating version 2 UUIDs I've found out there don't support multiple platforms.

In the long run, I think it would be better having UUID module generate UUIDs of all versions and having the users choose which one they want to use.

That said, for now, it's better having the random part as a simple fallback.

What do you think? Would you mind writing a patch for that?

Thanks

I've found this implementation:

http://www.shapeshifter.se/2008/09/29/uuid-generator-for-php/

Also, there's a PECL module for UUID generation we should use when avaiable:

http://pecl.php.net/package/uuid

I think we should avoid the MySQL function completely. I'm working on a patch, with an included class that will create 3 different uuid versions, selectable in the admin interface. This method does not require the PECL library, or the uuid_create functions to be enabled in PHP. I'll share it here once it's ready.

@arski thank you for the patch. The patch looks good. I'm just wondering, though, if we shouldn't hide a little of implementation from users. I.e. what if we just have two options for now in the selection fields: "Use the best method available" and "Always use random UUIDs". That way we can choose by ourselves in the code what method is better in each scenery. I think the better, if available, is the PECL implementation, so we should use that instead of MySQL when available. What do you think? Maybe later when we have all UUID versions available regardless of the platform, we can present uses with more choices.

@klktrk great news. A platform independent way of generating UUIDs was what I wanted when I was writing this module. But I think we should still give users the opportunity to use the PECL version, and maybe even the MySQL one, if they want to.

"Globally unique, optically similar" vs. "Locally unique, optically distinct", sounds ok to me.

We need to distinguish between uuid generating _methods_ and uuid _types_.

In the patch, the form radio buttons
* mysql
* pecl
* rfc4122_v4

mix those two concepts together. mysql and pecl are methods of generating UUIDs. rfc4122_v4 is a type of UUID.

People should be able to choose preferred type, and on the basis of the type chosen (v1, ... v5), they should be presented only with available methods that can create those types of uuids.

The patch also should also modify uuid.install hook_uninstall() to delete the new variables being set

There is a php class here http://drupal.org/node/918888 that gives some additional types of uuids, as well as possible suggestions regarding uuid generation and admin.

I'm sorry, I didn't mean to change the scope willy nilly on this. And you're right to limit the issue.

the patch looks good within the scope addressed by the ticket.

However, I still think there is an issue with blurring the difference between generation-method of a uuid and a uuid *type* (or format).

+  $gen_method_options = array();
+  if ($GLOBALS['db_type'] != 'pgsql' || function_exists('uuid_create')) {
+    $gen_method_options['universal'] = t('Globally unique, optically similar');
+  }
+  $gen_method_options['optical'] = t('Locally unique, optically distinct');

The 'universal' option basically equates to "use MYSQL/PECL if available" (if you still want to talk about Method.

Since your interface exposes to the user a "type/format" of uuid rather than a method (The method is chosen by the app, not by the user, based on the type the user selects), shouldn't the variables etc reflect that? e.g. uuid_type_options[], or, better, uuid_format_options[]

Also, if the user is on PGSQL and does not have the Pecl extension, why not just hide this from the user completely? Why show just one radio button? I guess this last bit is just a matter of taste. Perhaps its useful for a user to see what format uuid they are using, even if they have no option to change it.

Hi patch looks great. Though I think there's still room for improvements:

1. Please rename $gen_method_options to $method_options
2. The 'universal' and 'optical' seems meaningless in code context, maybe it should be the UUID versions, or 'mysql' and 'random
3. In uuid_uuid(), I think we could split the switch blocks into separate functions in preparation for abstracting the various ways of generate UUIDs into separate modules, this way the uuid_uuid() function would look like this:

function uuid_uuid() {
  $method = variable_get('uuid_generation_method', uuid_default_generation_method());
  $function = 'uuid_uuid_' . $method;
  if (function_exists($function)) {
    return $function();
  }
}

Thanks for working on this!

I agree with @klktrk on not showing only on radio button, but I don't care much about this.

Also, it's important to keep in mind that the PECL extension is capable of generating all UUID versions, and the default one used can change between php installs. So we can't ensure it'll always be 'universal'.

apologies arski, I can't undestand the 'optically similar' meaning. MD5 or SHA1 are very similar optically and nobody has a doubt in their validation. Does really that kind of separation have a sense for the admin user when configuring the module? What are the use cases for the user to chosse one method prior to another?

And no, I'm not following you in the issue queue ;)

Arski, I would not go that way. No, I'm not a module maintainer; no, I'm not answering on all the issues, and no, I'm not my intention is not to get all issues stucked, I'm trying to find the rationale behind the issues I don't understand. If this is bothering or you think a single comment in an issue would hang all the work already done until now, then the problem is not me comming to this module queue.

I'm, however, the one that is providing the token integration patch, I'm the one writting the testcase for this module and I'm one of the module users, position legitim enough to participate in the discussions but not to take decissions, that is not my job anyway, so I'm going to leave that task for others.

After re-reading the first post (and the last), I'm still thinking the same, so thanks for your reply. I'll wait a couple of days to see if someone points additional arguments.

For sure the options are needed, in fact, I'd say that it is required in this case, because depending on your uuid usage you may need one provider or another. What I don't understand is why have you considered this separation: "localy unique" or "optically similar", I'm pretty sure nobody will be able to remember more than 4 different uuids in the first attempt, so it is just an 'sthetic' issue, but I'm not opposite to this approach, which I've found better than the complex technical approach "mysql V-x" or "pecl V-y".. in fact, if of the uuid providers name is "Random", does that means the rest of methods are not random?

Thanks for all your work in this queue arski, and count with me to push your issues untill they get fixed. Apologies if my style has been rude, I'm just trying to understand the 'why' behind the issues, so we can argue enough to convince recidive to commit or not, I never meant to block, hang or drop the issues or the ideas at all.

Hello,

I made a fork on GitHub of the project, I've included some patches found on this issue list and especially this one and some improvements.

You can find it and improve it if you want, here: https://github.com/Polzme/uuid

It's just that it's easier to maintain... as I said, I don't want to get people frustrated by this fork.

I'm also a module developer and I never get the chance to have a cvs account, so I made my own repository on Github.

I think Github is also easier to use than CVS but that's not the main point.

There are also people who can't use CVS at work like me, so using Github on the web is perfect.

You're totally right of course... I'll see what we can do about it !

Thanks !

@arski, a late response to your question in #18: the PECL can be set to generate all versions of UUIDs, it's not well documented. I've found a forum topic on this. Despite it doesn't seem relevant, I think supporting this is good since this is already packaged for some linux distros.

@ilo, It's not about convincing me, you have to convince everyone here ;)

@Pol, if you have an improved patch, please post it here so there's a chance that everyone will benefit from that rather than only the ones tracking your github fork.

Let's work on an improved patch, adding what was discussed from #18 to now, and I promise I'll review and commit it when it's ready.

Thanks!

I'm considering this for the D7 port - subscribing

This has been implements in the D7 port, I don't think it would take too much effort to back port it for D6. The code is available at https://github.com/skwashd/uuid/blob/DRUPAL-7--1/uuid.inc At this stage I don't have much time (or interest) in back porting to D6 as most of my new work is D7 based.

I have been granted comaintainer status, but I started this months ago before d.o switched to git and haven't have time to migrate back. I am waiting to see there is some consensus develops around the current implementation before i move the code back.

What happened to this? In uuid.api.php there is documentation about hook_uuid_info() to define new generators but there is no example on how to switch it:

<?php
/**
 * Defines one or more UUID generators exposed by a module.
 *
 * @return
 *   An associative array with the key being the machine name for the
 *   implementation and the values being an array with the following keys:
 *     - title: The human readable name for the generator.
 *     - callback: The function to be called for generating the UUID.
 *
 * @see uuid_get_info()
 */
function hook_uuid_info() {
  $generators = array();
  $generators['my_module'] = array(
    'title' => t('My module UUID generator'),
    'callback' => 'my_module_generate_uuid',
  );
  return $generators;
}
?>

Looking at this code makes me think it is not possible:

<?php
/**
 * Generates an universally unique identifier.
 *
 * This function first checks for the PECL alternative for generating
 * universally unique identifiers. If that doesn't exist, then it falls back on
 * PHP for generating that.
 *
 * @return
 *   An UUID, made up of 32 hex digits and 4 hyphens.
 */
function uuid_generate() {
  $callback = drupal_static(__FUNCTION__);

  if (empty($callback)) {
    if (function_exists('uuid_create') && !function_exists('uuid_make')) {
      $callback = '_uuid_generate_pecl';
    }
    elseif (function_exists('com_create_guid')) {
      $callback = '_uuid_generate_com';
    }
    else {
      $callback = '_uuid_generate_php';
    }
  }
  return $callback();
}
?>

I created a patch. It currently just uses a variable to store the function name of the generator. Now we could introduce a settings form where this function can be configured.

38: 902176-generator-method-38.patch queued for re-testing. *edit* Sorry for that!

I am putting this back to 6.x. @recidive can decide if he wants to implement this functionality there.

We won't be implementing this in the D7 version of the module. We use the most efficient function available for generating the UUID.

Is there any chance, that you would consider having this in the 7.x version? I think it would be a very nice addition to this module.

I'm encountering the second project in a row in which the client needs identifiers to certain entities, which are not the primary entity identifiers. I ruled out having the UUID made up of 32 hex digits and 4 hyphens, as it produces not so nice URLs. Having the possibility to write our own UUID generator would gracefully solve this. We could use something like hashids or whatever we come up with in our own application. I know that producing nice identifiers that look great in URLs is not really what this module is about, but everything is already implemented to make this happen, it's just a matter of invoking a different callback. So I would hate to write something new for this use case.

The only thing that is remaining here is an administrative UI, which I'm willing to work on. I would focus on not imposing the configuration on those who don't care.

@balintk I second that idea. This module can be much more useful if it can be repurposed for URL aliasing/shortening and similar things. hashids look very nice for that.

UUIDs are an IETF standard. If you want to use a non compliant unique identifier system, such as hashlib, I recommend you start your own module.

@skawshd it's your module so you get to decide whatever you do with it. The fact that it's an IETF standard is irrelevant for this discussion because there are multiple ways to do unique IDs. For distributed systems causality plays a very important role and there this module as it is will be useless. If OTOH you provided the option to plug in other algos then we could implement a client of the ID generation service. I'm happy to code a module for that myself. I don't need it for tomorrow either, in a couple of months I will.

@perusio this module, like D8 core, is only interested in supporting RFC 4122 compliant UUIDs.