From a31de23bb16f58c26c1eb38ad6b2fdfcdc2e9607 Mon Sep 17 00:00:00 2001 From: Michael Spiegel Date: Thu, 15 Jan 2015 16:51:59 -0500 Subject: [PATCH 1/4] Support for device brand and model. --- src/main/java/ua_parser/Client.java | 8 +- src/main/java/ua_parser/Device.java | 35 ++++++--- src/main/java/ua_parser/DeviceParser.java | 93 ++++++++++++++++++----- src/main/java/ua_parser/OS.java | 14 ++-- src/main/java/ua_parser/UserAgent.java | 10 +-- src/test/java/ua_parser/DeviceTest.java | 6 +- src/test/java/ua_parser/ParserTest.java | 8 +- 7 files changed, 125 insertions(+), 49 deletions(-) diff --git a/src/main/java/ua_parser/Client.java b/src/main/java/ua_parser/Client.java index 0f682f4..575b95f 100644 --- a/src/main/java/ua_parser/Client.java +++ b/src/main/java/ua_parser/Client.java @@ -45,10 +45,10 @@ public boolean equals(Object other) { @Override public int hashCode() { - int h = userAgent == null ? 0 : userAgent.hashCode(); - h += os == null ? 0 : os.hashCode(); - h += device == null ? 0 : device.hashCode(); - return h; + int result = userAgent != null ? userAgent.hashCode() : 0; + result = 31 * result + (os != null ? os.hashCode() : 0); + result = 31 * result + (device != null ? device.hashCode() : 0); + return result; } @Override diff --git a/src/main/java/ua_parser/Device.java b/src/main/java/ua_parser/Device.java index e318ef2..8cc95a2 100644 --- a/src/main/java/ua_parser/Device.java +++ b/src/main/java/ua_parser/Device.java @@ -25,32 +25,47 @@ */ public class Device { public final String family; + public final String brand; + public final String model; - public Device(String family) { + public Device(String family, String brand, String model) { this.family = family; + this.brand = brand; + this.model = model; } public static Device fromMap(Map m) { - return new Device((String) m.get("family")); + return new Device(m.get("family"), m.get("brand"), m.get("model")); } @Override - public boolean equals(Object other) { - if (other == this) return true; - if (!(other instanceof Device)) return false; + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; - Device o = (Device) other; - return (this.family != null && this.family.equals(o.family)) || this.family == o.family; + Device device = (Device) o; + + if (brand != null ? !brand.equals(device.brand) : device.brand != null) return false; + if (this.family != null ? !this.family.equals(device.family) : device.family != null) return false; + if (model != null ? !model.equals(device.model) : device.model != null) return false; + + return true; } @Override public int hashCode() { - return family == null ? 0 : family.hashCode(); + int result = family != null ? family.hashCode() : 0; + result = 31 * result + (brand != null ? brand.hashCode() : 0); + result = 31 * result + (model != null ? model.hashCode() : 0); + return result; } @Override public String toString() { - return String.format("{\"family\": %s}", - family == null ? Constants.EMPTY_STRING : '"' + family + '"'); + return String.format("{\"family\": %s, \"brand\": %s, \"model\": %s}", + family == null ? Constants.EMPTY_STRING : '"' + family + '"', + brand == null ? Constants.EMPTY_STRING : '"' + brand + '"', + model == null ? Constants.EMPTY_STRING : '"' + model + '"'); } + } \ No newline at end of file diff --git a/src/main/java/ua_parser/DeviceParser.java b/src/main/java/ua_parser/DeviceParser.java index 04d9955..1196608 100644 --- a/src/main/java/ua_parser/DeviceParser.java +++ b/src/main/java/ua_parser/DeviceParser.java @@ -17,8 +17,10 @@ package ua_parser; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -39,15 +41,14 @@ public Device parse(String agentString) { return null; } - String device = null; + Device device; for (DevicePattern p : patterns) { if ((device = p.match(agentString)) != null) { - break; + return device; } } - if (device == null) device = "Other"; - return new Device(device); + return new Device("Other", null, null); } public static DeviceParser fromList(List> configList) { @@ -60,40 +61,96 @@ public static DeviceParser fromList(List> configList) { protected static DevicePattern patternFromMap(Map configMap) { String regex = configMap.get("regex"); + String regex_flag = configMap.get("regex_flag"); if (regex == null) { throw new IllegalArgumentException("Device is missing regex"); } - return new DevicePattern(Pattern.compile(regex), - configMap.get("device_replacement")); + + Pattern pattern; + if (regex_flag != null && regex_flag.equals("i")) { + pattern = Pattern.compile(regex, Pattern.CASE_INSENSITIVE); + } else { + pattern = Pattern.compile(regex); + } + + /** + * To maintains backwards compatibility the familyReplacement + * field has been named "device_replacement" + */ + return new DevicePattern(pattern, + configMap.get("device_replacement"), + configMap.get("brand_replacement"), + configMap.get("model_replacement")); } protected static class DevicePattern { private final Pattern pattern; private final String familyReplacement; + private final String brandReplacement; + private final String modelReplacement; - public DevicePattern(Pattern pattern, String familyReplacement) { + public DevicePattern(Pattern pattern, String familyReplacement, String brandReplacement, String modelReplacement) { this.pattern = pattern; this.familyReplacement = familyReplacement; + this.brandReplacement = brandReplacement; + this.modelReplacement = modelReplacement; + } + + private Set extractPositions(String input) { + int position = 0; + Set result = new HashSet(); + while ((position = input.indexOf('$', position)) != -1) { + char digit = input.charAt(position + 1); + if (Character.isDigit(digit)) { + result.add(Character.digit(digit, 10)); + } + position++; + } + return result; + } + + private String replace(Matcher matcher, String replacement, int position) { + if (replacement == null) { + if (position > 0) { + replacement = "$" + position; + } else { + return null; + } + } + if (replacement.contains("$")) { + Set positions = extractPositions(replacement); + int count = matcher.groupCount(); + for (Integer i : positions) { + String group = (i > count) ? null : matcher.group(i); + replacement = replacement.replace("$" + i, (group == null) ? "" : group); + } + } + replacement = replacement.trim(); + if (replacement.length() == 0) { + return null; + } else { + return replacement; + } } - public String match(String agentString) { + public Device match(String agentString) { + Matcher matcher = pattern.matcher(agentString); if (!matcher.find()) { return null; } - String family = null; - if (familyReplacement != null) { - if (familyReplacement.contains("$1") && matcher.groupCount() >= 1 && matcher.group(1) != null) { - family = familyReplacement.replaceFirst("\\$1", Matcher.quoteReplacement(matcher.group(1))); - } else { - family = familyReplacement; - } - } else if (matcher.groupCount() >= 1) { - family = matcher.group(1); + String family = replace(matcher, familyReplacement, 1); + String brand = replace(matcher, brandReplacement, -1); + String model = replace(matcher, modelReplacement, 1); + + if (family != null) { + return new Device(family, brand, model); + } else { + return null; } - return family; + } } diff --git a/src/main/java/ua_parser/OS.java b/src/main/java/ua_parser/OS.java index d51f261..05c2eea 100644 --- a/src/main/java/ua_parser/OS.java +++ b/src/main/java/ua_parser/OS.java @@ -53,15 +53,15 @@ public boolean equals(Object other) { @Override public int hashCode() { - int h = family == null ? 0 : family.hashCode(); - h += major == null ? 0 : major.hashCode(); - h += minor == null ? 0 : minor.hashCode(); - h += patch == null ? 0 : patch.hashCode(); - h += patchMinor == null ? 0 : patchMinor.hashCode(); - return h; + int result = family != null ? family.hashCode() : 0; + result = 31 * result + (major != null ? major.hashCode() : 0); + result = 31 * result + (minor != null ? minor.hashCode() : 0); + result = 31 * result + (patch != null ? patch.hashCode() : 0); + result = 31 * result + (patchMinor != null ? patchMinor.hashCode() : 0); + return result; } - @Override + @Override public String toString() { return String.format("{\"family\": %s, \"major\": %s, \"minor\": %s, \"patch\": %s, \"patch_minor\": %s}", family == null ? Constants.EMPTY_STRING : '"' + family + '"', diff --git a/src/main/java/ua_parser/UserAgent.java b/src/main/java/ua_parser/UserAgent.java index 5a5b530..d5d81ae 100644 --- a/src/main/java/ua_parser/UserAgent.java +++ b/src/main/java/ua_parser/UserAgent.java @@ -51,11 +51,11 @@ public boolean equals(Object other) { @Override public int hashCode() { - int h = family == null ? 0 : family.hashCode(); - h += major == null ? 0 : major.hashCode(); - h += minor == null ? 0 : minor.hashCode(); - h += patch == null ? 0 : patch.hashCode(); - return h; + int result = family != null ? family.hashCode() : 0; + result = 31 * result + (major != null ? major.hashCode() : 0); + result = 31 * result + (minor != null ? minor.hashCode() : 0); + result = 31 * result + (patch != null ? patch.hashCode() : 0); + return result; } @Override diff --git a/src/test/java/ua_parser/DeviceTest.java b/src/test/java/ua_parser/DeviceTest.java index 0ecfba4..4da3a97 100644 --- a/src/test/java/ua_parser/DeviceTest.java +++ b/src/test/java/ua_parser/DeviceTest.java @@ -24,11 +24,13 @@ public class DeviceTest extends DataTest { protected Device getRandomInstance(long seed, StringGenerator g) { random.setSeed(seed); String family = g.getString(256); - return new Device(family); + String brand = g.getString(256); + String model = g.getString(256); + return new Device(family, brand, model); } @Override protected Device getBlankInstance() { - return new Device(null); + return new Device(null, null, null); } } \ No newline at end of file diff --git a/src/test/java/ua_parser/ParserTest.java b/src/test/java/ua_parser/ParserTest.java index 525a092..13d3025 100644 --- a/src/test/java/ua_parser/ParserTest.java +++ b/src/test/java/ua_parser/ParserTest.java @@ -81,10 +81,10 @@ public void testParseAll() { Client expected1 = new Client(new UserAgent("Firefox", "3", "5", "5"), new OS("Mac OS X", "10", "4", null, null), - new Device("Other")); + new Device("Other", null, null)); Client expected2 = new Client(new UserAgent("Mobile Safari", "5", "1", null), new OS("iOS", "5", "1", "1", null), - new Device("iPhone")); + new Device("iPhone", "Apple", "iPhone")); assertThat(parser.parse(agentString1), is(expected1)); assertThat(parser.parse(agentString2), is(expected2)); @@ -100,7 +100,9 @@ public void testReplacementQuoting() throws Exception { + " os_replacement: 'CatOS 9000'\n" + "device_parsers:\n" + " - regex: 'CashPhone-([\\$0-9]+)\\.(\\d+)\\.(\\d+)'\n" - + " device_replacement: 'CashPhone $1'\n"; + + " device_replacement: 'CashPhone $1'\n" + + " brand_replacement: 'CashPhone'\n" + + " model_replacement: '$1'\n"; Parser testParser = parserFromStringConfig(testConfig); Client result = testParser.parse("ABC12\\34 (CashPhone-$9.0.1 CatOS OH-HAI=/^.^\\=)"); From d19aae01ff66f47381ee150ec3c4a9f73b3eabe2 Mon Sep 17 00:00:00 2001 From: Michael Spiegel Date: Thu, 15 Jan 2015 23:58:14 -0500 Subject: [PATCH 2/4] Simplify device parsing. Use Matcher#appendReplacement() and Matcher#appendTail() to perform a single pass through the device replacement in order to avoid edge cases where the order of replacement changes the generated string. --- src/main/java/ua_parser/DeviceParser.java | 40 +++++++++++++---------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/src/main/java/ua_parser/DeviceParser.java b/src/main/java/ua_parser/DeviceParser.java index 1196608..293e098 100644 --- a/src/main/java/ua_parser/DeviceParser.java +++ b/src/main/java/ua_parser/DeviceParser.java @@ -17,10 +17,8 @@ package ua_parser; import java.util.ArrayList; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -30,7 +28,7 @@ * @author Steve Jiang (@sjiang) */ public class DeviceParser { - List patterns; + final List patterns; public DeviceParser(List patterns) { this.patterns = patterns; @@ -89,6 +87,8 @@ protected static class DevicePattern { private final String brandReplacement; private final String modelReplacement; + private static final Pattern GROUP_PATTERN = Pattern.compile("\\$(\\d+)"); + public DevicePattern(Pattern pattern, String familyReplacement, String brandReplacement, String modelReplacement) { this.pattern = pattern; this.familyReplacement = familyReplacement; @@ -96,17 +96,25 @@ public DevicePattern(Pattern pattern, String familyReplacement, String brandRepl this.modelReplacement = modelReplacement; } - private Set extractPositions(String input) { - int position = 0; - Set result = new HashSet(); - while ((position = input.indexOf('$', position)) != -1) { - char digit = input.charAt(position + 1); - if (Character.isDigit(digit)) { - result.add(Character.digit(digit, 10)); + private String performReplacement(Matcher matcher, String replacement) { + int count = matcher.groupCount(); + StringBuffer buffer = new StringBuffer(); + Matcher replaceMatcher = GROUP_PATTERN.matcher(replacement); + while (replaceMatcher.find()) { + String group = null; + try { + int id = Integer.parseInt(replaceMatcher.group(1)); + if (id >= 0 && id <= count) { + group = matcher.group(id); + } } - position++; + catch (NumberFormatException ignored) {} + catch (IllegalArgumentException ignored) {} + catch (IndexOutOfBoundsException ignored) {} + replaceMatcher.appendReplacement(buffer, group == null ? "" : Matcher.quoteReplacement(group)); } - return result; + replacement = buffer.toString(); + return replacement; } private String replace(Matcher matcher, String replacement, int position) { @@ -117,13 +125,9 @@ private String replace(Matcher matcher, String replacement, int position) { return null; } } + if (replacement.contains("$")) { - Set positions = extractPositions(replacement); - int count = matcher.groupCount(); - for (Integer i : positions) { - String group = (i > count) ? null : matcher.group(i); - replacement = replacement.replace("$" + i, (group == null) ? "" : group); - } + replacement = performReplacement(matcher, replacement); } replacement = replacement.trim(); if (replacement.length() == 0) { From 6386a0855d8fd084dd1e1a261a967fd230afc278 Mon Sep 17 00:00:00 2001 From: Michael Spiegel Date: Fri, 16 Jan 2015 00:03:03 -0500 Subject: [PATCH 3/4] Revert unnecessary changes to equals method. --- src/main/java/ua_parser/Device.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/ua_parser/Device.java b/src/main/java/ua_parser/Device.java index 8cc95a2..ae93c4c 100644 --- a/src/main/java/ua_parser/Device.java +++ b/src/main/java/ua_parser/Device.java @@ -39,11 +39,11 @@ public static Device fromMap(Map m) { } @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; + public boolean equals(Object other) { + if (this == other) return true; + if (!(other instanceof Device)) return false; - Device device = (Device) o; + Device device = (Device) other; if (brand != null ? !brand.equals(device.brand) : device.brand != null) return false; if (this.family != null ? !this.family.equals(device.family) : device.family != null) return false; From 364375c95d88f864c6a3b0a701a95ff5b1b4b84a Mon Sep 17 00:00:00 2001 From: Michael Spiegel Date: Fri, 16 Jan 2015 00:04:59 -0500 Subject: [PATCH 4/4] Finish cleanup of equals() method. --- src/main/java/ua_parser/Device.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/ua_parser/Device.java b/src/main/java/ua_parser/Device.java index ae93c4c..46ea6c7 100644 --- a/src/main/java/ua_parser/Device.java +++ b/src/main/java/ua_parser/Device.java @@ -40,14 +40,14 @@ public static Device fromMap(Map m) { @Override public boolean equals(Object other) { - if (this == other) return true; + if (other == this) return true; if (!(other instanceof Device)) return false; - Device device = (Device) other; + Device o = (Device) other; - if (brand != null ? !brand.equals(device.brand) : device.brand != null) return false; - if (this.family != null ? !this.family.equals(device.family) : device.family != null) return false; - if (model != null ? !model.equals(device.model) : device.model != null) return false; + if (brand != null ? !brand.equals(o.brand) : o.brand != null) return false; + if (this.family != null ? !this.family.equals(o.family) : o.family != null) return false; + if (model != null ? !model.equals(o.model) : o.model != null) return false; return true; }