Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preset correctness fixes #2707

Merged
merged 3 commits into from
Oct 31, 2024
Merged

Preset correctness fixes #2707

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1532c56
Correctness fixes
simonpoole Oct 29, 2024
03effec
Add traffic_sign to hardwired object tags
simonpoole Oct 29, 2024
eee4773
Increase test timeout
simonpoole Oct 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -761,7 +761,7 @@ task jacocoTestReport(type:JacocoReport, dependsOn: "testCurrentDebugUnitTest")
// see https://marathonlabs.github.io/marathon/doc/configuration.html
marathon {
uncompletedTestRetryQuota = 6
testOutputTimeoutMillis = 720000
testOutputTimeoutMillis = 1080000
poolingStrategy {
operatingSystem = true
}
Expand Down
11 changes: 6 additions & 5 deletions src/main/java/de/blau/android/osm/Tags.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ public static boolean isLikeAName(@NonNull String key) {
public static final String KEY_SIDEWALK = "sidewalk";
public static final String KEY_BRIDGE = "bridge";
public static final String KEY_TUNNEL = "tunnel";
public static final String KEY_TRAFFIC_SIGN = "traffic_sign";
public static final String VALUE_CULVERT = "culvert";
public static final String KEY_BARRIER = "barrier";
public static final String VALUE_RETAINING_WALL = "retaining_wall";
Expand Down Expand Up @@ -298,7 +299,7 @@ public static String getDirectionKey(@Nullable PresetItem presetItem, @NonNull O
for (PresetField field : presetItem.getFields().values()) {
if (field instanceof PresetTagField && ((PresetTagField) field).getValueType() == ValueType.CARDINAL_DIRECTION) {
String key = ((PresetTagField) field).getKey();
if (key != null && e.hasTagKey(key)) {
if (e.hasTagKey(key)) {
return key;
}
}
Expand Down Expand Up @@ -577,10 +578,10 @@ public static boolean isWayMetric(@Nullable final String key) {
* An set of tags considered 'important'. These are typically tags that define real-world objects and not properties
* of such.
*/
public static final Set<String> IMPORTANT_TAGS = Collections.unmodifiableSet(
new HashSet<>(Arrays.asList(KEY_HIGHWAY, KEY_BARRIER, KEY_WATERWAY, KEY_RAILWAY, KEY_AEROWAY, KEY_AERIALWAY, KEY_POWER, KEY_MAN_MADE, KEY_BUILDING,
KEY_LEISURE, KEY_AMENITY, KEY_OFFICE, KEY_SHOP, KEY_CRAFT, KEY_EMERGENCY, KEY_TOURISM, KEY_HISTORIC, KEY_LANDUSE, KEY_MILITARY, KEY_NATURAL,
KEY_BOUNDARY, KEY_PLACE, KEY_TYPE, KEY_ENTRANCE, KEY_PIPELINE, KEY_HEALTHCARE, KEY_GEOLOGICAL, KEY_ADDR_HOUSENUMBER, KEY_ADDR_HOUSENAME)));
public static final Set<String> IMPORTANT_TAGS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(KEY_HIGHWAY, KEY_BARRIER, KEY_WATERWAY,
KEY_RAILWAY, KEY_AEROWAY, KEY_AERIALWAY, KEY_POWER, KEY_MAN_MADE, KEY_BUILDING, KEY_LEISURE, KEY_AMENITY, KEY_OFFICE, KEY_SHOP, KEY_CRAFT,
KEY_EMERGENCY, KEY_TOURISM, KEY_HISTORIC, KEY_LANDUSE, KEY_MILITARY, KEY_NATURAL, KEY_BOUNDARY, KEY_PLACE, KEY_TYPE, KEY_ENTRANCE, KEY_PIPELINE,
KEY_HEALTHCARE, KEY_GEOLOGICAL, KEY_ADDR_HOUSENUMBER, KEY_ADDR_HOUSENAME, KEY_TRAFFIC_SIGN)));
/** ways that we might want to render differently */
public static final Set<String> WAY_TAGS = Collections
.unmodifiableSet(new HashSet<>(Arrays.asList(KEY_BUILDING, KEY_RAILWAY, KEY_LEISURE, KEY_LANDUSE, KEY_WATERWAY, KEY_NATURAL, KEY_ADDR_INTERPOLATION,
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/de/blau/android/presets/Preset.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1182,7 +1182,7 @@ public static PresetItem findBestMatch(@Nullable Preset[] presets, @Nullable Map
* @return the best match or null
*/
private static PresetItem findBestMatch(Map<String, String> tags, List<String> regions, ElementType elementType, Set<PresetItem> possibleMatches) {
int bestMatchStrength = 0;
int bestMatchStrength = Integer.MIN_VALUE;
PresetItem bestMatch = null;
// Find best
// always prioritize presets with fixed keys that match
Expand Down
7 changes: 4 additions & 3 deletions src/main/java/de/blau/android/presets/PresetCheckField.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@
import de.blau.android.util.StringWithDescription;

public class PresetCheckField extends PresetTagField {
private static final long serialVersionUID = 1L;
private static final long serialVersionUID = 2L;

/**
* on value
*/
final StringWithDescription onValue;

/**
* on value
* off value
*/
private StringWithDescription offValue = null;

Expand All @@ -41,6 +41,7 @@ public PresetCheckField(@NonNull String key, @NonNull StringWithDescription onVa
public PresetCheckField(@NonNull PresetCheckField field) {
super(field);
this.onValue = field.onValue;
this.offValue = field.offValue;
}

/**
Expand Down Expand Up @@ -111,6 +112,6 @@ public void toXml(XmlSerializer s) throws IllegalArgumentException, IllegalState

@Override
public String toString() {
return super.toString() + " onValue: " + onValue;
return super.toString() + " onValue: " + onValue + " offValue " + offValue + " default " + getDefaultValue();
}
}
2 changes: 1 addition & 1 deletion src/main/java/de/blau/android/presets/PresetCheckGroupField.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import ch.poole.poparser.Po;

public class PresetCheckGroupField extends PresetTagField {
private static final long serialVersionUID = 1L;
private static final long serialVersionUID = 2L;

private Map<String, PresetCheckField> checks = new LinkedHashMap<>();

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/de/blau/android/presets/PresetChunk.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

public class PresetChunk extends PresetItem {

private static final long serialVersionUID = 1L;
private static final long serialVersionUID = 2L;

private List<StringWithDescription> listValues = null;

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/de/blau/android/presets/PresetComboField.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import de.blau.android.util.StringWithDescription;

public class PresetComboField extends PresetTagField implements PresetFieldJavaScript {
private static final long serialVersionUID = 1L;
private static final long serialVersionUID = 2L;

private StringWithDescription[] values;

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/de/blau/android/presets/PresetElement.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
*/
public abstract class PresetElement extends Regionalizable implements Serializable {

private static final long serialVersionUID = 1L;
private static final long serialVersionUID = 2L;

private static final int VIEW_PADDING = 4;
private static final int VIEW_SIDE_LENGTH = 72;
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/de/blau/android/presets/PresetFixedField.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import de.blau.android.util.StringWithDescription;

public class PresetFixedField extends PresetTagField {
private static final long serialVersionUID = 1L;
private static final long serialVersionUID = 2L;

private final StringWithDescription value;
private Boolean isObject;
Expand All @@ -33,7 +33,7 @@ public PresetFixedField(@NonNull String key, @NonNull StringWithDescription valu
*
* @param field PresetFixedField to copy
*/
public PresetFixedField(PresetFixedField field) {
public PresetFixedField(@NonNull PresetFixedField field) {
super(field);
this.value = field.value;
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/de/blau/android/presets/PresetFormattingField.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

public abstract class PresetFormattingField extends PresetField {

private static final long serialVersionUID = 1L;
private static final long serialVersionUID = 2L;

protected PresetFormattingField() {
super();
Expand Down
53 changes: 33 additions & 20 deletions src/main/java/de/blau/android/presets/PresetItem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
/** Represents a preset item (e.g. "footpath", "grocery store") */
public class PresetItem extends PresetElement {

private static final long serialVersionUID = 1L;
private static final long serialVersionUID = 2L;

private static final int TAG_LEN = Math.min(LOG_TAG_LEN, PresetItem.class.getSimpleName().length());
private static final String DEBUG_TAG = PresetItem.class.getSimpleName().substring(0, TAG_LEN);
Expand Down Expand Up @@ -231,26 +231,38 @@ synchronized void buildSearchIndex() {
}
}
}
if (tagField instanceof PresetCheckField) {
addCheckToSearchIndex((PresetCheckField) tagField);
}
} else {
for (PresetCheckField check : ((PresetCheckGroupField) tagField).getCheckFields()) {
preset.addToSearchIndex(check.getKey(), textContext, this);
String hint = tagField.getHint();
if (hint != null) {
preset.addToSearchIndex(hint, textContext, this);
}
StringWithDescription value = check.getOnValue();
String valueContext = check.getValueContext();
addValueAndDescriptionToSearchIndex(value.getValue(), value.getDescription(), valueContext);
value = check.getOffValue();
if (value != null && !"".equals(value.getValue())) {
addValueAndDescriptionToSearchIndex(value.getValue(), value.getDescription(), valueContext);
}
addCheckToSearchIndex(check);
}
}
}
}
}

/**
* Add a check values to the search index
*
* @param check the search field
*/
private void addCheckToSearchIndex(@NonNull PresetCheckField check) {
StringWithDescription value = check.getOnValue();
String valueContext = check.getValueContext();
addValueAndDescriptionToSearchIndex(value.getValue(), value.getDescription(), valueContext);
value = check.getOffValue();
if (value != null && !"".equals(value.getValue())) {
addValueAndDescriptionToSearchIndex(value.getValue(), value.getDescription(), valueContext);
}
}

/**
* Get A PresetTagField for a key
*
Expand Down Expand Up @@ -807,19 +819,20 @@ public List<PresetItem> getLinkedPresets(boolean noPrimary, @Nullable Preset[] o
presets.remove(preset);
}
presets.add(0, preset); // move this Preset to front
if (linkedPresetItems != null) {
for (PresetItemLink pl : linkedPresetItems) {
for (Preset preset : presets) {
if (preset != null) {
PresetItem candidateItem = preset.getItemByName(pl.getPresetName(), regions);
if (candidateItem != null) {
if (!noPrimary || !candidateItem.isObject(preset)) { // remove primary objects
result.add(candidateItem);
}
break;
} else {
Log.e(DEBUG_TAG, "Couldn't find linked preset " + pl.getPresetName() + " for regions " + regions);
if (linkedPresetItems == null) {
return result;
}
for (PresetItemLink pl : linkedPresetItems) {
for (Preset preset : presets) {
if (preset != null) {
PresetItem candidateItem = preset.getItemByName(pl.getPresetName(), regions);
if (candidateItem != null) {
if (!noPrimary || !candidateItem.isObject(preset)) { // remove primary objects
result.add(candidateItem);
}
break;
} else {
Log.e(DEBUG_TAG, "Couldn't find linked preset " + pl.getPresetName() + " for regions " + regions);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/de/blau/android/presets/PresetLabelField.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import ch.poole.poparser.Po;

public class PresetLabelField extends PresetFormattingField {
private static final long serialVersionUID = 1L;
private static final long serialVersionUID = 2L;

private String label;

Expand Down
8 changes: 5 additions & 3 deletions src/main/java/de/blau/android/presets/PresetMRUInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,11 @@ public static void addToPresetGroup(@NonNull PresetGroup group, @NonNull Preset[
}
Collections.sort(paths, (TimestampedPresetElementPath p1, TimestampedPresetElementPath p2) -> Long.compare(p2.getTimestamp(), p1.getTimestamp()));
for (PresetElementPath path : paths) {
final PresetItem item = (PresetItem) Preset.getElementByPath(parent.get(path).getRootGroup(), path, regions, false);
if (item != null) {
group.addElement(item, false);
final PresetElement element = Preset.getElementByPath(parent.get(path).getRootGroup(), path, regions, false);
if (element instanceof PresetItem) {
group.addElement(element, false);
} else {
Log.e(DEBUG_TAG, "Unexpected element for " + path);
}
}
}
Expand Down
68 changes: 35 additions & 33 deletions src/main/java/de/blau/android/presets/PresetParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,8 +278,8 @@ private void parseTop(String name, Attributes attr) throws SAXException {
throw new SAXException("Nested chunks are not allowed");
}
if (inOptionalSection) {
Log.e(DEBUG_TAG, "Chunk " + attr.getValue(ID) + " optional must be nested");
throw new SAXException("optional must be nexted");
Log.e(DEBUG_TAG, "Chunk " + attr.getValue(ID) + " optional must be nested"); // NOSONAR
throw new SAXException("optional must be nested");
}
type = attr.getValue(TYPE);
if (type == null) {
Expand Down Expand Up @@ -573,43 +573,45 @@ private void parseItem(@NonNull String name, @NonNull Attributes attr) throws SA
case REFERENCE:
PresetChunk chunk = chunks.get(attr.getValue(REF)); // note this assumes that there are no
// forward references
if (chunk != null) {
if (chunk.getListValues() != null) {
if (listValues != null) {
listValues.addAll(chunk.getListValues());
} else {
Log.d(DEBUG_TAG, "chunk with LIST_ENTRY sequence referenced outside of COMBO/MULTISELECT");
throw new SAXException("chunk with LIST_ENTRY sequence referenced outside of COMBO/MULTISELECT");
}
if (chunk == null) {
Log.e(DEBUG_TAG, "Chunk " + attr.getValue(REF) + " not found"); // NOSONAR
break;
}
if (chunk.getListValues() != null) {
if (listValues != null) {
listValues.addAll(chunk.getListValues());
} else {
if (inOptionalSection) {
// fixed tags don't make sense in an optional section, and doesn't seem to happen in
// practice
if (chunk.getFixedTagCount() > 0) {
Log.e(DEBUG_TAG, "Chunk " + chunk.name + " has fixed tags but is used in an optional section");
}
for (PresetField f : chunk.getFields().values()) {
if (f instanceof PresetTagField) {
key = ((PresetTagField) f).getKey();
// don't overwrite exiting fields
if (!currentItem.hasKey(key)) {
addOptionalCopy(f);
} else {
Log.w(DEBUG_TAG, "PresetItem " + currentItem.getName() + " chunk " + attr.getValue(REF) + " field " + key
+ " overwrites existing field");
}
} else {
Log.d(DEBUG_TAG, "chunk with LIST_ENTRY sequence referenced outside of COMBO/MULTISELECT");
throw new SAXException("chunk with LIST_ENTRY sequence referenced outside of COMBO/MULTISELECT");
}
} else {
if (inOptionalSection) {
// fixed tags don't make sense in an optional section, and doesn't seem to happen in
// practice
if (chunk.getFixedTagCount() > 0) {
Log.e(DEBUG_TAG, "Chunk " + chunk.name + " has fixed tags but is used in an optional section"); // NOSONAR
}
for (PresetField f : chunk.getFields().values()) {
if (f instanceof PresetTagField) {
key = ((PresetTagField) f).getKey();
// don't overwrite exiting fields
if (!currentItem.hasKey(key)) {
addOptionalCopy(f);
} else {
Log.w(DEBUG_TAG, "PresetItem " + currentItem.getName() + " chunk " + attr.getValue(REF) + " field " + key
+ " overwrites existing field");
}
} else {
addOptionalCopy(f);
}
} else {
currentItem.addAllFixedFields(chunk.getFixedTags());
currentItem.addAllFields(chunk.getFields());
}
currentItem.addAllRoles(chunk.getRoles());
currentItem.addAllLinkedPresetItems(chunk.getLinkedPresetItems());
currentItem.addAllAlternativePresetItems(chunk.getAlternativePresetItems());
} else {
currentItem.addAllFixedFields(chunk.getFixedTags());
currentItem.addAllFields(chunk.getFields());
}
currentItem.addAllRoles(chunk.getRoles());
currentItem.addAllLinkedPresetItems(chunk.getLinkedPresetItems());
currentItem.addAllAlternativePresetItems(chunk.getAlternativePresetItems());
}
break;
case LIST_ENTRY:
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/de/blau/android/presets/PresetSpaceField.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import ch.poole.poparser.Po;

public class PresetSpaceField extends PresetFormattingField implements FieldHeight {
private static final long serialVersionUID = 1L;
private static final long serialVersionUID = 2L;

private int height;

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/de/blau/android/presets/PresetTagField.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import ch.poole.poparser.Po;

public abstract class PresetTagField extends PresetField {
private static final long serialVersionUID = 1L;
private static final long serialVersionUID = 2L;

/**
* Key this field is for
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/de/blau/android/presets/PresetTextField.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import androidx.annotation.NonNull;

public class PresetTextField extends PresetTagField implements PresetFieldJavaScript {
private static final long serialVersionUID = 1L;
private static final long serialVersionUID = 2L;

/**
* Script for pre-filling text fields
Expand Down
Loading
Loading