From 481209218c9021cfa62e9a2d6f14e2dc4eb49096 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Sun, 24 Aug 2014 10:04:20 +0200 Subject: [PATCH] Moved type lookup to generic model. --- app/models/activity_stream.rb | 43 ++-------- app/models/object_lookup.rb | 24 +++--- app/models/type_lookup.rb | 38 +++++++++ db/migrate/20120101000001_create_base.rb | 9 +- ...date_recent_viewed_create_object_lookup.rb | 2 +- ...date_activity_stream_create_type_lookup.rb | 24 ++++++ test/unit/object_lookup_test.rb | 34 -------- test/unit/object_type_lookup_test.rb | 85 +++++++++++++++++++ 8 files changed, 173 insertions(+), 86 deletions(-) create mode 100644 app/models/type_lookup.rb create mode 100644 db/migrate/20140824000001_update_activity_stream_create_type_lookup.rb delete mode 100644 test/unit/object_lookup_test.rb create mode 100644 test/unit/object_type_lookup_test.rb diff --git a/app/models/activity_stream.rb b/app/models/activity_stream.rb index 9c13d562f..5468724fe 100644 --- a/app/models/activity_stream.rb +++ b/app/models/activity_stream.rb @@ -2,11 +2,9 @@ class ActivityStream < ApplicationModel self.table_name = 'activity_streams' - belongs_to :activity_stream_type, :class_name => 'ActivityStream::Type' + belongs_to :activity_stream_type, :class_name => 'TypeLookup' belongs_to :activity_stream_object, :class_name => 'ObjectLookup' - @@cache_type = {} - =begin add a new activity entry for an object @@ -26,7 +24,7 @@ add a new activity entry for an object # lookups if data[:type] - type = self.type_lookup( data[:type] ) + type_id = TypeLookup.by_name( data[:type] ) end if data[:object] object_id = ObjectLookup.by_name( data[:object] ) @@ -44,7 +42,7 @@ add a new activity entry for an object # check newest entry - is needed result = ActivityStream.where( :o_id => data[:o_id], - # :activity_stream_type_id => type.id, + # :activity_stream_type_id => type_id, :role_id => role_id, :activity_stream_object_id => object_id, :created_by_id => data[:created_by_id] @@ -56,7 +54,7 @@ add a new activity entry for an object # create history record = { :o_id => data[:o_id], - :activity_stream_type_id => type.id, + :activity_stream_type_id => type_id, :activity_stream_object_id => object_id, :role_id => role_id, :group_id => data[:group_id], @@ -112,7 +110,7 @@ return all activity entries of an user stream.each do |item| data = item.attributes data['object'] = ObjectLookup.by_id( data['activity_stream_object_id'] ) - data['type'] = self.type_lookup_id( data['activity_stream_type_id'] ).name + data['type'] = TypeLookup.by_id( data['activity_stream_type_id'] ) data.delete('activity_stream_object_id') data.delete('activity_stream_type_id') list.push data @@ -122,37 +120,6 @@ return all activity entries of an user private - def self.type_lookup_id( id ) - - # use cache - return @@cache_type[ id ] if @@cache_type[ id ] - - # lookup - type = ActivityStream::Type.lookup( :id => id ) - @@cache_type[ id ] = type - type - end - - def self.type_lookup( name ) - - # use cache - return @@cache_type[ name ] if @@cache_type[ name ] - - # lookup - type = ActivityStream::Type.lookup( :name => name ) - if type - @@cache_type[ name ] = type - return type - end - - # create - type = ActivityStream::Type.create( - :name => name - ) - @@cache_type[ name ] = type - type - end - class Object < ApplicationModel end diff --git a/app/models/object_lookup.rb b/app/models/object_lookup.rb index f32f477a4..456b6100c 100644 --- a/app/models/object_lookup.rb +++ b/app/models/object_lookup.rb @@ -9,10 +9,10 @@ class ObjectLookup < ApplicationModel return @@cache_object[ id ] if @@cache_object[ id ] # lookup - object_lookup = ObjectLookup.lookup( :id => id ) - return if !object_lookup - @@cache_object[ id ] = object_lookup.name - object_lookup.name + lookup = self.lookup( :id => id ) + return if !lookup + @@cache_object[ id ] = lookup.name + lookup.name end def self.by_name( name ) @@ -21,18 +21,18 @@ class ObjectLookup < ApplicationModel return @@cache_object[ name ] if @@cache_object[ name ] # lookup - object_lookup = ObjectLookup.lookup( :name => name ) - if object_lookup - @@cache_object[ name ] = object_lookup.id - return object_lookup.id + lookup = self.lookup( :name => name ) + if lookup + @@cache_object[ name ] = lookup.id + return lookup.id end # create - object_lookup = ObjectLookup.create( + lookup = self.create( :name => name ) - @@cache_object[ name ] = object_lookup.id - object_lookup.id + @@cache_object[ name ] = lookup.id + lookup.id end -end +end \ No newline at end of file diff --git a/app/models/type_lookup.rb b/app/models/type_lookup.rb new file mode 100644 index 000000000..05f9a6da6 --- /dev/null +++ b/app/models/type_lookup.rb @@ -0,0 +1,38 @@ +# Copyright (C) 2012-2014 Zammad Foundation, http://zammad-foundation.org/ + +class TypeLookup < ApplicationModel + @@cache_object = {} + + def self.by_id( id ) + + # use cache + return @@cache_object[ id ] if @@cache_object[ id ] + + # lookup + lookup = self.lookup( :id => id ) + return if !lookup + @@cache_object[ id ] = lookup.name + lookup.name + end + + def self.by_name( name ) + + # use cache + return @@cache_object[ name ] if @@cache_object[ name ] + + # lookup + lookup = self.lookup( :name => name ) + if lookup + @@cache_object[ name ] = lookup.id + return lookup.id + end + + # create + lookup = self.create( + :name => name + ) + @@cache_object[ name ] = lookup.id + lookup.id + end + +end \ No newline at end of file diff --git a/db/migrate/20120101000001_create_base.rb b/db/migrate/20120101000001_create_base.rb index 2343ec5c2..003b568ea 100644 --- a/db/migrate/20120101000001_create_base.rb +++ b/db/migrate/20120101000001_create_base.rb @@ -1,6 +1,6 @@ class CreateBase < ActiveRecord::Migration def up - + create_table :sessions do |t| t.string :session_id, :null => false t.text :data @@ -149,6 +149,13 @@ class CreateBase < ActiveRecord::Migration t.timestamps end add_index :object_lookups, [:name], :unique => true + + create_table :type_lookups do |t| + t.column :name, :string, :limit => 250, :null => false + t.timestamps + end + add_index :type_lookups, [:name], :unique => true + end end diff --git a/db/migrate/20140823000001_update_recent_viewed_create_object_lookup.rb b/db/migrate/20140823000001_update_recent_viewed_create_object_lookup.rb index 3e40cb87e..85a721de8 100644 --- a/db/migrate/20140823000001_update_recent_viewed_create_object_lookup.rb +++ b/db/migrate/20140823000001_update_recent_viewed_create_object_lookup.rb @@ -21,4 +21,4 @@ class UpdateRecentViewedCreateObjectLookup < ActiveRecord::Migration def down end -end +end \ No newline at end of file diff --git a/db/migrate/20140824000001_update_activity_stream_create_type_lookup.rb b/db/migrate/20140824000001_update_activity_stream_create_type_lookup.rb new file mode 100644 index 000000000..415c9c487 --- /dev/null +++ b/db/migrate/20140824000001_update_activity_stream_create_type_lookup.rb @@ -0,0 +1,24 @@ +class UpdateActivityStreamCreateTypeLookup < ActiveRecord::Migration + def up + + if !ActiveRecord::Base.connection.table_exists? 'type_lookups' + create_table :type_lookups do |t| + t.column :name, :string, :limit => 250, :null => false + t.timestamps + end + add_index :type_lookups, [:name], :unique => true + end + ActivityStream.all.each {|entry| + ro = ActivityStream::Type.find(entry.activity_stream_type_id) + lookup_id = TypeLookup.by_name( ro.name ) + entry.update_attribute( :activity_stream_type_id, lookup_id ) + entry.cache_delete + } + + drop_table :activity_stream_types + Cache.clear + end + + def down + end +end diff --git a/test/unit/object_lookup_test.rb b/test/unit/object_lookup_test.rb deleted file mode 100644 index 312a69561..000000000 --- a/test/unit/object_lookup_test.rb +++ /dev/null @@ -1,34 +0,0 @@ -# encoding: utf-8 -require 'test_helper' - -class ObjectLookupTest < ActiveSupport::TestCase - - test 'simple tests' do - - object_lookup_id = ObjectLookup.by_name( 'SomeObject' ) - assert( object_lookup_id, 'first by_name' ) - - object_lookup_name = ObjectLookup.by_id( object_lookup_id ) - assert( object_lookup_name, 'first by_id' ) - assert_equal( object_lookup_name, 'SomeObject' ) - - - object_lookup_id2 = ObjectLookup.by_name( 'Some_Object' ) - assert( object_lookup_id2, 'by_name - Some_Object' ) - - object_lookup_name2 = ObjectLookup.by_id( object_lookup_id2 ) - assert( object_lookup_name2, 'by_id - Some_Object' ) - assert_equal( object_lookup_name2, 'Some_Object' ) - - - object_lookup_id3 = ObjectLookup.by_name( 'SomeObject' ) - assert( object_lookup_id3, 'by_name 2 - SomeObject' ) - - object_lookup_name3 = ObjectLookup.by_id( object_lookup_id3 ) - assert( object_lookup_name3, 'by_id 2 - SomeObject' ) - assert_equal( object_lookup_name3, 'SomeObject' ) - assert_equal( object_lookup_name3, object_lookup_name, 'SomeObject' ) - assert_equal( object_lookup_id3, object_lookup_id, 'SomeObject' ) - - end -end diff --git a/test/unit/object_type_lookup_test.rb b/test/unit/object_type_lookup_test.rb new file mode 100644 index 000000000..2f8ddf948 --- /dev/null +++ b/test/unit/object_type_lookup_test.rb @@ -0,0 +1,85 @@ +# encoding: utf-8 +require 'test_helper' + +class ObjectTypeLookupTest < ActiveSupport::TestCase + + test 'object tests' do + + object_lookup_id = ObjectLookup.by_name( 'SomeObject' ) + assert( object_lookup_id, 'first by_name' ) + + object_lookup_name = ObjectLookup.by_id( object_lookup_id ) + assert( object_lookup_name, 'first by_id' ) + assert_equal( object_lookup_name, 'SomeObject' ) + + + object_lookup_id2 = ObjectLookup.by_name( 'Some_Object' ) + assert( object_lookup_id2, 'by_name - Some_Object' ) + + object_lookup_name2 = ObjectLookup.by_id( object_lookup_id2 ) + assert( object_lookup_name2, 'by_id - Some_Object' ) + assert_equal( object_lookup_name2, 'Some_Object' ) + + + object_lookup_id3 = ObjectLookup.by_name( 'SomeObject' ) + assert( object_lookup_id3, 'by_name 2 - SomeObject' ) + + object_lookup_name3 = ObjectLookup.by_id( object_lookup_id3 ) + assert( object_lookup_name3, 'by_id 2 - SomeObject' ) + assert_equal( object_lookup_name3, 'SomeObject' ) + assert_equal( object_lookup_name3, object_lookup_name, 'SomeObject' ) + assert_equal( object_lookup_id3, object_lookup_id, 'SomeObject' ) + + end + + test 'type tests' do + + type_lookup_id = TypeLookup.by_name( 'SomeType' ) + assert( type_lookup_id, 'first by_name' ) + + type_lookup_name = TypeLookup.by_id( type_lookup_id ) + assert( type_lookup_name, 'first by_id' ) + assert_equal( type_lookup_name, 'SomeType' ) + + + type_lookup_id2 = TypeLookup.by_name( 'Some_Type' ) + assert( type_lookup_id2, 'by_name - Some_Type' ) + + type_lookup_name2 = TypeLookup.by_id( type_lookup_id2 ) + assert( type_lookup_name2, 'by_id - Some_Type' ) + assert_equal( type_lookup_name2, 'Some_Type' ) + + + type_lookup_id3 = TypeLookup.by_name( 'SomeType' ) + assert( type_lookup_id3, 'by_name 2 - SomeType' ) + + type_lookup_name3 = TypeLookup.by_id( type_lookup_id3 ) + assert( type_lookup_name3, 'by_id 2 - SomeType' ) + assert_equal( type_lookup_name3, 'SomeType' ) + assert_equal( type_lookup_name3, type_lookup_name, 'SomeType' ) + assert_equal( type_lookup_id3, type_lookup_id, 'SomeType' ) + + end + + test 'type and object tests' do + + object_lookup_id = ObjectLookup.by_name( 'SomeObject' ) + assert( object_lookup_id, 'first by_name' ) + + object_lookup_name = ObjectLookup.by_id( object_lookup_id ) + assert( object_lookup_name, 'first by_id' ) + assert_equal( object_lookup_name, 'SomeObject' ) + + type_lookup_id = TypeLookup.by_name( 'SomeType' ) + assert( type_lookup_id, 'first by_name' ) + + type_lookup_name = TypeLookup.by_id( type_lookup_id ) + assert( type_lookup_name, 'first by_id' ) + assert_equal( type_lookup_name, 'SomeType' ) + + assert_not_equal( object_lookup_name, type_lookup_name, 'verify lookups' ) + +puts "ll #{ObjectLookup.cache_object.inspect}" + end + +end