Refactor common invalidation framework types

sync/notifier/object_id_invalidation_map.h

-// Copyright 2012 The Chromium Authors. All rights reserved.
+// Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef SYNC_NOTIFIER_OBJECT_ID_INVALIDATION_MAP_H_
 #define SYNC_NOTIFIER_OBJECT_ID_INVALIDATION_MAP_H_
 
 #include <map>
-#include <string>
+#include <vector>
 
-#include "base/basictypes.h"
-#include "base/memory/scoped_ptr.h"
-#include "google/cacheinvalidation/include/types.h"
 #include "sync/base/sync_export.h"
 #include "sync/internal_api/public/base/invalidation.h"
 #include "sync/notifier/invalidation_util.h"
-
-namespace base {
-class ListValue;
-}  // namespace base
+#include "sync/notifier/ordered_invalidation_list.h"
 
 namespace syncer {
 
-typedef std::map<invalidation::ObjectId,
-                 Invalidation,
-                 ObjectIdLessThan> ObjectIdInvalidationMap;
+class SYNC_EXPORT ObjectIdInvalidationMap {

[tim (not reviewing), 2013/09/20 21:53:46]

What is this class supposed to do?  Looking at it I have a hard time figuring
out what 'WithObjects' is doing... at first I thought it was a factory function.

[rlarocque, 2013/09/23 18:38:19]

This class is supposed to be a container for many invalidations which may or may
not have different ObjectIds.

I made it a map because there aren't many interesting things one can do with two
invalidations that have different object IDs.  It's likely that the first thing
most clients will want to do with this object is iterate over the sets of
invalidations that share object IDs.

WithObjects is a helper to select the subset of contents of this map that have
the same ObjectId.  That's often a useful operation, but I guess I didn't do a
very good job of naming it.

Maybe GetInvalidationsWithIdsMatching(...) would be a better name?

[tim (not reviewing), 2013/09/24 21:16:54]

I see.  So it's returning a subset / projection of the tuples in the full
mapping. I find it pretty informative to include the subset aspect of it in the
name (as opposed to builder/factory functions where you can create an object
from scratch WithObjects(...))

[Get]SubsetWith[Objects|Ids](...)?

I was also hunting for a class comment here :)

[rlarocque, 2013/09/25 00:40:00]

GetSubsetWithObjectIds() makes sense to me.
I've added one.  Check patch set 12 or later.

+  public:
+   static ObjectIdInvalidationMap InvalidateAll(const ObjectIdSet& ids);
 
-// Converts between ObjectIdInvalidationMaps and ObjectIdSets.
-ObjectIdSet ObjectIdInvalidationMapToSet(
-    const ObjectIdInvalidationMap& invalidation_map);
-SYNC_EXPORT ObjectIdInvalidationMap ObjectIdSetToInvalidationMap(
-    const ObjectIdSet& ids, int64 version, const std::string& payload);
+   ObjectIdInvalidationMap();
+   ~ObjectIdInvalidationMap();
 
-SYNC_EXPORT bool ObjectIdInvalidationMapEquals(
-    const ObjectIdInvalidationMap& invalidation_map1,
-    const ObjectIdInvalidationMap& invalidation_map2);
+   ObjectIdSet GetObjectIds() const;
+   bool Empty() const;
+   bool operator==(const ObjectIdInvalidationMap& other) const;
 
-scoped_ptr<base::ListValue> ObjectIdInvalidationMapToValue(
-    const ObjectIdInvalidationMap& invalidation_map);
+   void Insert(const Invalidation& invalidation);
 
-bool ObjectIdInvalidationMapFromValue(const base::ListValue& value,
-                               ObjectIdInvalidationMap* out);
+   ObjectIdInvalidationMap WithObjects(const ObjectIdSet& ids) const;

[tim (not reviewing), 2013/09/20 21:53:46]

Are there any basic tests for this filtering?

[rlarocque, 2013/09/23 18:38:19]

No, there are no tests for any part of this class.  I'll fix that.

[tim (not reviewing), 2013/09/24 21:16:54]

Great!

+   const OrderedInvalidationList& ForObject(invalidation::ObjectId id) const;
+   void GetAllInvalidations(std::vector<syncer::Invalidation>* out) const;
+
+   scoped_ptr<base::ListValue> ToValue() const;
+   bool ResetFromValue(const base::ListValue& value);
+
+   std::string ToString() const;
+
+  private:
+   typedef std::map<invalidation::ObjectId,
+                    OrderedInvalidationList,
+                    ObjectIdLessThan> IdToListMap;
+
+   ObjectIdInvalidationMap(const IdToListMap& map);
+
+   IdToListMap map_;
+};
 
 }  // namespace syncer
 
 #endif  // SYNC_NOTIFIER_OBJECT_ID_INVALIDATION_MAP_H_