From 71b8570a2a2420b5a68113a8a97546f13f1eb4c7 Mon Sep 17 00:00:00 2001 From: Tamas Payer Date: Wed, 17 Mar 2021 14:50:19 +0100 Subject: [PATCH 1/3] Refactor setStatus method Change-Id: Ibadd3a94d1fc0035e6afa17efdcbdf56a98d565a --- .../apache/ambari/server/state/host/HostImpl.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java index f2a9e88e8f1..b500c4e740e 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java @@ -957,18 +957,24 @@ public void setTimeInState(long timeInState) { } } - @Override public String getStatus() { return status; } + /** + * Sets the Host's status. + * + * @param status Host Status string representation of the Host's status + * @throws IllegalArgumentException if status is null or {@link HealthStatus} enum has no such constant name. + */ @Override public void setStatus(String status) { - if (this.status != status) { - ambariEventPublisher.publish(new HostStatusUpdateEvent(getHostName(), status)); + String newStatus = HealthStatus.valueOf(status).name(); + if (!newStatus.equals(this.status)) { + ambariEventPublisher.publish(new HostStatusUpdateEvent(getHostName(), newStatus)); + this.status = newStatus; } - this.status = status; } @Override From e49a12fba023155228ba7404e0736011a835f150 Mon Sep 17 00:00:00 2001 From: Tamas Payer Date: Fri, 19 Mar 2021 16:36:23 +0100 Subject: [PATCH 2/3] Add package private constructor for testing Change-Id: Idb5d032460ce67c351aafc28dfe9e8146a4befc5 --- .../apache/ambari/server/state/host/HostImpl.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java index b500c4e740e..e56395584f7 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java @@ -85,6 +85,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.common.annotations.VisibleForTesting; import com.google.gson.Gson; import com.google.gson.reflect.TypeToken; import com.google.inject.Inject; @@ -320,6 +321,12 @@ public HostImpl(@Assisted HostEntity hostEntity, Gson gson, HostDAO hostDAO, Hos maintMap = ensureMaintMap(hostEntity.getHostStateEntity()); } + @VisibleForTesting + HostImpl(HostEntity hostEntity, Gson gson, HostDAO hostDAO, HostStateDAO hostStateDAO, AmbariEventPublisher publisher) { + this(hostEntity, gson, hostDAO, hostStateDAO); + this.ambariEventPublisher = publisher; + } + @Override public int compareTo(Object o) { if ((o != null ) && (o instanceof Host)) { @@ -970,6 +977,9 @@ public String getStatus() { */ @Override public void setStatus(String status) { + if (status == null) { + throw new IllegalArgumentException("Host status cannot be null"); + } String newStatus = HealthStatus.valueOf(status).name(); if (!newStatus.equals(this.status)) { ambariEventPublisher.publish(new HostStatusUpdateEvent(getHostName(), newStatus)); @@ -1329,8 +1339,8 @@ public boolean isRepositoryVersionCorrect(RepositoryVersionEntity repositoryVers // for every host component, if it matches the desired repo and has reported // the correct version then we're good for (HostComponentStateEntity hostComponentState : hostComponentStates) { - ServiceComponentDesiredStateEntity desiredComponmentState = hostComponentState.getServiceComponentDesiredStateEntity(); - RepositoryVersionEntity desiredRepositoryVersion = desiredComponmentState.getDesiredRepositoryVersion(); + ServiceComponentDesiredStateEntity desiredComponentState = hostComponentState.getServiceComponentDesiredStateEntity(); + RepositoryVersionEntity desiredRepositoryVersion = desiredComponentState.getDesiredRepositoryVersion(); ComponentInfo componentInfo = ambariMetaInfo.getComponent( desiredRepositoryVersion.getStackName(), desiredRepositoryVersion.getStackVersion(), From c6632864a25800676cb71d07cca28fbc34f74955 Mon Sep 17 00:00:00 2001 From: Tamas Payer Date: Fri, 19 Mar 2021 16:37:20 +0100 Subject: [PATCH 3/3] Add unit tests Change-Id: I7fa4d936359502d27caf3381912ad696f829b96a --- .../server/state/host/HostImplTest.java | 69 ++++++++++++++++++- 1 file changed, 68 insertions(+), 1 deletion(-) diff --git a/ambari-server/src/test/java/org/apache/ambari/server/state/host/HostImplTest.java b/ambari-server/src/test/java/org/apache/ambari/server/state/host/HostImplTest.java index d8195c20937..c914c053b92 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/state/host/HostImplTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/state/host/HostImplTest.java @@ -17,15 +17,24 @@ */ package org.apache.ambari.server.state.host; +import static org.easymock.EasyMock.anyObject; import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.expectLastCall; import static org.junit.Assert.assertEquals; import java.util.Map; + +import org.apache.ambari.server.events.HostStatusUpdateEvent; +import org.apache.ambari.server.events.publishers.AmbariEventPublisher; import org.apache.ambari.server.orm.dao.HostDAO; import org.apache.ambari.server.orm.dao.HostStateDAO; import org.apache.ambari.server.orm.entities.HostEntity; import org.apache.ambari.server.orm.entities.HostStateEntity; + +import org.apache.ambari.server.state.Host; +import org.apache.ambari.server.state.HostHealthStatus; + import org.easymock.EasyMockSupport; import org.junit.Test; @@ -66,7 +75,7 @@ public void testGetHostAttributes() throws Exception { } @Test - public void testGetHealthStatus() throws Exception { + public void testGetHealthStatus() { HostEntity hostEntity = createNiceMock(HostEntity.class); HostStateEntity hostStateEntity = createNiceMock(HostStateEntity.class); @@ -93,4 +102,62 @@ public void testGetHealthStatus() throws Exception { verifyAll(); } + + @Test(expected = IllegalArgumentException.class) + public void testSetStatusWithNull() { + HostEntity hostEntity = createNiceMock(HostEntity.class); + HostDAO hostDAO = createNiceMock(HostDAO.class); + HostStateDAO hostStateDAO = createNiceMock(HostStateDAO.class); + + expect(hostEntity.getHostId()).andReturn(1L).anyTimes(); + replayAll(); + + Host host = new HostImpl(hostEntity, new Gson(), hostDAO, hostStateDAO); + host.setStatus(null); + } + + @Test(expected = IllegalArgumentException.class) + public void testSetStatusWithIllegalState() { + HostEntity hostEntity = createNiceMock(HostEntity.class); + HostDAO hostDAO = createNiceMock(HostDAO.class); + HostStateDAO hostStateDAO = createNiceMock(HostStateDAO.class); + + expect(hostEntity.getHostId()).andReturn(1L).anyTimes(); + replayAll(); + + Host host = new HostImpl(hostEntity, new Gson(), hostDAO, hostStateDAO); + host.setStatus("illegal status"); + } + + @Test + public void testSetStatusWithValidArgs() { + HostEntity hostEntity = createNiceMock(HostEntity.class); + HostDAO hostDAO = createNiceMock(HostDAO.class); + HostStateDAO hostStateDAO = createNiceMock(HostStateDAO.class); + AmbariEventPublisher eventPublisher = createNiceMock(AmbariEventPublisher.class); + + expect(hostEntity.getHostName()).andReturn("host1").anyTimes(); + expect(hostEntity.getHostId()).andReturn(1L).anyTimes(); + + eventPublisher.publish(anyObject(HostStatusUpdateEvent.class)); + expectLastCall().times(4); + + replayAll(); + + Host host = new HostImpl(hostEntity, new Gson(), hostDAO, hostStateDAO, eventPublisher); + + host.setStatus(HostHealthStatus.HealthStatus.UNHEALTHY.name()); + assertEquals(HostHealthStatus.HealthStatus.UNHEALTHY.name(), host.getStatus()); + + host.setStatus(HostHealthStatus.HealthStatus.HEALTHY.name()); + assertEquals(HostHealthStatus.HealthStatus.HEALTHY.name(), host.getStatus()); + + host.setStatus(HostHealthStatus.HealthStatus.ALERT.name()); + assertEquals(HostHealthStatus.HealthStatus.ALERT.name(), host.getStatus()); + + host.setStatus(HostHealthStatus.HealthStatus.UNKNOWN.name()); + assertEquals(HostHealthStatus.HealthStatus.UNKNOWN.name(), host.getStatus()); + + verifyAll(); + } }