New password element

src/Contract/ValueCandidates.php

@@ -2,6 +2,11 @@
 
 namespace ipl\Html\Contract;
 
+/**
+ * @deprecated Only {@see \ipl\Html\FormElement\PasswordElement} ever used this even
+ * though {@see \ipl\Html\FormElement\BaseFormElement} implements it. No replacement
+ * will be provided.
+ */
 interface ValueCandidates
 {
     /**

src/Form.php

@@ -311,6 +311,18 @@ public function isValid()
         return $this->isValid;
     }
 
+    /**
+     * Get whether the form has already been validated
+     *
+     * Unlike {@see isValid()}, this never triggers validation.
+     *
+     * @return bool
+     */
+    public function hasBeenValidated()
+    {
+        return $this->isValid !== null;
+    }
+
     /**
      * Validate all elements
      *

src/FormElement/BaseFormElement.php

@@ -8,7 +8,6 @@
 use ipl\Html\Contract\DecorableFormElement;
 use ipl\Html\Contract\FormElement;
 use ipl\Html\Contract\FormElementDecoration;
-use ipl\Html\Contract\ValueCandidates;
 use ipl\Html\Form;
 use ipl\Html\FormDecoration\DecoratorChain;
 use ipl\Html\FormDecoration\FormElementDecorationResult;
@@ -22,7 +21,7 @@
  *
  * @phpstan-import-type decoratorsFormat from DecoratorChain
  */
-abstract class BaseFormElement extends BaseHtmlElement implements FormElement, ValueCandidates, DecorableFormElement
+abstract class BaseFormElement extends BaseHtmlElement implements FormElement, DecorableFormElement
 {
     use Messages;
     use Translation;
@@ -51,9 +50,6 @@ abstract class BaseFormElement extends BaseHtmlElement implements FormElement, V
     /** @var mixed Value of the element */
     protected $value;
 
-    /** @var array<int, mixed> Value candidates of the element */
-    protected $valueCandidates = [];
-
     /** @var ?DecoratorChain<FormElementDecoration> All registered decorators */
     protected ?DecoratorChain $decorators = null;
 
@@ -249,18 +245,6 @@ public function setValue($value)
         return $this;
     }
 
-    public function getValueCandidates()
-    {
-        return $this->valueCandidates;
-    }
-
-    public function setValueCandidates(array $values)
-    {
-        $this->valueCandidates = $values;
-
-        return $this;
-    }
-
     public function onRegistered(Form $form)
     {
     }

src/FormElement/FileElement.php

@@ -224,6 +224,16 @@ protected function storeFiles(UploadedFileInterface ...$files): array
                 continue;
             }
 
+            if (isset($this->files[$name]) && is_file($path)) {
+                // The file has already been stored, e.g. because setValue() was called more than
+                // once for the same upload ({@see FormElements::registerElement()} replays every
+                // populated value). Re-moving the original upload would fail as its stream has
+                // already been moved.
+                $storedFiles[] = $this->files[$name];
+
+                continue;
+            }
+
             $file->moveTo($path);
 
             // Re-created to ensure moveTo() still works if called externally

src/FormElement/FormElements.php

@@ -8,7 +8,6 @@
 use ipl\Html\Contract\FormElement;
 use ipl\Html\Contract\FormElementDecoration;
 use ipl\Html\Contract\FormElementDecorator;
-use ipl\Html\Contract\ValueCandidates;
 use ipl\Html\Form;
 use ipl\Html\FormDecoration\DecoratorChain;
 use ipl\Html\FormDecorator\DecoratorInterface;
@@ -240,10 +239,8 @@ public function registerElement(FormElement $element)
         $this->elements[$name] = $element;
 
         if (array_key_exists($name, $this->populatedValues)) {
-            $element->setValue($this->populatedValues[$name][count($this->populatedValues[$name]) - 1]);
-
-            if ($element instanceof ValueCandidates) {
-                $element->setValueCandidates($this->populatedValues[$name]);
+            foreach ($this->populatedValues[$name] as $value) {
+                $element->setValue($value);
             }
         }
 

src/FormElement/PasswordElement.php

@@ -12,56 +12,150 @@ class PasswordElement extends InputElement
 
     protected $type = 'password';
 
-    /** @var bool Status of the form */
-    protected $isFormValid = true;
+    /** @var mixed Stored value, preserved across DUMMYPASSWORD submissions */
+    protected mixed $storedValue = null;
 
-    /** @var bool Status indicating if the form got submitted */
-    protected $isFormSubmitted = false;
+    /**
+     * Value from the very first setValue() call, used as the baseline to detect a change
+     *
+     * `null` is a legitimate baseline (no pre-existing secret); {@see $seeded} distinguishes that
+     * from "not captured yet".
+     *
+     * @var mixed
+     */
+    protected mixed $initialValue = null;
 
-    protected function registerAttributeCallbacks(Attributes $attributes)
+    /** @var bool Whether {@see $initialValue} has been captured */
+    protected bool $seeded = false;
+
+    /**
+     * Number of setValue() calls received so far, dummy placeholder included
+     *
+     * A pre-existing value seeded before the current request's own value lands on top of it takes
+     * at least two calls; an element that only ever got the request's own value never does, and
+     * thus has nothing worth masking.
+     *
+     * @var int
+     */
+    protected int $assignments = 0;
+
+    /** @var ?Form The form this element is registered with */
+    protected ?Form $form = null;
+
+    public function getValue()
     {
-        parent::registerAttributeCallbacks($attributes);
+        if ($this->hasFailedValidation()) {
+            return null;
+        }
 
-        $attributes->registerAttributeCallback(
-            'value',
-            function () {
-                if (
-                    $this->hasValue()
-                    && count($this->getValueCandidates()) === 1
-                    && $this->isFormValid
-                    && ! $this->isFormSubmitted
-                ) {
-                    return self::DUMMYPASSWORD;
-                }
-
-                if (parent::getValue() === self::DUMMYPASSWORD && count($this->getValueCandidates()) > 1) {
-                    return self::DUMMYPASSWORD;
-                }
-
-                return null;
-            }
-        );
+        return $this->storedValue;
     }
 
-    public function onRegistered(Form $form)
+    /**
+     * Get whether a changed value has failed to validate
+     *
+     * Peeks at an already known form validity only, never asks for it. Calling {@see isValid()}
+     * would trigger validation, which may lead to an infinite loop.
+     *
+     * @return bool
+     */
+    protected function hasFailedValidation(): bool
     {
-        $form->on(Form::ON_VALIDATE, function ($form) {
-            $this->isFormValid = $form->isValid();
-        });
+        $value = parent::getValue();
 
-        $form->on(Form::ON_SENT, function ($form) {
-            $this->isFormSubmitted = $form->hasBeenSent();
-        });
+        return $value !== null
+            && $value !== static::DUMMYPASSWORD
+            && (
+                $this->valid === false
+                || ($this->form !== null && $this->form->hasBeenValidated() && ! $this->form->isValid())
+            );
     }
 
-    public function getValue()
+    public function setValue($value)
     {
+        $this->assignments++;
+        parent::setValue($value);
         $value = parent::getValue();
-        $candidates = $this->getValueCandidates();
-        while ($value === self::DUMMYPASSWORD) {
-            $value = array_pop($candidates);
+        if ($value !== static::DUMMYPASSWORD) {
+            $this->storedValue = $value;
+
+            if (! $this->seeded) {
+                $this->seeded = true;
+                $this->initialValue = $value;
+            }
+        }
+
+        return $this;
+    }
+
+    /**
+     * Validate the element using all registered validators
+     *
+     * The dummy placeholder isn't a real password, just a marker for "unchanged", so it's exempt
+     * from validators meant for freshly typed input.
+     *
+     * @return $this
+     */
+    public function validate(): static
+    {
+        $this->ensureAssembled();
+
+        if (parent::getValue() === static::DUMMYPASSWORD) {
+            $this->valid = true;
+            $this->clearMessages();
+
+            return $this;
         }
 
-        return $value;
+        return parent::validate();
+    }
+
+    /**
+     * Get the value to render into the `value` attribute
+     *
+     * In order:
+     * - Nothing stored, or a changed value that just failed validation: `null`.
+     * - No request to compare against yet (e.g. an initial GET render): masked with
+     *   {@see DUMMYPASSWORD}.
+     * - Nothing was seeded before this request's own value, i.e. no secret to protect (see
+     *   {@see $assignments}): shown while still editing, cleared once actually submitted.
+     * - Otherwise: masked if unchanged or actually submitted, shown while still editing.
+     *
+     * @return ?string
+     */
+    public function getValueAttribute()
+    {
+        if ($this->storedValue === null) {
+            return null;
+        }
+
+        $stillEditing = $this->form !== null && ! $this->form->hasBeenSubmitted();
+        if (! $stillEditing && $this->hasFailedValidation()) {
+            return null;
+        }
+
+        if ($this->form === null || ! $this->form->hasBeenSent()) {
+            return static::DUMMYPASSWORD;
+        }
+
+        if ($this->assignments < 2) {
+            return $stillEditing ? $this->storedValue : null;
+        }
+
+        $unchanged = $this->storedValue === $this->initialValue;
+
+        return $unchanged || ! $stillEditing ? static::DUMMYPASSWORD : $this->storedValue;
+    }
+
+    public function onRegistered(Form $form)
+    {
+        parent::onRegistered($form);
+        $this->form = $form;
+    }
+
+    protected function registerAttributeCallbacks(Attributes $attributes)
+    {
+        parent::registerAttributeCallbacks($attributes);
+        $attributes->registerAttributeCallback('value', $this->getValueAttribute(...), $this->setValue(...));
     }
 }

tests/FormElement/FileElementTest.php

@@ -143,6 +143,52 @@ protected function assemble()
         $this->assertNull($thirdForm->getValue('test'));
     }
 
+    public function testUploadedFileIsStoredOnlyOnceWhenSetValueIsReplayed()
+    {
+        // FormElements::registerElement() replays every populated value through setValue().
+        // A file element that is populated more than once before it is registered must not
+        // attempt to move its already-moved upload a second time.
+
+        $form = new class extends Form {
+            protected function assemble()
+            {
+                $this->addElement('file', 'test', [
+                    'destination' => sys_get_temp_dir()
+                ]);
+            }
+        };
+
+        $file = new UploadedFile(
+            Utils::streamFor('lorem ipsum dolorem'),
+            19,
+            0,
+            'test.txt',
+            'text/plain'
+        );
+
+        $storedPath = implode(DIRECTORY_SEPARATOR, [sys_get_temp_dir(), sha1('test.txt')]);
+        @unlink($storedPath);
+
+        // Two populate() calls for the same name accumulate two values, so registerElement()
+        // calls setValue() twice on the same upload once the element is added during assemble().
+        $form->populate(['test' => $file]);
+        $form->populate(['test' => $file]);
+
+        $form->ensureAssembled();
+
+        $this->assertSame(
+            'test.txt',
+            $form->getElement('test')->getValue()->getClientFilename()
+        );
+
+        $this->assertSame(
+            'lorem ipsum dolorem',
+            $form->getValue('test')->getStream()->getContents()
+        );
+
+        @unlink($storedPath);
+    }
+
     public function testUploadedFileCanBeMoved()
     {
         $form = new class extends Form {