Refactor `is_valid_uuid` method

[Abuelodelanada]

Today the is_valid_uuid method in JujuTopology class, utilises a regex to validate UUID v4:

def is_valid_uuid(self, uuid):
    """Validates the supplied UUID against the Juju Model UUID pattern."""
    regex = re.compile(
        "^[a-f0-9]{8}-?[a-f0-9]{4}-?4[a-f0-9]{3}-?[89ab][a-f0-9]{3}-?[a-f0-9]{12}$"
    )
    return bool(regex.match(uuid))

An old friend told me many years ago:

If you have a problem and you use regular expressions to solve it, you now have two problems.

Jokes aside, regular expressions are powerful but complex to read sometimes.

We can avoid regex in this case by using the python uuid module. The result is much easy to read and maintain.

def is_valid_uuid(self, _uuid) -> bool:
     """Validate the supplied UUID against the Juju Model UUID pattern."""
     try:
         return True if uuid.UUID(_uuid).version == 4 else False
     except (TypeError, ValueError):
         return False

[Abuelodelanada]

The method was updated:

    def is_valid_uuid(self, uuid):
        """Validate the supplied UUID against the Juju Model UUID pattern."""
        # TODO:
        # Harness is harcoding an UUID that is v1 not v4: f2c1b2a6-e006-11eb-ba80-0242ac130004
        # See: https://github.com/canonical/operator/issues/779
        #
        # >>> uuid.UUID("f2c1b2a6-e006-11eb-ba80-0242ac130004").version
        # 1
        #
        # we changed the validation of the 3ed UUID block: 4[a-f0-9]{3} -> [a-f0-9]{4}
        # See: https://github.com/canonical/operator/blob/main/ops/testing.py#L1094
        #
        # Juju in fact generates a UUID v4: https://github.com/juju/utils/blob/master/uuid.go#L62
        # but does not validate it is actually v4:
        # See:
        # - https://github.com/juju/utils/blob/master/uuid.go#L22
        # - https://github.com/juju/schema/blob/master/strings.go#L79
        #
        # Once Harness fixes this, we should remove this comment and refactor the regex or
        # the entire method using the uuid module to validate UUIDs
        regex = re.compile(
            "^[a-f0-9]{8}-?[a-f0-9]{4}-?[a-f0-9]{4}-?[89ab][a-f0-9]{3}-?[a-f0-9]{12}$"
        )
        return bool(regex.match(uuid))

The fix in the Operator framework was merged in this PR

[gqcpm]

I would like to work on this issue

[Abuelodelanada]

Hi @grequa

We are happy that you want to work on this issue!

Let me know if you need any guidance. You can chat with us in this Mattermost channel

[gqcpm]

how exactly do I run tests for this?

[Abuelodelanada]

You need to install tox and the run tox -e unit for unit tests. In this file you can see all the tasks you can run with tox