You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by hu...@apache.org on 2014/02/25 15:18:47 UTC
[1/2] git commit: updated refs/heads/master to afc188c
Repository: cloudstack
Updated Branches:
refs/heads/master abf3c055c -> afc188cb5
Forward port the fix by David from 0c2ad0338e34f6117cecc24ec00c7746dd481465
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/afc188cb
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/afc188cb
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/afc188cb
Branch: refs/heads/master
Commit: afc188cb5c72e316975799c95529e8692ddcb94b
Parents: 3a7e410
Author: Hugo Trippaers <ht...@schubergphilis.com>
Authored: Tue Feb 25 15:17:16 2014 +0100
Committer: Hugo Trippaers <ht...@schubergphilis.com>
Committed: Tue Feb 25 15:18:16 2014 +0100
----------------------------------------------------------------------
client/tomcatconf/catalina.properties.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/afc188cb/client/tomcatconf/catalina.properties.in
----------------------------------------------------------------------
diff --git a/client/tomcatconf/catalina.properties.in b/client/tomcatconf/catalina.properties.in
index dc2db35..ce03ff6 100644
--- a/client/tomcatconf/catalina.properties.in
+++ b/client/tomcatconf/catalina.properties.in
@@ -44,7 +44,7 @@ package.definition=sun.,java.,org.apache.catalina.,org.apache.coyote.,org.apache
# "foo/*.jar": Add all the JARs of the specified folder as class
# repositories
# "foo/bar.jar": Add bar.jar as a class repository
-common.loader=${catalina.base}/lib,${catalina.base}/lib/*.jar,${catalina.home}/lib,${catalina.home}/lib/*.jar
+common.loader=${catalina.base}/lib,${catalina.base}/lib/*.jar,${catalina.home}/lib,${catalina.home}/lib/*.jar,/usr/share/java/mysql-connector-java.jar
#
# List of comma-separated paths defining the contents of the "server"
[2/2] git commit: updated refs/heads/master to afc188c
Posted by hu...@apache.org.
FindBugs findings: fixing equals() methods in 2 classes; commenting out dead variable in 1 class; adding 5 tests to cover the changes in the equals() methods.
Signed-off-by: Hugo Trippaers <ht...@schubergphilis.com>
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/3a7e4103
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/3a7e4103
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/3a7e4103
Branch: refs/heads/master
Commit: 3a7e4103fc2ca54e82d597288acd76332b054b98
Parents: abf3c05
Author: wrodrigues <wr...@schubergphilis.com>
Authored: Sat Feb 15 13:23:35 2014 +0100
Committer: Hugo Trippaers <ht...@schubergphilis.com>
Committed: Tue Feb 25 15:18:16 2014 +0100
----------------------------------------------------------------------
.../com/cloud/agent/manager/AgentAttache.java | 40 +++++-----
.../agent/manager/ClusteredAgentAttache.java | 18 ++---
.../agent/manager/ConnectedAgentAttache.java | 33 ++++++--
.../service/api/ProvisioningServiceImpl.java | 34 ++++----
.../manager/ConnectedAgentAttacheTest.java | 82 ++++++++++++++++++++
5 files changed, 155 insertions(+), 52 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3a7e4103/engine/orchestration/src/com/cloud/agent/manager/AgentAttache.java
----------------------------------------------------------------------
diff --git a/engine/orchestration/src/com/cloud/agent/manager/AgentAttache.java b/engine/orchestration/src/com/cloud/agent/manager/AgentAttache.java
index 3ebaf4a..fd1531e 100755
--- a/engine/orchestration/src/com/cloud/agent/manager/AgentAttache.java
+++ b/engine/orchestration/src/com/cloud/agent/manager/AgentAttache.java
@@ -32,9 +32,8 @@ import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
-import org.apache.log4j.Logger;
-
import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.log4j.Logger;
import com.cloud.agent.Listener;
import com.cloud.agent.api.Answer;
@@ -72,7 +71,7 @@ public abstract class AgentAttache {
protected static final Comparator<Request> s_reqComparator = new Comparator<Request>() {
@Override
- public int compare(Request o1, Request o2) {
+ public int compare(final Request o1, final Request o2) {
long seq1 = o1.getSequence();
long seq2 = o2.getSequence();
if (seq1 < seq2) {
@@ -87,7 +86,7 @@ public abstract class AgentAttache {
protected static final Comparator<Object> s_seqComparator = new Comparator<Object>() {
@Override
- public int compare(Object o1, Object o2) {
+ public int compare(final Object o1, final Object o2) {
long seq1 = ((Request)o1).getSequence();
long seq2 = (Long)o2;
if (seq1 < seq2) {
@@ -122,7 +121,7 @@ public abstract class AgentAttache {
Arrays.sort(s_commandsNotAllowedInConnectingMode);
}
- protected AgentAttache(AgentManagerImpl agentMgr, final long id, final String name, boolean maintenance) {
+ protected AgentAttache(final AgentManagerImpl agentMgr, final long id, final String name, final boolean maintenance) {
_id = id;
_name = name;
_waitForList = new ConcurrentHashMap<Long, Listener>();
@@ -180,13 +179,13 @@ public abstract class AgentAttache {
}
}
- protected synchronized void addRequest(Request req) {
+ protected synchronized void addRequest(final Request req) {
int index = findRequest(req);
assert (index < 0) : "How can we get index again? " + index + ":" + req.toString();
_requests.add(-index - 1, req);
}
- protected void cancel(Request req) {
+ protected void cancel(final Request req) {
long seq = req.getSequence();
cancel(seq);
}
@@ -205,11 +204,11 @@ public abstract class AgentAttache {
}
}
- protected synchronized int findRequest(Request req) {
+ protected synchronized int findRequest(final Request req) {
return Collections.binarySearch(_requests, req, s_reqComparator);
}
- protected synchronized int findRequest(long seq) {
+ protected synchronized int findRequest(final long seq) {
return Collections.binarySearch(_requests, seq, s_seqComparator);
}
@@ -331,17 +330,20 @@ public abstract class AgentAttache {
}
@Override
- public boolean equals(Object obj) {
- try {
- AgentAttache that = (AgentAttache)obj;
- return _id == that._id;
- } catch (ClassCastException e) {
- assert false : "Who's sending an " + obj.getClass().getSimpleName() + " to AgentAttache.equals()? ";
+ public boolean equals(final Object obj) {
+ // Return false straight away.
+ if (obj == null) {
+ return false;
+ }
+ // No need to handle a ClassCastException. If the classes are different, then equals can return false straight ahead.
+ if (this.getClass() != obj.getClass()) {
return false;
}
+ AgentAttache that = (AgentAttache)obj;
+ return _id == that._id;
}
- public void send(Request req, final Listener listener) throws AgentUnavailableException {
+ public void send(final Request req, final Listener listener) throws AgentUnavailableException {
checkAvailability(req.getCommands());
long seq = req.getSequence();
@@ -387,7 +389,7 @@ public abstract class AgentAttache {
}
}
- public Answer[] send(Request req, int wait) throws AgentUnavailableException, OperationTimedoutException {
+ public Answer[] send(final Request req, final int wait) throws AgentUnavailableException, OperationTimedoutException {
SynchronousListener sl = new SynchronousListener(null);
long seq = req.getSequence();
@@ -478,7 +480,7 @@ public abstract class AgentAttache {
_currentSequence = req.getSequence();
}
- public void process(Answer[] answers) {
+ public void process(final Answer[] answers) {
//do nothing
}
@@ -505,7 +507,7 @@ public abstract class AgentAttache {
protected class Alarm extends ManagedContextRunnable {
long _seq;
- public Alarm(long seq) {
+ public Alarm(final long seq) {
_seq = seq;
}
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3a7e4103/engine/orchestration/src/com/cloud/agent/manager/ClusteredAgentAttache.java
----------------------------------------------------------------------
diff --git a/engine/orchestration/src/com/cloud/agent/manager/ClusteredAgentAttache.java b/engine/orchestration/src/com/cloud/agent/manager/ClusteredAgentAttache.java
index 23c3f76..306c47f 100755
--- a/engine/orchestration/src/com/cloud/agent/manager/ClusteredAgentAttache.java
+++ b/engine/orchestration/src/com/cloud/agent/manager/ClusteredAgentAttache.java
@@ -42,17 +42,17 @@ public class ClusteredAgentAttache extends ConnectedAgentAttache implements Rout
protected final LinkedList<Request> _transferRequests;
protected boolean _transferMode = false;
- static public void initialize(ClusteredAgentManagerImpl agentMgr) {
+ static public void initialize(final ClusteredAgentManagerImpl agentMgr) {
s_clusteredAgentMgr = agentMgr;
}
- public ClusteredAgentAttache(AgentManagerImpl agentMgr, long id, String name) {
+ public ClusteredAgentAttache(final AgentManagerImpl agentMgr, final long id, final String name) {
super(agentMgr, id, name, null, false);
_forward = true;
_transferRequests = new LinkedList<Request>();
}
- public ClusteredAgentAttache(AgentManagerImpl agentMgr, long id, String name, Link link, boolean maintenance) {
+ public ClusteredAgentAttache(final AgentManagerImpl agentMgr, final long id, final String name, final Link link, final boolean maintenance) {
super(agentMgr, id, name, link, maintenance);
_forward = link == null;
_transferRequests = new LinkedList<Request>();
@@ -84,7 +84,7 @@ public class ClusteredAgentAttache extends ConnectedAgentAttache implements Rout
}
@Override
- public void cancel(long seq) {
+ public void cancel(final long seq) {
if (forForward()) {
Listener listener = getListener(seq);
if (listener != null && listener instanceof SynchronousListener) {
@@ -106,7 +106,7 @@ public class ClusteredAgentAttache extends ConnectedAgentAttache implements Rout
}
@Override
- public void routeToAgent(byte[] data) throws AgentUnavailableException {
+ public void routeToAgent(final byte[] data) throws AgentUnavailableException {
if (s_logger.isDebugEnabled()) {
s_logger.debug(log(Request.getSequence(data), "Routing from " + Request.getManagementServerId(data)));
}
@@ -136,7 +136,7 @@ public class ClusteredAgentAttache extends ConnectedAgentAttache implements Rout
}
@Override
- public void send(Request req, Listener listener) throws AgentUnavailableException {
+ public void send(final Request req, final Listener listener) throws AgentUnavailableException {
if (_link != null) {
super.send(req, listener);
return;
@@ -220,7 +220,7 @@ public class ClusteredAgentAttache extends ConnectedAgentAttache implements Rout
_transferMode = transfer;
}
- public boolean getTransferMode() {
+ public synchronized boolean getTransferMode() {
return _transferMode;
}
@@ -232,13 +232,13 @@ public class ClusteredAgentAttache extends ConnectedAgentAttache implements Rout
}
}
- protected synchronized void addRequestToTransfer(Request req) {
+ protected synchronized void addRequestToTransfer(final Request req) {
int index = findTransferRequest(req);
assert (index < 0) : "How can we get index again? " + index + ":" + req.toString();
_transferRequests.add(-index - 1, req);
}
- protected synchronized int findTransferRequest(Request req) {
+ protected synchronized int findTransferRequest(final Request req) {
return Collections.binarySearch(_transferRequests, req, s_reqComparator);
}
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3a7e4103/engine/orchestration/src/com/cloud/agent/manager/ConnectedAgentAttache.java
----------------------------------------------------------------------
diff --git a/engine/orchestration/src/com/cloud/agent/manager/ConnectedAgentAttache.java b/engine/orchestration/src/com/cloud/agent/manager/ConnectedAgentAttache.java
index 00d54bb..f11a105 100755
--- a/engine/orchestration/src/com/cloud/agent/manager/ConnectedAgentAttache.java
+++ b/engine/orchestration/src/com/cloud/agent/manager/ConnectedAgentAttache.java
@@ -33,13 +33,13 @@ public class ConnectedAgentAttache extends AgentAttache {
protected Link _link;
- public ConnectedAgentAttache(AgentManagerImpl agentMgr, final long id, final String name, final Link link, boolean maintenance) {
+ public ConnectedAgentAttache(final AgentManagerImpl agentMgr, final long id, final String name, final Link link, final boolean maintenance) {
super(agentMgr, id, name, maintenance);
_link = link;
}
@Override
- public synchronized void send(Request req) throws AgentUnavailableException {
+ public synchronized void send(final Request req) throws AgentUnavailableException {
try {
_link.send(req.toBytes());
} catch (ClosedChannelException e) {
@@ -67,14 +67,31 @@ public class ConnectedAgentAttache extends AgentAttache {
}
@Override
- public boolean equals(Object obj) {
- try {
- ConnectedAgentAttache that = (ConnectedAgentAttache)obj;
- return super.equals(obj) && _link == that._link && _link != null;
- } catch (ClassCastException e) {
- assert false : "Who's sending an " + obj.getClass().getSimpleName() + " to " + this.getClass().getSimpleName() + ".equals()? ";
+ public int hashCode() {
+ final int prime = 31;
+ int result = 1;
+ result = prime * result + ((_link == null) ? 0 : _link.hashCode());
+ return result;
+ }
+
+ @Override
+ public boolean equals(final Object obj) {
+ // Return false straight away.
+ if (obj == null) {
+ return false;
+ }
+ // No need to handle a ClassCastException. If the classes are different, then equals can return false straight ahead.
+ if (this.getClass() != obj.getClass()) {
+ return false;
+ }
+ // This should not be part of the equals() method, but I'm keeping it because it is expected behaviour based
+ // on the previous implementation. The link attribute of the other object should be checked here as well
+ // to verify if it's not null whilst the this is null.
+ if (_link == null) {
return false;
}
+ ConnectedAgentAttache that = (ConnectedAgentAttache)obj;
+ return super.equals(obj) && _link == that._link;
}
@Override
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3a7e4103/engine/orchestration/src/org/apache/cloudstack/engine/service/api/ProvisioningServiceImpl.java
----------------------------------------------------------------------
diff --git a/engine/orchestration/src/org/apache/cloudstack/engine/service/api/ProvisioningServiceImpl.java b/engine/orchestration/src/org/apache/cloudstack/engine/service/api/ProvisioningServiceImpl.java
index 86eab58..51e8766 100644
--- a/engine/orchestration/src/org/apache/cloudstack/engine/service/api/ProvisioningServiceImpl.java
+++ b/engine/orchestration/src/org/apache/cloudstack/engine/service/api/ProvisioningServiceImpl.java
@@ -25,9 +25,6 @@ import java.util.Map;
import javax.inject.Inject;
import javax.ws.rs.Path;
-import org.springframework.stereotype.Component;
-import org.springframework.stereotype.Service;
-
import org.apache.cloudstack.engine.datacenter.entity.api.ClusterEntity;
import org.apache.cloudstack.engine.datacenter.entity.api.ClusterEntityImpl;
import org.apache.cloudstack.engine.datacenter.entity.api.DataCenterResourceManager;
@@ -38,6 +35,8 @@ import org.apache.cloudstack.engine.datacenter.entity.api.PodEntityImpl;
import org.apache.cloudstack.engine.datacenter.entity.api.StorageEntity;
import org.apache.cloudstack.engine.datacenter.entity.api.ZoneEntity;
import org.apache.cloudstack.engine.datacenter.entity.api.ZoneEntityImpl;
+import org.springframework.stereotype.Component;
+import org.springframework.stereotype.Service;
import com.cloud.host.Host;
import com.cloud.host.Status;
@@ -52,13 +51,13 @@ public class ProvisioningServiceImpl implements ProvisioningService {
DataCenterResourceManager manager;
@Override
- public StorageEntity registerStorage(String name, List<String> tags, Map<String, String> details) {
+ public StorageEntity registerStorage(final String name, final List<String> tags, final Map<String, String> details) {
// TODO Auto-generated method stub
return null;
}
@Override
- public ZoneEntity registerZone(String zoneUuid, String name, String owner, List<String> tags, Map<String, String> details) {
+ public ZoneEntity registerZone(final String zoneUuid, final String name, final String owner, final List<String> tags, final Map<String, String> details) {
ZoneEntityImpl zoneEntity = new ZoneEntityImpl(zoneUuid, manager);
zoneEntity.setName(name);
zoneEntity.setOwner(owner);
@@ -68,7 +67,7 @@ public class ProvisioningServiceImpl implements ProvisioningService {
}
@Override
- public PodEntity registerPod(String podUuid, String name, String owner, String zoneUuid, List<String> tags, Map<String, String> details) {
+ public PodEntity registerPod(final String podUuid, final String name, final String owner, final String zoneUuid, final List<String> tags, final Map<String, String> details) {
PodEntityImpl podEntity = new PodEntityImpl(podUuid, manager);
podEntity.setOwner(owner);
podEntity.setName(name);
@@ -77,7 +76,7 @@ public class ProvisioningServiceImpl implements ProvisioningService {
}
@Override
- public ClusterEntity registerCluster(String clusterUuid, String name, String owner, List<String> tags, Map<String, String> details) {
+ public ClusterEntity registerCluster(final String clusterUuid, final String name, final String owner, final List<String> tags, final Map<String, String> details) {
ClusterEntityImpl clusterEntity = new ClusterEntityImpl(clusterUuid, manager);
clusterEntity.setOwner(owner);
clusterEntity.setName(name);
@@ -86,7 +85,7 @@ public class ProvisioningServiceImpl implements ProvisioningService {
}
@Override
- public HostEntity registerHost(String hostUuid, String name, String owner, List<String> tags, Map<String, String> details) {
+ public HostEntity registerHost(final String hostUuid, final String name, final String owner, final List<String> tags, final Map<String, String> details) {
HostEntityImpl hostEntity = new HostEntityImpl(hostUuid, manager);
hostEntity.setOwner(owner);
hostEntity.setName(name);
@@ -97,38 +96,38 @@ public class ProvisioningServiceImpl implements ProvisioningService {
}
@Override
- public void deregisterStorage(String uuid) {
+ public void deregisterStorage(final String uuid) {
// TODO Auto-generated method stub
}
@Override
- public void deregisterZone(String uuid) {
+ public void deregisterZone(final String uuid) {
ZoneEntityImpl zoneEntity = new ZoneEntityImpl(uuid, manager);
zoneEntity.disable();
}
@Override
- public void deregisterPod(String uuid) {
+ public void deregisterPod(final String uuid) {
PodEntityImpl podEntity = new PodEntityImpl(uuid, manager);
podEntity.disable();
}
@Override
- public void deregisterCluster(String uuid) {
+ public void deregisterCluster(final String uuid) {
ClusterEntityImpl clusterEntity = new ClusterEntityImpl(uuid, manager);
clusterEntity.disable();
}
@Override
- public void deregisterHost(String uuid) {
+ public void deregisterHost(final String uuid) {
HostEntityImpl hostEntity = new HostEntityImpl(uuid, manager);
hostEntity.disable();
}
@Override
- public void changeState(String type, String entity, Status state) {
+ public void changeState(final String type, final String entity, final Status state) {
// TODO Auto-generated method stub
}
@@ -141,7 +140,10 @@ public class ProvisioningServiceImpl implements ProvisioningService {
@Override
public List<PodEntity> listPods() {
- List<PodEntity> pods = new ArrayList<PodEntity>();
+ /*
+ * Not in use now, just commented out.
+ */
+ //List<PodEntity> pods = new ArrayList<PodEntity>();
//pods.add(new PodEntityImpl("pod-uuid-1", "pod1"));
//pods.add(new PodEntityImpl("pod-uuid-2", "pod2"));
return null;
@@ -162,7 +164,7 @@ public class ProvisioningServiceImpl implements ProvisioningService {
}
@Override
- public ZoneEntity getZone(String uuid) {
+ public ZoneEntity getZone(final String uuid) {
ZoneEntityImpl impl = new ZoneEntityImpl(uuid, manager);
return impl;
}
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3a7e4103/engine/orchestration/test/com/cloud/agent/manager/ConnectedAgentAttacheTest.java
----------------------------------------------------------------------
diff --git a/engine/orchestration/test/com/cloud/agent/manager/ConnectedAgentAttacheTest.java b/engine/orchestration/test/com/cloud/agent/manager/ConnectedAgentAttacheTest.java
new file mode 100644
index 0000000..e0e97c5
--- /dev/null
+++ b/engine/orchestration/test/com/cloud/agent/manager/ConnectedAgentAttacheTest.java
@@ -0,0 +1,82 @@
+// 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 com.cloud.agent.manager;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+
+import org.junit.Test;
+
+import com.cloud.utils.nio.Link;
+
+public class ConnectedAgentAttacheTest {
+
+ @Test
+ public void testEquals() throws Exception {
+
+ Link link = mock(Link.class);
+
+ ConnectedAgentAttache agentAttache1 = new ConnectedAgentAttache(null, 0, null, link, false);
+ ConnectedAgentAttache agentAttache2 = new ConnectedAgentAttache(null, 0, null, link, false);
+
+ assertTrue(agentAttache1.equals(agentAttache2));
+ }
+
+ @Test
+ public void testEqualsFalseNull() throws Exception {
+
+ Link link = mock(Link.class);
+
+ ConnectedAgentAttache agentAttache1 = new ConnectedAgentAttache(null, 0, null, link, false);
+
+ assertFalse(agentAttache1.equals(null));
+ }
+
+ @Test
+ public void testEqualsFalseDiffLink() throws Exception {
+
+ Link link1 = mock(Link.class);
+ Link link2 = mock(Link.class);
+
+ ConnectedAgentAttache agentAttache1 = new ConnectedAgentAttache(null, 0, null, link1, false);
+ ConnectedAgentAttache agentAttache2 = new ConnectedAgentAttache(null, 0, null, link2, false);
+
+ assertFalse(agentAttache1.equals(agentAttache2));
+ }
+
+ @Test
+ public void testEqualsFalseDiffId() throws Exception {
+
+ Link link1 = mock(Link.class);
+
+ ConnectedAgentAttache agentAttache1 = new ConnectedAgentAttache(null, 1, null, link1, false);
+ ConnectedAgentAttache agentAttache2 = new ConnectedAgentAttache(null, 2, null, link1, false);
+
+ assertFalse(agentAttache1.equals(agentAttache2));
+ }
+
+ @Test
+ public void testEqualsFalseDiffClass() throws Exception {
+
+ Link link1 = mock(Link.class);
+
+ ConnectedAgentAttache agentAttache1 = new ConnectedAgentAttache(null, 1, null, link1, false);
+
+ assertFalse(agentAttache1.equals("abc"));
+ }
+}
\ No newline at end of file