This is an automated email from the ASF dual-hosted git repository.

mpapirkovskyy pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ambari.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 4c877c8  AMBARI-24074. Guarantee STOMP update contains not null 
hash/timestamp if exists. (mpapirkovskyy) (#1517)
4c877c8 is described below

commit 4c877c8f7e57b68068a41dff38c5b7cab3fbb8ce
Author: Myroslav Papirkovskyi <[email protected]>
AuthorDate: Wed Jun 13 15:02:09 2018 +0300

    AMBARI-24074. Guarantee STOMP update contains not null hash/timestamp if 
exists. (mpapirkovskyy) (#1517)
---
 .../server/agent/stomp/AgentClusterDataHolder.java |  29 +-----
 .../server/agent/stomp/AgentConfigsHolder.java     |   2 -
 .../ambari/server/agent/stomp/AgentDataHolder.java |  19 +++-
 .../stomp/dto/HashAndTimestampIgnoreMixIn.java     |  32 +++++++
 .../server/agent/stomp/dto/HashIgnoreMixIn.java    |  29 ++++++
 .../server/agent/stomp/AgentDataHolderTest.java    | 104 +++++++++++++++++++++
 6 files changed, 186 insertions(+), 29 deletions(-)

diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/agent/stomp/AgentClusterDataHolder.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/agent/stomp/AgentClusterDataHolder.java
index e5f49e1..afb4273 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/agent/stomp/AgentClusterDataHolder.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/agent/stomp/AgentClusterDataHolder.java
@@ -19,8 +19,6 @@
 package org.apache.ambari.server.agent.stomp;
 
 import java.util.Objects;
-import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantLock;
 
 import javax.inject.Inject;
 
@@ -40,17 +38,9 @@ public abstract class AgentClusterDataHolder<T extends 
STOMPEvent & Hashable> ex
 
   private T data;
 
-  //TODO perhaps need optimization
-  private Lock lock = new ReentrantLock();
-
   public T getUpdateIfChanged(String agentHash) throws AmbariException {
-    try {
-      lock.lock();
-      initializeDataIfNeeded(true);
-      return !Objects.equals(agentHash, data.getHash()) ? data : 
getEmptyData();
-    } finally {
-      lock.unlock();
-    }
+    initializeDataIfNeeded(true);
+    return !Objects.equals(agentHash, data.getHash()) ? data : getEmptyData();
   }
 
   /**
@@ -73,32 +63,23 @@ public abstract class AgentClusterDataHolder<T extends 
STOMPEvent & Hashable> ex
     initializeDataIfNeeded(false);
     boolean changed = handleUpdate(update);
     if (changed) {
-      regenerateHash();
+      regenerateDataIdentifiers(data);
       update.setHash(getData().getHash());
       STOMPUpdatePublisher.publish(update);
     } else {
       // in case update does not have changes empty identifiers should be 
populated anyway
       if (!isIdentifierValid(data)) {
-        regenerateHash();
+        regenerateDataIdentifiers(data);
       }
     }
     return changed;
   }
 
-  protected final void regenerateHash() {
-    try {
-      lock.lock();
-      regenerateDataIdentifiers(data);
-    } finally {
-      lock.unlock();
-    }
-  }
-
   protected final void initializeDataIfNeeded(boolean regenerateHash) throws 
AmbariException {
     if (data == null) {
       data = getCurrentData();
       if (regenerateHash) {
-        regenerateHash();
+        regenerateDataIdentifiers(data);
       }
     }
   }
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/agent/stomp/AgentConfigsHolder.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/agent/stomp/AgentConfigsHolder.java
index e8db31d..44a6e7f 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/agent/stomp/AgentConfigsHolder.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/agent/stomp/AgentConfigsHolder.java
@@ -91,8 +91,6 @@ public class AgentConfigsHolder extends 
AgentHostDataHolder<AgentConfigsUpdateEv
 
   @Override
   protected void regenerateDataIdentifiers(AgentConfigsUpdateEvent data) {
-    data.setHash(null);
-    data.setTimestamp(null);
     data.setHash(getHash(data));
     data.setTimestamp(System.currentTimeMillis());
   }
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/agent/stomp/AgentDataHolder.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/agent/stomp/AgentDataHolder.java
index ec41447..e005fe0 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/agent/stomp/AgentDataHolder.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/agent/stomp/AgentDataHolder.java
@@ -22,10 +22,14 @@ import java.io.UnsupportedEncodingException;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 
+import org.apache.ambari.server.agent.stomp.dto.HashAndTimestampIgnoreMixIn;
+import org.apache.ambari.server.agent.stomp.dto.HashIgnoreMixIn;
 import org.apache.ambari.server.agent.stomp.dto.Hashable;
+import org.apache.ambari.server.events.AgentConfigsUpdateEvent;
 import org.apache.commons.lang.StringUtils;
 
-import com.google.gson.Gson;
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
 
 /**
  * Is used to hash generating for event
@@ -33,11 +37,15 @@ import com.google.gson.Gson;
  */
 public abstract class AgentDataHolder<T extends Hashable> {
   private final String salt = "";
+  private final static ObjectMapper MAPPER = new ObjectMapper();
+  static {
+    MAPPER.addMixIn(Hashable.class, HashIgnoreMixIn.class);
+    MAPPER.addMixIn(AgentConfigsUpdateEvent.class, 
HashAndTimestampIgnoreMixIn.class);
+  }
 
   protected abstract T getEmptyData();
 
   protected void regenerateDataIdentifiers(T data) {
-    data.setHash(null);
     data.setHash(getHash(data));
   }
 
@@ -46,7 +54,12 @@ public abstract class AgentDataHolder<T extends Hashable> {
   }
 
   protected String getHash(T data) {
-    String json = new Gson().toJson(data);
+    String json = null;
+    try {
+      json = MAPPER.writeValueAsString(data);
+    } catch (JsonProcessingException e) {
+      throw new RuntimeException("Error during mapping message to calculate 
hash", e);
+    }
     String generatedPassword = null;
     try {
       MessageDigest md = MessageDigest.getInstance("SHA-512");
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/agent/stomp/dto/HashAndTimestampIgnoreMixIn.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/agent/stomp/dto/HashAndTimestampIgnoreMixIn.java
new file mode 100644
index 0000000..11f33d8
--- /dev/null
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/agent/stomp/dto/HashAndTimestampIgnoreMixIn.java
@@ -0,0 +1,32 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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.ambari.server.agent.stomp.dto;
+
+import com.fasterxml.jackson.annotation.JsonIgnore;
+
+/**
+ * Mix-in annotations class for {@link 
org.apache.ambari.server.events.AgentConfigsUpdateEvent}.
+ * Is used to hide hash and timestamp properties during mapping to json.
+ */
+public abstract class HashAndTimestampIgnoreMixIn {
+  @JsonIgnore
+  abstract String getHash();
+  @JsonIgnore
+  abstract String getTimestamp();
+}
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/agent/stomp/dto/HashIgnoreMixIn.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/agent/stomp/dto/HashIgnoreMixIn.java
new file mode 100644
index 0000000..d8b7de9
--- /dev/null
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/agent/stomp/dto/HashIgnoreMixIn.java
@@ -0,0 +1,29 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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.ambari.server.agent.stomp.dto;
+
+import com.fasterxml.jackson.annotation.JsonIgnore;
+
+/**
+ * Mix-in annotations class for {@link Hashable}. Is used to hide hash 
property during mapping to json.
+ */
+public abstract class HashIgnoreMixIn {
+  @JsonIgnore
+  abstract String getHash();
+}
diff --git 
a/ambari-server/src/test/java/org/apache/ambari/server/agent/stomp/AgentDataHolderTest.java
 
b/ambari-server/src/test/java/org/apache/ambari/server/agent/stomp/AgentDataHolderTest.java
new file mode 100644
index 0000000..bb5c988
--- /dev/null
+++ 
b/ambari-server/src/test/java/org/apache/ambari/server/agent/stomp/AgentDataHolderTest.java
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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.ambari.server.agent.stomp;
+
+import static org.easymock.EasyMock.createNiceMock;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+
+import org.apache.ambari.server.events.AgentConfigsUpdateEvent;
+import org.apache.ambari.server.events.MetadataUpdateEvent;
+import org.apache.ambari.server.events.UpdateEventType;
+import org.apache.ambari.server.events.publishers.AmbariEventPublisher;
+import org.apache.commons.collections.MapUtils;
+import org.junit.Test;
+
+public class AgentDataHolderTest {
+
+  @Test
+  public void testGetHashWithTimestamp() {
+    AmbariEventPublisher ambariEventPublisher = 
createNiceMock(AmbariEventPublisher.class);
+    AgentConfigsHolder agentConfigsHolder = new 
AgentConfigsHolder(ambariEventPublisher);
+
+    AgentConfigsUpdateEvent event1 = new AgentConfigsUpdateEvent(null);
+    event1.setHash("01");
+    event1.setTimestamp(1L);
+    String eventHash1 = agentConfigsHolder.getHash(event1);
+
+    // difference in hash only
+    AgentConfigsUpdateEvent event2 = new AgentConfigsUpdateEvent(null);
+    event2.setHash("02");
+    event2.setTimestamp(1L);
+    String eventHash2 = agentConfigsHolder.getHash(event2);
+
+    // difference in timestamp only
+    AgentConfigsUpdateEvent event3 = new AgentConfigsUpdateEvent(null);
+    event3.setHash("01");
+    event3.setTimestamp(2L);
+    String eventHash3 = agentConfigsHolder.getHash(event3);
+
+    // difference in both hash and timestamp
+    AgentConfigsUpdateEvent event4 = new AgentConfigsUpdateEvent(null);
+    event4.setHash("02");
+    event4.setTimestamp(2L);
+    String eventHash4 = agentConfigsHolder.getHash(event4);
+
+    // hash and timestamp are the same, changes in body
+    AgentConfigsUpdateEvent event5 = new 
AgentConfigsUpdateEvent(MapUtils.EMPTY_SORTED_MAP);
+    event5.setHash("01");
+    event5.setTimestamp(1L);
+    String eventHash5 = agentConfigsHolder.getHash(event5);
+
+    assertEquals(eventHash1, eventHash2);
+    assertEquals(eventHash1, eventHash3);
+    assertEquals(eventHash1, eventHash4);
+    assertFalse(eventHash1.equals(eventHash5));
+  }
+
+  @Test
+  public void testGetHash() {
+    AmbariEventPublisher ambariEventPublisher = 
createNiceMock(AmbariEventPublisher.class);
+    MetadataHolder metadataHolder = new MetadataHolder(ambariEventPublisher);
+
+    MetadataUpdateEvent event1 = new MetadataUpdateEvent(null,
+        null,
+        null,
+        UpdateEventType.CREATE);
+    event1.setHash("01");
+    String eventHash1 = metadataHolder.getHash(event1);
+
+    // difference in hash only
+    MetadataUpdateEvent event2 = new MetadataUpdateEvent(null,
+        null,
+        null,
+        UpdateEventType.CREATE);
+    event2.setHash("02");
+    String eventHash2 = metadataHolder.getHash(event2);
+
+    // the same hash, but the body was changed
+    MetadataUpdateEvent event3 = new MetadataUpdateEvent(null,
+        null,
+        null,
+        UpdateEventType.UPDATE);
+    event3.setHash("01");
+    String eventHash3 = metadataHolder.getHash(event3);
+
+    assertEquals(eventHash1, eventHash2);
+    assertFalse(eventHash1.equals(eventHash3));
+  }
+}

-- 
To stop receiving notification emails like this one, please contact
[email protected].

Reply via email to