Modify

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#12437 closed defect (fixed)

support multiple etag values in `no-tile-header`

Reported by: bastiK Owned by: team
Priority: normal Milestone: 16.02
Component: Core imagery Version:
Keywords: Cc:

Description

Currently, the following does not work:

<entry>
    <name>Mapbox Satellite</name>
    ...
    <no-tile-header name="ETag" value="&quot;067736a547cafe90014b4e59b6510abe&quot;" />
    <no-tile-header name="ETag" value="&quot;ee1f6802b0234046b553cbbc672ac7d9&quot;" />
    <no-tile-header name="ETag" value="&quot;9f5a2f1d7cc131e58befc2052c71c827&quot;" />
</entry>

It only respects the last entry.

(As a workaround one can use regexp, e.g.

    <no-tile-header name='ETag' value='"067736a547cafe90014b4e59b6510abe"|"ee1f6802b0234046b553cbbc672ac7d9"|"9f5a2f1d7cc131e58befc2052c71c827"' />

)

Build-Date:2016-01-24 13:27:02
Revision:9576
Is-Local-Build:true

Identification: JOSM/1.5 (9576 SVN en) Linux Ubuntu 14.04.3 LTS
Memory Usage: 614 MB / 1764 MB (344 MB allocated, but free)
Java version: 1.8.0_45-b14, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM

Attachments (0)

Change History (19)

comment:1 Changed 4 years ago by stoecker

Milestone: 16.02
Resolution: fixed
Status: newclosed

See #12425.

comment:2 Changed 4 years ago by stoecker

In 9613/josm:

fix #12437 - see #12425 - fix handling of equal no tile headers with different values - hope the related josm config change does not break anything

comment:3 Changed 4 years ago by stoecker

Resolution: fixed
Status: closedreopened

Preferences saving needs to be fixed.

comment:4 Changed 4 years ago by stoecker

In 9617/josm:

see #12437 - support new Map<String, List<String>> type in preferences

comment:5 Changed 4 years ago by stoecker

Now only conversion of old preferences is needed. But as this setting is anyway overwritten with the Maps stuff maybe that's unnecessary?

comment:6 Changed 4 years ago by wiktorn

In 9619/josm:

Finish removal of tile-size workaround code started in [9617].

Now WMTS TileSources with other than 256px tile size should work properly.
See #12437, #12186

Fix name of "valid-georeference", so it will be properly loaded from preferences

comment:7 Changed 4 years ago by stoecker

Resolution: fixed
Status: reopenedclosed

Seems nobody complains :-)

comment:8 Changed 4 years ago by wiktorn

Please check #12474

comment:9 Changed 4 years ago by simon04

In 9713/josm:

see #12437 see #12474 - Add (failing) unit tests

comment:10 Changed 4 years ago by simon04

Resolution: fixed
Status: closedreopened

What about using MultiMap<String, String> instead of Map<String, List<String>> (→r9617)?

comment:11 Changed 4 years ago by stoecker

Ah I overlooked that we have our own implementation. Yes. Why not.

I'd leave the Prefs implementation of Map<String, List<String>>, as it does no harm.

Is MultiMap correctly handled in prefs?

comment:12 Changed 4 years ago by bastiK

I would stick to Map<String, List<String>> as it uses only standard collections and does the same job as MultiMap.

comment:13 Changed 4 years ago by simon04

The problem with supporting Map<String, String> as well as Map<String, List<String>> are unchecked assignments as demonstrated with testDeserializeStructTicket12474 in r9713. The shape of the string (in the preference XML file) determines the type in Java which leads to #12474/ClassCastException w/o any compiler error/warning.

comment:14 in reply to:  13 Changed 4 years ago by stoecker

Replying to simon04:

The problem with supporting Map<String, String> as well as Map<String, List<String>> are unchecked assignments as demonstrated with testDeserializeStructTicket12474 in r9713. The shape of the string (in the preference XML file) determines the type in Java which leads to #12474/ClassCastException w/o any compiler error/warning.

That's only an issue when like here the preferences are changed and not updated as I left out the rewriting of existing prefs. Simple solution is to add a rewrite of the old entry in our preferences update section. But I'm still not sure how the two reports managed to get into that state, as the wrong entry normally gets immediately overwritten with a new correct one from the maps file.

You failing test simply says: Our deserializer isn't stable against JSON not matching the expectations. That's independent from the question what structure we use.

comment:15 Changed 4 years ago by stoecker

I fixed the prefs, but not your failing test.

comment:16 in reply to:  13 Changed 4 years ago by bastiK

Replying to simon04:

The problem with supporting Map<String, String> as well as Map<String, List<String>> are unchecked assignments as demonstrated with testDeserializeStructTicket12474 in r9713. The shape of the string (in the preference XML file) determines the type in Java which leads to #12474/ClassCastException w/o any compiler error/warning.

You are right, this doesn't work. It is kind of funny, we cannot even catch this kind of error proactively. Instead it will blow up somewhere in the calling code like a parcel bomb. This can be easily avoided by switching to MultiMap (or any other class != Map).

Type erasure is really one of the worst things in Java...

comment:17 Changed 4 years ago by bastiK

Resolution: fixed
Status: reopenedclosed

In 9755/josm:

add preference type MultiMap (closes #12437)

comment:18 Changed 4 years ago by bastiK

comment:19 Changed 4 years ago by bastiK

In 9756/josm:

fix test (see #12437)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.