Skip to content

[Uid] Add Generate and Inspect commands #39883

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

Merged
merged 1 commit into from
Feb 25, 2021

Conversation

fancyweb
Copy link
Contributor

@fancyweb fancyweb commented Jan 18, 2021

Q A
Branch? 5.x
Bug fix? no
New feature? yes
Deprecations? no
Tickets -
License MIT
Doc PR -

After some time using the component, I realized I often needed to quickly generate new ulids or to convert them from one format to another so I thought having those commands would be useful.

Usage

Generate a ULID - outputs N ULID(s) separated by new lines in base 32, base 58 or RFC 4122 format

Generate 1 ULID now

php bin/console ulid:generate

Generate 1 ULID with a specific timestamp

php bin/console ulid:generate --time="2021-02-02 14:00:00"

Generate 2 ULIDs and ouput the RFC4122 format

php bin/console ulid:generate --count=2 --format=rfc4122

Generate a UUID - outputs N UUID(s) separated by new lines in RFC 4122, base 58 or base 32 format

Generate 1 UUID (defaults from the underlying factory)

php bin/console uuid:generate

Generate 1 time-based UUID now

php bin/console uuid:generate --time-based=now

Generate 1 time-based UUID with a specific timestamp

php bin/console uuid:generate --time-based="2021-02-02 14:00:00"

Generate 1 time-based UUID with a specific node

php bin/console uuid:generate --time-based=now --node=fb3502dc-137e-4849-8886-ac90d07f64a7

Generate 1 name-based UUID (there must be a default namespace in the underlying factory)

php bin/console uuid:generate --name-based=foo

Generate 1 name-based UUID with a specific namespace (overrides the default namespace from the underlying factory)

php bin/console uuid:generate --name-based=foo --namespace=fb3502dc-137e-4849-8886-ac90d07f64a7

Generate 1 random-based UUID

php bin/console uuid:generate --random-based

Generate 2 UUIDs and output their base 58 format

php bin/console uuid:generate --count=2 --format=base58

Inspect a ULID - outputs base32, base58 and RFC 4122 formats of a ULID and its humand readable timestamp if it is time-based

php bin/console ulid:inspect 01EWAKBCMWQ2C94EXNN60ZBS0Q
php bin/console ulid:inspect 1BVdfLn3ERmbjYBLCdaaLW
php bin/console ulid:inspect 01771535-b29c-b898-923b-b5a981f5e417

Inspect a UUID - outputs RFC 4122, base 58 and base 32 formats of a UUID and its human readable timestamp

php bin/console uuid:inspect a7613e0a-5986-11eb-a861-2bf05af69e52
php bin/console uuid:inspect MfnmaUvvQ1h8B14vTwt6dX
php bin/console uuid:inspect 57C4Z0MPC627NTGR9BY1DFD7JJ

Register the commands

YAML

# services.yaml
services:
    Symfony\Component\Uid\Command\GenerateUlidCommand: ~
    Symfony\Component\Uid\Command\GenerateUuidCommand: ~
    Symfony\Component\Uid\Command\InspectUlidCommand: ~
    Symfony\Component\Uid\Command\InspectUuidCommand: ~

PHP

<?php

// services.php

namespace Symfony\Component\DependencyInjection\Loader\Configurator;

use Symfony\Component\Uid\Command\GenerateUlidCommand;
use Symfony\Component\Uid\Command\GenerateUuidCommand;
use Symfony\Component\Uid\Command\InspectUlidCommand;
use Symfony\Component\Uid\Command\InspectUuidCommand;

return static function (ContainerConfigurator $configurator): void {
    $services = $configurator->services()
        ->defaults()
        ->autowire()
        ->autoconfigure();

    $services
        ->set(GenerateUlidCommand::class)
        ->set(GenerateUuidCommand::class)
        ->set(InspectUlidCommand::class)
        ->set(InspectUuidCommand::class);
};

@stof
Copy link
Member

stof commented Jan 18, 2021

See #36405 for previous discussion about that topic.

@jderusse jderusse added this to the 5.x milestone Jan 18, 2021
@fabpot
Copy link
Member

fabpot commented Jan 19, 2021

Still 👎 for me.

@nicolas-grekas
Copy link
Member

The commands could be provided as a third party bundle.

Alternatively, we could provide them in the source, but not register them as commands, so that ppl could enable them by adding a line in their services.yaml file:

services:
    Symfony\Component\Uid\Command\GenerateUlidCommand: ~

This approach could enable us to provide commands without cluttering the list command by default.

WDYT @fabpot?

@fabpot
Copy link
Member

fabpot commented Jan 19, 2021

Sure, if they are hidden by default, no problems for me.

@fancyweb fancyweb force-pushed the uid-commands branch 3 times, most recently from 7d4b5db to bedc3f5 Compare January 19, 2021 12:15
@fancyweb
Copy link
Contributor Author

fancyweb commented Jan 19, 2021

Unfortunately I did not know there was already another PR for that feature in the past. I still think it's an useful feature to offer. I have removed the commands registration in the FWB and fixed the other comments. I also changed the timestamp display to be in milliseconds. I still need to fix the tests.

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some thougths

@maxhelias
Copy link
Contributor

Is it possible to have a format option (rfc, binary..) ?

@fancyweb fancyweb force-pushed the uid-commands branch 2 times, most recently from 836350b to 325d898 Compare February 17, 2021 12:56
@fancyweb
Copy link
Contributor Author

I pushed a new implementation that leverages the new factories.

@maxhelias I added a --format option to the generate commands.

@fancyweb fancyweb force-pushed the uid-commands branch 2 times, most recently from b9b5b7b to 1eff44f Compare February 17, 2021 16:57
@fancyweb fancyweb requested a review from dunglas as a code owner February 17, 2021 16:57
@ro0NL
Copy link
Contributor

ro0NL commented Feb 23, 2021

Sure, if they are hidden by default, no problems for me.

arent we registering them as hidden? being able to default advertise bin/console .. seems reasonable

public const NORMALIZATION_FORMAT_RFC_4122 = 'rfc_4122';
public const NORMALIZATION_FORMAT_BASE58 = 'base58';
public const NORMALIZATION_FORMAT_BASE32 = 'base32';
public const NORMALIZATION_FORMAT_RFC4122 = 'rfc4122';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i feel like we should move the consts to AbstractUid

Copy link
Member

@nicolas-grekas nicolas-grekas Feb 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? AbstractUid nor the Uid component wouldn't use them anywhere.

Copy link
Contributor

@ro0NL ro0NL Feb 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because the formats are leaking now: The ULID output format: base32, base58 or rfc4122

Copy link
Member

@nicolas-grekas nicolas-grekas Feb 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these consts in Serializer are legit.
Then, the duplication in Uid is legit also - these are from different domains.
Also, the commands (on the cli), use inline values.
In the end, these consts in Uid won't serve any purpose for the devs.
They would actually make the public API of the component more complex to me.
I'd prefer not to add them as you might have understood :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think both serializer+console are duplicating what could be AbstractUid::toFormat($format)

Copy link
Member

@nicolas-grekas nicolas-grekas Feb 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, then I'd recommend splitting this discussion into a separate PR or issue.

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Feb 23, 2021

arent we registering them as hidden?

Nope: we're not registering them by default, so that we'll ask ppl to opt-in to list them.

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't miss updating the description of the PR :)

@fancyweb fancyweb force-pushed the uid-commands branch 2 times, most recently from 002f62b to 8036d86 Compare February 24, 2021 17:24
Co-authored-by: Nicolas Grekas <nicolas.grekas@gmail.com>
@nicolas-grekas
Copy link
Member

Thank you @fancyweb.

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

Successfully merging this pull request may close these issues.

10 participants