Merge pull request #110 from podigee/fg/improve-device-brand-detection

Fix device brand detection

lib/device_detector.rb

@@ -15,6 +15,7 @@
 require 'device_detector/os'
 require 'device_detector/browser'
 require 'device_detector/client_hint'
+require 'device_detector/vendor_fragment'
 
 class DeviceDetector
   attr_reader :client_hint, :user_agent
@@ -71,9 +72,12 @@ def device_name
   end
 
   def device_brand
+    return if fake_ua?
+
     # Assume all devices running iOS / Mac OS are from Apple
     brand = device.brand
     brand = 'Apple' if brand.nil? && ['Apple TV', 'iOS', 'Mac'].include?(os_name)
+
     brand
   end
 
@@ -222,7 +226,7 @@ def os
 
   # https://github.com/matomo-org/device-detector/blob/827a3fab7e38c3274c18d2f5f5bc2a78b7ef4a3a/DeviceDetector.php#L921C5-L921C5
   def fake_ua?
-    os_name == 'Android' && device_brand == 'Apple'
+    os_name == 'Android' && device.brand == 'Apple'
   end
 
   # https://github.com/matomo-org/device-detector/blob/be1c9ef486c247dc4886668da5ed0b1c49d90ba8/Parser/Client/Browser.php#L772

lib/device_detector/device.rb

@@ -1724,11 +1724,24 @@ def type
     end
 
     def brand
-      regex_meta[:brand]
+      return regex_meta[:brand] if regex_meta[:brand] == 'Sony Ericsson'
+
+      brand = regex_meta[:regex_name] || regex_meta[:brand] || vendor_fragment.name || fix_for_x_music
+      return if brand == 'Unknown'
+
+      brand
     end
 
     private
 
+    def fix_for_x_music
+      user_agent&.include?('X-music Ⅲ') ? 'OneClick' : nil
+    end
+
+    def vendor_fragment
+      ::DeviceDetector::VendorFragment.new(user_agent)
+    end
+
     # The order of files needs to be the same as the order of device
     # parser classes used in the piwik project.
     def filenames

lib/device_detector/parser.rb

@@ -58,7 +58,29 @@ def regexes_for(file_paths)
     end
 
     def load_regexes(file_paths)
-      file_paths.map { |path, full_path| [path, symbolize_keys!(YAML.load_file(full_path))] }
+      file_paths.map do |path, full_path|
+        object = YAML.load_file(full_path)
+        object = rewrite_device_object!(object) if is_device_yml_file?(full_path)
+        object = rewrite_vendor_object!(object) if is_vendor_yml_file?(full_path)
+
+        [path, symbolize_keys!(object)]
+      end
+    end
+
+    def is_device_yml_file?(file_path)
+      file_path.include?('/regexes/device/')
+    end
+
+    def is_vendor_yml_file?(file_path)
+      file_path.include?('/regexes/vendorfragments')
+    end
+
+    def rewrite_vendor_object!(object)
+      object.map { |key, values| values.map { |v| { 'regex_name' => key, 'regex' => v } } }.flatten
+    end
+
+    def rewrite_device_object!(object)
+      object.map { |key, value| [key, { 'regex_name' => key }.merge!(value)] }.to_h
     end
 
     def symbolize_keys!(object)
@@ -88,8 +110,8 @@ def build_regex(src)
       Regexp.new('(?:^|[^A-Z0-9\-_]|[^A-Z0-9\-]_|sprd-|MZ-)(?:' + src + ')', Regexp::IGNORECASE)
     end
 
-    def from_cache(key)
-      DeviceDetector.cache.get_or_set(key) { yield }
+    def from_cache(key, &block)
+      DeviceDetector.cache.get_or_set(key, &block)
     end
   end
 end

lib/device_detector/vendor_fragment.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+require 'set'
+
+class DeviceDetector
+  class VendorFragment < Parser
+    def name
+      vendor_fragment_info
+    end
+
+    private
+
+    def vendor_fragment_info
+      from_cache(['vendor_fragment', self.class.name, user_agent]) do
+        return if regex_meta.nil? || regex_meta.empty?
+
+        regex_meta[:regex_name]
+      end
+    end
+
+    def filenames
+      ['vendorfragments.yml']
+    end
+  end
+end

spec/device_detector/detector_fixtures_spec.rb

@@ -91,6 +91,14 @@ def str_or_nil(string)
                 else
                   assert_equal model, detector.device_name, 'failed device name detection'
                 end
+
+                brand = str_or_nil(f['device']['brand'])
+                brand = brand.to_s unless brand.nil?
+                if brand.nil?
+                  assert_nil detector.device_brand, 'failed brand name detection'
+                else
+                  assert_equal brand, detector.device_brand, 'failed brand name detection'
+                end
               end
             end
           end

spec/device_detector/device_fixtures_spec.rb

@@ -21,6 +21,10 @@
             assert_equal f['device']['model'], device.name, 'failed model detection'
           end
 
+          it 'should have the expected brand' do
+            assert_equal f['device']['brand'], device.brand, 'failed brand detection'
+          end
+
           it 'should have the expected type' do
             expected_device_type = DeviceDetector::Device::DEVICE_NAMES[f['device']['type']]
             assert_equal expected_device_type, device.type, 'failed device name detection'

spec/device_detector/device_spec.rb

@@ -116,7 +116,7 @@
       it 'identifies the device' do
         value(device.name).must_equal 'Wii'
         value(device.type).must_equal 'console'
-        value(device.brand).must_be_nil
+        value(device.brand).must_equal 'Nintendo'
       end
     end