Committed by
Ray Milkey
Refactor to avoid trivial errors in transactions
Include Fixing a bug not to commit the transaction on success Change-Id: Ie1f823ab6d8fc6f54091d443d24ecc61336155da
Showing
1 changed file
with
29 additions
and
11 deletions
... | @@ -96,16 +96,14 @@ public class ConsistentResourceStore implements ResourceStore { | ... | @@ -96,16 +96,14 @@ public class ConsistentResourceStore implements ResourceStore { |
96 | ResourceConsumer existing = txMap.putIfAbsent(resource, consumer); | 96 | ResourceConsumer existing = txMap.putIfAbsent(resource, consumer); |
97 | // if the resource is already allocated to another consumer, the whole allocation fails | 97 | // if the resource is already allocated to another consumer, the whole allocation fails |
98 | if (existing != null) { | 98 | if (existing != null) { |
99 | - tx.abort(); | 99 | + return abortTransaction(tx); |
100 | - return false; | ||
101 | } | 100 | } |
102 | } | 101 | } |
103 | - tx.commit(); | 102 | + |
104 | - return true; | 103 | + return commitTransaction(tx); |
105 | } catch (TransactionException e) { | 104 | } catch (TransactionException e) { |
106 | log.error("Exception thrown, abort the transaction", e); | 105 | log.error("Exception thrown, abort the transaction", e); |
107 | - tx.abort(); | 106 | + return abortTransaction(tx); |
108 | - return false; | ||
109 | } | 107 | } |
110 | } | 108 | } |
111 | 109 | ||
... | @@ -130,16 +128,14 @@ public class ConsistentResourceStore implements ResourceStore { | ... | @@ -130,16 +128,14 @@ public class ConsistentResourceStore implements ResourceStore { |
130 | // if this single release fails (because the resource is allocated to another consumer, | 128 | // if this single release fails (because the resource is allocated to another consumer, |
131 | // the whole release fails | 129 | // the whole release fails |
132 | if (!txMap.remove(resource, consumer)) { | 130 | if (!txMap.remove(resource, consumer)) { |
133 | - tx.abort(); | 131 | + return abortTransaction(tx); |
134 | - return false; | ||
135 | } | 132 | } |
136 | } | 133 | } |
137 | 134 | ||
138 | - return true; | 135 | + return commitTransaction(tx); |
139 | } catch (TransactionException e) { | 136 | } catch (TransactionException e) { |
140 | log.error("Exception thrown, abort the transaction", e); | 137 | log.error("Exception thrown, abort the transaction", e); |
141 | - tx.abort(); | 138 | + return abortTransaction(tx); |
142 | - return false; | ||
143 | } | 139 | } |
144 | } | 140 | } |
145 | 141 | ||
... | @@ -169,4 +165,26 @@ public class ConsistentResourceStore implements ResourceStore { | ... | @@ -169,4 +165,26 @@ public class ConsistentResourceStore implements ResourceStore { |
169 | .map(x -> (Resource<S, T>) x.getKey()) | 165 | .map(x -> (Resource<S, T>) x.getKey()) |
170 | .collect(Collectors.toList()); | 166 | .collect(Collectors.toList()); |
171 | } | 167 | } |
168 | + | ||
169 | + /** | ||
170 | + * Abort the transaction. | ||
171 | + * | ||
172 | + * @param tx transaction context | ||
173 | + * @return always false | ||
174 | + */ | ||
175 | + private boolean abortTransaction(TransactionContext tx) { | ||
176 | + tx.abort(); | ||
177 | + return false; | ||
178 | + } | ||
179 | + | ||
180 | + /** | ||
181 | + * Commit the transaction. | ||
182 | + * | ||
183 | + * @param tx transaction context | ||
184 | + * @return always true | ||
185 | + */ | ||
186 | + private boolean commitTransaction(TransactionContext tx) { | ||
187 | + tx.commit(); | ||
188 | + return true; | ||
189 | + } | ||
172 | } | 190 | } | ... | ... |
-
Please register or login to post a comment