Committed by
Gerrit Code Review
New API for specifying an executor when registering a map listener
Change-Id: I1fc92e0a3da576d88d5ece4a666af8ad1c1fb9d8
Showing
9 changed files
with
62 additions
and
19 deletions
| ... | @@ -22,6 +22,7 @@ import java.util.Objects; | ... | @@ -22,6 +22,7 @@ import java.util.Objects; |
| 22 | import java.util.Set; | 22 | import java.util.Set; |
| 23 | import java.util.concurrent.CompletableFuture; | 23 | import java.util.concurrent.CompletableFuture; |
| 24 | import java.util.concurrent.ExecutionException; | 24 | import java.util.concurrent.ExecutionException; |
| 25 | +import java.util.concurrent.Executor; | ||
| 25 | import java.util.concurrent.TimeUnit; | 26 | import java.util.concurrent.TimeUnit; |
| 26 | import java.util.concurrent.TimeoutException; | 27 | import java.util.concurrent.TimeoutException; |
| 27 | import java.util.function.BiFunction; | 28 | import java.util.function.BiFunction; |
| ... | @@ -178,8 +179,8 @@ public class DefaultConsistentMap<K, V> extends Synchronous<AsyncConsistentMap<K | ... | @@ -178,8 +179,8 @@ public class DefaultConsistentMap<K, V> extends Synchronous<AsyncConsistentMap<K |
| 178 | } | 179 | } |
| 179 | 180 | ||
| 180 | @Override | 181 | @Override |
| 181 | - public void addListener(MapEventListener<K, V> listener) { | 182 | + public void addListener(MapEventListener<K, V> listener, Executor executor) { |
| 182 | - complete(asyncMap.addListener(listener)); | 183 | + complete(asyncMap.addListener(listener, executor)); |
| 183 | } | 184 | } |
| 184 | 185 | ||
| 185 | @Override | 186 | @Override | ... | ... |
| ... | @@ -17,6 +17,7 @@ | ... | @@ -17,6 +17,7 @@ |
| 17 | package org.onosproject.store.primitives; | 17 | package org.onosproject.store.primitives; |
| 18 | 18 | ||
| 19 | import com.google.common.base.Throwables; | 19 | import com.google.common.base.Throwables; |
| 20 | + | ||
| 20 | import org.onosproject.store.service.ConsistentMapException; | 21 | import org.onosproject.store.service.ConsistentMapException; |
| 21 | import org.onosproject.store.service.AsyncConsistentTreeMap; | 22 | import org.onosproject.store.service.AsyncConsistentTreeMap; |
| 22 | import org.onosproject.store.service.MapEventListener; | 23 | import org.onosproject.store.service.MapEventListener; |
| ... | @@ -30,6 +31,7 @@ import java.util.NavigableSet; | ... | @@ -30,6 +31,7 @@ import java.util.NavigableSet; |
| 30 | import java.util.Set; | 31 | import java.util.Set; |
| 31 | import java.util.concurrent.CompletableFuture; | 32 | import java.util.concurrent.CompletableFuture; |
| 32 | import java.util.concurrent.ExecutionException; | 33 | import java.util.concurrent.ExecutionException; |
| 34 | +import java.util.concurrent.Executor; | ||
| 33 | import java.util.concurrent.TimeUnit; | 35 | import java.util.concurrent.TimeUnit; |
| 34 | import java.util.concurrent.TimeoutException; | 36 | import java.util.concurrent.TimeoutException; |
| 35 | import java.util.function.BiFunction; | 37 | import java.util.function.BiFunction; |
| ... | @@ -258,8 +260,8 @@ public class DefaultConsistentTreeMap<K, V> extends Synchronous<AsyncConsistentT | ... | @@ -258,8 +260,8 @@ public class DefaultConsistentTreeMap<K, V> extends Synchronous<AsyncConsistentT |
| 258 | } | 260 | } |
| 259 | 261 | ||
| 260 | @Override | 262 | @Override |
| 261 | - public void addListener(MapEventListener<K, V> listener) { | 263 | + public void addListener(MapEventListener<K, V> listener, Executor executor) { |
| 262 | - complete(treeMap.addListener(listener)); | 264 | + complete(treeMap.addListener(listener, executor)); |
| 263 | } | 265 | } |
| 264 | 266 | ||
| 265 | @Override | 267 | @Override | ... | ... |
| ... | @@ -21,6 +21,7 @@ import java.util.Objects; | ... | @@ -21,6 +21,7 @@ import java.util.Objects; |
| 21 | import java.util.Map.Entry; | 21 | import java.util.Map.Entry; |
| 22 | import java.util.Set; | 22 | import java.util.Set; |
| 23 | import java.util.concurrent.CompletableFuture; | 23 | import java.util.concurrent.CompletableFuture; |
| 24 | +import java.util.concurrent.Executor; | ||
| 24 | import java.util.function.BiFunction; | 25 | import java.util.function.BiFunction; |
| 25 | import java.util.function.Function; | 26 | import java.util.function.Function; |
| 26 | import java.util.function.Predicate; | 27 | import java.util.function.Predicate; |
| ... | @@ -28,6 +29,8 @@ import java.util.function.Predicate; | ... | @@ -28,6 +29,8 @@ import java.util.function.Predicate; |
| 28 | import org.onosproject.store.primitives.DefaultConsistentMap; | 29 | import org.onosproject.store.primitives.DefaultConsistentMap; |
| 29 | import org.onosproject.store.primitives.TransactionId; | 30 | import org.onosproject.store.primitives.TransactionId; |
| 30 | 31 | ||
| 32 | +import com.google.common.util.concurrent.MoreExecutors; | ||
| 33 | + | ||
| 31 | /** | 34 | /** |
| 32 | * A distributed, strongly consistent map whose methods are all executed asynchronously. | 35 | * A distributed, strongly consistent map whose methods are all executed asynchronously. |
| 33 | * <p> | 36 | * <p> |
| ... | @@ -308,7 +311,18 @@ public interface AsyncConsistentMap<K, V> extends DistributedPrimitive { | ... | @@ -308,7 +311,18 @@ public interface AsyncConsistentMap<K, V> extends DistributedPrimitive { |
| 308 | * @param listener listener to notify about map events | 311 | * @param listener listener to notify about map events |
| 309 | * @return future that will be completed when the operation finishes | 312 | * @return future that will be completed when the operation finishes |
| 310 | */ | 313 | */ |
| 311 | - CompletableFuture<Void> addListener(MapEventListener<K, V> listener); | 314 | + default CompletableFuture<Void> addListener(MapEventListener<K, V> listener) { |
| 315 | + return addListener(listener, MoreExecutors.directExecutor()); | ||
| 316 | + } | ||
| 317 | + | ||
| 318 | + /** | ||
| 319 | + * Registers the specified listener to be notified whenever the map is updated. | ||
| 320 | + * | ||
| 321 | + * @param listener listener to notify about map events | ||
| 322 | + * @param executor executor to use for handling incoming map events | ||
| 323 | + * @return future that will be completed when the operation finishes | ||
| 324 | + */ | ||
| 325 | + CompletableFuture<Void> addListener(MapEventListener<K, V> listener, Executor executor); | ||
| 312 | 326 | ||
| 313 | /** | 327 | /** |
| 314 | * Unregisters the specified listener such that it will no longer | 328 | * Unregisters the specified listener such that it will no longer | ... | ... |
| ... | @@ -20,10 +20,13 @@ import java.util.Collection; | ... | @@ -20,10 +20,13 @@ import java.util.Collection; |
| 20 | import java.util.Map; | 20 | import java.util.Map; |
| 21 | import java.util.Map.Entry; | 21 | import java.util.Map.Entry; |
| 22 | import java.util.Set; | 22 | import java.util.Set; |
| 23 | +import java.util.concurrent.Executor; | ||
| 23 | import java.util.function.BiFunction; | 24 | import java.util.function.BiFunction; |
| 24 | import java.util.function.Function; | 25 | import java.util.function.Function; |
| 25 | import java.util.function.Predicate; | 26 | import java.util.function.Predicate; |
| 26 | 27 | ||
| 28 | +import com.google.common.util.concurrent.MoreExecutors; | ||
| 29 | + | ||
| 27 | /** | 30 | /** |
| 28 | * {@code ConsistentMap} provides the same functionality as {@link AsyncConsistentMap} with | 31 | * {@code ConsistentMap} provides the same functionality as {@link AsyncConsistentMap} with |
| 29 | * the only difference that all its methods block until the corresponding operation completes. | 32 | * the only difference that all its methods block until the corresponding operation completes. |
| ... | @@ -270,7 +273,17 @@ public interface ConsistentMap<K, V> extends DistributedPrimitive { | ... | @@ -270,7 +273,17 @@ public interface ConsistentMap<K, V> extends DistributedPrimitive { |
| 270 | * | 273 | * |
| 271 | * @param listener listener to notify about map events | 274 | * @param listener listener to notify about map events |
| 272 | */ | 275 | */ |
| 273 | - void addListener(MapEventListener<K, V> listener); | 276 | + default void addListener(MapEventListener<K, V> listener) { |
| 277 | + addListener(listener, MoreExecutors.directExecutor()); | ||
| 278 | + } | ||
| 279 | + | ||
| 280 | + /** | ||
| 281 | + * Registers the specified listener to be notified whenever the map is updated. | ||
| 282 | + * | ||
| 283 | + * @param listener listener to notify about map events | ||
| 284 | + * @param executor executor to use for handling incoming map events | ||
| 285 | + */ | ||
| 286 | + void addListener(MapEventListener<K, V> listener, Executor executor); | ||
| 274 | 287 | ||
| 275 | /** | 288 | /** |
| 276 | * Unregisters the specified listener such that it will no longer | 289 | * Unregisters the specified listener such that it will no longer | ... | ... |
| ... | @@ -18,6 +18,7 @@ package org.onosproject.store.service; | ... | @@ -18,6 +18,7 @@ package org.onosproject.store.service; |
| 18 | import java.util.Collection; | 18 | import java.util.Collection; |
| 19 | import java.util.Map; | 19 | import java.util.Map; |
| 20 | import java.util.Set; | 20 | import java.util.Set; |
| 21 | +import java.util.concurrent.Executor; | ||
| 21 | import java.util.function.BiFunction; | 22 | import java.util.function.BiFunction; |
| 22 | import java.util.function.Function; | 23 | import java.util.function.Function; |
| 23 | import java.util.function.Predicate; | 24 | import java.util.function.Predicate; |
| ... | @@ -149,7 +150,7 @@ public class ConsistentMapAdapter<K, V> implements ConsistentMap<K, V> { | ... | @@ -149,7 +150,7 @@ public class ConsistentMapAdapter<K, V> implements ConsistentMap<K, V> { |
| 149 | } | 150 | } |
| 150 | 151 | ||
| 151 | @Override | 152 | @Override |
| 152 | - public void addListener(MapEventListener<K, V> listener) { | 153 | + public void addListener(MapEventListener<K, V> listener, Executor executor) { |
| 153 | 154 | ||
| 154 | } | 155 | } |
| 155 | 156 | ... | ... |
| ... | @@ -23,6 +23,7 @@ import java.util.Map.Entry; | ... | @@ -23,6 +23,7 @@ import java.util.Map.Entry; |
| 23 | import java.util.Objects; | 23 | import java.util.Objects; |
| 24 | import java.util.Set; | 24 | import java.util.Set; |
| 25 | import java.util.concurrent.CompletableFuture; | 25 | import java.util.concurrent.CompletableFuture; |
| 26 | +import java.util.concurrent.Executor; | ||
| 26 | import java.util.function.BiFunction; | 27 | import java.util.function.BiFunction; |
| 27 | import java.util.function.Consumer; | 28 | import java.util.function.Consumer; |
| 28 | import java.util.function.Predicate; | 29 | import java.util.function.Predicate; |
| ... | @@ -33,6 +34,7 @@ import org.onosproject.store.service.AsyncConsistentMap; | ... | @@ -33,6 +34,7 @@ import org.onosproject.store.service.AsyncConsistentMap; |
| 33 | import org.onosproject.store.service.MapEventListener; | 34 | import org.onosproject.store.service.MapEventListener; |
| 34 | import org.onosproject.store.service.MapTransaction; | 35 | import org.onosproject.store.service.MapTransaction; |
| 35 | import org.onosproject.store.service.Versioned; | 36 | import org.onosproject.store.service.Versioned; |
| 37 | + | ||
| 36 | import com.google.common.base.MoreObjects; | 38 | import com.google.common.base.MoreObjects; |
| 37 | 39 | ||
| 38 | /** | 40 | /** |
| ... | @@ -153,8 +155,8 @@ public class DelegatingAsyncConsistentMap<K, V> implements AsyncConsistentMap<K, | ... | @@ -153,8 +155,8 @@ public class DelegatingAsyncConsistentMap<K, V> implements AsyncConsistentMap<K, |
| 153 | } | 155 | } |
| 154 | 156 | ||
| 155 | @Override | 157 | @Override |
| 156 | - public CompletableFuture<Void> addListener(MapEventListener<K, V> listener) { | 158 | + public CompletableFuture<Void> addListener(MapEventListener<K, V> listener, Executor executor) { |
| 157 | - return delegateMap.addListener(listener); | 159 | + return delegateMap.addListener(listener, executor); |
| 158 | } | 160 | } |
| 159 | 161 | ||
| 160 | @Override | 162 | @Override | ... | ... |
| ... | @@ -24,6 +24,7 @@ import java.util.Map.Entry; | ... | @@ -24,6 +24,7 @@ import java.util.Map.Entry; |
| 24 | import java.util.Set; | 24 | import java.util.Set; |
| 25 | import java.util.TreeMap; | 25 | import java.util.TreeMap; |
| 26 | import java.util.concurrent.CompletableFuture; | 26 | import java.util.concurrent.CompletableFuture; |
| 27 | +import java.util.concurrent.Executor; | ||
| 27 | import java.util.concurrent.atomic.AtomicBoolean; | 28 | import java.util.concurrent.atomic.AtomicBoolean; |
| 28 | import java.util.concurrent.atomic.AtomicInteger; | 29 | import java.util.concurrent.atomic.AtomicInteger; |
| 29 | import java.util.function.BiFunction; | 30 | import java.util.function.BiFunction; |
| ... | @@ -39,6 +40,7 @@ import org.onosproject.store.service.AsyncConsistentMap; | ... | @@ -39,6 +40,7 @@ import org.onosproject.store.service.AsyncConsistentMap; |
| 39 | import org.onosproject.store.service.MapEventListener; | 40 | import org.onosproject.store.service.MapEventListener; |
| 40 | import org.onosproject.store.service.MapTransaction; | 41 | import org.onosproject.store.service.MapTransaction; |
| 41 | import org.onosproject.store.service.Versioned; | 42 | import org.onosproject.store.service.Versioned; |
| 43 | + | ||
| 42 | import com.google.common.collect.Lists; | 44 | import com.google.common.collect.Lists; |
| 43 | import com.google.common.collect.Maps; | 45 | import com.google.common.collect.Maps; |
| 44 | import com.google.common.collect.Sets; | 46 | import com.google.common.collect.Sets; |
| ... | @@ -190,9 +192,9 @@ public class PartitionedAsyncConsistentMap<K, V> implements AsyncConsistentMap<K | ... | @@ -190,9 +192,9 @@ public class PartitionedAsyncConsistentMap<K, V> implements AsyncConsistentMap<K |
| 190 | } | 192 | } |
| 191 | 193 | ||
| 192 | @Override | 194 | @Override |
| 193 | - public CompletableFuture<Void> addListener(MapEventListener<K, V> listener) { | 195 | + public CompletableFuture<Void> addListener(MapEventListener<K, V> listener, Executor executor) { |
| 194 | return CompletableFuture.allOf(getMaps().stream() | 196 | return CompletableFuture.allOf(getMaps().stream() |
| 195 | - .map(map -> map.addListener(listener)) | 197 | + .map(map -> map.addListener(listener, executor)) |
| 196 | .toArray(CompletableFuture[]::new)); | 198 | .toArray(CompletableFuture[]::new)); |
| 197 | } | 199 | } |
| 198 | 200 | ... | ... |
| ... | @@ -21,6 +21,7 @@ import java.util.Map; | ... | @@ -21,6 +21,7 @@ import java.util.Map; |
| 21 | import java.util.Map.Entry; | 21 | import java.util.Map.Entry; |
| 22 | import java.util.Set; | 22 | import java.util.Set; |
| 23 | import java.util.concurrent.CompletableFuture; | 23 | import java.util.concurrent.CompletableFuture; |
| 24 | +import java.util.concurrent.Executor; | ||
| 24 | import java.util.function.BiFunction; | 25 | import java.util.function.BiFunction; |
| 25 | import java.util.function.Consumer; | 26 | import java.util.function.Consumer; |
| 26 | import java.util.function.Function; | 27 | import java.util.function.Function; |
| ... | @@ -34,6 +35,7 @@ import org.onosproject.store.service.MapEvent; | ... | @@ -34,6 +35,7 @@ import org.onosproject.store.service.MapEvent; |
| 34 | import org.onosproject.store.service.MapEventListener; | 35 | import org.onosproject.store.service.MapEventListener; |
| 35 | import org.onosproject.store.service.MapTransaction; | 36 | import org.onosproject.store.service.MapTransaction; |
| 36 | import org.onosproject.store.service.Versioned; | 37 | import org.onosproject.store.service.Versioned; |
| 38 | + | ||
| 37 | import com.google.common.collect.Maps; | 39 | import com.google.common.collect.Maps; |
| 38 | 40 | ||
| 39 | /** | 41 | /** |
| ... | @@ -235,11 +237,11 @@ public class TranscodingAsyncConsistentMap<K1, V1, K2, V2> implements AsyncConsi | ... | @@ -235,11 +237,11 @@ public class TranscodingAsyncConsistentMap<K1, V1, K2, V2> implements AsyncConsi |
| 235 | } | 237 | } |
| 236 | 238 | ||
| 237 | @Override | 239 | @Override |
| 238 | - public CompletableFuture<Void> addListener(MapEventListener<K1, V1> listener) { | 240 | + public CompletableFuture<Void> addListener(MapEventListener<K1, V1> listener, Executor executor) { |
| 239 | synchronized (listeners) { | 241 | synchronized (listeners) { |
| 240 | InternalBackingMapEventListener backingMapListener = | 242 | InternalBackingMapEventListener backingMapListener = |
| 241 | listeners.computeIfAbsent(listener, k -> new InternalBackingMapEventListener(listener)); | 243 | listeners.computeIfAbsent(listener, k -> new InternalBackingMapEventListener(listener)); |
| 242 | - return backingMap.addListener(backingMapListener); | 244 | + return backingMap.addListener(backingMapListener, executor); |
| 243 | } | 245 | } |
| 244 | } | 246 | } |
| 245 | 247 | ... | ... |
| ... | @@ -22,10 +22,12 @@ import io.atomix.resource.ResourceTypeInfo; | ... | @@ -22,10 +22,12 @@ import io.atomix.resource.ResourceTypeInfo; |
| 22 | import java.util.Collection; | 22 | import java.util.Collection; |
| 23 | import java.util.ConcurrentModificationException; | 23 | import java.util.ConcurrentModificationException; |
| 24 | import java.util.List; | 24 | import java.util.List; |
| 25 | +import java.util.Map; | ||
| 25 | import java.util.Map.Entry; | 26 | import java.util.Map.Entry; |
| 26 | import java.util.Properties; | 27 | import java.util.Properties; |
| 27 | import java.util.Set; | 28 | import java.util.Set; |
| 28 | import java.util.concurrent.CompletableFuture; | 29 | import java.util.concurrent.CompletableFuture; |
| 30 | +import java.util.concurrent.Executor; | ||
| 29 | import java.util.concurrent.atomic.AtomicReference; | 31 | import java.util.concurrent.atomic.AtomicReference; |
| 30 | import java.util.function.BiFunction; | 32 | import java.util.function.BiFunction; |
| 31 | import java.util.function.Consumer; | 33 | import java.util.function.Consumer; |
| ... | @@ -54,7 +56,9 @@ import org.onosproject.store.service.MapEvent; | ... | @@ -54,7 +56,9 @@ import org.onosproject.store.service.MapEvent; |
| 54 | import org.onosproject.store.service.MapEventListener; | 56 | import org.onosproject.store.service.MapEventListener; |
| 55 | import org.onosproject.store.service.MapTransaction; | 57 | import org.onosproject.store.service.MapTransaction; |
| 56 | import org.onosproject.store.service.Versioned; | 58 | import org.onosproject.store.service.Versioned; |
| 59 | + | ||
| 57 | import com.google.common.collect.ImmutableSet; | 60 | import com.google.common.collect.ImmutableSet; |
| 61 | +import com.google.common.collect.Maps; | ||
| 58 | import com.google.common.collect.Sets; | 62 | import com.google.common.collect.Sets; |
| 59 | 63 | ||
| 60 | /** | 64 | /** |
| ... | @@ -65,7 +69,7 @@ public class AtomixConsistentMap extends AbstractResource<AtomixConsistentMap> | ... | @@ -65,7 +69,7 @@ public class AtomixConsistentMap extends AbstractResource<AtomixConsistentMap> |
| 65 | implements AsyncConsistentMap<String, byte[]> { | 69 | implements AsyncConsistentMap<String, byte[]> { |
| 66 | 70 | ||
| 67 | private final Set<Consumer<Status>> statusChangeListeners = Sets.newCopyOnWriteArraySet(); | 71 | private final Set<Consumer<Status>> statusChangeListeners = Sets.newCopyOnWriteArraySet(); |
| 68 | - private final Set<MapEventListener<String, byte[]>> mapEventListeners = Sets.newCopyOnWriteArraySet(); | 72 | + private final Map<MapEventListener<String, byte[]>, Executor> mapEventListeners = Maps.newIdentityHashMap(); |
| 69 | 73 | ||
| 70 | public static final String CHANGE_SUBJECT = "changeEvents"; | 74 | public static final String CHANGE_SUBJECT = "changeEvents"; |
| 71 | 75 | ||
| ... | @@ -87,7 +91,8 @@ public class AtomixConsistentMap extends AbstractResource<AtomixConsistentMap> | ... | @@ -87,7 +91,8 @@ public class AtomixConsistentMap extends AbstractResource<AtomixConsistentMap> |
| 87 | } | 91 | } |
| 88 | 92 | ||
| 89 | private void handleEvent(List<MapEvent<String, byte[]>> events) { | 93 | private void handleEvent(List<MapEvent<String, byte[]>> events) { |
| 90 | - events.forEach(event -> mapEventListeners.forEach(listener -> listener.event(event))); | 94 | + events.forEach(event -> |
| 95 | + mapEventListeners.forEach((listener, executor) -> executor.execute(() -> listener.event(event)))); | ||
| 91 | } | 96 | } |
| 92 | 97 | ||
| 93 | @Override | 98 | @Override |
| ... | @@ -250,18 +255,19 @@ public class AtomixConsistentMap extends AbstractResource<AtomixConsistentMap> | ... | @@ -250,18 +255,19 @@ public class AtomixConsistentMap extends AbstractResource<AtomixConsistentMap> |
| 250 | } | 255 | } |
| 251 | 256 | ||
| 252 | @Override | 257 | @Override |
| 253 | - public synchronized CompletableFuture<Void> addListener(MapEventListener<String, byte[]> listener) { | 258 | + public synchronized CompletableFuture<Void> addListener(MapEventListener<String, byte[]> listener, |
| 259 | + Executor executor) { | ||
| 254 | if (mapEventListeners.isEmpty()) { | 260 | if (mapEventListeners.isEmpty()) { |
| 255 | - return submit(new Listen()).thenRun(() -> mapEventListeners.add(listener)); | 261 | + return submit(new Listen()).thenRun(() -> mapEventListeners.putIfAbsent(listener, executor)); |
| 256 | } else { | 262 | } else { |
| 257 | - mapEventListeners.add(listener); | 263 | + mapEventListeners.put(listener, executor); |
| 258 | return CompletableFuture.completedFuture(null); | 264 | return CompletableFuture.completedFuture(null); |
| 259 | } | 265 | } |
| 260 | } | 266 | } |
| 261 | 267 | ||
| 262 | @Override | 268 | @Override |
| 263 | public synchronized CompletableFuture<Void> removeListener(MapEventListener<String, byte[]> listener) { | 269 | public synchronized CompletableFuture<Void> removeListener(MapEventListener<String, byte[]> listener) { |
| 264 | - if (mapEventListeners.remove(listener) && mapEventListeners.isEmpty()) { | 270 | + if (mapEventListeners.remove(listener) != null && mapEventListeners.isEmpty()) { |
| 265 | return submit(new Unlisten()).thenApply(v -> null); | 271 | return submit(new Unlisten()).thenApply(v -> null); |
| 266 | } | 272 | } |
| 267 | return CompletableFuture.completedFuture(null); | 273 | return CompletableFuture.completedFuture(null); | ... | ... |
-
Please register or login to post a comment