Improve errors when an item no longer exists

Two main user visible improvements:
 * If they keep a ref to an item, and that item disappears,
   calling a method on that item will now raise a helpful
   StaleProxyError, instead of a NoMethodError on nil
 * Enumerable and predicate methods will now behave better
   when they refer to stale proxies - filtering methods will
   just remove them. I run into this occasionally when I have
   multiple rules firing, and some of them are adding or
   removing items - and calling `items.locations` would raise
   a NoMethodError (now a StaleProxyError) for `#location?`
   due to a race condition. This should no longer happen -
   it will just ignore the now-stale items.

The proxy code was significantly refactored, and Thing
proxies will also inherit the same stale proxy handling.
I also removed the $things registry lookup on every access
of a thing, using the same event subscriber mechanism as Item.

I also moved several methods from GenericItem to Item, so that
Proxy can detect that those methods should exist even for
dummy/stale items.

Signed-off-by: Cody Cutrer <[email protected]>

CHANGELOG.md

@@ -723,7 +723,7 @@ Here is a non-exhaustive list of significant departures from the original gem:
 - {OpenHAB::Core::Events::ItemStateEvent} and {OpenHAB::Core::Events::ItemStateChangedEvent} now have full sets of predicate methods.
 - {OpenHAB::DSL::Rules::Terse terse rules} now have an `on_load` parameter.
 - {Item#all_groups Item#all_groups}, {Enumerable#all_members}, {Enumerable#groups}, {Enumerable#all_groups}.
-- {GenericItem#formatted_state GenericItem#formatted_state}.
+- {Item#formatted_state Item#formatted_state}.
 - {OpenHAB::DSL.transform} now available at top-level, like Rules DSL.
 - {Item#member_of? Item#member_of?}, {Item#tagged? Item#tagged?}.
 - {OpenHAB::DSL::Rules::BuilderDSL.watch watch} can now be used to monitor subdirectories

lib/openhab/core/entity_lookup.rb

@@ -160,7 +160,7 @@ class << self
         def lookup_entity(name, create_dummy_items: false)
           # make sure we have a nil return
           create_dummy_items = nil if create_dummy_items == false
-          lookup_item(name) || lookup_thing_const(name) || (create_dummy_items && Items::Proxy.new(name.to_sym))
+          lookup_item(name) || lookup_thing_const(name) || (create_dummy_items && Items::Proxy.new(name.to_s))
         end
 
         #

lib/openhab/core/items/generic_item.rb

@@ -38,24 +38,6 @@ def ACCEPTED_DATA_TYPES
           def ===(other)
             other.is_a?(self)
           end
-
-          # @!visibility private
-          def item_states_event_builder
-            @item_states_event_builder ||=
-              OpenHAB::OSGi.service("org.openhab.core.io.rest.sse.internal.SseItemStatesEventBuilder")&.tap do |builder|
-                m = builder.class.java_class.get_declared_method("getDisplayState", Item, java.util.Locale)
-                m.accessible = true
-                builder.instance_variable_set(:@getDisplayState, m)
-                # Disable "singleton on non-persistent Java type"
-                original_verbose = $VERBOSE
-                $VERBOSE = nil
-                def builder.get_display_state(item)
-                  @getDisplayState.invoke(self, item, nil)
-                end
-              ensure
-                $VERBOSE = original_verbose
-              end
-          end
         end
         # rubocop:enable Naming/MethodName
 
@@ -89,21 +71,6 @@ def state?
           !raw_state.is_a?(Types::UnDefType)
         end
 
-        # @!attribute [r] formatted_state
-        #
-        # Format the item's state according to its state description
-        #
-        # This may include running a transformation.
-        #
-        # @return [String] The formatted state
-        #
-        # @example
-        #   logger.info(Exterior_WindDirection.formatted_state) # => "NE (36°)"
-        #
-        def formatted_state
-          GenericItem.item_states_event_builder.get_display_state(self)
-        end
-
         #
         # @!attribute [r] state
         # @return [State, nil]
@@ -124,124 +91,6 @@ def state
         #   Check if the item state == {UNDEF}
         #   @return [true,false]
 
-        #
-        # Send a command to this item
-        #
-        # When this method is chained after the {OpenHAB::DSL::Items::Ensure::Ensurable#ensure ensure}
-        # method, or issued inside an {OpenHAB::DSL.ensure_states ensure_states} block, or after
-        # {OpenHAB::DSL.ensure_states! ensure_states!} have been called,
-        # the command will only be sent if the item is not already in the same state.
-        #
-        # The similar method `command!`, however, will always send the command regardless of the item's state.
-        #
-        # @param [Command, #to_s] command command to send to the item.
-        #   When given a {Command} argument, it will be passed directly.
-        #   Otherwise, the result of `#to_s` will be parsed into a {Command}.
-        # @param [String, nil] source Optional string to identify what sent the event.
-        # @return [self, nil] nil when `ensure` is in effect and the item was already in the same state,
-        #   otherwise the item.
-        #
-        # @see DSL::Items::TimedCommand#command Timed Command
-        # @see OpenHAB::DSL.ensure_states ensure_states
-        # @see OpenHAB::DSL.ensure_states! ensure_states!
-        # @see DSL::Items::Ensure::Ensurable#ensure ensure
-        #
-        # @example Sending a {Command} to an item
-        #   MySwitch.command(ON) # The preferred method is `MySwitch.on`
-        #   Garage_Door.command(DOWN) # The preferred method is `Garage_Door.down`
-        #   SetTemperature.command 20 | "°C"
-        #
-        # @example Sending a plain number to a {NumberItem}
-        #   SetTemperature.command(22.5) # if it accepts a DecimalType
-        #
-        # @example Sending a string to a dimensioned {NumberItem}
-        #   SetTemperature.command("22.5 °C") # The string will be parsed and converted to a QuantityType
-        #
-        def command(command, source: nil)
-          command = format_command(command)
-          logger.trace { "Sending Command #{command} to #{name}" }
-          if source
-            Events.publisher.post(Events::ItemEventFactory.create_command_event(name, command, source.to_s))
-          else
-            $events.send_command(self, command)
-          end
-          Proxy.new(self)
-        end
-        alias_method :command!, :command
-
-        # not an alias to allow easier stubbing and overriding
-        def <<(command)
-          command(command)
-        end
-
-        # @!parse alias_method :<<, :command
-
-        # @!method refresh
-        #   Send the {REFRESH} command to the item
-        #   @return [Item] `self`
-
-        #
-        # Send an update to this item
-        #
-        # @param [State, #to_s, nil] state the state to update the item.
-        #   When given a {State} argument, it will be passed directly.
-        #   Otherwise, the result of `#to_s` will be parsed into a {State} first.
-        #   If `nil` is passed, the item will be updated to {NULL}.
-        # @return [self, nil] nil when `ensure` is in effect and the item was already in the same state,
-        #   otherwise the item.
-        #
-        # @example Updating to a {State}
-        #   DoorStatus.update(OPEN)
-        #   InsideTemperature.update 20 | "°C"
-        #
-        # @example Updating to {NULL}, the two following are equivalent:
-        #   DoorStatus.update(nil)
-        #   DoorStatus.update(NULL)
-        #
-        # @example Updating with a plain number
-        #   PeopleCount.update(5) # A plain NumberItem
-        #
-        # @example Updating with a string to a dimensioned {NumberItem}
-        #   InsideTemperature.update("22.5 °C") # The string will be parsed and converted to a QuantityType
-        #
-        def update(state)
-          state = format_update(state)
-          logger.trace { "Sending Update #{state} to #{name}" }
-          $events.post_update(self, state)
-          Proxy.new(self)
-        end
-        alias_method :update!, :update
-
-        # @!visibility private
-        def format_command(command)
-          command = format_type(command)
-          return command if command.is_a?(Types::Command)
-
-          command = command.to_s
-          org.openhab.core.types.TypeParser.parse_command(getAcceptedCommandTypes, command) || command
-        end
-
-        # @!visibility private
-        def format_update(state)
-          state = format_type(state)
-          return state if state.is_a?(Types::State)
-
-          state = state.to_s
-          org.openhab.core.types.TypeParser.parse_state(getAcceptedDataTypes, state) || StringType.new(state)
-        end
-
-        # formats a {Types::Type} to send to the event bus
-        # @!visibility private
-        def format_type(type)
-          # actual Type types can be sent directly without conversion
-          # make sure to use Type, because this method is used for both
-          # #update and #command
-          return type if type.is_a?(Types::Type)
-          return NULL if type.nil?
-
-          type.to_s
-        end
-
         #
         # @method time_series=(time_series)
         #   Set a new time series.

lib/openhab/core/items/item.rb

@@ -19,6 +19,24 @@ def ===(other)
             other.is_a?(self)
           end
 
+          # @!visibility private
+          def item_states_event_builder
+            @item_states_event_builder ||=
+              OpenHAB::OSGi.service("org.openhab.core.io.rest.sse.internal.SseItemStatesEventBuilder")&.tap do |builder|
+                m = builder.class.java_class.get_declared_method("getDisplayState", Item, java.util.Locale)
+                m.accessible = true
+                builder.instance_variable_set(:@getDisplayState, m)
+                # Disable "singleton on non-persistent Java type"
+                original_verbose = $VERBOSE
+                $VERBOSE = nil
+                def builder.get_display_state(item)
+                  @getDisplayState.invoke(self, item, nil)
+                end
+              ensure
+                $VERBOSE = original_verbose
+              end
+          end
+
           private
 
           # @!macro def_type_predicate
@@ -55,6 +73,139 @@ def to_s
           label || name
         end
 
+        # @!attribute [r] formatted_state
+        #
+        # Format the item's state according to its state description
+        #
+        # This may include running a transformation.
+        #
+        # @return [String] The formatted state
+        #
+        # @example
+        #   logger.info(Exterior_WindDirection.formatted_state) # => "NE (36°)"
+        #
+        def formatted_state
+          Item.item_states_event_builder.get_display_state(self)
+        end
+
+        #
+        # Send a command to this item
+        #
+        # When this method is chained after the {OpenHAB::DSL::Items::Ensure::Ensurable#ensure ensure}
+        # method, or issued inside an {OpenHAB::DSL.ensure_states ensure_states} block, or after
+        # {OpenHAB::DSL.ensure_states! ensure_states!} have been called,
+        # the command will only be sent if the item is not already in the same state.
+        #
+        # The similar method `command!`, however, will always send the command regardless of the item's state.
+        #
+        # @param [Command, #to_s] command command to send to the item.
+        #   When given a {Command} argument, it will be passed directly.
+        #   Otherwise, the result of `#to_s` will be parsed into a {Command}.
+        # @param [String, nil] source Optional string to identify what sent the event.
+        # @return [self, nil] nil when `ensure` is in effect and the item was already in the same state,
+        #   otherwise the item.
+        #
+        # @see DSL::Items::TimedCommand#command Timed Command
+        # @see OpenHAB::DSL.ensure_states ensure_states
+        # @see OpenHAB::DSL.ensure_states! ensure_states!
+        # @see DSL::Items::Ensure::Ensurable#ensure ensure
+        #
+        # @example Sending a {Command} to an item
+        #   MySwitch.command(ON) # The preferred method is `MySwitch.on`
+        #   Garage_Door.command(DOWN) # The preferred method is `Garage_Door.down`
+        #   SetTemperature.command 20 | "°C"
+        #
+        # @example Sending a plain number to a {NumberItem}
+        #   SetTemperature.command(22.5) # if it accepts a DecimalType
+        #
+        # @example Sending a string to a dimensioned {NumberItem}
+        #   SetTemperature.command("22.5 °C") # The string will be parsed and converted to a QuantityType
+        #
+        def command(command, source: nil)
+          command = format_command(command)
+          logger.trace { "Sending Command #{command} to #{name}" }
+          if source
+            Events.publisher.post(Events::ItemEventFactory.create_command_event(name, command, source.to_s))
+          else
+            $events.send_command(self, command)
+          end
+          Proxy.new(self)
+        end
+        alias_method :command!, :command
+
+        # not an alias to allow easier stubbing and overriding
+        def <<(command)
+          command(command)
+        end
+
+        # @!parse alias_method :<<, :command
+
+        # @!method refresh
+        #   Send the {REFRESH} command to the item
+        #   @return [Item] `self`
+
+        #
+        # Send an update to this item
+        #
+        # @param [State, #to_s, nil] state the state to update the item.
+        #   When given a {State} argument, it will be passed directly.
+        #   Otherwise, the result of `#to_s` will be parsed into a {State} first.
+        #   If `nil` is passed, the item will be updated to {NULL}.
+        # @return [self, nil] nil when `ensure` is in effect and the item was already in the same state,
+        #   otherwise the item.
+        #
+        # @example Updating to a {State}
+        #   DoorStatus.update(OPEN)
+        #   InsideTemperature.update 20 | "°C"
+        #
+        # @example Updating to {NULL}, the two following are equivalent:
+        #   DoorStatus.update(nil)
+        #   DoorStatus.update(NULL)
+        #
+        # @example Updating with a plain number
+        #   PeopleCount.update(5) # A plain NumberItem
+        #
+        # @example Updating with a string to a dimensioned {NumberItem}
+        #   InsideTemperature.update("22.5 °C") # The string will be parsed and converted to a QuantityType
+        #
+        def update(state)
+          state = format_update(state)
+          logger.trace { "Sending Update #{state} to #{name}" }
+          $events.post_update(self, state)
+          Proxy.new(self)
+        end
+        alias_method :update!, :update
+
+        # @!visibility private
+        def format_command(command)
+          command = format_type(command)
+          return command if command.is_a?(Types::Command)
+
+          command = command.to_s
+          org.openhab.core.types.TypeParser.parse_command(getAcceptedCommandTypes, command) || command
+        end
+
+        # @!visibility private
+        def format_update(state)
+          state = format_type(state)
+          return state if state.is_a?(Types::State)
+
+          state = state.to_s
+          org.openhab.core.types.TypeParser.parse_state(getAcceptedDataTypes, state) || StringType.new(state)
+        end
+
+        # formats a {Types::Type} to send to the event bus
+        # @!visibility private
+        def format_type(type)
+          # actual Type types can be sent directly without conversion
+          # make sure to use Type, because this method is used for both
+          # #update and #command
+          return type if type.is_a?(Types::Type)
+          return NULL if type.nil?
+
+          type.to_s
+        end
+
         #
         # @!attribute [r] groups
         #

lib/openhab/core/items/persistence.rb

@@ -62,7 +62,7 @@ module Items
       #   logger.info("Max power usage today: #{max}, at: #{max.timestamp})
       #
       module Persistence
-        GenericItem.prepend(self)
+        Item.prepend(self)
 
         #
         # A wrapper for {org.openhab.core.persistence.HistoricItem HistoricItem} that delegates to its state.