Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

problemy z systemem pluginów #1243

Open
maciejlew opened this issue Apr 16, 2018 · 12 comments
Open

problemy z systemem pluginów #1243

maciejlew opened this issue Apr 16, 2018 · 12 comments

Comments

@maciejlew
Copy link
Contributor

@chilek tu możemy omówić problemy z systemem pluginów

@chilek
Copy link
Owner

chilek commented Apr 16, 2018

  1. Biblioteki porzucone/osierocone:
    phine/exception 1.0.0 A PHP library for improving the use of exceptions.
    phine/observer 2.0.0 A PHP library that implements the observer pattern.
  2. phone/observer nie pozwala na rekursywne wywoływanie ExecuteHook().

@maciejlew
Copy link
Contributor Author

ad 1. tak mogło się stać z każdą zależnością, jeśli nie ma w niej bugów to nie jest źle;
ad 2. to akurat dobrze, plugin mógłby zapetlić wykonywanie hooków, przydałby się jakiś przykład kiedy taka funkcjonalność jest niezbędna.

Raczj bym tego nie ruszał.

@chilek
Copy link
Owner

chilek commented Apr 18, 2018

ad 2. to akurat dobrze, plugin mógłby zapetlić wykonywanie hooków, przydałby się jakiś przykład kiedy taka funkcjonalność jest niezbędna.

  1. Podpinamy się pod:
    https://github.com/lmsgit/lms/blob/master/lib/LMSManagers/LMSCashManager.php#L405
  2. W obsłudze 1 robimy powiadomienie mailowe w oparciu o LMS::SendMail().
  3. W SendMail() mamy:
    https://github.com/lmsgit/lms/blob/master/lib/LMS.class.php#L2015
    (chcemy mieć możliwość wpłynięcia na treść listu przed wysyłką każdego listu).

@maciejlew
Copy link
Contributor Author

Dzięki za przykład. Nie rozumiem jednak dlaczego SendMail nie mógłby dostać wszystkiego co powinien już podczas wywoływania tej metody. Mam wrażenie że hooki trochę wymknęły się spod kontroli i mogą być wszędzie...

Pewnie ten hook będzie musiał tam już zostać na jakiś czas bo ktoś z tego korzysta, ale trzeba by przemyśleć czy pozwalać w przyszłości na takie coś.

Z perspektywy czasu widzę, że lepiej by było jakby hooki/eventy (czy jakkolwiek to nazwiemy) nie wpływały na wykonanie standardowej ścieżki programu tylko powodowały rozpoczęcie kolejnej po tym jak ta pierwsza się już zakończy. Wygląda to na sporo roboty ale może uda mi się coś takiego przygotować.

@chilek
Copy link
Owner

chilek commented Apr 19, 2018

Dzięki za przykład. Nie rozumiem jednak dlaczego SendMail nie mógłby dostać wszystkiego co powinien już podczas wywoływania tej metody. Mam wrażenie że hooki trochę wymknęły się spod kontroli i mogą być wszędzie...

Stare hooki (nie wiem czy z tego kiedykolwiek korzystałeś) takich ograniczeń nie miały.

@interduo
Copy link
Collaborator

interduo commented Apr 25, 2020

@maciejlew dorzucę jedną rzecz do tematu który jest ogólny:
3. krytyczny błąd w kodzie wtyczki jest w stanie unieruchomić całego LMS'a np. die(); w kodzie wtyczki. Zamiast zakończyć kod wtyczki sprawia że "website is down"

@ZdanowskiS
Copy link
Contributor

2\. phone/observer nie pozwala na rekursywne wywoływanie ExecuteHook().

Próba wywołania NotifyUsers w nowej wtyczce powoduje błąd phone/observer w funkcji SendSMS.
Możliwości ominięcia tego jest kilka.
Być może dobre było by dodanie do klasy LMS dodatkowej zmiennej i ustawienie ExecuteHook wewnątrz warunku if.
Wówczas we wtyczkach można by wyłączać hooki kiedy powodują problem. Dalej niemożliwa będzie interakcja między wtyczkami ale możliwe będzie używanie bazowych funkcji LMS bez robienia kopii funkcji.

@chilek
Copy link
Owner

chilek commented Feb 21, 2021

W pliku vendor/phine/observer/src/lib/Phine/Observer/Subject.php
wystarczy zakomentować fragment:

    public function notifyObservers()
    {
        if ($this->updating) {
            throw SubjectException::alreadyUpdating();
        }

        $this->resetInterrupt();

(blok z if ($this->updating).
Kod phine już od wielu lat nie jest od kilku lat utrzymywany przez autora i stanowi niezły balast.

@ZdanowskiS
Copy link
Contributor

Zrobiłem to przez dziedziczenie i własną wersję funkcji.(Wiem że przy aktualizacjach LMS może się to zemścić.)
Konieczność modyfikacji żeby skorzystać z wtyczki, raczej kiepska propozycja.
Będzie trzeba się przyglądnąć Phine w takim razie.

@chilek
Copy link
Owner

chilek commented Feb 21, 2021

Będzie trzeba się przyglądnąć Phine w takim razie.

W jakim sensie? Moim zdaniem to już trup ;-)

@ZdanowskiS
Copy link
Contributor

Czy warto go ożywić?

@interduo
Copy link
Collaborator

@maciejlew jest jeszcze jedna drobna rzecz - brakuje w systemie pluginów miejsca gdzie można zdefiniować:

  • kod wykonywany tylko raz przy instalacji wtyczki,
  • kod wykonywany przy kliknięciu przycisku wyłącz,
  • kod wykonywany przy kliknięciu przycisku włącz,
  • kod uruchamiany przy permanentnym odinstalowaniu wtyczki,

Obecnie to jest rozsiane po instrukcjach, oddzielnych plikach SQL lub w ogóle nie ma. Może to nie stanowi, żadnej przeszkody w kodzie ale każda wtyczka nie była robiona na swoją modłę to: "fajnie by było gdyby to było".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants