Return-Path: X-Original-To: apmail-devicemap-dev-archive@www.apache.org Delivered-To: apmail-devicemap-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 4490918B85 for ; Mon, 13 Jul 2015 16:06:51 +0000 (UTC) Received: (qmail 75486 invoked by uid 500); 13 Jul 2015 16:06:51 -0000 Delivered-To: apmail-devicemap-dev-archive@devicemap.apache.org Received: (qmail 75445 invoked by uid 500); 13 Jul 2015 16:06:51 -0000 Mailing-List: contact dev-help@devicemap.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@devicemap.apache.org Delivered-To: mailing list dev@devicemap.apache.org Received: (qmail 75434 invoked by uid 99); 13 Jul 2015 16:06:50 -0000 Received: from mail-relay.apache.org (HELO mail-relay.apache.org) (140.211.11.15) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 13 Jul 2015 16:06:50 +0000 Received: from mail-ob0-f169.google.com (mail-ob0-f169.google.com [209.85.214.169]) by mail-relay.apache.org (ASF Mail Server at mail-relay.apache.org) with ESMTPSA id A20CD1A0323 for ; Mon, 13 Jul 2015 16:06:50 +0000 (UTC) Received: by obre1 with SMTP id e1so3652500obr.1 for ; Mon, 13 Jul 2015 09:06:49 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.202.212.151 with SMTP id l145mr29595939oig.54.1436803609721; Mon, 13 Jul 2015 09:06:49 -0700 (PDT) Received: by 10.202.85.136 with HTTP; Mon, 13 Jul 2015 09:06:49 -0700 (PDT) In-Reply-To: <0000014e87e83db5-f2a552b6-4dc2-44e2-9b8f-a2201e770acf-000000@us-west-2.amazonses.com> References: <55A019E6.1080005@stefan-seelmann.de>

<0000014e87e83db5-f2a552b6-4dc2-44e2-9b8f-a2201e770acf-000000@us-west-2.amazonses.com> Date: Mon, 13 Jul 2015 12:06:49 -0400 Message-ID: Subject: Re: Uncommitted patch DMAP-107 From: Reza Naghibi To: DeviceMap Dev Content-Type: multipart/alternative; boundary=001a113c9d2cc50543051ac3e811 --001a113c9d2cc50543051ac3e811 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Looks like "HTC One X" and "HTC One X+" match the same since 1.0 normalizes out the regex. Otherwise, im guessing the + is an error because its likely meant to be a literal \+ and not a regex. Since the patterns are identical, the device choosen is the first one reached during iteration. The patch changes this because the data structure changes from a HashMap to a HashSet, so iteration order is different. I could add logic to the ranking function to fix this, but at this point there is no use. Getting matching to properly work on the ODDR data will never be perfect because the data has many errors like the one above. So as always, the solution here is the fix the bad pattern. Also, some problems with your split() method. It shouldn't be static and you can remove the reference to Apache commons by using String.isEmpty(). Not sure we need the null check, but null is allowed in normalize(), so its best to err on the side of safety. Below is the corrected version: --- private List split(String text) { List nonemptyParts =3D new ArrayList(); String[] parts =3D TEXT_SPLIT_PATTERN.split(text); for (String part : parts) { String normalizedPart =3D Pattern.normalize(part); if (normalizedPart !=3D null && !normalizedPart.isEmpty()) { nonemptyParts.add(normalizedPart); } } return nonemptyParts; } --- Also, the style of the 1.0 Java client is to be explicit with imports and not use the wildcard. Just a small style nitpick. So if you can correct the above split() function (and fix the imports), your patch should be good to go with the HTC One X tests removed. I will put this in the ticket. Thanks On Mon, Jul 13, 2015 at 10:53 AM, Reza Naghibi wrote: > This sounds good. Why does the patch remove 2 user agents from the test > file? > > On Mon, Jul 13, 2015 at 6:06 AM, Volkan Yaz=C4=B1c=C4=B1 > wrote: > > > Then if there are no objects, I will proceed patching 1.x this week. > > > > On Mon, Jul 13, 2015 at 11:59 AM, Werner Keil > > wrote: > > > > > If it improves the performance, as Stefan mentioned, I see no reason > why > > > you should wait. > > > 2.x is very likely to be much different, most importantly using JSON, > so > > > the patch could not even match there as it does now;-) > > > > > > Cheers, > > > Werner > > > > > > On Mon, Jul 13, 2015 at 11:39 AM, Volkan Yaz=C4=B1c=C4=B1 < > volkan.yazici@gmail.com > > > > > > wrote: > > > > > > > Hello Reza, > > > > > > > > I was waiting for the 2.0 release of the client to commit those > > changes. > > > Do > > > > you want me to commit them to the 1.x branch? > > > > > > > > Best. > > > > > > > > On Fri, Jul 10, 2015 at 9:40 PM, Reza Naghibi > > wrote: > > > > > > > > > No reason. This patch was submitted by Volkan before he had commi= t > > > > rights, > > > > > so he posted it on JIRA. Volkan is now a committer and he is > allowed > > to > > > > > work on the 1.0 clients, so im pretty sure he is free to commit t= he > > > > patch. > > > > > > > > > > On Fri, Jul 10, 2015 at 3:15 PM, Stefan Seelmann < > > > > mail@stefan-seelmann.de> > > > > > wrote: > > > > > > > > > > > Hi, > > > > > > > > > > > > I browsed through the devicemap-client Java code and saw some > > > > > > performance optimizations in DeviceMapClient.classify(). Then I > saw > > > > that > > > > > > there is already an issue and patch available since last > December. > > Is > > > > > > there a reason why it is not yet committed? The patch currently > > has a > > > > > > compile error as commons-lang is not in dependencies but that's > > easy > > > to > > > > > > fix. > > > > > > > > > > > > Kind Regards, > > > > > > Stefan > > > > > > > > > > > > [1] https://issues.apache.org/jira/browse/DMAP-107 > > > > > > > > > > > > > > > > > > > > > --001a113c9d2cc50543051ac3e811--