Entities with names differing only in case are allowed; causes errors later

wjt commented

2015-04-23 10:14:51 +00:00

If I try to create two entities with the same name, eg "Test Case Insensitive Duplicate", the second one has [2] appended to its name. But if I create an entity with name "test case insensitive duplicate", its name is not deduplicated. Later, trying to edit annotations for this entity will crash in Entity.get_by_name with:

MultipleObjectsReturned: get() returned more than one Entity -- it returned 2!
Lookup parameters were {
    'name_find__icontains': u'|Test Case Insensitive Duplicate|',
    'type': 'locations'
}

(wrapped for clarity).

Even if this is fixed (untested patch to follow) to forbid you creating entities whose names differ only in case, I think there's still a problem if one entity has name Foo, and another entity has additionalName Foo: nothing guards against this situation, but it will break Entity.get_by_name in the same way.

If I try to create two entities with the same name, eg "Test Case Insensitive Duplicate", the second one has ` [2]` appended to its name. But if I create an entity with name "test case insensitive duplicate", its name is not deduplicated. Later, trying to edit annotations for this entity will crash in `Entity.get_by_name` with: ``` MultipleObjectsReturned: get() returned more than one Entity -- it returned 2! Lookup parameters were { 'name_find__icontains': u'|Test Case Insensitive Duplicate|', 'type': 'locations' } ``` (wrapped for clarity). Even if this is fixed (untested patch to follow) to forbid you creating entities whose names differ only in case, I think there's still a problem if one entity has name Foo, and another entity has additionalName Foo: nothing guards against this situation, but it will break `Entity.get_by_name` in the same way.

wjt added the

backend

label 2015-04-23 10:14:51 +00:00

wjt added this to the 14.04 milestone 2015-04-23 10:14:51 +00:00

j was assigned by wjt

2015-04-23 10:14:51 +00:00

wjt added the

normal

defect

labels 2015-04-23 10:14:51 +00:00

wjt commented

2015-04-23 10:16:37 +00:00

Author

Attachment 0001-Entity-ignore-case-when-uniquifying-name.patch (959 bytes) added

**Attachment** 0001-Entity-ignore-case-when-uniquifying-name.patch (959 bytes) added

0001-Entity-ignore-case-when-uniquifying-name.patch

959 B

wjt commented

2015-04-23 10:18:46 +00:00

Author

Do annotations refer back to entities by name, or by id? I believe it's by name (hence this bug), so I think it's safe for me to just rename or delete the offending duplicate entity.

j commented

2015-04-23 10:31:09 +00:00

Owner

Replying to [wjt]comment:1:

Do annotations refer back to entities by name, or by id? I believe it's by name (hence this bug), so I think it's safe for me to just rename or delete the offending duplicate entity.

its referenced by id, before deleting possibly run sumething like this:

Annotation.objects.filter(value=).update(value=)

Replying to [wjt]comment:1: > Do annotations refer back to entities by name, or by id? I believe it's by name (hence this bug), so I think it's safe for me to just rename or delete the offending duplicate entity. its referenced by id, before deleting possibly run sumething like this: > Annotation.objects.filter(value=<old id>).update(value=<new id>)

j commented

2015-04-23 10:33:24 +00:00

Owner

additionalName are not alternativeNames. alternativeNames + name are unique.
additional names are just additional names and can overlap.

patch looks better but should also check for alternativeNames, will have a look...

additionalName are not alternativeNames. alternativeNames + name are unique. additional names are just additional names and can overlap. patch looks better but should also check for alternativeNames, will have a look...

j commented

2015-04-23 10:55:18 +00:00

Owner

In []changeset:pandora,4935:

#!CommitTicketReference repository="" revision="pandora,4935"
ignore case checking for existing entities, check for alternative names too, migrate existing double entries, fixes #2754

In []changeset:pandora,4935: ``` #!CommitTicketReference repository="" revision="pandora,4935" ignore case checking for existing entities, check for alternative names too, migrate existing double entries, fixes #2754 ```

j added the

fixed

label 2015-04-23 10:55:18 +00:00

j closed this issue

2015-04-23 10:55:18 +00:00

wjt commented

2015-04-23 11:17:38 +00:00

Author

Sorry, I meant to say alternativeNames not additional_names above.

I still don't think this prevents alternativeNames overlapping name – I think you need the same uniquifying logic for each name here:

            elif key == 'alternativeNames':
                self.alternativeNames = tuple([ox.escape_html(v) for v in data[key]])

Suppose entity ABC has name: "Foo"; and I now create entity DEF with name: "Bar", alternativeNames: ["Foo"]. Then name_find for ABC will be |Foo|; name_find for DEF will be `|Bar|Foo|

        return cls.objects.get(name_find__icontains=u'|Foo|', type=type)

will get both rows back and throw the same exception as above. (If I tried to create ABC after DEF, your patch would correctly uniquify ABC's name to "Foo [2]".)

Sorry, I meant to say `alternativeNames` not `additional_names` above. I still don't think this prevents `alternativeNames` overlapping `name` – I think you need the same uniquifying logic for each name here: ``` elif key == 'alternativeNames': self.alternativeNames = tuple([ox.escape_html(v) for v in data[key]]) ``` Suppose entity ABC has `name: "Foo"`; and I now create entity DEF with `name: "Bar", alternativeNames: ["Foo"]`. Then `name_find` for ABC will be `|Foo|`; `name_find` for DEF will be `|Bar|Foo| ``` return cls.objects.get(name_find__icontains=u'|Foo|', type=type) ``` will get both rows back and throw the same exception as above. (If I tried to create ABC after DEF, your patch would correctly uniquify ABC's name to "Foo [2]".)

j commented

2015-04-23 11:50:05 +00:00

Owner

In []changeset:pandora,4937:

#!CommitTicketReference repository="" revision="pandora,4937"
make sure alternative names are unique too, also fixes #2754

In []changeset:pandora,4937: ``` #!CommitTicketReference repository="" revision="pandora,4937" make sure alternative names are unique too, also fixes #2754 ```

Entities with names differing only in case are allowed; causes errors later #2754