From 8723469e689277fc5dc511f0546ffead739e97fc Mon Sep 17 00:00:00 2001 From: Bert Massop Date: Mon, 17 Nov 2014 16:13:06 +0100 Subject: [PATCH 1/3] Subscriptions: improve usage of Java generics * Parametrize Notification with generic argument * Use proper generic types instead of wherever possible on Subscription and Notification * Reduce the amount of (now unnecessary) casting * Don't write Subscription: the EventType is not exposed directly anywhere and there cannot exist any Subscription where the EventType does not extend EventSource, given the class signature of Subscription. Write Subscription instead when the EventType is not known. * Classes are of type Class where XXX is the non-generic superclass of XXX. Hence there is no need to ever write Class>, and doing so requires unchecked casts all over the place. --- .../WebOfTrust/SubscriptionManager.java | 159 +++++++++--------- .../WebOfTrust/ui/fcp/FCPInterface.java | 13 +- 2 files changed, 83 insertions(+), 89 deletions(-) diff --git a/src/plugins/WebOfTrust/SubscriptionManager.java b/src/plugins/WebOfTrust/SubscriptionManager.java index 460bc7d6d..06dc6ce7e 100644 --- a/src/plugins/WebOfTrust/SubscriptionManager.java +++ b/src/plugins/WebOfTrust/SubscriptionManager.java @@ -250,11 +250,11 @@ protected boolean sendNotifications(SubscriptionManager manager) switch(getType()) { case FCP: - for(final Notification notification : manager.getNotifications(this)) { + for(final Notification notification : manager.getNotifications(this)) { if(SubscriptionManager.logDEBUG) Logger.debug(manager, "Sending notification via FCP: " + notification); try { try { - notification.getSubscription().notifySubscriberByFCP(notification); + notification.notifySubscriberByFCP(); notification.deleteWithoutCommit(); } catch(FCPCallFailedException | IOException | RuntimeException e) { Persistent.checkedRollback(mDB, this, e, LogLevel.WARNING); @@ -312,7 +312,7 @@ protected boolean sendNotifications(SubscriptionManager manager) return true; } - + /** * Sends a message to the client which indicates that a {@link Subscription} has been forcefully terminated. * This can happen if the client exceeds the limit of {@link SubscriptionManager#DISCONNECT_CLIENT_AFTER_FAILURE_COUNT} failures @@ -320,9 +320,8 @@ protected boolean sendNotifications(SubscriptionManager manager) * * Any {@link Throwable}s which happen if sending the message fails are swallowed. */ - @SuppressWarnings("unchecked") private void notifyClientAboutDeletion( - final Subscription deletedSubscriptoin) { + final Subscription deletedSubscriptoin) { try { Logger.warning(getSubscriptionManager(), "notifyClientAboutDeletion() for " + deletedSubscriptoin); @@ -330,8 +329,7 @@ private void notifyClientAboutDeletion( switch(getType()) { case FCP: mWebOfTrust.getFCPInterface().sendUnsubscribedMessage(getFCP_ID(), - (Class>) deletedSubscriptoin - .getClass(), + deletedSubscriptoin.getClass(), deletedSubscriptoin.getID()); break; default: @@ -352,8 +350,7 @@ private void notifyClientAboutDeletion( * @param subscriptionManager The {@link SubscriptionManager} to which this Client belongs. */ protected void deleteWithoutCommit(final SubscriptionManager subscriptionManager) { - for(final Subscription subscription - : subscriptionManager.getSubscriptions(this)) { + for(final Subscription subscription : subscriptionManager.getSubscriptions(this)) { subscription.deleteWithoutCommit(subscriptionManager); notifyClientAboutDeletion(subscription); @@ -471,7 +468,7 @@ protected void deleteWithoutCommit() { * @param manager The {@link SubscriptionManager} to which this Subscription belongs. */ protected void deleteWithoutCommit(final SubscriptionManager manager) { - for(final Notification notification : manager.getNotifications(this)) { + for(final Notification notification : manager.getNotifications(this)) { notification.deleteWithoutCommit(); } super.deleteWithoutCommit(); @@ -590,7 +587,7 @@ abstract void storeNotificationWithoutCommit( * The {@link Notification} objects which this function receives contain serialized clones of the objects from WebOfTrust. * Therefore, the notifications are self-contained and this function should and must NOT call any database query functions of the WebOfTrust. * - * @param notification + * @param notification * The {@link Notification} to send out via FCP. Must be cast-able to one of: *
* - {@link ObjectChangedNotification} containing objects matching the type @@ -617,7 +614,7 @@ abstract void storeNotificationWithoutCommit( * Notification at the next iteration of the notification sending thread * - until the limit is reached. */ - protected abstract void notifySubscriberByFCP(Notification notification) + protected abstract void notifySubscriberByFCP(Notification notification) throws FCPCallFailedException, IOException, InterruptedException; /** {@inheritDoc} */ @@ -636,7 +633,7 @@ public String toString() { * The SubscriptionManager will wake up some time after that, pull all notifications from the database and process them. */ @SuppressWarnings("serial") - public static abstract class Notification extends Persistent { + public static abstract class Notification extends Persistent { /** * The {@link Client} to which this Notification belongs @@ -648,7 +645,7 @@ public static abstract class Notification extends Persistent { * The {@link Subscription} to which this Notification belongs */ @IndexedField - private final Subscription mSubscription; + private final Subscription mSubscription; /** * The index of this Notification in the queue of its {@link Client}: @@ -664,7 +661,7 @@ public static abstract class Notification extends Persistent { * * @param mySubscription The {@link Subscription} which requested this type of Notification. */ - Notification(final Subscription mySubscription) { + Notification(final Subscription mySubscription) { mSubscription = mySubscription; mClient = mSubscription.getClient(); mIndex = mClient.takeFreeNotificationIndexWithoutCommit(); @@ -696,7 +693,7 @@ public String getID() { /** * @return The {@link Subscription} which requested this type of Notification. */ - public Subscription getSubscription() { + public Subscription getSubscription() { checkedActivate(1); mSubscription.initializeTransient(mWebOfTrust); return mSubscription; @@ -706,6 +703,14 @@ public Subscription getSubscription() { @Override protected void activateFully() { checkedActivate(1); } + + /** + * @see Subscription#notifySubscriberByFCP(Notification) + */ + void notifySubscriberByFCP() + throws InterruptedException, FCPCallFailedException, IOException { + getSubscription().notifySubscriberByFCP(this); + } } /** @@ -720,7 +725,8 @@ public Subscription getSubscription() { * NOTICE: Both Persistent objects are not stored in the database and must not be stored there to prevent duplicates! */ @SuppressWarnings("serial") - public static abstract class ObjectChangedNotification extends Notification { + public static abstract class ObjectChangedNotification extends + Notification { /** * A serialized copy of the changed {@link Persistent} object before the change. @@ -753,7 +759,7 @@ public static abstract class ObjectChangedNotification extends Notification { * @param newObject The version of the changed {@link Persistent} object after the change. * @see Notification#Notification(Subscription) This parent constructor is also called. */ - ObjectChangedNotification(final Subscription mySubscription, + ObjectChangedNotification(final Subscription mySubscription, final Persistent oldObject, final Persistent newObject) { super(mySubscription); @@ -842,7 +848,7 @@ public String toString() { */ @SuppressWarnings("serial") public static class BeginSynchronizationNotification - extends Notification { + extends Notification { /** * All {@link EventSource} objects which are stored inside of * {@link ObjectChangedNotification} as part of the synchronization which is marked by this @@ -898,9 +904,8 @@ public String toString() { public static class EndSynchronizationNotification extends BeginSynchronizationNotification { - @SuppressWarnings("unchecked") EndSynchronizationNotification(BeginSynchronizationNotification begin) { - super((Subscription) begin.getSubscription(), begin.getID()); + super(begin.getSubscription(), begin.getID()); assert !(begin instanceof EndSynchronizationNotification); } @@ -922,7 +927,7 @@ public static class EndSynchronizationNotification { /** * Only one of oldIentity and newIdentity may be null. If both are non-null, their {@link Identity#getID()} must match. * @@ -951,7 +956,7 @@ protected IdentityChangedNotification(final Subscription mySubscriptio * @see TrustsSubscription The type of {@link Subscription} which deploys this notification. */ @SuppressWarnings("serial") - public static final class TrustChangedNotification extends ObjectChangedNotification { + public static final class TrustChangedNotification extends ObjectChangedNotification { /** * Only one of oldTrust and newTrust may be null. If both are non-null, their {@link Trust#getID()} must match. * @@ -959,7 +964,7 @@ public static final class TrustChangedNotification extends ObjectChangedNotifica * @param oldTrust The version of the {@link Trust} before the change. * @param newTrust The version of the {@link Trust} after the change. */ - protected TrustChangedNotification(final Subscription mySubscription, + protected TrustChangedNotification(final Subscription mySubscription, final Trust oldTrust, final Trust newTrust) { super(mySubscription, oldTrust, newTrust); } @@ -980,7 +985,7 @@ protected TrustChangedNotification(final Subscription mySubscription, * @see ScoresSubscription The type of {@link Subscription} which deploys this notification. */ @SuppressWarnings("serial") - public static final class ScoreChangedNotification extends ObjectChangedNotification { + public static final class ScoreChangedNotification extends ObjectChangedNotification { /** * Only one of oldScore and newScore may be null. If both are non-null, their {@link Score#getID()} must match. * @@ -1020,9 +1025,10 @@ protected IdentitiesSubscription(final Client myClient) { // TODO: Code quality: This function is almost the same in TrustsSubscription and // ScoresSubscription. Add a type parameter to Notification and use // it to move this function upwards to the Subscription base class. - /** {@inheritDoc} */ + /** {@inheritDoc} + * @param notification*/ @Override - protected void notifySubscriberByFCP(Notification notification) + protected void notifySubscriberByFCP(Notification notification) throws FCPCallFailedException, IOException, InterruptedException { final UUID clientID = getClient().getFCP_ID(); @@ -1084,9 +1090,10 @@ protected TrustsSubscription(final Client myClient) { return mWebOfTrust.getAllTrusts(); } - /** {@inheritDoc} */ + /** {@inheritDoc} + * @param notification*/ @Override - protected void notifySubscriberByFCP(final Notification notification) + protected void notifySubscriberByFCP(final Notification notification) throws FCPCallFailedException, IOException, InterruptedException { final UUID clientID = getClient().getFCP_ID(); @@ -1146,9 +1153,10 @@ protected ScoresSubscription(final Client myClient) { return mWebOfTrust.getAllScores(); } - /** {@inheritDoc} */ + /** {@inheritDoc} + * @param notification*/ @Override - protected void notifySubscriberByFCP(final Notification notification) + protected void notifySubscriberByFCP(final Notification notification) throws FCPCallFailedException, IOException, InterruptedException { final UUID clientID = getClient().getFCP_ID(); @@ -1255,10 +1263,10 @@ public SubscriptionManager(WebOfTrust myWoT) { */ @SuppressWarnings("serial") public static final class SubscriptionExistsAlreadyException extends Exception { - public final Subscription existingSubscription; + public final Subscription existingSubscription; public SubscriptionExistsAlreadyException( - Subscription existingSubscription) { + Subscription existingSubscription) { this.existingSubscription = existingSubscription; } @@ -1301,20 +1309,14 @@ public UnknownSubscriptionException(String message) { * @param subscription The new subscription which the client is trying to create. The database is checked for an existing one with similar properties as specified above. * @throws SubscriptionExistsAlreadyException If a {@link Subscription} exists which matches the attributes of the given Subscription as specified in the description of the function. */ - @SuppressWarnings("unchecked") - private synchronized void throwIfSimilarSubscriptionExists( - final Subscription subscription) - throws SubscriptionExistsAlreadyException { + private synchronized void throwIfSimilarSubscriptionExists(final Subscription subscription) + throws SubscriptionExistsAlreadyException { try { final Client client = subscription.getClient(); if(!mDB.isStored(client)) - return; // The client was newly created just for this subscription so there cannot be any similar subscriptions on it. - final Subscription existing - = getSubscription( - (Class>) subscription.getClass() - , client - ); + return; // The client was newly created just for this subscription so there cannot be any similar subscriptions on it. + final Subscription existing = getSubscription(subscription.getClass(), client); throw new SubscriptionExistsAlreadyException(existing); } catch (UnknownSubscriptionException e) { return; @@ -1349,8 +1351,7 @@ private synchronized void throwIfSimilarSubscriptionExists( * calling the FCP interface's * {@link FredPluginFCPMessageHandler.ServerSideFCPMessageHandler}. */ - private void storeNewSubscriptionWithoutCommit( - final Subscription subscription) + private void storeNewSubscriptionWithoutCommit(final Subscription subscription) throws InterruptedException, SubscriptionExistsAlreadyException { subscription.initializeTransient(mWoT); @@ -1486,12 +1487,11 @@ public ScoresSubscription subscribeToScores(UUID fcpID) * @return The class of the terminated {@link Subscription} * @throws UnknownSubscriptionException If no subscription with the given ID exists. */ - @SuppressWarnings("unchecked") - public Class> unsubscribe(String subscriptionID) + public Class unsubscribe(String subscriptionID) throws UnknownSubscriptionException { synchronized(this) { - final Subscription subscription = getSubscription(subscriptionID); + final Subscription subscription = getSubscription(subscriptionID); synchronized(Persistent.transactionLock(mDB)) { try { subscription.deleteWithoutCommit(this); @@ -1508,7 +1508,7 @@ public Class> unsubscribe(String subscriptio Persistent.checkedRollbackAndThrow(mDB, this, e); } } - return (Class>) subscription.getClass(); + return subscription.getClass(); } } @@ -1556,20 +1556,20 @@ private Client getOrCreateClient(final UUID fcpID) { * * @return All existing {@link Subscription}s. */ - private ObjectSet> getAllSubscriptions() { + private ObjectSet> getAllSubscriptions() { final Query q = mDB.query(); q.constrain(Subscription.class); - return new Persistent.InitializingObjectSet>(mWoT, q); + return new Persistent.InitializingObjectSet>(mWoT, q); } /** * @return All subscriptions of the given {@link Client}. */ - private ObjectSet> getSubscriptions(final Client client) { + private ObjectSet> getSubscriptions(final Client client) { final Query q = mDB.query(); q.constrain(Subscription.class); q.descend("mClient").constrain(client).identity(); - return new Persistent.InitializingObjectSet>(mWoT, q); + return new Persistent.InitializingObjectSet>(mWoT, q); } /** @@ -1583,12 +1583,11 @@ private ObjectSet> getSubscriptions(final Cl * @param clazz The type of {@link EventSource} to filter by. * @return Get all {@link Subscription}s to a certain {@link EventSource} type. */ - private ObjectSet> getSubscriptions( - final Class> clazz) { - + private ObjectSet getSubscriptions(final Class clazz) { + final Query q = mDB.query(); q.constrain(clazz); - return new Persistent.InitializingObjectSet>(mWoT, q); + return new Persistent.InitializingObjectSet(mWoT, q); } /** @@ -1596,14 +1595,13 @@ private ObjectSet> getSubscription * @return The {@link Subscription} with the given ID. Only one Subscription can exist for a single ID. * @throws UnknownSubscriptionException If no {@link Subscription} exists with the given ID. */ - private Subscription getSubscription(final String id) - throws UnknownSubscriptionException { + private Subscription getSubscription(final String id) throws UnknownSubscriptionException { final Query q = mDB.query(); q.constrain(Subscription.class); q.descend("mID").constrain(id); - ObjectSet> result - = new Persistent.InitializingObjectSet>(mWoT, q); + ObjectSet> result + = new Persistent.InitializingObjectSet>(mWoT, q); switch(result.size()) { case 1: return result.next(); @@ -1627,15 +1625,13 @@ private Subscription getSubscription(final String id) * @return See description. * @throws UnknownSubscriptionException If no matching {@link Subscription} exists. */ - private Subscription getSubscription( - final Class> clazz, final Client client) + private S getSubscription(final Class clazz, final Client client) throws UnknownSubscriptionException { final Query q = mDB.query(); q.constrain(clazz); q.descend("mClient").constrain(client).identity(); - ObjectSet> result - = new Persistent.InitializingObjectSet>(mWoT, q); + ObjectSet result = new Persistent.InitializingObjectSet(mWoT, q); switch(result.size()) { case 1: return result.next(); @@ -1658,11 +1654,11 @@ protected synchronized final void deleteAllClients() { synchronized(Persistent.transactionLock(mDB)) { try { - for(Notification n : getAllNotifications()) { + for(Notification n : getAllNotifications()) { n.deleteWithoutCommit(); } - for(Subscription s : getAllSubscriptions()) { + for(Subscription s : getAllSubscriptions()) { s.deleteWithoutCommit(); } @@ -1683,10 +1679,10 @@ protected synchronized final void deleteAllClients() { * * @return All objects of class Notification which are stored in the database. */ - private ObjectSet getAllNotifications() { + private ObjectSet> getAllNotifications() { final Query q = mDB.query(); q.constrain(Notification.class); - return new Persistent.InitializingObjectSet(mWoT, q); + return new Persistent.InitializingObjectSet>(mWoT, q); } /** @@ -1698,13 +1694,12 @@ private ObjectSet getAllNotifications() { * @param subscription The {@link Subscription} of whose queue to return notifications from. * @return All {@link Notification}s on the queue of the subscription. */ - private ObjectSet getNotifications( - final Subscription subscription) { - + private ObjectSet> + getNotifications(final Subscription subscription) { final Query q = mDB.query(); q.constrain(Notification.class); q.descend("mSubscription").constrain(subscription).identity(); - return new Persistent.InitializingObjectSet(mWoT, q); + return new Persistent.InitializingObjectSet>(mWoT, q); } /** @@ -1718,12 +1713,13 @@ private ObjectSet getNotifications( * @param subscription The {@link Client} of whose queue to return notifications from. * @return All {@link Notification}s on the queue of the {@link Client}, ordered ascending by time of happening of their inducing event. */ - private ObjectSet getNotifications(final Client client) { + private ObjectSet> getNotifications(final Client + client) { final Query q = mDB.query(); q.constrain(Notification.class); q.descend("mClient").constrain(client).identity(); q.descend("mIndex").orderAscending(); - return new Persistent.InitializingObjectSet(mWoT, q); + return new Persistent.InitializingObjectSet>(mWoT, q); } /** @@ -1743,8 +1739,7 @@ private ObjectSet getNotifications(final Client client) protected void storeIdentityChangedNotificationWithoutCommit(final Identity oldIdentity, final Identity newIdentity) { if(logDEBUG) Logger.debug(this, "storeIdentityChangedNotificationWithoutCommit(): old=" + oldIdentity + "; new=" + newIdentity); - @SuppressWarnings("unchecked") - final ObjectSet subscriptions = (ObjectSet)getSubscriptions(IdentitiesSubscription.class); + final ObjectSet subscriptions = getSubscriptions(IdentitiesSubscription.class); for(IdentitiesSubscription subscription : subscriptions) { subscription.storeNotificationWithoutCommit(oldIdentity, newIdentity); @@ -1768,9 +1763,8 @@ protected void storeIdentityChangedNotificationWithoutCommit(final Identity oldI */ protected void storeTrustChangedNotificationWithoutCommit(final Trust oldTrust, final Trust newTrust) { if(logDEBUG) Logger.debug(this, "storeTrustChangedNotificationWithoutCommit(): old=" + oldTrust + "; new=" + newTrust); - - @SuppressWarnings("unchecked") - final ObjectSet subscriptions = (ObjectSet)getSubscriptions(TrustsSubscription.class); + + final ObjectSet subscriptions = getSubscriptions(TrustsSubscription.class); for(TrustsSubscription subscription : subscriptions) { subscription.storeNotificationWithoutCommit(oldTrust, newTrust); @@ -1794,9 +1788,8 @@ protected void storeTrustChangedNotificationWithoutCommit(final Trust oldTrust, */ protected void storeScoreChangedNotificationWithoutCommit(final Score oldScore, final Score newScore) { if(logDEBUG) Logger.debug(this, "storeScoreChangedNotificationWithoutCommit(): old=" + oldScore + "; new=" + newScore); - - @SuppressWarnings("unchecked") - final ObjectSet subscriptions = (ObjectSet)getSubscriptions(ScoresSubscription.class); + + final ObjectSet subscriptions = getSubscriptions(ScoresSubscription.class); for(ScoresSubscription subscription : subscriptions) { subscription.storeNotificationWithoutCommit(oldScore, newScore); diff --git a/src/plugins/WebOfTrust/ui/fcp/FCPInterface.java b/src/plugins/WebOfTrust/ui/fcp/FCPInterface.java index 4f1cfb1dc..caf3d9d99 100644 --- a/src/plugins/WebOfTrust/ui/fcp/FCPInterface.java +++ b/src/plugins/WebOfTrust/ui/fcp/FCPInterface.java @@ -1216,7 +1216,7 @@ private FCPPluginMessage handleSubscribe(final FCPPluginClient client, final FCP try { FCPPluginMessage reply = FCPPluginMessage.constructSuccessReply(message); - Subscription subscription; + Subscription subscription; if(to.equals("Identities")) { subscription = mSubscriptionManager.subscribeToIdentities(client.getID()); @@ -1280,13 +1280,13 @@ private FCPPluginMessage handleUnsubscribe(final FCPPluginMessage request) throws InvalidParameterException, UnknownSubscriptionException { final String subscriptionID = getMandatoryParameter(request.params, "SubscriptionID"); - final Class> clazz + final Class clazz = mSubscriptionManager.unsubscribe(subscriptionID); return handleUnsubscribe(request, clazz, subscriptionID); } public void sendUnsubscribedMessage(final UUID clientID, - final Class> clazz, final String subscriptionID) + final Class clazz, final String subscriptionID) throws IOException { mPluginRespirator.getPluginClientByID(clientID).send(SendDirection.ToClient, @@ -1302,7 +1302,8 @@ public void sendUnsubscribedMessage(final UUID clientID, * due to an original client message. */ private FCPPluginMessage handleUnsubscribe(final FCPPluginMessage request, - final Class> clazz, final String subscriptionID) { + final Class clazz, final String + subscriptionID) { final FCPPluginMessage result = request != null ? FCPPluginMessage.constructSuccessReply(request) : @@ -1338,10 +1339,10 @@ public void sendBeginOrEndSynchronizationNotification(final UUID clientID, final FCPPluginMessage fcpMessage = FCPPluginMessage.construct(); fcpMessage.params.putOverwrite("Message", - notification instanceof EndSynchronizationNotification + notification instanceof EndSynchronizationNotification ? "EndSynchronizationEvent" : "BeginSynchronizationEvent"); - Subscription subscription = notification.getSubscription(); + Subscription subscription = notification.getSubscription(); String to; // The type parameter of the BeginSynchronizationNotification is not known at runtime From 11f690ff41709b2c78f397373ab77023b89f803e Mon Sep 17 00:00:00 2001 From: Bert Massop Date: Mon, 17 Nov 2014 17:33:26 +0100 Subject: [PATCH 2/3] Subscriptions: move notifySubscriberByFCP to Notification The Notification has knowledge of which FCP method to invoke. Moving this responsibility to Notification instead of the Subscription considerably reduces the amount of FCP related code duplication and number of instanceof checks. --- .../WebOfTrust/SubscriptionManager.java | 231 +++++++----------- 1 file changed, 85 insertions(+), 146 deletions(-) diff --git a/src/plugins/WebOfTrust/SubscriptionManager.java b/src/plugins/WebOfTrust/SubscriptionManager.java index 06dc6ce7e..784e9f5d7 100644 --- a/src/plugins/WebOfTrust/SubscriptionManager.java +++ b/src/plugins/WebOfTrust/SubscriptionManager.java @@ -9,6 +9,7 @@ import java.util.UUID; import plugins.WebOfTrust.exceptions.DuplicateObjectException; +import plugins.WebOfTrust.ui.fcp.FCPInterface; import plugins.WebOfTrust.ui.fcp.FCPInterface.FCPCallFailedException; import com.db4o.ObjectSet; @@ -439,6 +440,14 @@ protected final Client getClient() { mClient.initializeTransient(mWebOfTrust); return mClient; } + + /** + * Gets the FCP {@link UUID} for the {@link Client} which created this {@link Subscription}. + * @see #mClient + */ + protected final UUID getClientID() { + return getClient().getFCP_ID(); + } /** * @return The UUID of this Subscription. Stored as String for db4o performance, but must be valid in terms of the UUID class. @@ -563,60 +572,6 @@ protected final void storeSynchronizationWithoutCommit() { abstract void storeNotificationWithoutCommit( final EventType oldEventSource, final EventType newEventSource); - /** - * Called by this Subscription when the type of it is FCP and a {@link Notification} shall be sent via FCP. - * The implementation MUST throw a {@link FCPCallFailedException} if the client did not signal that the processing was successful: - * Not only shall the {@link Notification} be resent if transmission fails but also if the client fails processing it. This - * will allow the client to use failing database transactions in the event handlers and just rollback and throw if the transaction - * fails. - * The implementation MUST use synchronous FCP communication to allow the client to signal an error. - * Also, synchronous communication is necessary for guaranteeing the notifications to arrive in proper order at the client. - *

- * - * If this function fails by an exception, the caller MUST NOT proceed to deploy subsequent - * {@link Notification}s to the client. Instead, it must retry calling this function upon - * the same Notification until it succeeds or the retry limit is exceeded.
- * This is necessary because Notifications must be deployed in proper order to guarantee - * that they make sense to the client.
- * Notice that the above does not apply to all types of exceptions. See the JavaDoc of - * the various thrown exceptions for details.

- * - * Thread safety:
- * This must be called while locking upon the SubscriptionManager.
- * The {@link WebOfTrust} object shall NOT be locked: - * The {@link Notification} objects which this function receives contain serialized clones of the objects from WebOfTrust. - * Therefore, the notifications are self-contained and this function should and must NOT call any database query functions of the WebOfTrust. - * - * @param notification - * The {@link Notification} to send out via FCP. Must be cast-able to one of: - *
- * - {@link ObjectChangedNotification} containing objects matching the type - * of the type parameter EventType of this Subscription.
- * - {@link BeginSynchronizationNotification} with type parameter matching the - * type of the type parameter EventType of this Subscription.
- * - {@link EndSynchronizationNotification} with type parameter matching the - * type of the type parameter EventType of this Subscription.
- * @throws IOException - * If the FCP client has disconnected. The SubscriptionManager then should not - * retry deploying this Notification, the {@link Subscription} should be - * terminated instead. - * @throws InterruptedException - * If an external thread requested the current thread to terminate via - * {@link Thread#interrupt()} while the data was being transfered to the client. - *
This is a necessary shutdown mechanism as clients can be attached by - * network and thus transfers can take a long time. Please honor it by - * terminating the thread so WOT can shutdown quickly. - * @throws FCPCallFailedException - * If processing failed at the client.
- * The SubscriptionManager should call - * {@link Client#incrementSendNotificationsFailureCountWithoutCommit()} upon - * {@link #mClient} then, and retry calling this function upon the same - * Notification at the next iteration of the notification sending thread - * - until the limit is reached. - */ - protected abstract void notifySubscriberByFCP(Notification notification) - throws FCPCallFailedException, IOException, InterruptedException; - /** {@inheritDoc} */ @Override protected void activateFully() { checkedActivate(1); @@ -705,12 +660,62 @@ public Subscription getSubscription() { } /** - * @see Subscription#notifySubscriberByFCP(Notification) + * Used when this Notification shall be sent via FCP. + * The implementation MUST throw a {@link FCPCallFailedException} if the client did not signal that the processing was successful: + * Not only shall the Notification be resent if transmission fails but also if the client fails processing it. This + * will allow the client to use failing database transactions in the event handlers and just rollback and throw if the transaction + * fails. + * The implementation MUST use synchronous FCP communication to allow the client to signal an error. + * Also, synchronous communication is necessary for guaranteeing the notifications to arrive in proper order at the client. + *

+ * + * If this function fails by an exception, the caller MUST NOT proceed to deploy subsequent + * Notifications to the client. Instead, it must retry calling this function upon + * the same Notification until it succeeds or the retry limit is exceeded.
+ * This is necessary because Notifications must be deployed in proper order to guarantee + * that they make sense to the client.
+ * Notice that the above does not apply to all types of exceptions. See the JavaDoc of + * the various thrown exceptions for details.

+ * + * Thread safety:
+ * This must be called while locking upon the {@link SubscriptionManager}.
+ * The {@link WebOfTrust} object shall NOT be locked: + * The Notification objects which this function receives contain serialized clones of the objects from WebOfTrust. + * Therefore, the notifications are self-contained and this function should and must NOT call any database query functions of the WebOfTrust. + * + * @throws IOException + * If the FCP client has disconnected. The SubscriptionManager then should not + * retry deploying this Notification, the {@link Subscription} should be + * terminated instead. + * @throws InterruptedException + * If an external thread requested the current thread to terminate via + * {@link Thread#interrupt()} while the data was being transferred to the + * client. + *
This is a necessary shutdown mechanism as clients can be attached by + * network and thus transfers can take a long time. Please honor it by + * terminating the thread so WOT can shutdown quickly. + * @throws FCPCallFailedException + * If processing failed at the client.
+ * The SubscriptionManager should call + * {@link Client#incrementSendNotificationsFailureCountWithoutCommit()} upon + * {@link #mClient} then, and retry calling this function upon the same + * Notification at the next iteration of the notification sending thread + * - until the limit is reached. */ - void notifySubscriberByFCP() + protected final void notifySubscriberByFCP() throws InterruptedException, FCPCallFailedException, IOException { - getSubscription().notifySubscriberByFCP(this); + UUID clientID = getSubscription().getClientID(); + notifyClientByFCP(clientID, mWebOfTrust.getFCPInterface()); } + + /** + * Notification type specific implementation of {@link #notifySubscriberByFCP()}. + * @see Notification#notifySubscriberByFCP() + * @param clientID the client to send this notification to + * @param fcp the FCP connection interface to send over + */ + protected abstract void notifyClientByFCP(UUID clientID, FCPInterface fcp) + throws InterruptedException, FCPCallFailedException, IOException; } /** @@ -890,7 +895,13 @@ public void startupDatabaseIntegrityTest() throws Exception { UUID.fromString(getID()); // Will throw if ID is no valid UUID. } - + + @Override + protected void notifyClientByFCP(UUID clientID, FCPInterface fcp) + throws InterruptedException, FCPCallFailedException, IOException { + fcp.sendBeginOrEndSynchronizationNotification(clientID, this); + } + @Override public String toString() { return super.toString() + " { mVersionID=" + getID() + " }"; @@ -940,6 +951,11 @@ protected IdentityChangedNotification(final Subscription mySubscriptio super(mySubscription, oldIdentity, newIdentity); } + @Override + protected void notifyClientByFCP(UUID clientID, FCPInterface fcp) + throws InterruptedException, FCPCallFailedException, IOException { + fcp.sendIdentityChangedNotification(clientID, this); + } } /** @@ -968,7 +984,12 @@ protected TrustChangedNotification(final Subscription mySubscription, final Trust oldTrust, final Trust newTrust) { super(mySubscription, oldTrust, newTrust); } - + + @Override + protected void notifyClientByFCP(UUID clientID, FCPInterface fcp) + throws InterruptedException, FCPCallFailedException, IOException { + fcp.sendTrustChangedNotification(clientID, this); + } } /** @@ -998,6 +1019,11 @@ protected ScoreChangedNotification(final Subscription mySubscription, super(mySubscription, oldScore, newScore); } + @Override + protected void notifyClientByFCP(UUID clientID, FCPInterface fcp) + throws InterruptedException, FCPCallFailedException, IOException { + fcp.sendScoreChangedNotification(clientID, this); + } } /** @@ -1022,37 +1048,6 @@ protected IdentitiesSubscription(final Client myClient) { return mWebOfTrust.getAllIdentities(); } - // TODO: Code quality: This function is almost the same in TrustsSubscription and - // ScoresSubscription. Add a type parameter to Notification and use - // it to move this function upwards to the Subscription base class. - /** {@inheritDoc} - * @param notification*/ - @Override - protected void notifySubscriberByFCP(Notification notification) - throws FCPCallFailedException, IOException, InterruptedException { - - final UUID clientID = getClient().getFCP_ID(); - - if(notification instanceof IdentityChangedNotification) { - mWebOfTrust.getFCPInterface().sendIdentityChangedNotification( - clientID, (IdentityChangedNotification)notification); - } else if(notification instanceof BeginSynchronizationNotification) { - // EndSynchronizationNotification is a child of BeginSynchronizationNotification. - - // Would be a false positive: - // The type parameter is not known at runtime due to the way Java is implemented. - /* assert((BeginSynchronizationNotification)notification != null) - : "The IdentitiesSubscription should only receive Identity notifications"; */ - - mWebOfTrust.getFCPInterface().sendBeginOrEndSynchronizationNotification( - clientID, - (BeginSynchronizationNotification)notification); - } else { - throw new UnsupportedOperationException("Unknown notification type: " - + notification); - } - } - /** * Stores a {@link IdentityChangedNotification} to the {@link Notification} queue of this {@link Client}. * @@ -1090,34 +1085,6 @@ protected TrustsSubscription(final Client myClient) { return mWebOfTrust.getAllTrusts(); } - /** {@inheritDoc} - * @param notification*/ - @Override - protected void notifySubscriberByFCP(final Notification notification) - throws FCPCallFailedException, IOException, InterruptedException { - - final UUID clientID = getClient().getFCP_ID(); - - if(notification instanceof TrustChangedNotification) { - mWebOfTrust.getFCPInterface().sendTrustChangedNotification( - clientID, (TrustChangedNotification)notification); - } else if(notification instanceof BeginSynchronizationNotification) { - // EndSynchronizationNotification is a child of BeginSynchronizationNotification. - - // Would be a false positive: - // The type parameter is not known at runtime due to the way Java is implemented. - /* assert((BeginSynchronizationNotification)notification != null) - : "The TrustsSubscription should only receive Trust notifications"; */ - - mWebOfTrust.getFCPInterface().sendBeginOrEndSynchronizationNotification( - clientID, - (BeginSynchronizationNotification)notification); - } else { - throw new UnsupportedOperationException("Unknown notification type: " - + notification); - } - } - /** * Stores a {@link TrustChangedNotification} to the {@link Notification} queue of this {@link Client}. * @@ -1153,34 +1120,6 @@ protected ScoresSubscription(final Client myClient) { return mWebOfTrust.getAllScores(); } - /** {@inheritDoc} - * @param notification*/ - @Override - protected void notifySubscriberByFCP(final Notification notification) - throws FCPCallFailedException, IOException, InterruptedException { - - final UUID clientID = getClient().getFCP_ID(); - - if(notification instanceof ScoreChangedNotification) { - mWebOfTrust.getFCPInterface().sendScoreChangedNotification( - clientID, (ScoreChangedNotification)notification); - } else if(notification instanceof BeginSynchronizationNotification) { - // EndSynchronizationNotification is a child of BeginSynchronizationNotification. - - // Would be a false positive: - // The type parameter is not known at runtime due to the way Java is implemented. - /* assert((BeginSynchronizationNotification)notification != null) - : "The ScoresSubscription should only receive Score notifications"; */ - - mWebOfTrust.getFCPInterface().sendBeginOrEndSynchronizationNotification( - clientID, - (BeginSynchronizationNotification)notification); - } else { - throw new UnsupportedOperationException("Unknown notification type: " - + notification); - } - } - /** * Stores a {@link ScoreChangedNotification} to the {@link Notification} queue of this {@link Client}. * From a2343cf6d2f1087b6a589ed78e7b405b363c9c04 Mon Sep 17 00:00:00 2001 From: Bert Massop Date: Mon, 17 Nov 2014 18:37:33 +0100 Subject: [PATCH 3/3] ObjectChangedNotification: use EventType instead of Persistent --- src/plugins/WebOfTrust/EventSource.java | 8 +++-- src/plugins/WebOfTrust/Identity.java | 8 ++++- src/plugins/WebOfTrust/Score.java | 8 +++++ .../WebOfTrust/SubscriptionManager.java | 30 +++++++++++-------- src/plugins/WebOfTrust/Trust.java | 8 +++++ 5 files changed, 46 insertions(+), 16 deletions(-) diff --git a/src/plugins/WebOfTrust/EventSource.java b/src/plugins/WebOfTrust/EventSource.java index 96b4c31c9..4a2f1b531 100644 --- a/src/plugins/WebOfTrust/EventSource.java +++ b/src/plugins/WebOfTrust/EventSource.java @@ -32,9 +32,11 @@ * is about. */ public interface EventSource extends Cloneable, Serializable { - - /** {@inheritDoc} */ - public EventSource clone(); // Override because it is not public in class Object. + + /** + * Provides a clone of this event source. Implementations must return {@code this.clone()}. + */ + public EventSource cloned(); /** * When a {@link Notification} about an {@link EventSource} is being deployed to a diff --git a/src/plugins/WebOfTrust/Identity.java b/src/plugins/WebOfTrust/Identity.java index 5ff8d6776..e3435ece4 100644 --- a/src/plugins/WebOfTrust/Identity.java +++ b/src/plugins/WebOfTrust/Identity.java @@ -1005,8 +1005,14 @@ public Identity clone() { throw new IllegalStateException(e); } } - + /** + * @see Identity#clone() + */ + @Override + public Identity cloned() { + return clone(); + } /** * Stores this identity in the database without committing the transaction diff --git a/src/plugins/WebOfTrust/Score.java b/src/plugins/WebOfTrust/Score.java index 5a44a9701..fa2d209f2 100644 --- a/src/plugins/WebOfTrust/Score.java +++ b/src/plugins/WebOfTrust/Score.java @@ -390,6 +390,14 @@ public Score clone() { return clone; } + /** + * @see Score#clone() + */ + @Override + public Score cloned() { + return clone(); + } + @Override public void startupDatabaseIntegrityTest() throws Exception { activateFully(); diff --git a/src/plugins/WebOfTrust/SubscriptionManager.java b/src/plugins/WebOfTrust/SubscriptionManager.java index 784e9f5d7..be7092c10 100644 --- a/src/plugins/WebOfTrust/SubscriptionManager.java +++ b/src/plugins/WebOfTrust/SubscriptionManager.java @@ -532,7 +532,7 @@ protected final void storeSynchronizationWithoutCommit() { // main EventSource object stored in the mWebOfTrust. Thus, we clone() the // EventSource and call the setter upon the temporary clone. @SuppressWarnings("unchecked") - EventType eventSourceWithProperVersionID = (EventType) eventSource.clone(); + EventType eventSourceWithProperVersionID = (EventType) eventSource.cloned(); eventSourceWithProperVersionID.setVersionID(synchronizationID); storeNotificationWithoutCommit(null, eventSourceWithProperVersionID); @@ -562,8 +562,8 @@ protected final void storeSynchronizationWithoutCommit() { /** * Shall store a {@link ObjectChangedNotification} constructed via - * {@link ObjectChangedNotification#ObjectChangedNotification(Subscription, Persistent, - * Persistent)} with parameters oldObject = oldEventSource, newObject = newEventSource.
+ * {@link ObjectChangedNotification#ObjectChangedNotification(Subscription, EventType, + * EventType)} with parameters oldObject = oldEventSource, newObject = newEventSource.
*
* * The type parameter of the {@link ObjectChangedNotification} shall match the type @@ -730,8 +730,8 @@ protected abstract void notifyClientByFCP(UUID clientID, FCPInterface fcp) * NOTICE: Both Persistent objects are not stored in the database and must not be stored there to prevent duplicates! */ @SuppressWarnings("serial") - public static abstract class ObjectChangedNotification extends - Notification { + public static abstract class ObjectChangedNotification extends Notification { /** * A serialized copy of the changed {@link Persistent} object before the change. @@ -757,15 +757,13 @@ public static abstract class ObjectChangedNotification mySubscription, - final Persistent oldObject, final Persistent newObject) { + final EventType oldObject, final EventType newObject) { super(mySubscription); @@ -805,18 +803,26 @@ public void startupDatabaseIntegrityTest() throws Exception { * @return The changed {@link Persistent} object before the change. Null if the change was the creation of the object. * @see #mOldObject The backend member variable of this getter. */ - public final Persistent getOldObject() throws NoSuchElementException { + public final EventType getOldObject() throws NoSuchElementException { checkedActivate(1); // byte[] is a db4o primitive type so 1 is enough - return mOldObject != null ? Persistent.deserialize(mWebOfTrust, mOldObject) : null; + return deserialize(mOldObject); } /** * @return The changed {@link Persistent} object after the change. Null if the change was the deletion of the object. * @see #mNewObject The backend member variable of this getter. */ - public final Persistent getNewObject() throws NoSuchElementException { + public final EventType getNewObject() throws NoSuchElementException { checkedActivate(1); // byte[] is a db4o primitive type so 1 is enough - return mNewObject != null ? Persistent.deserialize(mWebOfTrust, mNewObject) : null; + return deserialize(mNewObject); + } + + @SuppressWarnings("unchecked") + private EventType deserialize(byte[] obj) { + if (obj == null) { + return null; + } + return (EventType) Persistent.deserialize(mWebOfTrust, mNewObject); } /** {@inheritDoc} */ diff --git a/src/plugins/WebOfTrust/Trust.java b/src/plugins/WebOfTrust/Trust.java index 464dc70c4..e7db7997e 100644 --- a/src/plugins/WebOfTrust/Trust.java +++ b/src/plugins/WebOfTrust/Trust.java @@ -418,6 +418,14 @@ public Trust clone() { } } + /** + * @see Trust#clone() + */ + @Override + public Trust cloned() { + return clone(); + } + @Override public void startupDatabaseIntegrityTest() throws Exception { activateFully();