Opened 10 years ago

Closed 8 years ago

#809 closed defect (fixed)

DBA automatic name handling vs user set names vs developer set names

Reported by: rambo Owned by: rambo
Priority: critical Milestone: 8.09.4 Ragnaroek
Component: MidCOM core Version: 8.09 Ragnaroek
Keywords: Cc:

Description

After a sit-down with bergie the following was drafted:

## Definitions:

  • URL-safe name: string containing nothing that would be encoded by rawurlencode()
  • Clean name: string that is not changed by midcom_generate_urlname_from_string()
  • Duplicate name: name that is shared by sibling of any MgdSchema? type
  • Title property: usually just the property 'title' but for legacy reasons some objects (notably midgard_topic and midgard_event) use different one.

## Goals

  1. Ensure names are unique
  2. Do not second-guess the developer (by overriding the names they have set without asking)
  3. Ensure we have URL-safe, preferably clean names for all objects
  4. Ensure we do not have weird update-loops caused by midcom_baseclasses_core_dbobject::generate_urlname() being called in _on_updated (see also #805)

## High-level constraints:

  1. name SHOULD NOT be empty. This would be MUST NOT, but there is one exception where we return later.
  2. name MUST NOT be duplicate
  3. name MUST BE URL-Safe or empty
  4. if name is set it SHOULD NOT be overridden. This would be MUST NOT, but there are exceptions.
  5. name SHOULD be clean
  6. name SHOULD be based on the objects title property

The idea is to separate handling of this to two levels:

  1. DataManager?, where new "urlname" datatype is created that can handle various validations
  2. DBA API which enforces the MUST constraints

## The datatype

  1. Write-enabled only if midcom:urlname privilege is granted
  2. If left empty, generates (unique) value from designated 'title' DM schema
  3. If provided raises validation error on following:

3.1. value is not clean

3.2. value is not unique

3.2.1 If configured to do so the datatype can generate an unique value by catenating an incrementing number to the value.

## The API

  1. Checks will be done in the pre-flight check phase (ie just after _on_creating/_on_updating)
  2. If name is not unique false is returned for pre-flight check, preventing create/update 2.2 UNLESS a property in the object ('allow_name_catenate') is set to true in which case unique one is generated by catenating an incrementing number to the name.
  3. if name is empty unique name is generated from title property (unless title is empty too)
  4. if name is not URL-safe false is returned

The midcom_baseclasses_core_dbobject::generate_urlname() will be changed to log error and return immediately, components calling it will be updated to remove the call

Change History (40)

comment:1 Changed 10 years ago by rambo

  • Status changed from new to assigned

See also #805, #806 & #807 (which means this *must* be backported to Thor)

comment:2 Changed 10 years ago by bergie

  • Priority changed from major to blocker

See also #769

comment:3 Changed 10 years ago by rambo

(In [20926]) some name related helpers, refs #809

comment:4 Changed 10 years ago by rambo

Actually the midgard_event has title and uses it correctly, but it does not have "name" -property (the "extra" is used in stead), anyways you get the idea.

comment:5 Changed 10 years ago by rambo

(In [20927]) some title related helpers, refs #809

comment:6 Changed 10 years ago by rambo

(In [20934]) methods for checking name uniqueness, refs #809

comment:7 Changed 10 years ago by rambo

(In [20949]) use dbfactory provided property_exists wrapper, refs #809, #940

comment:8 Changed 10 years ago by rambo

(In [20957]) fix typo (lazy check), remove some noisy debugs, name uniquness check now works for reflectors part, refs #809

comment:9 Changed 10 years ago by rambo

(In [20980]) check for name uniqueness before update/create on DBA level, deprecated generate_urlname(), refs #809 & #805

comment:10 Changed 10 years ago by rambo

(In [20981]) enforce URL-safe names for objects refs #809, cache name/title heuristics results per class

comment:11 Changed 10 years ago by rambo

(In [20988]) method for generating unique names for objects, refs #809

comment:12 Changed 10 years ago by rambo

(In [20990]) remove code we no longer reach, refs #805 & #809, see also r16377

comment:13 Changed 10 years ago by rambo

forgot to ref r20992 to here.

comment:14 Changed 10 years ago by rambo

(In [20994]) better docblock, refs #809

comment:15 Changed 10 years ago by rambo

(In [20996]) If automatic name catenation is allowed, try to create new unique name for object on the fly, refs #809

comment:16 Changed 10 years ago by rambo

(In [20997]) code used to test r20996, refs #809

comment:17 Changed 10 years ago by rambo

The API-level part should now be complete. Next up: the datatype.

comment:18 Changed 10 years ago by rambo

(In [21006]) safety against weird problem situations, refs #809

comment:19 Changed 10 years ago by rambo

(In [21007]) make sure the reflector component is loaded (we need the configs too), refs #809

comment:20 Changed 10 years ago by rambo

(In [21008]) initial url-name datatype, seems to work, needs more testing, refs #809

comment:21 Changed 10 years ago by rambo

(In [21010]) better documentation, refs #809

comment:22 Changed 10 years ago by rambo

(In [21011]) use the new urlname datatype for name, refs #809

comment:23 Changed 10 years ago by rambo

(In [21014]) midcom_baseclasses_core_dbobject::generate_urlname has been deprecated, refs #809, #805 & #807

comment:24 Changed 10 years ago by rambo

(In [21016]) use the urlname datatype for name fields, remove deprecated name handling logic, refs #809

comment:25 Changed 10 years ago by rambo

(In [21017]) use automatic name catenation on RSS imported objects, refs #809 & #562

comment:26 Changed 10 years ago by rambo

(In [21018]) use urlname datatype here as well, refs #809

comment:27 Changed 10 years ago by rambo

(In [21020]) use the urlname datatype for url-names, refs #809

comment:28 Changed 10 years ago by rambo

(In [21021]) use urlname datatype in scaffold example config, refs #809

comment:29 Changed 10 years ago by rambo

(In [21023]) use automatic name catenation when copying objects. refs #809

comment:30 Changed 10 years ago by rambo

(In [21025]) use urlname datatype for the name field, generate paths for name property on reflected choosers. refs #809 fixes #878. Also added some future TODOs

comment:31 Changed 10 years ago by rambo

  • Resolution set to fixed
  • Status changed from assigned to closed

Based on my (limited) testing this seems to work.

Made new ticket (#953) about backporting all the related stuff.

comment:32 Changed 10 years ago by rambo

(In [21031]) more robust name safeties, refs #809 & #562

comment:33 Changed 10 years ago by rambo

(In [21094]) do not check name cleanliness for certain types, refs #809 fixes #961

comment:34 Changed 10 years ago by rambo

r21218 should have been reffed here as well

comment:35 Changed 10 years ago by rambo

r21596 is related to this

comment:36 Changed 10 years ago by rambo

r21218 is also related.

comment:37 Changed 9 years ago by rambo

(In [22942]) do not check name uniqueness for certain midcom core types, refs #1271 and #809

comment:38 Changed 9 years ago by rambo

(In [22943]) forward port r22942, fixes #1271 refs #809

comment:39 Changed 8 years ago by ferenc

  • Priority changed from blocker to critical
  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Type changed from enhancement to defect

I think we ran into this issue again at maemo.org. Please see the following bug report for details: https://bugs.maemo.org/show_bug.cgi?id=11470

comment:40 Changed 8 years ago by bergie

  • Resolution set to fixed
  • Status changed from reopened to closed

The Maemo issue mentioned was with local config, closing.

Note: See TracTickets for help on using tickets.