"Curiosa" interacción entre permalink _fu y los campos tipo datetime


#1

Buenas.
He observado en mi aplicación que aunque hay modelos que si intento
acceder
a un atributo de tipo datetime me devuelven la fecha local, tal y como
espero:

Comment.find(:last).created_at
=> Tue, 21 Apr 2009 10:17:40 CEST +02:00

Hay también algunos modelos que no, y siempre me devuelven la fecha en
formato UTC:

Article.find(:last).created_at
=> Tue Apr 21 08:19:08 UTC 2009

En concreto, los modelos que funcionan de forma correcta han encapsulado
correctamente las fechas en un ActiveSupport::TimeWithZone, pero las que
no
siempre devuelven un Time:

Comment.find(:last).created_at.class
=> ActiveSupport::TimeWithZone

Article.find(:last).created_at.class
=> Time

Después de ir línea a línea del modelo para averiguar qué definición
podría
ser la que está causando el que Rails no llegase a asignar el
TimeWithZone
observé que TimeWithZone no se asignaba cuando estaba activa la
siguiente
línea:

has_permalink :title, :scope => :id

En la aplicación estoy utilizando un fork de permalink_fu, en concreto
este:

Por más vueltas que le he dado al permalink_fu.rb, no consigo ver en qué
momento se produce lo que impide que Rails cambie el campo a
TimeWithZone y
lo deje con la definición por defecto de Time. Por si no usais
permalink_fu
y queréis ver el código directamente desde permalink_fu

Cualquier ayuda o consejo será más que bien recibido :slight_smile:

Salu2


#2

El 21 de abril de 2009 12:54, Daniel R. Troitiño
removed_email_address@domain.invalidescribió:

Como prueba puedes ver que resultado obtienes de
Article.time_zone_aware_attributes y
Article.skip_time_zone_conversion_for_attributes (supongo que el
column.type sí está correcto).

Article.time_zone_aware_attributes
=> true

Article.skip_time_zone_conversion_for_attributes
=> []

Salu2


#3

2009/4/21 Luis M. removed_email_address@domain.invalid:

En concreto, los modelos que funcionan de forma correcta han encapsulado
has_permalink :title, :scope => :id
Cualquier ayuda o consejo será más que bien recibido :slight_smile:

Siento no poder darte una solución, realmente es una interacción
“curiosa”.

Rails para determinar si debe o no crear un atributo con TimeWithZone
utiliza el método create_time_zone_conversion_attribute?(name, column)
(linea #141 de attribute_methods.rb en ActiveRecord 2.3.2):

def create_time_zone_conversion_attribute?(name, column)
time_zone_aware_attributes
&& !skip_time_zone_conversion_for_attributes.include?(name.to_sym)
&& [:datetime, :timestamp].include?(column.type)
end

Como prueba puedes ver que resultado obtienes de
Article.time_zone_aware_attributes y
Article.skip_time_zone_conversion_for_attributes (supongo que el
column.type sí está correcto).

Suerte.


#4

Buenas, pues sí que era curioso.

Rails tiene una lista de métodos de atributos generados
(generated_methods), y si no está vacia no genera ninguno más. De
hecho los genera todos de una tacada recorriendo el array de columnas
en el método define_attribute_methods.

El problema es que permalink_fu hace uso de evaluate_attribute_method,
que actualiza generated_methods, con lo que impide que Rails genere
sus métodos cuando se hace la primera llamada a method_missing.

Mi solución pasa por invocar al método que genera los atributos de
Rails antes de invocar a evaluate_attribute_method.

—start—
diff --git a/vendor/plugins/permalink_fu/lib/permalink_fu.rb
b/vendor/plugins/pe
index 385824b…a163b0a 100644
— a/vendor/plugins/permalink_fu/lib/permalink_fu.rb
+++ b/vendor/plugins/permalink_fu/lib/permalink_fu.rb
@@ -64,6 +64,7 @@ module PermalinkFu
end

   before_validation :create_unique_permalink
  •  define_attribute_methods
     evaluate_attribute_method permalink_field, "def 
    

#{self.permalink_field}=(
extend PermalinkFinders

—end—

Creo que no debería romper nada.

Por cierto, no se si el fallo es de Permalink_fu o de Rails, aunque
creo que voto por este último: es extraño tener una lista de métodos
generados cuando se supone que se generan todos a la vez, y comprobar
si la lista está vacía para saber si los métodos tienen que ser
generados (sobre todo teniendo en cuenta que el programador puede
modificar la lista como le apetezca). A mi parecer la comprobación
debería ser atributo a atributo, pero quizá sea una optimización que
les ha salido un poco rara.

Suerte.


#5

El 22 de abril de 2009 0:42, Daniel R. Troitiño
removed_email_address@domain.invalidescribió:

Por cierto, no se si el fallo es de Permalink_fu o de Rails, aunque
creo que voto por este último: es extraño tener una lista de métodos
generados cuando se supone que se generan todos a la vez, y comprobar
si la lista está vacía para saber si los métodos tienen que ser
generados (sobre todo teniendo en cuenta que el programador puede
modificar la lista como le apetezca). A mi parecer la comprobación
debería ser atributo a atributo, pero quizá sea una optimización que
les ha salido un poco rara.

Parece la clave de todo esto la da la definición de method_missing un
poco
más abajo en el fichero:

  # If we haven't generated any methods yet, generate them, then
  # see if we've created the method we're looking for.
  if !self.class.generated_methods?
    self.class.define_attribute_methods
    if self.class.generated_methods.include?(method_name)
      return self.send(method_id, *args, &block)
    end
  end

El problema como bien comentabas es que generated_methods? es tal que:

  def generated_methods?
    !generated_methods.empty?
  end

Con lo cual teniendo en cuenta que en ese punto generated_methods ya no
es
vacío sino que tiene un elemento añadido por permalink_fu,
method_missing no
llama a define_attribute_methods.

El caso es que con todo esto me ha picado la curiosidad y me ha dado por
mirar cómo resolvieron este tema la gente de paperclip, que al fin y al
cabo
también añaden métodos al modelo en el que se usa (líneas 161 a 172 en
la
versión 2.1.5):

  define_method name do |*args|
    a = attachment_for(name)
    (args.length > 0) ? a.to_s(args.first) : a
  end

  define_method "#{name}=" do |file|
    attachment_for(name).assign(file)
  end

  define_method "#{name}?" do
    attachment_for(name).file?
  end

Es decir, prescinden del uso de métodos privados de ActiveRecord y
directamente utilizan el define_method de Ruby. El caso es que viendo la
definición de evaluate_attribute_method lo único interesante que aporta
frente al define_method es que si se encuentra con un SyntaxError
continúa
con la ejecución de la aplicación, aunque eso no lo veo como una ventaja
:confused:

El caso es que me gusta más el enfoque de paperclip, principalmente por
el
peligro que veo en basarte en métodos privados de una librería que no
sabes
como cambiará en el futuro. En cambio, el define_method sabes que
funcionará
siempre, pasen los años que pasen (a no ser que directamente cambien
Ruby).

Ahora mis dudas son… ¿mando al autor del fork de permalink_fu que uso
un
parche implementando esa parte del código con define_method? ¿creo mi
propio
fork en github con la nueva implementación? ¿abro ticket en el
lighthouse de
rails avisando de lo que puede que le ocurra a más gente que use plugins
que
utilicen evaluate_attribute_method?

Es lo que tiene la emoción del poder aportar tu granito de arena, que no
sabes por donde empezar xD

Salu2


#6

2009/4/22 Luis M. removed_email_address@domain.invalid:

debería ser atributo a atributo, pero quizá sea una optimización que
      return self.send(method_id, *args, &block)
     end
    end

El problema como bien comentabas es que generated_methods? es tal que:
    def generated_methods?
     !generated_methods.empty?
    end
Con lo cual teniendo en cuenta que en ese punto generated_methods ya no es
vacío sino que tiene un elemento añadido por permalink_fu, method_missing no
llama a define_attribute_methods.

Exactamente, esas son todas las razones y causas del fallo.

    end
como cambiará en el futuro. En cambio, el define_method sabes que funcionará
siempre, pasen los años que pasen (a no ser que directamente cambien Ruby).

Verdaderamente mejor solución, porque durante la ejecución de
define_attribute_methods (que sería posterior a has_permalink) tienen
cuidado de no sobreescribir los métodos ya existente. Por cierto, la
ventaja de continuar con la ejecución mostrando el warning es (como
ellos indican) si el nombre no es un identificador válido en Ruby.
Quizá lo mejor sería mezclar ambas soluciones, y tener una copia
privada de evaluate_attribute_method que lo único que no haga es
rellenar el generated_methods.

Ahora mis dudas son… ¿mando al autor del fork de permalink_fu que uso un
parche implementando esa parte del código con define_method? ¿creo mi propio
fork en github con la nueva implementación? ¿abro ticket en el lighthouse de
rails avisando de lo que puede que le ocurra a más gente que use plugins que
utilicen evaluate_attribute_method?

Sobre lo primero, deberías avisar al autor del fallo en su
implementación, sobre todo porque parece que el permalink_fu original
de technoweenie http://github.com/technoweenie/permalink_fu no
contiene ese fallo (tras una revisión rápida del código). Obviamente
si te gusta la versión que estás utilizando por algo en especial,
hacer un fork de technoweenie y ir “mergeando” cambios de otras
personas sería una gran idea para tener tu permalink_fu personalizado.

Sobre lo de avisar al grupo de Rails, habría que estudiar la historia
de código de ese archivo. La versión más vieja de Rails que tengo
instalada es la 2.0.2 y contiene el mismo código que hemos estado
comentando, pero desde luego es incorrecto en algunas ocasiones (como
hemos demostrado). Si abres un ticket en lighthouse recuerda
justificarlo bien y mostrar que no está directamente relacionado con
permalink_fu, sino que podría suceder en otros casos (mucho más
sencillos porque generated_methods es público y por lo tanto
modificable). Si te decides a abrir un ticket en lighthouse pasa el
enlace, quizá con un poco de apoyo tenga algo más de visibilidad, pero
yo no tendría muchas esperanzas de que modificasen el comportamiento.


#7

El 22 de abril de 2009 10:47, Daniel R. Troitiño
removed_email_address@domain.invalidescribió:

Sobre lo primero, deberías avisar al autor del fallo en su
implementación, sobre todo porque parece que el permalink_fu original
de technoweenie http://github.com/technoweenie/permalink_fu no
contiene ese fallo (tras una revisión rápida del código). Obviamente
si te gusta la versión que estás utilizando por algo en especial,
hacer un fork de technoweenie y ir “mergeando” cambios de otras
personas sería una gran idea para tener tu permalink_fu personalizado.

En su moment me decidí por el fork de djanowski por el tema de tratar
mejor
las cadenas con caracteres no ingleses y no depender tanto de iconv. He
estado probando el permalink_fu actual y después de ver que hace todo lo
que
necesitaba (los scopes de los permalinks y cosas de ese estilo) he hecho
un
fork y he adaptado la generación de permalinks de djanowski al
permalink_fu
original. El resultado lo tenéis en:

http://github.com/mayoral/permalink_fu

Iré manteniéndolo actualizado con el permalink_fu original.

Salu2