git commit: Fix logic error causing new slave attributes to be discarded.

[wf...@apache.org]

Repository: incubator-aurora
Updated Branches:
  refs/heads/master 065c92700 -> 3f6fba7e9


Fix logic error causing new slave attributes to be discarded.

Bugs closed: AURORA-582

Reviewed at https://reviews.apache.org/r/23653/


Project: http://git-wip-us.apache.org/repos/asf/incubator-aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-aurora/commit/3f6fba7e
Tree: http://git-wip-us.apache.org/repos/asf/incubator-aurora/tree/3f6fba7e
Diff: http://git-wip-us.apache.org/repos/asf/incubator-aurora/diff/3f6fba7e

Branch: refs/heads/master
Commit: 3f6fba7e9be112104e726f25cff1bdb74bcc5b57
Parents: 065c927
Author: Bill Farner <wf...@apache.org>
Authored: Thu Jul 17 14:11:53 2014 -0700
Committer: Bill Farner <wf...@apache.org>
Committed: Thu Jul 17 14:11:53 2014 -0700

----------------------------------------------------------------------
 .../storage/mem/MemAttributeStore.java          | 28 +++++---
 .../storage/mem/MemAttributeStoreTest.java      | 67 ++++++++++++++++++++
 2 files changed, 87 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/3f6fba7e/src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java b/src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java
index 4bb807c..82fcddd 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java
@@ -39,16 +39,28 @@ class MemAttributeStore implements Mutable {
 
   @Override
   public void saveHostAttributes(IHostAttributes attributes) {
-    hostAttributes.putIfAbsent(attributes.getHost(), attributes);
+    hostAttributes.put(
+        attributes.getHost(),
+        merge(attributes, Optional.fromNullable(hostAttributes.get(attributes.getHost()))));
+  }
 
-    IHostAttributes stored = hostAttributes.get(attributes.getHost());
-    HostAttributes updated = stored.newBuilder();
-    if (!stored.isSetMode()) {
-      updated.setMode(attributes.isSetMode() ? attributes.getMode() : MaintenanceMode.NONE);
+  private IHostAttributes merge(IHostAttributes newAttributes, Optional<IHostAttributes> previous) {
+    HostAttributes attributes = newAttributes.newBuilder();
+    if (!attributes.isSetMode()) {
+      // If the newly-saved value does not explicitly set the mode, use the previous value
+      // or the default.
+      MaintenanceMode mode;
+      if (previous.isPresent() && previous.get().isSetMode()) {
+        mode = previous.get().getMode();
+      } else {
+        mode = MaintenanceMode.NONE;
+      }
+      attributes.setMode(mode);
+    }
+    if (!attributes.isSetAttributes()) {
+      attributes.setAttributes(ImmutableSet.<Attribute>of());
     }
-    updated.setAttributes(updated.isSetAttributes()
-        ? updated.getAttributes() : ImmutableSet.<Attribute>of());
-    hostAttributes.replace(attributes.getHost(), stored, IHostAttributes.build(updated));
+    return IHostAttributes.build(attributes);
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/3f6fba7e/src/test/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStoreTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStoreTest.java b/src/test/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStoreTest.java
new file mode 100644
index 0000000..45fa43c
--- /dev/null
+++ b/src/test/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStoreTest.java
@@ -0,0 +1,67 @@
+/**
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.aurora.scheduler.storage.mem;
+
+import com.google.common.base.Optional;
+import com.google.common.collect.ImmutableSet;
+
+import org.apache.aurora.gen.Attribute;
+import org.apache.aurora.gen.HostAttributes;
+import org.apache.aurora.gen.MaintenanceMode;
+import org.apache.aurora.scheduler.storage.AttributeStore;
+import org.apache.aurora.scheduler.storage.entities.IHostAttributes;
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+public class MemAttributeStoreTest {
+
+  private static final IHostAttributes ATTRS = IHostAttributes.build(
+      new HostAttributes()
+          .setHost("hostA")
+          .setSlaveId("slaveA")
+          .setAttributes(ImmutableSet.of(
+              makeAttribute("host", "hostA"),
+              makeAttribute("rack", "rackA")))
+  );
+
+  private AttributeStore.Mutable store;
+
+  @Before
+  public void setUp() {
+    store = new MemAttributeStore();
+  }
+
+  @Test
+  public void testAttributeChange() {
+    store.saveHostAttributes(ATTRS);
+    assertEquals(Optional.of(defaultMode(ATTRS)), store.getHostAttributes(ATTRS.getHost()));
+    HostAttributes builder = ATTRS.newBuilder();
+    builder.addToAttributes(makeAttribute("foo", "bar"));
+    IHostAttributes updated = IHostAttributes.build(builder);
+    store.saveHostAttributes(updated);
+    assertEquals(Optional.of(defaultMode(updated)), store.getHostAttributes(ATTRS.getHost()));
+  }
+
+  private static Attribute makeAttribute(String name, String... values) {
+    return new Attribute()
+        .setName(name)
+        .setValues(ImmutableSet.<String>builder().add(values).build());
+  }
+
+  private static IHostAttributes defaultMode(IHostAttributes attrs) {
+    return IHostAttributes.build(attrs.newBuilder().setMode(MaintenanceMode.NONE));
+  }
+}