fix: PropertyStruct recursive wrapping and add Go tests to Ruby CI

- Fix PropertyStruct to recursively wrap nested hashes and arrays
- Fix respond_to_missing? to check if key exists in @table
- Fix ERB test variable name (obj -> ps) in property_struct_spec.rb
- Update hash operations test to use attribute access
- Add Go erbrenderer tests to Ruby CI workflow matrix

Author: rkoster

.github/workflows/ruby.yml

@@ -6,13 +6,23 @@ jobs:
     strategy:
       matrix:
         os: [ubuntu-latest, macos-latest]
-        ruby: [ '2.6', '2.7', '3.0', '3.1', '3.2', '3.3', '3.4', head]
+        ruby: ['2.6', '2.7', '3.0', '3.1', '3.2', '3.3', '3.4', head]
     runs-on: ${{ matrix.os }}
     steps:
       - uses: actions/checkout@v6
+      - name: Setup Image
+        if: matrix.os == 'ubuntu-latest'
+        run: |
+          sudo apt-get update && sudo apt-get install -y libpcap-dev
       - uses: ruby/setup-ruby@v1
         with:
           ruby-version: ${{ matrix.ruby }}
+      - uses: actions/setup-go@v6
+        with:
+          go-version-file: go.mod
+      - name: Run Go erbrenderer tests
+        run: go test ./templatescompiler/erbrenderer/...
+        continue-on-error: ${{ matrix.ruby == 'head' }}
       - run: bundle install
         working-directory: templatescompiler/erbrenderer/
       - run: bundle exec rake

templatescompiler/erbrenderer/erb_renderer.rb

@@ -1,29 +1,42 @@
 # Based on common/properties/template_evaluation_context.rb
-require "rubygems"
-require "json"
-require "erb"
-require "yaml"
+require 'rubygems'
+require 'json'
+require 'erb'
+require 'yaml'
 
 # Simple struct-like class to replace OpenStruct dependency
 # OpenStruct is being removed from Ruby standard library in Ruby 3.5+
 class PropertyStruct
   def initialize(hash = {})
     @table = {}
     hash.each do |key, value|
-      @table[key.to_sym] = value
+      @table[key.to_sym] = wrap_value(value)
     end
   end
 
   def method_missing(method_name, *args)
-    if method_name.to_s.end_with?("=")
-      @table[method_name.to_s.chomp("=").to_sym] = args.first
+    if method_name.to_s.end_with?('=')
+      @table[method_name.to_s.chomp('=').to_sym] = wrap_value(args.first)
     else
       @table[method_name.to_sym]
     end
   end
 
-  def respond_to_missing?(method_name, include_private = false)
-    true
+  def respond_to_missing?(method_name, _include_private = false)
+    @table.key?(method_name.to_sym) || method_name.to_s.end_with?('=')
+  end
+
+  private
+
+  def wrap_value(value)
+    case value
+    when Hash
+      PropertyStruct.new(value)
+    when Array
+      value.map { |item| wrap_value(item) }
+    else
+      value
+    end
   end
 end
 
@@ -41,22 +54,20 @@ def recursive_merge!(other)
 end
 
 class TemplateEvaluationContext
-  attr_reader :name, :index
-  attr_reader :properties, :raw_properties
-  attr_reader :spec
+  attr_reader :name, :index, :properties, :raw_properties, :spec
 
   def initialize(spec)
-    @name = spec["job"]["name"] if spec["job"].is_a?(Hash)
-    @index = spec["index"]
+    @name = spec['job']['name'] if spec['job'].is_a?(Hash)
+    @index = spec['index']
 
-    properties1 = if !spec["job_properties"].nil?
-      spec["job_properties"]
-    else
-      spec["global_properties"].recursive_merge!(spec["cluster_properties"])
-    end
+    properties1 = if !spec['job_properties'].nil?
+                    spec['job_properties']
+                  else
+                    spec['global_properties'].recursive_merge!(spec['cluster_properties'])
+                  end
 
     properties = {}
-    spec["default_properties"].each do |name, value|
+    spec['default_properties'].each do |name, value|
       copy_property(properties, properties1, name, value)
     end
 
@@ -78,28 +89,30 @@ def p(*args)
     end
 
     return args[1] if args.length == 2
+
     raise UnknownProperty.new(names)
   end
 
   def if_p(*names)
     values = names.map do |name|
       value = lookup_property(@raw_properties, name)
       return ActiveElseBlock.new(self) if value.nil?
+
       value
     end
 
     yield(*values)
     InactiveElseBlock.new
   end
 
-  def if_link(name)
+  def if_link(_name)
     false
   end
 
   private
 
   def copy_property(dst, src, name, default = nil)
-    keys = name.split(".")
+    keys = name.split('.')
     src_ref = src
     dst_ref = dst
 
@@ -120,9 +133,9 @@ def copy_property(dst, src, name, default = nil)
   def openstruct(object)
     case object
     when Hash
-      mapped = object.each_with_object({}) { |(k, v), h|
+      mapped = object.each_with_object({}) do |(k, v), h|
         h[k] = openstruct(v)
-      }
+      end
       PropertyStruct.new(mapped)
     when Array
       object.map { |item| openstruct(item) }
@@ -132,7 +145,7 @@ def openstruct(object)
   end
 
   def lookup_property(collection, name)
-    keys = name.split(".")
+    keys = name.split('.')
     ref = collection
 
     keys.each do |key|
@@ -161,24 +174,23 @@ def else
       yield
     end
 
-    def else_if_p(*names, &block) # rubocop:disable Style/ArgumentsForwarding
-      @context.if_p(*names, &block) # rubocop:disable Style/ArgumentsForwarding
+    def else_if_p(*names, &block)
+      @context.if_p(*names, &block)
     end
   end
 
   class InactiveElseBlock
-    def else
-    end
+    def else; end
 
-    def else_if_p(*names)
+    def else_if_p(*_names)
       InactiveElseBlock.new
     end
   end
 end
 
-# todo do not use JSON in releases
+# TODO: do not use JSON in releases
 class << JSON
-  alias_method :dump_array_or_hash, :dump
+  alias dump_array_or_hash dump
 
   def dump(*args)
     arg = args[0]
@@ -196,10 +208,10 @@ def initialize(json_context_path)
   end
 
   def render(src_path, dst_path)
-    erb = ERB.new(File.read(src_path), trim_mode: "-")
+    erb = ERB.new(File.read(src_path), trim_mode: '-')
     erb.filename = src_path
 
-    # Note: JSON.load_file was added in v2.3.1: https://github.com/ruby/json/blob/v2.3.1/lib/json/common.rb#L286
+    # NOTE: JSON.load_file was added in v2.3.1: https://github.com/ruby/json/blob/v2.3.1/lib/json/common.rb#L286
     context_hash = JSON.parse(File.read(@json_context_path))
     template_evaluation_context = TemplateEvaluationContext.new(context_hash)
 
@@ -208,7 +220,7 @@ def render(src_path, dst_path)
     name = "#{template_evaluation_context&.name}/#{template_evaluation_context&.index}"
 
     line_i = e.backtrace.index { |l| l.include?(erb&.filename.to_s) }
-    line_num = line_i ? e.backtrace[line_i].split(":")[1] : "unknown"
+    line_num = line_i ? e.backtrace[line_i].split(':')[1] : 'unknown'
     location = "(line #{line_num}: #{e.inspect})"
 
     raise("Error filling in template '#{src_path}' for #{name} #{location}")

templatescompiler/erbrenderer/spec/property_struct_spec.rb

@@ -1,86 +1,87 @@
-require "spec_helper"
-require "erb_renderer"
+require 'spec_helper'
+require 'erb_renderer'
 
-RSpec.describe "PropertyStruct" do
-  describe "initialization and attribute access" do
-    it "provides dynamic attribute access for hash keys" do
-      ps = PropertyStruct.new(name: "test", value: 42)
-      expect(ps.name).to eq("test")
+RSpec.describe 'PropertyStruct' do
+  describe 'initialization and attribute access' do
+    it 'provides dynamic attribute access for hash keys' do
+      ps = PropertyStruct.new(name: 'test', value: 42)
+      expect(ps.name).to eq('test')
       expect(ps.value).to eq(42)
     end
 
-    it "converts string keys to symbols" do
-      ps = PropertyStruct.new("name" => "test", "value" => 42)
-      expect(ps.name).to eq("test")
+    it 'converts string keys to symbols' do
+      ps = PropertyStruct.new('name' => 'test', 'value' => 42)
+      expect(ps.name).to eq('test')
       expect(ps.value).to eq(42)
     end
 
-    it "supports nested attribute access" do
-      ps = PropertyStruct.new(config: {database: {host: "localhost", port: 5432}})
+    it 'supports nested attribute access' do
+      ps = PropertyStruct.new(config: { database: { host: 'localhost', port: 5432 } })
       nested = ps.config
       expect(nested).to be_a(PropertyStruct)
       expect(nested.database).to be_a(PropertyStruct)
-      expect(nested.database.host).to eq("localhost")
+      expect(nested.database.host).to eq('localhost')
       expect(nested.database.port).to eq(5432)
     end
 
-    it "handles arrays of hashes" do
-      ps = PropertyStruct.new(servers: [{name: "web1", ip: "10.0.0.1"}, {name: "web2", ip: "10.0.0.2"}])
+    it 'handles arrays of hashes' do
+      ps = PropertyStruct.new(servers: [{ name: 'web1', ip: '10.0.0.1' }, { name: 'web2', ip: '10.0.0.2' }])
       servers = ps.servers
       expect(servers).to be_an(Array)
       expect(servers.length).to eq(2)
-      expect(servers.first.name).to eq("web1")
-      expect(servers.last.ip).to eq("10.0.0.2")
+      expect(servers.first.name).to eq('web1')
+      expect(servers.last.ip).to eq('10.0.0.2')
     end
 
-    it "responds to method queries correctly" do
-      ps = PropertyStruct.new(existing_key: "value")
+    it 'responds to method queries correctly' do
+      ps = PropertyStruct.new(existing_key: 'value')
       expect(ps.respond_to?(:existing_key)).to be true
       expect(ps.respond_to?(:nonexistent_key)).to be false
     end
   end
 
-  describe "Ruby standard library method pass-through" do
-    it "supports array operations like map" do
+  describe 'Ruby standard library method pass-through' do
+    it 'supports array operations like map' do
       ps = PropertyStruct.new(ports: [8080, 8081, 8082])
-      expect(ps.ports.map(&:to_s)).to eq(["8080", "8081", "8082"])
+      expect(ps.ports.map(&:to_s)).to eq(%w[8080 8081 8082])
     end
 
-    it "supports string operations" do
-      ps = PropertyStruct.new(url: "https://example.com")
-      expect(ps.url.start_with?("https")).to be true
-      expect(ps.url.split("://")).to eq(["https", "example.com"])
+    it 'supports string operations' do
+      ps = PropertyStruct.new(url: 'https://example.com')
+      expect(ps.url.start_with?('https')).to be true
+      expect(ps.url.split('://')).to eq(['https', 'example.com'])
     end
 
-    it "supports hash operations" do
-      ps = PropertyStruct.new(config: {a: 1, b: 2, c: 3})
-      expect(ps.config.keys.sort).to eq([:a, :b, :c])
-      expect(ps.config.values.sum).to eq(6)
+    it 'supports hash operations via direct access' do
+      ps = PropertyStruct.new(config: { a: 1, b: 2, c: 3 })
+      expect(ps.config.a).to eq(1)
+      expect(ps.config.b).to eq(2)
+      expect(ps.config.c).to eq(3)
     end
 
-    it "supports nil and empty checks" do
-      ps = PropertyStruct.new(empty_string: "", nil_value: nil, filled: "data")
+    it 'supports nil and empty checks' do
+      ps = PropertyStruct.new(empty_string: '', nil_value: nil, filled: 'data')
       expect(ps.empty_string.empty?).to be true
       expect(ps.nil_value.nil?).to be true
       expect(ps.filled.nil?).to be false
     end
   end
 
-  describe "compatibility across Ruby versions" do
-    it "works with ERB rendering" do
-      template = ERB.new("<%= obj.name.upcase %>: <%= obj.ports.join(',') %>")
-      ps = PropertyStruct.new(name: "service", ports: [80, 443, 8080])
+  describe 'compatibility across Ruby versions' do
+    it 'works with ERB rendering' do
+      template = ERB.new("<%= ps.name.upcase %>: <%= ps.ports.join(',') %>")
+      ps = PropertyStruct.new(name: 'service', ports: [80, 443, 8080])
       result = template.result(binding)
-      expect(result).to eq("SERVICE: 80,443,8080")
+      expect(result).to eq('SERVICE: 80,443,8080')
     end
 
-    it "maintains OpenStruct API compatibility" do
+    it 'maintains OpenStruct API compatibility' do
       # Test that PropertyStruct can be used as a drop-in replacement for OpenStruct
-      ps = PropertyStruct.new(field1: "value1", field2: "value2")
+      ps = PropertyStruct.new(field1: 'value1', field2: 'value2')
       expect(ps).to respond_to(:field1)
       expect(ps).to respond_to(:field2)
-      expect(ps.field1).to eq("value1")
-      expect(ps.field2).to eq("value2")
+      expect(ps.field1).to eq('value1')
+      expect(ps.field2).to eq('value2')
     end
   end
 end