Skip to content

Commit

Permalink
[GEOS-9023] SLDService range rules building does not follow RangedCla…
Browse files Browse the repository at this point in the history 
…ssifier specs, breaking quantile classification
  • Loading branch information
@aaime
aaime committed Nov 29, 2018
1 parent ccbce66 commit 6b97466
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 63 deletions.
53 changes: 16 additions & 37 deletions ...sion/sldService/src/main/java/org/geoserver/sldservice/utils/classifier/RulesBuilder.java
View file
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/ */
package org.geoserver.sldservice.utils.classifier; package org.geoserver.sldservice.utils.classifier;


import java.awt.Color; import java.awt.*;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Iterator; import java.util.Iterator;
import java.util.List; import java.util.List;
Expand Down Expand Up @@ -357,20 +357,9 @@ public List<Rule> openRangedRules(
try { try {
/* First class */ /* First class */
r = SF.createRule(); r = SF.createRule();
/*if (groups.getMin(0).equals(groups.getMax(0))) { f = FF.less(att, FF.literal(groups.getMax(0)));
f = ff.equals(att, ff.literal(groups.getMax(0)));
r.setFilter(f);
r.setTitle(ff.literal(groups.getMax(0)).toString());
list.add(r);
} else {
f = ff.lessOrEqual(att, ff.literal(groups.getMax(0)));
r.setFilter(f);
r.setTitle(" <= " + ff.literal(groups.getMax(0)));
list.add(r);
}*/
f = FF.lessOrEqual(att, FF.literal(groups.getMax(0)));
r.setFilter(f); r.setFilter(f);
r.setTitle(" <= " + FF.literal(groups.getMax(0))); r.setTitle(" < " + FF.literal(groups.getMax(0)));
list.add(r); list.add(r);
for (int i = 1; i < groups.getSize() - 1; i++) { for (int i = 1; i < groups.getSize() - 1; i++) {
r = SF.createRule(); r = SF.createRule();
Expand All @@ -382,33 +371,22 @@ public List<Rule> openRangedRules(
} else { } else {
f = f =
FF.and( FF.and(
FF.greater(att, FF.literal(groups.getMin(i))), FF.greaterOrEqual(att, FF.literal(groups.getMin(i))),
FF.lessOrEqual(att, FF.literal(groups.getMax(i)))); FF.less(att, FF.literal(groups.getMax(i))));
r.setTitle( r.setTitle(
" > " " >= "
+ FF.literal(groups.getMin(i)) + FF.literal(groups.getMin(i))
+ " AND <= " + " AND < "
+ FF.literal(groups.getMax(i))); + FF.literal(groups.getMax(i)));
r.setFilter(f); r.setFilter(f);
list.add(r); list.add(r);
} }
} }
/* Last class */ /* Last class */
r = SF.createRule(); r = SF.createRule();
/*if (groups.getMin(groups.getSize() - 1).equals(groups.getMax(groups.getSize() - 1))) { f = FF.greaterOrEqual(att, FF.literal(groups.getMin(groups.getSize() - 1)));
f = ff.equals(att, ff.literal(groups.getMin(groups.getSize() - 1)));
r.setFilter(f);
r.setTitle(ff.literal(groups.getMin(groups.getSize() - 1)).toString());
list.add(r);
} else {
f = ff.greater(att, ff.literal(groups.getMin(groups.getSize() - 1)));
r.setFilter(f);
r.setTitle(" > " + ff.literal(groups.getMin(groups.getSize() - 1)));
list.add(r);
}*/
f = FF.greater(att, FF.literal(groups.getMin(groups.getSize() - 1)));
r.setFilter(f); r.setFilter(f);
r.setTitle(" > " + FF.literal(groups.getMin(groups.getSize() - 1))); r.setTitle(" >= " + FF.literal(groups.getMin(groups.getSize() - 1)));
list.add(r); list.add(r);
return list; return list;
} catch (Exception e) { } catch (Exception e) {
Expand Down Expand Up @@ -449,7 +427,6 @@ public List<Rule> closedRangedRules(
r = SF.createRule(); r = SF.createRule();
for (int i = 0; i < groups.getSize(); i++) { for (int i = 0; i < groups.getSize(); i++) {
r = SF.createRule(); r = SF.createRule();
if (i > 0 && groups.getMax(i).equals(groups.getMax(i - 1))) continue;
if (groups.getMin(i).equals(groups.getMax(i))) { if (groups.getMin(i).equals(groups.getMax(i))) {
f = FF.equals(att, FF.literal(groups.getMin(i))); f = FF.equals(att, FF.literal(groups.getMin(i)));
r.setTitle(FF.literal(groups.getMin(i)).toString()); r.setTitle(FF.literal(groups.getMin(i)).toString());
Expand All @@ -458,14 +435,16 @@ public List<Rule> closedRangedRules(
} else { } else {
f = f =
FF.and( FF.and(
i == 0 FF.greaterOrEqual(att, FF.literal(groups.getMin(i))),
? FF.greaterOrEqual(att, FF.literal(groups.getMin(i))) i == (groups.getSize() - 1)
: FF.greater(att, FF.literal(groups.getMin(i))), ? FF.lessOrEqual(att, FF.literal(groups.getMax(i)))
FF.lessOrEqual(att, FF.literal(groups.getMax(i)))); : FF.less(att, FF.literal(groups.getMax(i))));

r.setTitle( r.setTitle(
" >= " " >= "
+ FF.literal(groups.getMin(i)) + FF.literal(groups.getMin(i))
+ " AND <= " + " AND "
+ (i == (groups.getSize() - 1) ? "<=" : "<")
+ FF.literal(groups.getMax(i))); + FF.literal(groups.getMax(i)));
r.setFilter(f); r.setFilter(f);
list.add(r); list.add(r);
Expand Down
24 changes: 12 additions & 12 deletions src/extension/sldService/src/test/java/org/geoserver/sldservice/rest/ClassifierTest.java
View file
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
import org.junit.AfterClass; import org.junit.AfterClass;
import org.junit.BeforeClass; import org.junit.BeforeClass;
import org.junit.Test; import org.junit.Test;
import org.opengis.filter.PropertyIsLessThan;
import org.opengis.geometry.Envelope; import org.opengis.geometry.Envelope;
import org.opengis.parameter.GeneralParameterDescriptor; import org.opengis.parameter.GeneralParameterDescriptor;
import org.opengis.parameter.GeneralParameterValue; import org.opengis.parameter.GeneralParameterValue;
Expand Down Expand Up @@ -378,9 +379,9 @@ public void testClassifyOpenRange() throws Exception {
Rule[] rules = Rule[] rules =
checkRules( checkRules(
resultXml.replace("<Rules>", sldPrefix).replace("</Rules>", sldPostfix), 3); resultXml.replace("<Rules>", sldPrefix).replace("</Rules>", sldPostfix), 3);
checkRule(rules[0], "#690000", org.opengis.filter.PropertyIsLessThanOrEqualTo.class); checkRule(rules[0], "#690000", org.opengis.filter.PropertyIsLessThan.class);
checkRule(rules[1], "#B40000", org.opengis.filter.And.class); checkRule(rules[1], "#B40000", org.opengis.filter.And.class);
checkRule(rules[2], "#FF0000", org.opengis.filter.PropertyIsGreaterThan.class); checkRule(rules[2], "#FF0000", org.opengis.filter.PropertyIsGreaterThanOrEqualTo.class);
} }


@Test @Test
Expand Down Expand Up @@ -425,10 +426,10 @@ public void testEqualArea() throws Exception {
resultXml.replace("<Rules>", sldPrefix).replace("</Rules>", sldPostfix), 4); resultXml.replace("<Rules>", sldPrefix).replace("</Rules>", sldPostfix), 4);


// not enough polygons to make 5 rules, only 4 // not enough polygons to make 5 rules, only 4
assertEquals(" <= 43.0", rules[0].getDescription().getTitle().toString()); assertEquals(" < 43.0", rules[0].getDescription().getTitle().toString());
assertEquals(" > 43.0 AND <= 61.0", rules[1].getDescription().getTitle().toString()); assertEquals(" >= 43.0 AND < 61.0", rules[1].getDescription().getTitle().toString());
assertEquals(" > 61.0 AND <= 90.0", rules[2].getDescription().getTitle().toString()); assertEquals(" >= 61.0 AND < 90.0", rules[2].getDescription().getTitle().toString());
assertEquals(" > 90.0", rules[3].getDescription().getTitle().toString()); assertEquals(" >= 90.0", rules[3].getDescription().getTitle().toString());
} }


@Test @Test
Expand All @@ -451,9 +452,9 @@ public void testEqualAreaWithinBounds() throws Exception {
resultXml.replace("<Rules>", sldPrefix).replace("</Rules>", sldPostfix), 3); resultXml.replace("<Rules>", sldPrefix).replace("</Rules>", sldPostfix), 3);


// also due to bbox restriction, not enough polygons to make 5 rules, only 3 // also due to bbox restriction, not enough polygons to make 5 rules, only 3
assertEquals(" <= 43.0", rules[0].getDescription().getTitle().toString()); assertEquals(" < 43.0", rules[0].getDescription().getTitle().toString());
assertEquals(" > 43.0 AND <= 90.0", rules[1].getDescription().getTitle().toString()); assertEquals(" >= 43.0 AND < 90.0", rules[1].getDescription().getTitle().toString());
assertEquals(" > 90.0", rules[2].getDescription().getTitle().toString()); assertEquals(" >= 90.0", rules[2].getDescription().getTitle().toString());
} }


@Test @Test
Expand Down Expand Up @@ -598,9 +599,8 @@ public void testNormalize() throws Exception {
checkRules( checkRules(
resultXml.replace("<Rules>", sldPrefix).replace("</Rules>", sldPostfix), 3); resultXml.replace("<Rules>", sldPrefix).replace("</Rules>", sldPostfix), 3);


checkRule(rules[0], "#690000", org.opengis.filter.PropertyIsLessThanOrEqualTo.class); checkRule(rules[0], "#690000", org.opengis.filter.PropertyIsLessThan.class);
org.opengis.filter.PropertyIsLessThanOrEqualTo filter = PropertyIsLessThan filter = (PropertyIsLessThan) rules[0].getFilter();
(org.opengis.filter.PropertyIsLessThanOrEqualTo) rules[0].getFilter();
assertTrue(filter.getExpression1() instanceof FilterFunction_parseDouble); assertTrue(filter.getExpression1() instanceof FilterFunction_parseDouble);
} }


Expand Down
19 changes: 5 additions & 14 deletions .../sldService/src/test/java/org/geoserver/sldservice/utils/classifier/RulesBuilderTest.java
View file
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -263,19 +263,10 @@ public void testEqualAreaClassification() throws Exception {
List<Rule> rules = List<Rule> rules =
builder.equalAreaClassification( builder.equalAreaClassification(
polygonCollection, "foo", Integer.class, 5, false, false); polygonCollection, "foo", Integer.class, 5, false, false);
// temporary invalid result due to GEOS-9023 assertEquals(4, rules.size());
assertEquals(3, rules.size()); assertEquals(CQL.toFilter("foo >= 4.0 AND foo < 43.0"), rules.get(0).getFilter());
assertEquals(CQL.toFilter("foo >= 4.0 AND foo <= 43.0"), rules.get(0).getFilter()); assertEquals(CQL.toFilter("foo >= 43.0 AND foo < 61.0"), rules.get(1).getFilter());
assertEquals(CQL.toFilter("foo > 43.0 AND foo <= 61.0"), rules.get(1).getFilter()); assertEquals(CQL.toFilter("foo >= 61.0 AND foo < 90.0"), rules.get(2).getFilter());
assertEquals(CQL.toFilter("foo > 61.0 AND foo <= 90.0"), rules.get(2).getFilter()); assertEquals(CQL.toFilter("foo = 90.0"), rules.get(3).getFilter());

// this would be the right result, to be fixed for GEOS-9023
// assertEquals(4, rules.size());
// assertEquals(CQL.toFilter("foo >= 4.0 AND foo < 43.0"), rules.get(0).getFilter());
// assertEquals(CQL.toFilter("foo >= 43.0 AND foo < 61.0"),
// rules.get(1).getFilter());
// assertEquals(CQL.toFilter("foo >= 61.0 AND foo < 90.0"),
// rules.get(2).getFilter());
// assertEquals(CQL.toFilter("foo = 90.0"), rules.get(3).getFilter());
} }
} }

0 comments on commit 6b97466

Please sign in to comment.