OsInfo to implement Writeable rather than Streamable

This allows to make all instance members final. Also added serialization tests and sorted out inizialization that was scattered in two places.

core/src/main/java/org/elasticsearch/action/admin/cluster/node/info/NodeInfo.java

@@ -37,10 +37,6 @@ import org.elasticsearch.threadpool.ThreadPoolInfo;
 import org.elasticsearch.transport.TransportInfo;
 
 import java.io.IOException;
-import java.util.HashMap;
-import java.util.Map;
-
-import static java.util.Collections.unmodifiableMap;
 
 /**
  * Node information (static, does not change over time).
@@ -206,7 +202,7 @@ public class NodeInfo extends BaseNodeResponse {
             settings = Settings.readSettingsFromStream(in);
         }
         if (in.readBoolean()) {
-            os = OsInfo.readOsInfo(in);
+            os = new OsInfo(in);
         }
         if (in.readBoolean()) {
             process = ProcessInfo.readProcessInfo(in);

core/src/main/java/org/elasticsearch/monitor/os/DummyOsInfo.java

@@ -21,13 +21,8 @@ package org.elasticsearch.monitor.os;
 
 public class DummyOsInfo extends OsInfo {
 
-    DummyOsInfo() {
+    private DummyOsInfo() {
-        refreshInterval = 0;
+        super(0, 0, 0, "dummy_name", "dummy_arch", "dummy_version");
-        availableProcessors = 0;
-        allocatedProcessors = 0;
-        name = "dummy_name";
-        arch = "dummy_arch";
-        version = "dummy_version";
     }
 
     public static final DummyOsInfo INSTANCE = new DummyOsInfo();

core/src/main/java/org/elasticsearch/monitor/os/OsInfo.java

@@ -21,25 +21,47 @@ package org.elasticsearch.monitor.os;
 
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
-import org.elasticsearch.common.io.stream.Streamable;
+import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 
 import java.io.IOException;
 
-public class OsInfo implements Streamable, ToXContent {
+public class OsInfo implements Writeable, ToXContent {
 
-    long refreshInterval;
+    private final long refreshInterval;
+    private final int availableProcessors;
+    private final int allocatedProcessors;
+    private final String name;
+    private final String arch;
+    private final String version;
 
-    int availableProcessors;
+    public OsInfo(long refreshInterval, int availableProcessors, int allocatedProcessors, String name, String arch, String version) {
+        this.refreshInterval = refreshInterval;
+        this.availableProcessors = availableProcessors;
+        this.allocatedProcessors = allocatedProcessors;
+        this.name = name;
+        this.arch = arch;
+        this.version = version;
+    }
 
-    int allocatedProcessors;
+    public OsInfo(StreamInput in) throws IOException {
+        this.refreshInterval = in.readLong();
+        this.availableProcessors = in.readInt();
+        this.allocatedProcessors = in.readInt();
+        this.name = in.readOptionalString();
+        this.arch = in.readOptionalString();
+        this.version = in.readOptionalString();
+    }
 
-    String name = null;
+    @Override
-    String arch = null;
+    public void writeTo(StreamOutput out) throws IOException {
-    String version = null;
+        out.writeLong(refreshInterval);
-
+        out.writeInt(availableProcessors);
-    OsInfo() {
+        out.writeInt(allocatedProcessors);
+        out.writeOptionalString(name);
+        out.writeOptionalString(arch);
+        out.writeOptionalString(version);
     }
 
     public long getRefreshInterval() {
@@ -95,30 +117,4 @@ public class OsInfo implements Streamable, ToXContent {
         builder.endObject();
         return builder;
     }
-
-    public static OsInfo readOsInfo(StreamInput in) throws IOException {
-        OsInfo info = new OsInfo();
-        info.readFrom(in);
-        return info;
-    }
-
-    @Override
-    public void readFrom(StreamInput in) throws IOException {
-        refreshInterval = in.readLong();
-        availableProcessors = in.readInt();
-        allocatedProcessors = in.readInt();
-        name = in.readOptionalString();
-        arch = in.readOptionalString();
-        version = in.readOptionalString();
-    }
-
-    @Override
-    public void writeTo(StreamOutput out) throws IOException {
-        out.writeLong(refreshInterval);
-        out.writeInt(availableProcessors);
-        out.writeInt(allocatedProcessors);
-        out.writeOptionalString(name);
-        out.writeOptionalString(arch);
-        out.writeOptionalString(version);
-    }
 }

core/src/main/java/org/elasticsearch/monitor/os/OsProbe.java

@@ -163,13 +163,9 @@ public class OsProbe {
     private OsProbe() {
     }
 
-    public OsInfo osInfo() {
+    public OsInfo osInfo(long refreshInterval, int allocatedProcessors) {
-        OsInfo info = new OsInfo();
+        return new OsInfo(refreshInterval, Runtime.getRuntime().availableProcessors(),
-        info.availableProcessors = Runtime.getRuntime().availableProcessors();
+                allocatedProcessors, Constants.OS_NAME, Constants.OS_ARCH, Constants.OS_VERSION);
-        info.name = Constants.OS_NAME;
-        info.arch = Constants.OS_ARCH;
-        info.version = Constants.OS_VERSION;
-        return info;
     }
 
     public OsStats osStats() {

core/src/main/java/org/elasticsearch/monitor/os/OsService.java

@@ -27,16 +27,11 @@ import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.common.util.SingleObjectCache;
 import org.elasticsearch.common.util.concurrent.EsExecutors;
 
-/**
- *
- */
 public class OsService extends AbstractComponent {
 
     private final OsProbe probe;
-
     private final OsInfo info;
-
+    private final SingleObjectCache<OsStats> osStatsCache;
-    private SingleObjectCache<OsStats> osStatsCache;
 
     public static final Setting<TimeValue> REFRESH_INTERVAL_SETTING =
         Setting.timeSetting("monitor.os.refresh_interval", TimeValue.timeValueSeconds(1), TimeValue.timeValueSeconds(1),
@@ -45,14 +40,9 @@ public class OsService extends AbstractComponent {
     public OsService(Settings settings) {
         super(settings);
         this.probe = OsProbe.getInstance();
-
         TimeValue refreshInterval = REFRESH_INTERVAL_SETTING.get(settings);
-
+        this.info = probe.osInfo(refreshInterval.millis(), EsExecutors.boundedNumberOfProcessors(settings));
-        this.info = probe.osInfo();
+        this.osStatsCache = new OsStatsCache(refreshInterval, probe.osStats());
-        this.info.refreshInterval = refreshInterval.millis();
-        this.info.allocatedProcessors = EsExecutors.boundedNumberOfProcessors(settings);
-
-        osStatsCache = new OsStatsCache(refreshInterval, probe.osStats());
         logger.debug("using refresh_interval [{}]", refreshInterval);
     }
 

core/src/test/java/org/elasticsearch/monitor/os/OsInfoTests.java

@@ -0,0 +1,60 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch 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.elasticsearch.monitor.os;
+
+import org.elasticsearch.common.io.stream.BytesStreamOutput;
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.test.ESTestCase;
+
+import java.io.IOException;
+
+public class OsInfoTests extends ESTestCase {
+
+    public void testSerialization() throws IOException {
+        int availableProcessors = randomIntBetween(1, 64);
+        int allocatedProcessors = randomIntBetween(1, availableProcessors);
+        long refreshInterval;
+        if (randomBoolean()) {
+            refreshInterval = -1;
+        } else {
+            refreshInterval = randomLong();
+            while (refreshInterval == Long.MIN_VALUE) {
+                refreshInterval = randomLong();
+            }
+            refreshInterval = Math.abs(refreshInterval);
+        }
+        String name = randomAsciiOfLengthBetween(3, 10);
+        String arch = randomAsciiOfLengthBetween(3, 10);
+        String version = randomAsciiOfLengthBetween(3, 10);
+        OsInfo osInfo = new OsInfo(refreshInterval, availableProcessors, allocatedProcessors, name, arch, version);
+        try (BytesStreamOutput out = new BytesStreamOutput()) {
+            osInfo.writeTo(out);
+            try (StreamInput in = out.bytes().streamInput()) {
+                OsInfo deserializedOsInfo = new OsInfo(in);
+                assertEquals(osInfo.getRefreshInterval(), deserializedOsInfo.getRefreshInterval());
+                assertEquals(osInfo.getAvailableProcessors(), deserializedOsInfo.getAvailableProcessors());
+                assertEquals(osInfo.getAllocatedProcessors(), deserializedOsInfo.getAllocatedProcessors());
+                assertEquals(osInfo.getName(), deserializedOsInfo.getName());
+                assertEquals(osInfo.getArch(), deserializedOsInfo.getArch());
+                assertEquals(osInfo.getVersion(), deserializedOsInfo.getVersion());
+            }
+        }
+    }
+}

core/src/test/java/org/elasticsearch/monitor/os/OsProbeTests.java

@@ -32,16 +32,28 @@ import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.lessThanOrEqualTo;
 
 public class OsProbeTests extends ESTestCase {
-    OsProbe probe = OsProbe.getInstance();
+    private OsProbe probe = OsProbe.getInstance();
 
     public void testOsInfo() {
-        OsInfo info = probe.osInfo();
+        int allocatedProcessors = randomIntBetween(1, Runtime.getRuntime().availableProcessors());
+        long refreshInterval;
+        if (randomBoolean()) {
+            refreshInterval = -1;
+        } else {
+            refreshInterval = randomLong();
+            while (refreshInterval == Long.MIN_VALUE) {
+                refreshInterval = randomLong();
+            }
+            refreshInterval = Math.abs(refreshInterval);
+        }
+        OsInfo info = probe.osInfo(refreshInterval, allocatedProcessors);
         assertNotNull(info);
-        assertThat(info.getRefreshInterval(), anyOf(equalTo(-1L), greaterThanOrEqualTo(0L)));
+        assertEquals(refreshInterval, info.getRefreshInterval());
-        assertThat(info.getName(), equalTo(Constants.OS_NAME));
+        assertEquals(Constants.OS_NAME, info.getName());
-        assertThat(info.getArch(), equalTo(Constants.OS_ARCH));
+        assertEquals(Constants.OS_ARCH, info.getArch());
-        assertThat(info.getVersion(), equalTo(Constants.OS_VERSION));
+        assertEquals(Constants.OS_VERSION, info.getVersion());
-        assertThat(info.getAvailableProcessors(), equalTo(Runtime.getRuntime().availableProcessors()));
+        assertEquals(allocatedProcessors, info.getAllocatedProcessors());
+        assertEquals(Runtime.getRuntime().availableProcessors(), info.getAvailableProcessors());
     }
 
     public void testOsStats() {