Fix modification of map while iterating in SimpleJsonFormat
Review URL: https://codereview.chromium.org//12210151
git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@18527 260f80e4-7a28-3924-810f-c04153c831b5
diff --git a/pkg/pkg.status b/pkg/pkg.status
index a241322..69fbaee 100644
--- a/pkg/pkg.status
+++ b/pkg/pkg.status
@@ -49,7 +49,6 @@
[ $runtime == vm ]
intl/test/find_default_locale_browser_test: Skip
intl/test/date_time_format_http_request_test: Skip
-serialization/test/serialization_test: Pass, Fail # Temporary workaround for flakiness
[ $runtime == vm && $system == windows ]
intl/test/find_default_locale_standalone_test: Fail # Issue 8110
diff --git a/pkg/serialization/lib/src/format.dart b/pkg/serialization/lib/src/format.dart
index ab602c9..a1726f0 100644
--- a/pkg/serialization/lib/src/format.dart
+++ b/pkg/serialization/lib/src/format.dart
@@ -154,9 +154,15 @@
* to turn References into a nested List/Map.
*/
jsonifyEntry(map, Writer w) {
+ // Note, if this is a Map, and the key might be a reference, we need to
+ // bend over backwards to avoid concurrent modifications. Non-string keys
+ // won't actually work if we try to write this to json, but might happen
+ // if e.g. sending between isolates.
+ var updates = new Map();
keysAndValues(map).forEach((key, value) {
- if (value is Reference) map[key] = w.stateForReference(value);
+ if (value is Reference) updates[key] = w.stateForReference(value);
});
+ updates.forEach((k, v) => map[k] = v);
}
/**
diff --git a/pkg/serialization/test/serialization_test.dart b/pkg/serialization/test/serialization_test.dart
index d45f562..5a9cc49 100644
--- a/pkg/serialization/test/serialization_test.dart
+++ b/pkg/serialization/test/serialization_test.dart
@@ -386,6 +386,20 @@
expect(a2.city, "Seattle");
});
+ test("Straight JSON format, non-string key", () {
+ // This tests what happens if we have a key that's not a string. That's
+ // not allowed by json, so we don't actually turn it into a json string,
+ // but someone might reasonably convert to a json-able structure without
+ // going through the string representation.
+ var p1 = new Person()..name = 'Alice'..address = a1;
+ var s = new Serialization()
+ ..addRule(new PersonRuleReturningMapWithNonStringKey());
+ var p2 = writeAndReadBack(s,
+ new SimpleJsonFormat(storeRoundTripInfo: true), p1);
+ expect(p2.name, "Alice");
+ expect(p2.address.street, "N 34th");
+ });
+
test("Root is a Map", () {
// Note that we can't use the usual round-trip test because it has cycles.
var p1 = new Person()..name = 'Alice'..address = a1;
@@ -704,3 +718,31 @@
node.children = state[2];
}
}
+
+/**
+ * This is a rather silly rule which stores the address data in a map,
+ * but inverts the keys and values, so we look up values and find the
+ * corresponding key. This will lead to maps that aren't allowed in JSON,
+ * and which have keys that need to be dereferenced.
+ */
+class PersonRuleReturningMapWithNonStringKey extends CustomRule {
+ appliesTo(instance, _) => instance is Person;
+ getState(instance) {
+ return new Map()
+ ..[instance.name] = "name"
+ ..[instance.address] = "address";
+ }
+ create(state) => new Person();
+ setState(Person a, state) {
+ a.name = findValue("name", state);
+ a.address = findValue("address", state);
+ }
+ findValue(String key, Map state) {
+ var answer;
+ for (var each in state.keys) {
+ var value = state[each];
+ if (value == key) return each;
+ }
+ return null;
+ }
+}
