Ankündigung

Einklappen
Keine Ankündigung bisher.

Controller <-> Model: Wohin mit der Logik?

Einklappen

Neue Werbung 2019

Einklappen
X
  • Filter
  • Zeit
  • Anzeigen
Alles löschen
neue Beiträge

  • Controller <-> Model: Wohin mit der Logik?

    Hallo,

    Nachdem mir empfohlen wurde mich in ein PHP Framework einzuarbeiten, bin ich gerade dabei mit Laravel herumzuspielen. Nun habe ich eine einfache Registrierung programmiert, wobei ein "UsersController" mit der Methode "store" das speichern übernimmt.
    Dieser sieht folgendermaßen aus:
    PHP-Code:
    class UsersController extends \BaseController {

    //...
    //...

        
    public function store()
        {
            
    $input Input::all();

            if ( ! 
    $this->user->fill($input)->isValid()) 
            {
                return 
    Redirect::back()->withInput()->withErrors($this->user->errors);
            }

            
    $this->user->save();

            return 
    Redirect::route('users.index');
        } 
    Das User-Model enthält die Validation:
    PHP-Code:
    class User extends Eloquent implements UserInterfaceRemindableInterface {

        public function 
    isValid()
        {
            
    $validation Validator::make($this->attributes, static::$rules);
            
    $this->hashPassword();

            if(
    $validation->passes()) return true;

            
            
    $this->errors $validation->errors();

            return 
    false;
        }

        public function 
    hashPassword()
        {
            
    // TODO Prevent from double hashing! 
            
    $this->attributes["password"] = Hash::make($this->attributes["password"]);
        } 
    Mein Problem nun:
    Wenn ein User sein Profil editiert, das Passwort dabei aber nicht ändert liegt das verhashte Passwort in $this->attributes["password"], wird also nocheinmal verhashet. Hat jemand eine Idee wie ich das elegant umgehen kann? Mein erster Gedanke war wenn die Länge > 20 ist (gleichzeitig Länge des Passworts auf 20 begrenzen) einfach zu sagen es ist schon ein Hash und nicht mehr verhashen. Aber das ist alles andere als sauber.

    Sollte ich die Validation überhaupt woanders hinpacken? Ich habe zumindest gelesen, dass der Controller möglichst wenig Logik enthalten soll.

    Ich bin für jeden Tipp dankbar!

    lg
    Manuel


  • #2
    Bei mir ist in diesem speziellen Fall das Passwort nie Teil des User-Models.
    Passwort-Handling mache ich dann gesondert.
    Standards - Best Practices - AwesomePHP - Guideline für WebApps

    Kommentar


    • #3
      Zum Einen hat das Hashing in der Validierung absolut nix verloren, zum Anderen halte ich von umgehbarer Validierung sowieso nichts, da somit die Möglichkeit besteht, ein invalides Model zu haben, ohne es zu merken.
      Für dein konkretes Problem: hashen und validieren beim Setzen der Daten, setzen eben auch nur, wenn vorhanden/geändert.
      VokeIT GmbH & Co. KG - VokeIT-oss @ github

      Kommentar


      • #4
        Bei mir ist in diesem speziellen Fall das Passwort nie Teil des User-Models.
        Passwort-Handling mache ich dann gesondert.
        Verstehe nicht ganz wie das gemeint ist. Welcher Vorteil ergibt sich wenn das Passwort nicht Teil des User-Models ist?

        Zitat von G.Schuster Beitrag anzeigen
        Zum Einen hat das Hashing in der Validierung absolut nix verloren, zum Anderen halte ich von umgehbarer Validierung sowieso nichts, da somit die Möglichkeit besteht, ein invalides Model zu haben, ohne es zu merken.
        Für dein konkretes Problem: hashen und validieren beim Setzen der Daten, setzen eben auch nur, wenn vorhanden/geändert.
        Edit: glaube doch dass ich verstehe was du meinst

        Ist es so besser?
        PHP-Code:
        // Model User
        public function fillAndCheck($input)
            {
                
        $validation Validator::make($input, static::$rules);

                if(
        $validation->passes()) 
                { 
                    
        $this->fill($input)
                    return 
        true;
                }
                
                
        $this->errors $validation->errors();

                return 
        false;
            } 
        Wobei es dann möglich wäre über eine magische "setPasswordAttribute" (Mutator) das Passwort automatisch zu hashen:
        PHP-Code:
        // Model User
            
        public function setPasswordAttribute($password)
            {
                
        $this->attributes["password"] = Hash::make($password);
            } 
        Hierbei würde der Code im Controller sich nur unwesentlich ändern
        PHP-Code:
            public function store()
            {
                
        $input Input::all();

                if ( ! 
        $this->user->fillAndCheck($input)) 
                {
                    return 
        Redirect::back()->withInput()->withErrors($this->user->errors);
                }

                
        $this->user->save();

                return 
        Redirect::route('users.index');
            } 
        Gefällt mir eigentlich schon viel besser

        Kommentar


        • #5
          Gefällt mir eigentlich schon viel besser
          Man kann es sicherlich so machen, aber schön finde ich es nicht.

          Wieso schmeißt Du checkAndFill() nicht komplett aus dem Modell raus und validierst stattdessen außerhalb? Auf die User-Properties greifst Du dann ausschließlich über Setter/Getter zu. Die Setter sind dabei so abgesichert, daß bei ungültigen Werten eine z.B. InvalidArgumentException geworfen wird. Wenn Du mehrere Werte auf einmal einfügen willst, dann könntest Du dem Modell entweder eine setByArray($properties) spendieren (die intern das Modell über Setter befüllt!) oder (vielleicht besser) einen UserArrayMapper::update($user, $properties). So ist die Konsistenz des Modells immer gewährleistet und das Modell wird nicht als Validator zweckentfremdet.

          Vg
          jack
          -

          Kommentar


          • #6
            Zitat von jack88 Beitrag anzeigen
            Wieso schmeißt Du checkAndFill() nicht komplett aus dem Modell raus und validierst stattdessen außerhalb?
            Das ist natürlich die Frage. WO wäre die Validierung der Daten deiner Meinung nach am Besten aufgehoben? Warum nicht dem Model überlassen? Ich finde es eigentlich gut wenn das Model direkt weis was es erlaubt und was nicht, die Validierung selbst geschieht ja in einer eigenen Klasse!

            Kommentar


            • #7
              Die Validierung ist Teil des Eingabeprozesses, also der Komponente, die Form-View, Form-Action etc. abhandelt. Ich würde die Validierung konfigurierbar gestalten und ggf. diese Konfiguration verbindlich für Validierung und Datenbankstruktur machen.
              --

              „Emoticons machen einen Beitrag etwas freundlicher. Deine wirken zwar fachlich richtig sein, aber meist ziemlich uninteressant.
              Wenn man nur Text sieht, haben viele junge Entwickler keine interesse, diese stumpfen Texte zu lesen.“


              --

              Kommentar


              • #8
                Zitat von jack88 Beitrag anzeigen
                Die Setter sind dabei so abgesichert, daß bei ungültigen Werten eine z.B. InvalidArgumentException geworfen wird.
                Und das ist keine Validierung?

                In meinen Augen hat eine Validierung immer dort stattzufinden, wo anschließend (auf "normalem" Weg) keine unvalidierte Änderung mehr stattfinden kann, andernfalls macht es keinen Sinn, sich überhaupt Gedanken über Datenkonsistenz zu machen.
                Ein von mir entwickeltes Model-Konzept setzt dazu eben auf Settern auf, wobei die Validatoren beim Init des Models angelegt werden und damit auch für eine "Array-Befüllung" problemlos zur Verfügung stehen.
                Nebenbei spare ich mir damit Code-Duplication: egal woher Daten kommen, ich pack sie als Array-Param in den Constructor-Aufruf und kann sicher sein, dass alles korrekt ist.
                Das schließt auch z.B. "Pflichtfelder" ein, die bei einer externen Validierung niemals zuverlässig sein können.
                VokeIT GmbH & Co. KG - VokeIT-oss @ github

                Kommentar


                • #9
                  Das Problem ist die Abhängigkeitvon der Validator-Klasse.
                  --

                  „Emoticons machen einen Beitrag etwas freundlicher. Deine wirken zwar fachlich richtig sein, aber meist ziemlich uninteressant.
                  Wenn man nur Text sieht, haben viele junge Entwickler keine interesse, diese stumpfen Texte zu lesen.“


                  --

                  Kommentar


                  • #10
                    Ich seh da kein Problem.
                    Für mich zählen valide Daten durchaus auch zur Geschäftslogik und für die Erfüllung von Zusicherungen (=definierter Zustand der Models) nehme ich Design-Kritik gern in Kauf.
                    VokeIT GmbH & Co. KG - VokeIT-oss @ github

                    Kommentar


                    • #11
                      Jetzt bin ich etwas ausgestiegen.

                      Die Validierung ist Teil des Eingabeprozesses, also der Komponente, die Form-View, Form-Action etc. abhandelt. Ich würde die Validierung konfigurierbar gestalten und ggf. diese Konfiguration verbindlich für Validierung und Datenbankstruktur machen.
                      Form-View und Form-Action wird ja vom Controller (in Laravel, In PHP Design Patterns heißen die glaubich "Commands") abgehandelt, oder verstehe ich das falsch?

                      In meinen Augen hat eine Validierung immer dort stattzufinden, wo anschließend (auf "normalem" Weg) keine unvalidierte Änderung mehr stattfinden kann, andernfalls macht es keinen Sinn, sich überhaupt Gedanken über Datenkonsistenz zu machen.
                      Was verstehst du unter "keine unvalidierte Änderung"?

                      Vielen Dank!!

                      Kommentar


                      • #12
                        Zitat von G.Schuster Beitrag anzeigen
                        Ich seh da kein Problem.
                        Für mich zählen valide Daten durchaus auch zur Geschäftslogik und für die Erfüllung von Zusicherungen (=definierter Zustand der Models) nehme ich Design-Kritik gern in Kauf.
                        Daten und Datenvalidität sind zwei unterschiedliche Concerns und sollten auch getrennt behandelt werden (können).
                        Zum Thema: Was spricht denn gegen meinen Vorschlag?
                        Standards - Best Practices - AwesomePHP - Guideline für WebApps

                        Kommentar


                        • #13
                          Zitat von rkr Beitrag anzeigen
                          Daten und Datenvalidität sind zwei unterschiedliche Concerns und sollten auch getrennt behandelt werden (können).
                          Zum Thema: Was spricht denn gegen meinen Vorschlag?
                          Ich sehe den Vorteil nicht welcher sich ergibt wenn das Passwort außerhalb des Models gehandelt wird. Aber ich freue mich über jeden Input, eure Tipps helfen mir sehr!

                          Kommentar


                          • #14
                            Weil das eine Modell-Eigenschaft ist, die nicht zu den anderen passt - bzw. ein special treatment benötigt. Welcher Nachteil ergibt sich denn das Passwort gesondert zu handhaben?

                            Bitte beachte zudem, dass das Passwort eigentlich keine richtige User-Eigenschaft ist.
                            Es ist ein Zugangsschlüssel. Wie beispielsweise ein SingleSignOn-Token oder ein LDAP-DN. Muss nicht mal zwangsläufig in der gleichen Tabelle gespeichert werden. Nicht mal in der gleichen Datenbank. Nicht mal auf dem gleichen Server.
                            Standards - Best Practices - AwesomePHP - Guideline für WebApps

                            Kommentar


                            • #15
                              Zitat von Leichti Beitrag anzeigen
                              Was verstehst du unter "keine unvalidierte Änderung"?
                              Setter ohne Validierung bzw. public properties - da kann ich dann lustig reinschieben, was ich möchte.
                              Über den Umweg mit Reflections wäre das zwar prinzipiell immer möglich, aber das wäre kein Grund, die anderen einfachen Möglichkeiten zu unterbinden.

                              Im Übrigen sind zwei Validierungen per se nichts Schlechtes, der Kontext einer Form-Validierung ist immerhin ein entscheidend anderer als der zur Sicherstellung von Datenintegrität.
                              Bei einem Formular komtm es z.B. darauf an, ob ein Geburtsdatum einem gewissen Alter entspricht, für das Model ist dagegen eher entscheidend, dass es ein valides Datum ist.
                              Oder ein Formular erfasst Daten im Zusammenhang, die auf mehrere Models abgebildet werden - dennoch muss jedes Model in meinen Augen sicherstellen, dass der Datenteil, den es beherbergt, den Regeln entspricht.
                              Ich glaube, das kann man jahrelang diskutieren, von daher klink ich mich an der Stelle aus.
                              VokeIT GmbH & Co. KG - VokeIT-oss @ github

                              Kommentar

                              Lädt...
                              X