Allow updating things/items/sitemaps by recreating them

lib/openhab/core/emulate_hash.rb

@@ -103,7 +103,12 @@ def merge(*others, &block)
       def merge!(*others, &block)
         return self if others.empty?
 
-        replace(merge(*others, &block))
+        # don't call replace here so we don't touch other keys
+        others.shift.merge(*others, &block).each do |key, value|
+          value = yield key, self[key], value if key?(key) && block
+          store(key, value)
+        end
+        self
       end
       alias_method :update, :merge!
 

lib/openhab/core/items/group_item.rb

@@ -106,6 +106,9 @@ def inspect
           alias_method :to_s, :inspect
         end
 
+        # @!attribute [r] function
+        # @return [GroupFunction] Returns the function of this GroupItem
+
         # Override because we want to send them to the base item if possible
         %i[command update].each do |method|
           define_method(method) do |command|
@@ -152,6 +155,19 @@ def #{type}_item?                      # def date_time_item?
           RUBY
         end
 
+        #
+        # Compares all attributes of the item with another item.
+        #
+        # @param other [Item] The item to compare with
+        # @return [true,false] true if all attributes are equal, false otherwise
+        #
+        # @!visibility private
+        def config_eql?(other)
+          return false unless super
+
+          base_item&.type == other.base_item&.type && function&.inspect == other.function&.inspect
+        end
+
         private
 
         # Add base type and function details

lib/openhab/core/items/item.rb

@@ -264,12 +264,17 @@ def thing
         #
         # @return [Array<Thing>] An array of things or an empty array
         def things
-          registry = Things::Links::Provider.registry
-          channels = registry.get_bound_channels(name).to_a
-          channels.map(&:thing_uid).uniq.map { |tuid| EntityLookup.lookup_thing(tuid) }.compact
+          Things::Links::Provider.registry.get_bound_things(name).map { |thing| Things::Proxy.new(thing) }
         end
         alias_method :all_linked_things, :things
 
+        # Returns all of the item's links (channels and link configurations).
+        #
+        # @return [Array<ItemChannelLink>] An array of ItemChannelLink or an empty array
+        def links
+          Things::Links::Provider.registry.get_links(name)
+        end
+
         # @return [String]
         def inspect
           s = "#<OpenHAB::Core::Items::#{type}Item#{type_details} #{name} #{label.inspect} state=#{raw_state.inspect}"
@@ -286,6 +291,23 @@ def provider
           Provider.registry.provider_for(self)
         end
 
+        #
+        # Compares all attributes except metadata and channels/links of the item with another item.
+        #
+        # @param other [Item] The item to compare with
+        # @return [true,false] true if all attributes are equal, false otherwise
+        #
+        # @!visibility private
+        def config_eql?(other)
+          # GenericItem#equals checks whether other has the same name and class
+          return false unless equals(other)
+
+          %i[label category tags group_names].all? do |method|
+            # Don't use #send here. It is defined in GenericItem for sending commands
+            public_send(method) == other.public_send(method)
+          end
+        end
+
         def_type_predicate(:color)
         def_type_predicate(:contact)
         def_type_predicate(:date_time)

lib/openhab/core/items/number_item.rb

@@ -48,6 +48,11 @@ def format_type(command)
           super
         end
 
+        # @!visibility private
+        def config_eql?(other)
+          super && dimension == other.dimension
+        end
+
         protected
 
         # Adds the unit dimension

lib/openhab/core/items/registry.rb

@@ -45,13 +45,19 @@ def to_a
         # Enter the Item Builder DSL.
         #
         # @param (see Core::Provider.current)
+        # @param update [true, false]  Update existing items with the same name.
+        #   When false, an error will be raised if an item with the same name already exists.
         # @yield Block executed in the context of a {DSL::Items::Builder}
         # @return [Object] The return value of the block.
+        # @raise [ArgumentError] if an item with the same name already exists and `update` is false.
+        # @raise [FrozenError] if `update` is true but the existing item with the same name
+        #   wasn't created by the current provider.
         #
         # @see DSL::Items::Builder
         #
-        def build(preferred_provider = nil, &block)
-          DSL::Items::BaseBuilderDSL.new(preferred_provider).instance_eval_with_dummy_items(&block)
+        def build(preferred_provider = nil, update: true, &block)
+          DSL::Items::BaseBuilderDSL.new(preferred_provider, update: update)
+                                    .instance_eval_with_dummy_items(&block)
         end
 
         #

lib/openhab/core/registry.rb

@@ -20,12 +20,20 @@ class Registry
       def provider_for(key)
         elementReadLock.lock
         if key.is_a?(org.openhab.core.common.registry.Identifiable)
-          element = key
-        else
-          return nil unless (element = identifierToElement[key])
-        end
+          return unless (provider = elementToProvider[key])
+
+          # The HashMap lookup above uses java's hashCode() which has been overridden
+          # by GenericItem and ThingImpl to return object's uid, e.g. item's name, thing uid
+          # so it will return a provider even for an unmanaged object having the same uid
+          # as an existing managed object.
 
-        elementToProvider[element]
+          # So take an extra step to verify that the provider really holds the given instance.
+          # by using equal? to compare the object's identity.
+          # Only ManagedProviders have a #get method to look up the object by uid.
+          provider if !provider.is_a?(ManagedProvider) || provider.get(key.uid).equal?(key)
+        elsif (element = identifierToElement[key])
+          elementToProvider[element]
+        end
       ensure
         elementReadLock.unlock
       end

lib/openhab/core/sitemaps/provider.rb

@@ -57,6 +57,7 @@ def unregister
         #
         # Enter the Sitemap Builder DSL.
         #
+        # @param update [true, false]  When true, existing sitemaps with the same name will be updated.
         # @yield Block executed in the context of a {DSL::Sitemaps::Builder}
         # @return [void]
         #
@@ -82,8 +83,8 @@ def unregister
         #     end
         #   end
         #
-        def build(&block)
-          DSL::Sitemaps::Builder.new(self).instance_eval(&block)
+        def build(update: true, &block)
+          DSL::Sitemaps::Builder.new(self, update: update).instance_eval(&block)
         end
         # rubocop:enable Layout/LineLength
 
@@ -123,7 +124,7 @@ def notify_listeners_about_removed_element(element)
           @listeners.each { |l| l.model_changed(element.name, org.openhab.core.model.core.EventType::REMOVED) }
         end
 
-        def notify_listeners_about_updated_element(element)
+        def notify_listeners_about_updated_element(_old_element, element)
           @listeners.each { |l| l.model_changed(element.name, org.openhab.core.model.core.EventType::MODIFIED) }
         end
       end

lib/openhab/core/things/links/provider.rb

@@ -24,13 +24,11 @@ def registry
             end
 
             # @!visibility private
-            def link(item, channel, config = {})
+            def create_link(item, channel, config = {})
               config = Configuration.new(config.transform_keys(&:to_s))
               channel = ChannelUID.new(channel) if channel.is_a?(String)
               channel = channel.uid if channel.is_a?(Channel)
-              link = org.openhab.core.thing.link.ItemChannelLink.new(item.name, channel, config)
-
-              current.add(link)
+              org.openhab.core.thing.link.ItemChannelLink.new(item.name, channel, config)
             end
           end
 

lib/openhab/core/things/registry.rb

@@ -39,11 +39,17 @@ def to_a
         #
         # Enter the Thing Builder DSL.
         # @param (see Core::Provider.current)
+        # @param update [true, false]
+        #   When true, existing things with the same name will be redefined if they're different.
+        #   When false, an error will be raised if a thing with the same uid already exists.
         # @yield Block executed in the context of a {DSL::Things::Builder}.
         # @return [Object] The result of the block.
+        # @raise [ArgumentError] if a thing with the same uid already exists and `update` is false.
+        # @raise [FrozenError] if `update` is true but the existing thing with the same uid
+        #   wasn't created by the current provider.
         #
-        def build(preferred_provider = nil, &block)
-          DSL::Things::Builder.new(preferred_provider).instance_eval(&block)
+        def build(preferred_provider = nil, update: true, &block)
+          DSL::Things::Builder.new(preferred_provider, update: update).instance_eval(&block)
         end
 
         #

lib/openhab/core/things/thing.rb

@@ -197,6 +197,16 @@ def actions(scope = nil)
           $actions.get(scope || uid.binding_id, uid.to_s)
         end
 
+        #
+        # Compares all attributes of the thing with another thing.
+        #
+        # @param other [Thing] The thing to compare with
+        # @return [true,false] true if all attributes are equal, false otherwise
+        #
+        def config_eql?(other)
+          %i[uid label channels bridge_uid location configuration].all? { |method| send(method) == other.send(method) }
+        end
+
         #
         # Delegate missing methods to the thing's default actions scope.
         #

lib/openhab/dsl/items/builder.rb

@@ -118,23 +118,68 @@ class BaseBuilderDSL
         class ProviderWrapper
           attr_reader :provider
 
-          def initialize(provider)
+          def initialize(provider, update:)
             @provider = provider
+            @update = update
           end
 
           # @!visibility private
           def add(builder)
-            item = builder.build
-            provider.add(item)
+            if DSL.items.key?(builder.name)
+              raise ArgumentError, "Item #{builder.name} already exists" unless @update
+
+              # Use provider.get because openHAB's ManagedProvider does not support the #[] method.
+              unless (old_item = provider.get(builder.name))
+                raise FrozenError, "Item #{builder.name} is managed by #{DSL.items[builder.name].provider}"
+              end
+
+              item = builder.build
+              if item.config_eql?(old_item)
+                logger.debug { "Not replacing existing item #{item.uid} because it is identical" }
+                item = old_item
+              else
+                logger.debug { "Replacing existing item #{item.uid} because it is not identical" }
+                provider.update(item)
+              end
+              item.metadata.merge!(builder.metadata)
+              item.metadata
+                  .reject { |namespace, _| builder.metadata.key?(namespace) }
+                  .each do |namespace, metadata|
+                item.metadata.delete(namespace) if metadata.provider == Core::Items::Metadata::Provider.current
+              end
+            else
+              item = builder.build
+              item.metadata.merge!(builder.metadata)
+              provider.add(item)
+            end
+
+            item.update(builder.state) unless builder.state.nil?
+
             # make sure to add the item to the registry before linking it
-            builder.channels.each do |(channel, config)|
+            provider = Core::Things::Links::Provider.current
+            channel_uids = builder.channels.to_set do |(channel, config)|
+              # fill in partial channel names from group's thing id
               if !channel.include?(":") &&
                  (group = builder.groups.find { |g| g.is_a?(GroupItemBuilder) && g.thing })
                 thing = group.thing
                 channel = "#{thing}:#{channel}"
               end
-              Core::Things::Links::Provider.link(item, channel, config)
+
+              new_link = Core::Things::Links::Provider.create_link(item, channel, config)
+              if !(current_link = provider.get(new_link.uid))
+                provider.add(new_link)
+              elsif current_link.configuration != config
+                provider.update(new_link)
+              end
+
+              new_link.linked_uid
             end
+
+            # remove links not in the new item
+            provider.all.each do |link|
+              provider.remove(link.uid) if link.item_name == item.name && !channel_uids.include?(link.linked_uid)
+            end
+
             item
           end
         end
@@ -143,8 +188,8 @@ def add(builder)
         # @return [org.openhab.core.items.ItemProvider]
         attr_reader :provider
 
-        def initialize(provider)
-          @provider = ProviderWrapper.new(Core::Items::Provider.current(provider))
+        def initialize(provider, update:)
+          @provider = ProviderWrapper.new(Core::Items::Provider.current(provider), update: update)
         end
       end
 
@@ -549,12 +594,10 @@ def build
           tags.each do |tag|
             item.add_tag(tag)
           end
-          item.metadata.merge!(metadata)
-          item.metadata["autoupdate"] = autoupdate.to_s unless autoupdate.nil?
-          item.metadata["expire"] = expire if expire
-          item.metadata["stateDescription"] = { "pattern" => format } if format
-          item.metadata["unit"] = unit if unit
-          item.state = item.format_update(state) unless state.nil?
+          metadata["autoupdate"] = autoupdate.to_s unless autoupdate.nil?
+          metadata["expire"] = expire if expire
+          metadata["stateDescription"] = { "pattern" => format } if format
+          metadata["unit"] = unit if unit
           item
         end
 

lib/openhab/dsl/sitemaps/builder.rb

@@ -12,8 +12,9 @@ module Sitemaps
       # Base sitemap builder DSL
       class Builder
         # @!visibility private
-        def initialize(provider)
+        def initialize(provider, update:)
           @provider = provider
+          @update = update
         end
 
         # (see SitemapBuilder#initialize)
@@ -23,8 +24,12 @@ def initialize(provider)
         def sitemap(name, label = nil, icon: nil, &block)
           sitemap = SitemapBuilder.new(name, label, icon: icon)
           sitemap.instance_eval_with_dummy_items(&block) if block
-          @provider.add(sitemap.build)
-          sitemap
+          sitemap = sitemap.build
+          if @update && @provider.get(sitemap.uid)
+            @provider.update(sitemap)
+          else
+            @provider.add(sitemap)
+          end
         end
       end