• Najnowsze pytania
  • Bez odpowiedzi
  • Zadaj pytanie
  • Kategorie
  • Tagi
  • Zdobyte punkty
  • Ekipa ninja
  • IRC
  • FAQ
  • Regulamin
  • Książki warte uwagi

Sprawdzenie kodu Symfony 4

+1 głos
92 wizyt
pytanie zadane 3 lutego w PHP, Symfony, Zend przez `Krzychuu Stary wyjadacz (12,940 p.)

Witam

Chciałbym żebyście sprawdzili kod na razie jest tylko rejestracja z dokumentacji symfony ale trochę przerobiłem:

<?php

namespace App\Controller;

use App\Form\UserType;
use App\Service\FormErrorsConverter;
use App\Service\Security;
use FOS\RestBundle\Controller\AbstractFOSRestController;
use FOS\RestBundle\Controller\Annotations as Rest;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Security\Core\Encoder\UserPasswordEncoderInterface;

class SecurityController extends AbstractFOSRestController
{
    /**
     * @Rest\Post("/register", name="user_register")
     *
     * @param Request $request
     * @param Security $security
     * @param FormErrorsConverter $converter
     * @param UserPasswordEncoderInterface $encoder
     *
     * @return Response
     */
    public function register(
        Request $request,
        Security $security,
        FormErrorsConverter $converter,
        UserPasswordEncoderInterface $encoder
    ): Response
    {
        $form = $this->createForm(UserType::class, null);

        $data = json_decode($request->getContent(), true);
        $form->submit($data);

        if($form->isValid()) {
            $data = $form->getData();

            $registerAction = $security->registerUser($data, $encoder);

            return $this->json([
                'success' => $registerAction
            ], Response::HTTP_OK);
        }

        return $this->json([
            'error' => "Form is not valid",
            'errors_fields' => $converter->convertErrorsFromFrom($form),
        ], Response::HTTP_BAD_REQUEST);
    }
}
<?php

namespace App\Form;

use App\Entity\User;
use Symfony\Component\Form\AbstractType;
use Symfony\Component\Form\Extension\Core\Type\EmailType;
use Symfony\Component\Form\Extension\Core\Type\PasswordType;
use Symfony\Component\Form\Extension\Core\Type\TextType;
use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\OptionsResolver\OptionsResolver;

class UserType extends AbstractType
{
    public function buildForm(FormBuilderInterface $builder, array $options)
    {
        $builder
            ->add('username', TextType::class)
            ->add('password', PasswordType::class)
            ->add('email', EmailType::class)
        ;
    }

    public function configureOptions(OptionsResolver $resolver)
    {
        $resolver->setDefaults([
            'data_class' => User::class,
        ]);
    }

    public function getName()
    {
        return 'user';
    }
}
<?php

namespace App\Service;

use App\Entity\User;
use App\Repository\UserRepository;
use Doctrine\ORM\EntityManagerInterface;
use Symfony\Component\Security\Core\Encoder\UserPasswordEncoderInterface;

class Security
{
    private $entityManager;
    private $userRepository;

    public function __construct(
        EntityManagerInterface $entityManager,
        UserRepository $userRepository
    )
    {
        $this->entityManager = $entityManager;
        $this->userRepository = $userRepository;
    }
    
    public function registerUser(User $user, UserPasswordEncoderInterface $encoder)
    {
        $password = $encoder->encodePassword($user, $user->getPassword());

        $user->setPassword($password);
        $this->entityManager->persist($user);
        $this->entityManager->flush();

        return $user->getId();
    }
}
<?php

namespace App\Entity;

use Doctrine\ORM\Mapping as ORM;
use Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntity;
use Symfony\Component\Security\Core\User\UserInterface;
use Symfony\Component\Validator\Constraints as Assert;

/**
 * @ORM\Table(name="users")
 * @ORM\Entity(repositoryClass="App\Repository\UserRepository")
 * @UniqueEntity(
 *     fields={"username", "email"}
 * )
 *
 * @ORM\HasLifecycleCallbacks()
 */
class User implements UserInterface, \Serializable
{
    /**
     * @ORM\Id()
     * @ORM\GeneratedValue()
     * @ORM\Column(type="integer")
     */
    private $id;

    /**
     * @ORM\Column(type="string", length=50, unique=true)
     * @Assert\Length(min=3, max="50")
     * @Assert\NotBlank()
     */
    private $username;

    /**
     * @ORM\Column(type="string", length=255)
     * @Assert\NotBlank()
     * @Assert\Length(min=8)
     */
    private $password;

    /**
     * @ORM\Column(type="string", length=255, unique=true)
     * @Assert\NotBlank()
     * @Assert\Email(
     *     message = "The email '{{ value }}' is not a valid email.",
     *     checkMX = true
     * )
     */
    private $email;

    /**
     * @ORM\Column(type="boolean")
     * @Assert\NotBlank()
     */
    private $isActive;

    /**
     * @ORM\Column(type="datetime")
     * @Assert\DateTime()
     */
    private $createdAt;

    public function __construct()
    {
        $this->isActive = true;
    }

    public function getId(): ?int
    {
        return $this->id;
    }

    public function getUsername(): ?string
    {
        return $this->username;
    }

    public function setUsername(string $username): self
    {
        $this->username = $username;

        return $this;
    }

    public function getPassword(): ?string
    {
        return $this->password;
    }

    public function setPassword(string $password): self
    {
        $this->password = $password;

        return $this;
    }

    public function getEmail(): ?string
    {
        return $this->email;
    }

    public function setEmail(string $email): self
    {
        $this->email = $email;

        return $this;
    }

    public function getIsActive(): ?bool
    {
        return $this->isActive;
    }

    public function setIsActive(bool $isActive): self
    {
        $this->isActive = $isActive;

        return $this;
    }

    public function getSalt()
    {
        return null;
    }

    public function getRoles()
    {
        return array('ROLE_USER');
    }

    /**
     * @ORM\PrePersist()
     */
    public function setCreatedAt()
    {
        $this->createdAt = new \DateTime();
    }
    
    public function eraseCredentials()
    {
    }

    /** @see \Serializable::serialize() */
    public function serialize()
    {
        return serialize(array(
            $this->id,
            $this->username,
            $this->password,
        ));
    }

    /** @see \Serializable::unserialize() */
    public function unserialize($serialized)
    {
        list (
            $this->id,
            $this->username,
            $this->password,
            ) = unserialize($serialized, array('allowed_classes' => false));
    }
}

 

1 odpowiedź

+3 głosów
odpowiedź 4 lutego przez Ehlert Mędrzec (166,360 p.)
wybrane 4 lutego przez `Krzychuu
 
Najlepsza
  1. json_decode w kontrolerze nie jest potrzebne. Po to mamy pole request i metodę all(). Opcjonalnie handleRequest na form.
  2. Klasa o nazwie Security. Trochę kiepsko, Wujek Bob płakałby.
  3. Dlaczego metoda z klasy Security przyjmuje passwordEncoder jako argument? Daj to do tej klasy jako zależność.
  4. Z kolei nie wiem po co w tej klasie repozytorium skoro z niego nie korzystasz. 
  5. getName jeszcze istnieje w formularzach? Na pewno nie potrzebnie. 
  6. Brakuje docblocków, ale nie wiem jakie masz konwencje. 
  7. A i strict types przydałoby się bez względu na konwencje.
  8. Duży błąd. Zauważ, że podczas całego flow rejestracji w polu usera w password raz siedzi plain password a raz hash. To nie jest dobry pomysł, w późniejszych zmianach może to prowadzić do niebezpiecznych sytuacji. Dołoż zwykłe pole na plain Password i czyść je w eraseCredentials. 
  9. Password length=255. Strzelam że korzystasz z bcrypta więc 60 znaków maksymalnie. A pro pos korzystaj z bcrypta, lub argona.
  10. Doctrine w annotations ok, ale kiedy dochodzą do tego jeszcze asercje robi się mały syf. To moje osobiste preferencje, rób jak Ci wygodniej... Żartowałem wywal asercje do ymla laugh​​​​​​
  11. Zapewne błędnie użyłeś UniqueEntity. W obecnej sytuacji email, lub username mogą się duplikować. Ich kombinacja nie może się powtórzyć.
  12. FormErrorsConverter to zupełnie zbędny byt biorąc pod uwagę, że mamy JMSSerializerBundle. Serializacja formularzy Symfony idzie mu wyśmienicie.
  13. Polecam zainstalować phpcs.
  14. Przyszłościowo dobrze byłoby trzymać role w polu. Ale to taka sugestia.
  15. Nie wszędzie masz Type hinty. 
  16. Bądź konsekwentny w typach. isActive masz domyślnie na true, asercja NotBlank(psst zmień na NotNull, bo false nie przejdzie walidacji), setter przyjmuje bool. Jakim cudem więc getter zwraca nullable.
  17. Dopisz testy devil

Ogólnie spoko. yes

komentarz 4 lutego przez `Krzychuu Stary wyjadacz (12,940 p.)
Przyznaje z nazewnictwem klas mam problem, masz rację co do UniqueEntity, teraz zauważyłem że tylko sprawdza mi username a emailu nie sprawdza i mogę dodać taki sam. Co do json_decode to trochę z tym walczyłem, dane przesyłam przez Postman w formie json i tylko z json_decode dane przeszły, gdy dodawałem bez json_decode to wywalało null. 14: tzn dodać do formularzu pole z rolami?. Mam jeszcze pytanie co do formularzy gdy mam teraz gotowy formularz User, to jakbym chciał zrobić zmianę hasła to wtedy muszę zrobić nowy formularz np ChangePasswordType?
komentarz 4 lutego przez Ehlert Mędrzec (166,360 p.)

Tak, to byłoby najlepsze rozwiązanie. Od razu nakierowuje na RepeatedType.

komentarz 4 lutego przez `Krzychuu Stary wyjadacz (12,940 p.)
Mógłbyś rozwinąć punkt 14?
komentarz 4 lutego przez `Krzychuu Stary wyjadacz (12,940 p.)

@Ehlert, FormErrorsConverter zwraca tablice w której znajduje się pole i wiadomość błędu, tak wygląda kod:

{
    $errors = array();

    foreach ($form->getErrors() as $error) {
        $errors[$form->getName()][] = $error->getMessage();
    }

    foreach ($form as $child /** @var Form $child */) {
        if (!$child->isValid()) {
            foreach ($child->getErrors() as $error) {
                $errors[$child->getName()][] = $error->getMessage();
            }
        }
    }

    return $errors;
}

 

1
komentarz 4 lutego przez Ehlert Mędrzec (166,360 p.)
Ty masz bardzo prostą strukturę formularza, ale zdarzają się takie bardziej skomplikowane. Nie wiem czy taki for zdałby egzamin. Dlatego instalujesz ten Serializer i wszystko śmiga.

Co do pkt 14. Dobrze jest trzymać role w polu. Może to być zwykły Varchar i masz tam wtedy jsona albo Serialize phpowy, albo doctrine ma coś takiego jak Array.
1
komentarz 4 lutego przez `Krzychuu Stary wyjadacz (12,940 p.)
teraz wszystko rozumiem, dziękuje za pomoc i poświęcony czas :)

Podobne pytania

0 głosów
1 odpowiedź 41 wizyt
0 głosów
0 odpowiedzi 37 wizyt
0 głosów
1 odpowiedź 50 wizyt
pytanie zadane 21 maja w PHP, Symfony, Zend przez hiper007 Stary wyjadacz (10,880 p.)
Porady nie od parady
Komentarze do pytań nie służą do odpowiadania, od tego jest wydzielona sekcja odpowiedzi. Funkcją komentarzy jest natomiast możliwość uzyskania dodatkowych informacji na temat samego posta.Komentarze

65,683 zapytań

112,321 odpowiedzi

237,045 komentarzy

46,657 pasjonatów

Przeglądających: 214
Pasjonatów: 7 Gości: 207

Motyw:

Akcja Pajacyk

Pajacyk od wielu lat dożywia dzieci. Pomóż klikając w zielony brzuszek na stronie. Dziękujemy! ♡

Oto dwie polecane książki warte uwagi. Pełną listę znajdziesz tutaj.

...