Opened 4 months ago

Last modified 3 weeks ago

#1852 new Bug/Fehler

Optimierung (und Bug ?) in /admin/modules.php

Reported by: noRiddle Owned by: somebody
Priority: normal Milestone:
Component: Admin Version: 2.0.5.1

Description

  • Sortierung:
    Wenn man z.B. eine Klassenerweiterung baut und den diversen Einstellungsmöglichkeiten eine Sortierung gibt (Feld sort_order in DB-Tabelle configuration) wird diese bei der Darstellung im Edit-Modus nicht berücksichtigt. Statt dessen wird die Reihenfolge im Array das die Methode keys() zurückgibt als Sortierung verwendet (Funktion get_module_info()).
    Das ist nicht nur kontra-intuitiv sondern das DB-Feld sort_order wird auch ad absurdum geführt weil es keine Wirkung hat.
    Das würde ich als eine Art Bug bezeichnen wollen.
  • Query:
    Die Query in o.g. Funktion get_module_info() wird innerhalb des for-Loops und somit mehrfach ausgeführt, was bei vielen Konfigurations-Parameteren sicherlich performance-technisch nicht optimal ist.
    Man könnte die Query, um sort_order ergänzt, außerhalb eines Loops mittels
    WHERE configuration_key IN('".implode('\',\'', $module_keys)."') ORDER BY sort_order ASC");
    machen und in einer while-Loop dann das Array $module_info füllen.

Mein Vorschlag inkl. auskommentiertem alten Code:

  function get_module_info($module) {
    $module_info = array('code' => $module->code,
                         'title' => $module->title,
                         'description' => $module->description,
                         'extended_description' => isset($module->extended_description) ? $module->extended_description : '',
                         'status' => $module->check());
    $module_info['properties'] = isset($module->properties) ? $module->properties : array();
    $module_keys = method_exists($module,'keys') ? $module->keys() : array();
    $keys_extra = array();
    
    $key_value_query = xtc_db_query("SELECT configuration_key,
                                            configuration_value,
                                            sort_order,
                                            use_function,
                                            set_function
                                       FROM " . TABLE_CONFIGURATION . "
                                      WHERE configuration_key IN('".implode('\',\'', $module_keys)."')
                                   ORDER BY sort_order ASC");
    
    while($key_value = xtc_db_fetch_array($key_value_query)) {
        if ($key_value['configuration_key'] !='') {
            $keys_extra[$key_value['sort_order']]['title'] = constant(strtoupper($key_value['configuration_key'] .'_TITLE'));
        }
        $keys_extra[$key_value['sort_order']]['value'] = $key_value['configuration_value'];
        if ($key_value['configuration_key'] !='') {
            $keys_extra[$key_value['sort_order']]['description'] = constant(strtoupper($key_value['configuration_key'] .'_DESC'));
        }
        $keys_extra[$key_value['sort_order']]['use_function'] = $key_value['use_function'];
        $keys_extra[$key_value['sort_order']]['set_function'] = $key_value['set_function'];
    }
    
    /*for ($j = 0, $k = sizeof($module_keys); $j < $k; $j++) {
      $key_value_query = xtc_db_query("SELECT configuration_key,
                                              configuration_value,
                                              use_function,
                                              set_function
                                         FROM " . TABLE_CONFIGURATION . "
                                        WHERE configuration_key = '" . $module_keys[$j] . "'");
      $key_value = xtc_db_fetch_array($key_value_query);
      if ($key_value['configuration_key'] !='') {
        $keys_extra[$module_keys[$j]]['title'] = constant(strtoupper($key_value['configuration_key'] .'_TITLE'));
      }
      $keys_extra[$module_keys[$j]]['value'] = $key_value['configuration_value'];
      if ($key_value['configuration_key'] !='') {
        $keys_extra[$module_keys[$j]]['description'] = constant(strtoupper($key_value['configuration_key'] .'_DESC'));
      }
      $keys_extra[$module_keys[$j]]['use_function'] = $key_value['use_function'];
      $keys_extra[$module_keys[$j]]['set_function'] = $key_value['set_function'];
    }*/
    $module_info['keys'] = $keys_extra;
    
    return $module_info;
  }

Damit hätten wir dem Feld sort_order wieder zur Geltung verholfen und die DB-Query optimiert.

Gruß,
noRiddle

Attachments (0)

Change History (8)

comment:1 Changed 4 months ago by noRiddle

*NACHTRAG*

Das Gesagte gilt auch für die /admin/module_export.php .

Gruß,
noRiddle

comment:2 Changed 4 months ago by Tomcraft

  • Milestone set to modified-shop-2.0.5.2

comment:3 Changed 4 months ago by GTB

das würde aber bedeuten, dass die vorgegebene Sortierung im keys array nicht mehr greift und das ist nicht optimal.

comment:4 Changed 4 months ago by noRiddle

Naja, wo aber steht, daß die Reihenfolge im keys-Array entscheidend ist ?
(...und wieso "vorgegebene" ... und wieso "nicht mehr", wann war das so ?)
Wofür ist den das configuration-Feld sort_order ?

Ich meine, okay, wenn man's weiß, die sort_order wirkt halt bei den Modulen allgemein (was ich noch nicht mal geprüft habe)), aber eben nicht bei den Konfigurations-Werten eines Modules. Kann man drüber streiten. Aber das Feld sort_order ist nun mal da.

Die Query-Optimierung halte ich jedenfalls für gut. :-)

Gruß,
noRiddle

comment:5 Changed 3 months ago by GTB

  • Milestone modified-shop-2.0.5.2 deleted

Ich habe mir den COde nochmal im Detail angeschaut.
Ich sehe darin mehrere Probleme.

  • alle Module bei denen keine Sortierung gepflegt wurde funktionieren nicht mehr, da die Reihenfolge verwendet wird als Index für das Array.
  • auch für die Zukunft finde ich diese Lösung als gefährlich, denn eine Sortierreihenfolge muss dann unique sein
  • dei der SQL Optimierung sehe ich den Vorteil der Performance, aber auch das Problem, dass die Reihenfolge nicht mehr stimmt.

Das Ticket wird deshalb vorerst nicht berücksichtigt.

comment:6 Changed 3 months ago by anonymous

Nicht gepflegte Sortierung bei Modulen ist ein Argument.

Die Query lässt sich allerdings trotzdem optimieren.
Du lässt das ORDER BY in meiner Query weg und um an den Array-Key der einzelnen Values in $module_keys zu kommen, also das was du im for-Loop mit j machst, machst du zusätzlich zu diesem

$module_keys = method_exists($module,'keys') ? $module->keys() : array();

das

$module_keys = method_exists($module,'keys') ? $module->keys() : array();
$keys_module = array_flip($module_keys);

und kannst dann im while-Loop so definieren

$keys_extra[$module_keys[$keys_module[$key_value['configuration_key']]]]['title'] = constant(strtoupper($key_value['configuration_key'] .'_TITLE'));

oder von mir aus aus Gründen der Lesbarkeit so

$k = $keys_module[$key_value['configuration_key']];
$keys_extra[$module_keys[$k]]['title'] = constant(strtoupper($key_value['configuration_key'] .'_TITLE'));

Damit könnte die Query außerhalb des for-Loops laufen.

Gruß,
noRiddle

comment:7 Changed 3 weeks ago by noRiddle

GTB, verfolgst du das hier noch ?
Was hältst du von meinem letzten Vorschlag Queries in einem Loop zu machen ?

Gruß,
noRiddle

comment:8 Changed 3 weeks ago by noRiddle

Korrektur: Ich meinet natürlich "zu vermeiden Queries in einem Loop zu machen..."

Add Comment

Modify Ticket

Action
as new
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.