current_user w modelu

Zależy czy logowanie jest częścią logiki biznesowej, czy nie. Tutaj wygląda na to, że jest.

Chyba dyskusja odbiega nieco od tematu.
Oczywiście, że observer przyszedł mi do głowy jako jedno z pierwszych rozwiązań ale z Rails4 wyrzucono observery (zapewne nie bez powodu) więc to rozwiązanie odrzuciłem.
Co do rozwiązań, kŧorych w Rails nie ma … Jest taka ciekawa pozycja “Antywzorce języka SQL”, jeden z rozdziałów wyraźnie odradza korzystanie z ORM. I dalej, migracje railsowe nie uwzględniają tworzenia kluczy obcych w bazie danych. Czy mam z tego powodu odrzucać rozwiązania zawarte w Rails i szukać czegoś innego. Stanowczo NIE. Tworzę aplikacje w Ruby on Rails i chcę wykorzystywać to co Rails-y oferują. Dlatego odrzucam observery tak samo jak activerecord-deprecated_finders.

Wracając do problemu. Aplikacja tworzona w środowisku Ruby 2.0, Rails 4.0.
Zadaniem jest logowanie aktywności użytkowników i zapisywanie jej w bazie danych (kto, akcja, obiekt, opis z linkiem do “rejestru zmian”).

Dotychczasowe propozycje rozwiązania:

  • Thread.current[:user] - równoznaczne z tworzeniem zmiennej globalnej, problem z wywołaniami spoza aplikacji (np. rake), odświeżanie pomiędzy requestami
  • services - duży nakład pracy
  • nadpisanie metod #save oraz #update, umieszczenie ich w module includowanym do modeli (w Rails 4 można wykorzystać concerns)
  • includowanie do modelu Log helperów i wykorzystanie ich do generownia wpisu - naruszenie zasad MVC, problemy z metodą current_user z Devise

Jeśli macie jeszcze jakieś propozycje oparte na Rails4 to chętnie je poznam. Również uwagi do powyższych. Proszę jednak o skupienie się na rozwiązaniu problemu a nie dyskusjach ideologicznych.

[quote=Tuptus]Chyba dyskusja odbiega nieco od tematu.
Oczywiście, że observer przyszedł mi do głowy jako jedno z pierwszych rozwiązań ale z Rails4 wyrzucono observery (zapewne nie bez powodu) więc to rozwiązanie odrzuciłem.[/quote]
Observery są dostępne jako gem, tutaj: https://github.com/rails/rails-observers i zostały usunięte z railsów głownie z tego powodu, że nie są już używane tak często jak kiedyś.

Osobiście nie lubię observerów, bo w zachowaniu, to właściwie takie callbacki tylko wyjęte z modelu, ale mogą mieć swoje zastosowania - to że zostały usunięte z railsów nie znaczy, że nie należy ich używać.

Pamiętaj, że railsy są “opinionated”, nie trzeba się zgadzać z każdym wyborem twórców frameworka. Moim zdaniem (i wydaje mi się, że zdaniem większości doświadczonych programistów), klucze obce są bardzo przydatne i pomagają uniknąć bajzlu w bazie danych, to, że nie ma ich w railsach to tylko kwestia tego, żę DHH ma niezdrowe (moim zdaniem oczywiście) podejście do baz danych.

Pamiętaj, że railsy są “opinionated”, nie trzeba się zgadzać z każdym wyborem twórców frameworka. Moim zdaniem (i wydaje mi się, że zdaniem większości doświadczonych programistów), klucze obce są bardzo przydatne i pomagają uniknąć bajzlu w bazie danych, to, że nie ma ich w railsach to tylko kwestia tego, żę DHH ma niezdrowe (moim zdaniem oczywiście) podejście do baz danych.[/quote]
Jak najbardziej się z Tobą zgadzam. Gdybym pisał np. w ZF gdzie i tak muszę ręcznie tworzyć bazę danych to zapewne zdefiniowałbym klucze obce. Ale skoro Rails-y tego nie przewidują … Chcę pisać aplikację w zgodzie z duchem tego framework-a. Zresztą problem kluczy obcych jest świetnie opisany w “Rails. Projektowanie systemów klasy enterprise” i możliwe, że wrócę do tego tematu kiedy już będę miał “prototyp” aplikacji.
Ale znowu odchodzimy od tematu wątku :frowning:

Nie zgodzę się zupełnie że Thread.current[:user] jest tworzeniem zmiennej globalnej. Jest to dokładnie tworzenie zmiennej lokalnej dla wątku (threadlocal), i jest jak najbardziej w porządku.
Nawet jeśli traktować to jako w pewnym sensie zmienną globalną, to akurat w tym wypadku taka zmienna jest jak najbardziej porządana.
Problem wywołania z rake nie jest problemem tak naprawdę, a nawet jeśli - to przedstawiłem gotowe i skuteczne rozwiązanie.

Ortodoksje i proste powtarzanie formułek jest najbardziej szkodliwym zachowaniem jakie spotkałem w programowaniu. Jeżeli coś jest skuteczne w danym miejscu, i pomaga osiągnąć cel małym nakładem pracy, należy z tego skorzystać chyba że istnieją istotne powody żeby tego nie robić (opinie że coś jest szkodliwe, bo tak nie jest istotnym powodem).

Potem bedziesz chcial wytestowac taka klase bez loggera i okaze sie ze masz problem :frowning:

Taka klasa zmiesci sie w 30 linijkach wiec o jakim nakladzie pracy mowimy ?

https://gist.github.com/lefty313/fac610d9283409f12ccc (napisane na kolanie w przegladarce)

  1. Mozesz uzywac roznych zrodel danych
  2. Mozesz uzywac roznych loggerow
  3. Mozesz to latwo wytestowac w izolacji

[quote=Świstak]Nie zgodzę się zupełnie że Thread.current[:user] jest tworzeniem zmiennej globalnej. Jest to dokładnie tworzenie zmiennej lokalnej dla wątku (threadlocal), i jest jak najbardziej w porządku.
Nawet jeśli traktować to jako w pewnym sensie zmienną globalną, to akurat w tym wypadku taka zmienna jest jak najbardziej porządana.[/quote]
Fajnie tylko ze mozemy ja wyeleminowac uzywajac podstawowego OOP

Jasne ale po co 20 linijek kodu jak wystarczy 4?

OOP w tym wypadku jest srednio przydatne. Ale jak da sie komus do reki mlotek, to wszedzie bedzie widzial gwozdzie :frowning:

[quote=Świstak]Jasne ale po co 20 linijek kodu jak wystarczy 4?

OOP w tym wypadku jest srednio przydatne. Ale jak da sie komus do reki mlotek, to wszedzie bedzie widzial gwozdzie :([/quote]
Po to, że dzisiaj potrzebujesz używać do danej funkcjonalności (logiki biznesowej, nieważne jak skomplikowanej) dla current usera, ale może się w przyszłości zdarzyć, że jednak chcesz stworzyć coś z perspektywy innego użytkownika.
Przykład: mająć sklep internetowy, chcę, żeby konsultant mógł wykonać daną rzecz za użytkownika.
Powodzenia w odkręcaniu tego.
Kolejna rzecz - przypinanie do wątku. Co się stanie w przypadku, gdy będziesz chciał przerzucić daną rzecz np do delayed joba? Nie zadziała.

[quote]1. Mozesz uzywac roznych zrodel danych
2. Mozesz uzywac roznych loggerow
3. Mozesz to latwo wytestowac w izolacji[/quote]
No przeciez napisalem po co :slight_smile:

no i dodatkowo warstwa logiki biznesowej nic nie wie o ActiveRecord ani SQL ani innych Persistance

http://confreaks.com/videos/1314-rubyconf2012-boundaries

Nie odbierz tego zle ale nie masz racji :frowning:

+1

Wszyscy wiemy ze “zmiana” to jedyne czego mozna byc pewnym w naszej pracy dlatego naprawde dla dobra waszych projektow, kolegow i klientow zawsze warto o tym pamietac

http://david.heinemeierhansson.com/2012/the-parley-letter.html

Taka klasa zmiesci sie w 30 linijkach wiec o jakim nakladzie pracy mowimy ?

https://gist.github.com/lefty313/fac610d9283409f12ccc (napisane na kolanie w przegladarce)

  1. Mozesz uzywac roznych zrodel danych
  2. Mozesz uzywac roznych loggerow
  3. Mozesz to latwo wytestowac w izolacji[/quote]
    Zauważ, że nie podważam samej idei services a jedynie jej pracochłonność.
    Przykład, który pokazałeś jest ciekawy ale wykorzystałeś jedynie Model#create. Teraz dodaj do tego #save i #update i pomnóż przez 6 bo tyle aktualnie mam modeli, które podlegają logowaniu. We wszystkich wypadkach metody będą bardzo podobne, różnice może 10% kodu. Dlatego uważam, że w tym przypadku services to nadmierny nakład pracy.
    Niemniej dziękuję za te uwagi i przy bardziej skomplikowanej aplikacji będę o nich pamiętał.

Wlasnie ci pokazalem ze to nie jest pracochlonne. Pracochlonna jest zmiana implementacji tworzenia rekordow rozrzuconej po calej aplikacji :wink:

Musisz jedynie dodac #update i #destroy

Po to uzywamy OOP aby nie trzeba bylo kopiowac tego samego kodu do wielu plikow. Mozesz lekko zmodyfikowac kod aby spelnial twoje nowe zalozenie bez zmiany api

https://gist.github.com/lefty313/32f7e6763922261fdb02

mozna pojsc nawet dalej i wykorzystac metaprogramming aby domyslnie klasy dostawaly odpowiednia fabryke :slight_smile:

Doszlismy do momentu kiedy klasa domenowa ktora pozwala nam tworzyc rekordy zajmuje 3 linijki

@lewy nie odbieram tego źle, prowadzimy dyskusję. Oczywiście się z tobą nie zgadzam, niestety rozmawiamy o rzeczach gdzie cięzko przytoczyć “twarde” argumenty, typu dane statystyczne.
Niestety to co ty wyprawiasz to jest nazywane lekko z angielska “overengineering” i jest złem porównywalnym tylko i jedynie z przedwczesną optymalizacją.
A ile to przestępstw przeciw kodowi powstało w związku z “testowalnością” i rozdzielnością zobowiązań, to już nawet nie zliczę. Pięknie to ilustruje http://alarmingdevelopment.org/?p=805

PS. Generalnie z mojego doświadczenia:
Prostszy kod = łatwiejszy do zrozumienia kod = łatwiejszy w refaktoringu = łatwiejszy w utrzymaniu.
Każda kolejna warstwa abstrakcji ( w tym wypadku warstwa serwisu) = Bardziej skomplikowany kod = trudniej zrozumieć = ciężej utrzymywać.

PPS. OOP to nie jest prawda objawiona i bywają też inne paradygmaty programowania :slight_smile:

PPPS. Jest co prawda dosyć późno, ale próbując uprościć twój przykład x.rb doszedłem do wniosku że za bardzo nie wiem co twój kod robi, co też wiele mówi. Na przykład nie mam pojęcia po kiego grzyba jest ta cała ActiveRecordFactory, ale wskazuje że kiedyś chyba programowałeś w Javie, za co współczuję.

P4S. Im dłużej o tym myślę tym bardziej twój kod odpowiada mi logicznie:

class Article < AR:Base
scope :with_creator, lambda{|a| where(creator: a) }

def after_create
logger.info(“article #{id} was created by #{creator}”)
end
end

Article.where(params).with_creator(x).create

Daje ± ten sam efekt. 7 linijek kodu vs prawie 70. 10 razy prostsze?

Sorry ale ja uzylem tylko depedency injection i delegation

Oczywiscie ze nie poniewaz ty na tym etapie stwierdziles ze tworzac artykul zawsze bedziesz logowal identyczna wiadomosc.

Dodatkowo wprowadziles powiazanie Article z #creator mimo ze jednym z wymagan jest

[quote=Tuptus]Oczywiście mogę napisać taką metodę zakładając, że obiekt jest w jakiś sposób powiązany z jego twórcą np. Article posiada atrybut user_id.
Zresztą to i tak nie rozwiązuje problemu bo przecież artykuł może być aktualizowany nie tylko przez jego twórcę (np. “moderator” poprawia “redaktora”) i taką aktualizację również muszę “zalogować”.[/quote]

To zaden argument ale moglbys trzymac sie faktow i naprawde policzyc ile linijek wykorzystalem do zaimplentowania tej funkcjonalnosci :wink:

Tak tylko ze ty nie proponujesz wykorzystania innego paradygmatu tylko forsujesz programowanie przez koincydencje

To tylko przyklad napisany na potrzeby tematu ale moze najpierw przejrzyj linki ktore wrzucilem w poprzednim poscie i wtedy bedziemy mogli kontynuowac nasza merytoryczna dyskusje

Stworzylem prosty gist abys mogl latwo zrozumiec za co odpowiada konkretna warstwa. Wyrzucilem modul i zostalem przy zwyklych klasach

https://gist.github.com/lefty313/0f0d7d4a72ec9297646d

mam nadzieje ze teraz widzisz ze rozmawiamy tylko i wylacznie o Dependency Injection :slight_smile:

Klasa Persistance nie wie gdzie i jak jest zapisywany dany rekord jest jedynie interfejsem pomiedzy domena a czyms co przechowuje dane

ActiveRecordFactory -> zawiera logike dodawania rekordu do relacyjnej bazy danych
InMemoryFactory -> zawiera logike dodawania rekordu do zwyklej tablicy

dzieki temu moge uzywac jednego i drugiego w zaleznosci od sytuacji

Teraz mozemy teoretyczne zastanowic sie ile pracy wymaga wprowadzenie ponizszych zmian do mojego i twojego kodu

  1. Chce moc tworzyc artykul z pliku CSV
  2. W trakcie tworzenia artykulu z pliku CSV chce to odpowiednio zalogowac

Jezeli mamy dalej dyskutowac na ten temat prosze abys pokazal jak zaimplementowalbys wyzej wymienione funkcjonalnosci

ps: Co do Javy i wykorzystawania OOP polecam http://www.amazon.com/Practical-Object-Oriented-Design-Ruby-Addison-Wesley/dp/0321721330

ps2: http://andrzejonsoftware.blogspot.com/2013/06/activerecord-overdose.html

O, argument z Javy w dyskusji o Ruby. Skoro już jedziesz w tę stronę to proponuję na całość:

[quote]O, argument z Javy w dyskusji o Ruby. Skoro już jedziesz w tę stronę to proponuję na całość:
http://it.slashdot.org/comments.pl?sid= … d=15916702[/quote]
? w tej ksiazce nie ma nic o javie

Nawiązywałem do całego podwątku o Javie.
Serio, nie idźcie w tę stronę, w Javie nie sposób odróżnić absurdu umyślnego od przypadkowego :wink:

[quote]Nawiązywałem do całego podwątku o Javie.
Serio, nie idźcie w tę stronę, w Javie nie sposób odróżnić absurdu umyślnego od przypadkowego wink[/quote]
Jedyne co bylo o Javie to wycieczka osobista w stosunku do mnie napisana przez Swistaka :wink:

dlatego Tomku byloby swietnie gdybys odniosl sie do dyskusji a nie tylko do jednego rzeczownika. Masz bardzo duze doswiadczenie jako programista i pracodawca dlatego napewno wniesiesz do niej wartosc dodatnia :slight_smile:

Dzięki za miłe słowa, ale nie potrafię się dołożyć do dyskusji bardziej niż banalnym “to zależy”. Bo na początek i do prostych rzeczy callbacki czy observery mają naprawdę najlepszy stosunek efektu do nakładu pracy, a do tego nie komplikują testowania. Natomiast przy bardziej złożonych rzeczach wolałbym już mieć osobną klasę i luźniejsze parowanie. Najchętniej na tyle luźne żeby zapisywanie się na zdarzenia i ich chwytanie było explicite, jak pozwala to robić jeden z wielu genialnych gemów Nicka Sutterera:

Uważam że bardzo cenną cechą programisty Rails jest nie tyle umiejętność używania obserwerów czy bardziej złożonych rozwiązań – ale umiejętność oceny momentu w którym od prostego, dostarczonego przez Rails rozwiązania lepszym (albo gorszym) będzie rozwiązanie bardziej złożone.
Spór “observers vs. services” w oderwaniu od konkretnych sytuacji jest sporem nie tylko z dupy, ale wręcz szkodliwym, bo czytający go mniej zaawansowani programiści mogą nabrać przekonania że istnieją rozwiązania obiektywnie lepsze lub gorsze, niezależnie od kontekstu.

A potem dostaję projekt napisany przez developera tak bardzo zachłyśniętego ideologią abstrahowania i separacji że nawet formularz do logowania (dwa pola, znikoma logika biznesowa) jest zrobiony Form Objectem…

+1 z tym ze bardzo nie lubie kiedy w aplikacji obiekty ActiveRecord pojawiaja sie w kazdym mozliwym miejscu

Kiedys mialem bardzo duze opory aby przestac uzywac DHH Rails Way. Kiedy jednak zaczalem pracowac nad roznymi projektami dla roznych klientow zawsze przychodzil moment kiedy dostalem zadanie ktorego implementacja byla bardzo trudna poniewaz nie wprowadzilem odpowiedniej abstrakcji

np:

Prosze dodac unikalny numer identyfikacyjny dla kazdego rekordu ktory nastepnie bedzie uzywany do stworzenia url

Gdybym mial warstwe ktora jest odpowiedzialna za szukanie rekordow moglbym to bardzo prosto zaimplementowac jednak poniewaz w roznych miejscach w aplikacji uzywalem

Model.find(:id)

musialem zmienic bardzo wiele roznych metod aby spelnic wymagania klienta. Od tamtej pory staram sie zawsze miec warstwe posrednia miedzy biblotekami a moja domena

:frowning:

Tak jak mozna przesadzic z Rails Way tak samo mozna przesadzic z OOP way

ps: Moze jest to dobry temat abysmy porozmawiali o nim na DrugCamp ?