ONOS-4380 Refactor AlarmId and Alarm construction and update

Change-Id: I0117afda723ba27aadb1db306f7ce15b666f102d
diff --git a/apps/faultmanagement/fmgui/src/main/java/org/onosproject/faultmanagement/alarms/gui/AlarmTableMessageHandler.java b/apps/faultmanagement/fmgui/src/main/java/org/onosproject/faultmanagement/alarms/gui/AlarmTableMessageHandler.java
index b0ebbad..f9ad3da 100644
--- a/apps/faultmanagement/fmgui/src/main/java/org/onosproject/faultmanagement/alarms/gui/AlarmTableMessageHandler.java
+++ b/apps/faultmanagement/fmgui/src/main/java/org/onosproject/faultmanagement/alarms/gui/AlarmTableMessageHandler.java
@@ -32,7 +32,6 @@
 import java.util.Collection;
 import java.util.Set;
 
-import static java.lang.Long.parseLong;
 import static org.onosproject.incubator.net.faultmanagement.alarm.AlarmId.alarmId;
 
 /**
@@ -123,7 +122,7 @@
         private void populateRow(TableModel.Row row, Alarm alarm) {
             log.debug("populateRow: row = {} alarm = {}", row, alarm);
 
-            row.cell(ID, alarm.id().fingerprint())
+            row.cell(ID, alarm.id())
                     .cell(DEVICE_ID_STR, alarm.deviceId())
                     .cell(DESCRIPTION, alarm.description())
                     .cell(SOURCE, alarm.source())
@@ -144,7 +143,7 @@
             log.debug("payload = {}", payload);
 
             String id = string(payload, ID, "(none)");
-            Alarm alarm = AlarmServiceUtil.lookupAlarm(alarmId(parseLong(id)));
+            Alarm alarm = AlarmServiceUtil.lookupAlarm(alarmId(id));
             ObjectNode rootNode = objectNode();
             ObjectNode data = objectNode();
             rootNode.set(DETAILS, data);
@@ -156,13 +155,13 @@
             } else {
                 rootNode.put(RESULT, "Found item with id '" + id + "'");
 
-                data.put(ID, alarm.id().fingerprint());
+                data.put(ID, alarm.id().toString());
                 data.put(DESCRIPTION, alarm.description());
                 data.put(DEVICE_ID_STR, alarm.deviceId().toString());
                 data.put(SOURCE, alarm.source().toString());
                 long timeRaised = alarm.timeRaised();
                 data.put(TIME_RAISED,
-                        formatTime(timeRaised)
+                         formatTime(timeRaised)
                 );
                 data.put(TIME_UPDATED, formatTime(alarm.timeUpdated()));
                 data.put(TIME_CLEARED, formatTime(alarm.timeCleared()));
diff --git a/apps/faultmanagement/fmmgr/src/main/java/org/onosproject/faultmanagement/api/AlarmStore.java b/apps/faultmanagement/fmmgr/src/main/java/org/onosproject/faultmanagement/api/AlarmStore.java
index 716449b..0b857f3 100644
--- a/apps/faultmanagement/fmmgr/src/main/java/org/onosproject/faultmanagement/api/AlarmStore.java
+++ b/apps/faultmanagement/fmmgr/src/main/java/org/onosproject/faultmanagement/api/AlarmStore.java
@@ -53,11 +53,12 @@
     Collection<Alarm> getAlarms(DeviceId deviceId);
 
     /**
-     * Stores an alarm.
+     * Stores or updates an alarm.
      *
      * @param alarm alarm
      */
-    void setAlarm(Alarm alarm);
+
+    void createOrUpdateAlarm(Alarm alarm);
 
     /**
      * Removes an alarm.
diff --git a/apps/faultmanagement/fmmgr/src/main/java/org/onosproject/faultmanagement/impl/AlarmManager.java b/apps/faultmanagement/fmmgr/src/main/java/org/onosproject/faultmanagement/impl/AlarmManager.java
index 7f78cda..bfe7627 100644
--- a/apps/faultmanagement/fmmgr/src/main/java/org/onosproject/faultmanagement/impl/AlarmManager.java
+++ b/apps/faultmanagement/fmmgr/src/main/java/org/onosproject/faultmanagement/impl/AlarmManager.java
@@ -16,7 +16,6 @@
 package org.onosproject.faultmanagement.impl;
 
 import com.google.common.collect.ImmutableSet;
-import org.apache.commons.collections.CollectionUtils;
 import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Deactivate;
@@ -95,33 +94,52 @@
         return true;
     }
 
+    //TODO maybe map for field to update ?
     @Override
     public Alarm updateBookkeepingFields(AlarmId id, boolean isAcknowledged, String assignedUser) {
-
         Alarm found = store.getAlarm(id);
         if (found == null) {
             throw new ItemNotFoundException("Alarm with id " + id + " found");
         }
-
         Alarm updated = new DefaultAlarm.Builder(found)
                 .withId(found.id())
                 .withAcknowledged(isAcknowledged)
                 .withAssignedUser(assignedUser).build();
-        store.setAlarm(updated);
+        store.createOrUpdateAlarm(updated);
         return updated;
     }
 
-    public Alarm clear(AlarmId id) {
+    @Override
+    public Alarm updateBookkeepingFields(AlarmId id, boolean clear, boolean isAcknowledged,
+                                         String assignedUser) {
+        checkNotNull(id, "Alarm id is null");
         Alarm found = store.getAlarm(id);
         if (found == null) {
-            log.warn("Alarm {} is not present", id);
-            return null;
+            throw new ItemNotFoundException("Alarm with id " + id + " found");
         }
-        Alarm updated = new DefaultAlarm.Builder(found).withId(id).clear().build();
-        store.setAlarm(updated);
+        long now = System.currentTimeMillis();
+        DefaultAlarm.Builder alarmBuilder = new DefaultAlarm.Builder(found).withTimeUpdated(now);
+        if (found.cleared() != clear) {
+            alarmBuilder.clear().withTimeCleared(now);
+        }
+        if (found.acknowledged() != isAcknowledged) {
+            alarmBuilder.withAcknowledged(isAcknowledged);
+        }
+        if (assignedUser != null && !found.assignedUser().equals(assignedUser)) {
+            alarmBuilder.withAssignedUser(assignedUser);
+        }
+        DefaultAlarm updated = alarmBuilder.build();
+        store.createOrUpdateAlarm(updated);
         return updated;
     }
 
+    //TODO move to AlarmAdminService
+    @Override
+    public void remove(AlarmId id) {
+        checkNotNull(id, "Alarm id is null");
+        store.removeAlarm(id);
+    }
+
     @Override
     public Map<Alarm.SeverityLevel, Long> getAlarmCounts(DeviceId deviceId) {
         return getAlarms(deviceId).stream().collect(
@@ -185,35 +203,6 @@
         return new InternalAlarmProviderService(provider);
     }
 
-    // Synchronised to prevent duplicate NE alarms being raised
-    protected synchronized void updateAlarms(DeviceId deviceId, Set<Alarm> discoveredSet) {
-        Set<Alarm> storedSet = getActiveAlarms(deviceId);
-        log.debug("CurrentNeAlarms={}. DiscoveredAlarms={}", storedSet, discoveredSet);
-
-        if (CollectionUtils.isEqualCollection(storedSet, discoveredSet)) {
-            log.debug("No update for {}.", deviceId);
-            return;
-        }
-        //TODO implement distinction between UPDATED and CLEARED ALARMS
-        storedSet.stream().filter(
-                (stored) -> (!discoveredSet.contains(stored))).forEach((stored) -> {
-            log.debug("Alarm will be Cleared as it is not on the device. Cleared alarm: {}.", stored);
-            clear(stored.id());
-        });
-
-        discoveredSet.stream().filter(
-                (discovered) -> (!storedSet.contains(discovered))).forEach((discovered) -> {
-            log.info("New alarm raised {}", discovered);
-            AlarmId id = generateAlarmId();
-            store.setAlarm(new DefaultAlarm.Builder(discovered).withId(id).build());
-        });
-    }
-
-    //TODO improve implementation of AlarmId
-    private AlarmId generateAlarmId() {
-        return AlarmId.alarmId(alarmIdGenerator.incrementAndGet());
-    }
-
     private class InternalAlarmProviderService extends AbstractProviderService<AlarmProvider>
             implements AlarmProviderService {
 
@@ -223,7 +212,7 @@
 
         @Override
         public void updateAlarmList(DeviceId deviceId, Collection<Alarm> alarms) {
-            updateAlarms(deviceId, ImmutableSet.copyOf(alarms));
+            alarms.forEach(alarm -> store.createOrUpdateAlarm(alarm));
         }
     }
 }
diff --git a/apps/faultmanagement/fmmgr/src/main/java/org/onosproject/faultmanagement/impl/DistributedAlarmStore.java b/apps/faultmanagement/fmmgr/src/main/java/org/onosproject/faultmanagement/impl/DistributedAlarmStore.java
index caea52b..db1052d 100644
--- a/apps/faultmanagement/fmmgr/src/main/java/org/onosproject/faultmanagement/impl/DistributedAlarmStore.java
+++ b/apps/faultmanagement/fmmgr/src/main/java/org/onosproject/faultmanagement/impl/DistributedAlarmStore.java
@@ -43,6 +43,7 @@
 
 import java.util.Collection;
 import java.util.Map;
+import java.util.Objects;
 import java.util.stream.Collectors;
 
 import static org.slf4j.LoggerFactory.getLogger;
@@ -109,13 +110,18 @@
     public Collection<Alarm> getAlarms(DeviceId deviceId) {
         //FIXME: this is expensive, need refactoring when core maps provide different indexes.
         return ImmutableSet.copyOf(alarmsMap.values().stream()
-                .filter(alarm -> alarm.deviceId().equals(deviceId))
-                .collect(Collectors.toSet()));
+                                           .filter(alarm -> alarm.deviceId().equals(deviceId))
+                                           .collect(Collectors.toSet()));
     }
 
     @Override
-    public void setAlarm(Alarm alarm) {
-        alarms.put(alarm.id(), alarm);
+    public void createOrUpdateAlarm(Alarm alarm) {
+        Alarm existing = alarmsMap.get(alarm.id());
+        if (Objects.equals(existing, alarm)) {
+            log.info("Received identical alarm, no operation needed on {}", alarm.id());
+        } else {
+            alarms.put(alarm.id(), alarm);
+        }
     }
 
     @Override
@@ -136,7 +142,7 @@
                     alarm = mapEvent.newValue().value();
                     break;
                 case UPDATE:
-                    type = AlarmEvent.Type.CREATED;
+                    type = AlarmEvent.Type.UPDATED;
                     alarm = mapEvent.newValue().value();
                     break;
                 case REMOVE:
diff --git a/apps/faultmanagement/fmmgr/src/test/java/org/onosproject/faultmanagement/impl/AlarmManagerTest.java b/apps/faultmanagement/fmmgr/src/test/java/org/onosproject/faultmanagement/impl/AlarmManagerTest.java
index 87cf28e..a7a2573 100644
--- a/apps/faultmanagement/fmmgr/src/test/java/org/onosproject/faultmanagement/impl/AlarmManagerTest.java
+++ b/apps/faultmanagement/fmmgr/src/test/java/org/onosproject/faultmanagement/impl/AlarmManagerTest.java
@@ -31,9 +31,14 @@
 import org.onosproject.incubator.net.faultmanagement.alarm.AlarmEvent;
 import org.onosproject.incubator.net.faultmanagement.alarm.AlarmId;
 import org.onosproject.incubator.net.faultmanagement.alarm.AlarmListener;
+import org.onosproject.incubator.net.faultmanagement.alarm.AlarmProvider;
+import org.onosproject.incubator.net.faultmanagement.alarm.AlarmProviderRegistry;
+import org.onosproject.incubator.net.faultmanagement.alarm.AlarmProviderService;
 import org.onosproject.incubator.net.faultmanagement.alarm.DefaultAlarm;
 import org.onosproject.net.DeviceId;
+import org.onosproject.net.MastershipRole;
 import org.onosproject.net.NetTestTools;
+import org.onosproject.net.provider.AbstractProvider;
 import org.onosproject.store.service.TestStorageService;
 
 import java.util.Collections;
@@ -46,6 +51,7 @@
 import static org.junit.Assert.assertTrue;
 import static org.onosproject.incubator.net.faultmanagement.alarm.Alarm.SeverityLevel.CLEARED;
 import static org.onosproject.incubator.net.faultmanagement.alarm.Alarm.SeverityLevel.CRITICAL;
+import static org.onosproject.net.NetTestTools.PID;
 
 /**
  * Alarm manager test suite.
@@ -53,17 +59,28 @@
 public class AlarmManagerTest {
 
     private static final DeviceId DEVICE_ID = DeviceId.deviceId("foo:bar");
-    private static final DefaultAlarm ALARM_A = new DefaultAlarm.Builder(
+    private static final String UNIQUE_ID_1 = "unique_id_1";
+    private static final String UNIQUE_ID_2 = "unique_id_2";
+    private static final AlarmId A_ID = AlarmId.alarmId(DEVICE_ID, UNIQUE_ID_1);
+    private static final AlarmId B_ID = AlarmId.alarmId(DEVICE_ID, UNIQUE_ID_2);
+    private static final DefaultAlarm ALARM_A = new DefaultAlarm.Builder(A_ID,
             DEVICE_ID, "aaa", Alarm.SeverityLevel.CRITICAL, 0).build();
 
+    private static final DefaultAlarm ALARM_A_CLEARED = new DefaultAlarm.Builder(ALARM_A)
+            .clear().build();
+
     private static final DefaultAlarm ALARM_A_WITHSRC = new DefaultAlarm.Builder(
             ALARM_A).forSource(AlarmEntityId.alarmEntityId("port:foo")).build();
 
-    private static final DefaultAlarm ALARM_B = new DefaultAlarm.Builder(
+    private static final DefaultAlarm ALARM_B = new DefaultAlarm.Builder(B_ID,
             DEVICE_ID, "bbb", Alarm.SeverityLevel.CRITICAL, 0).build();
 
+
     private AlarmManager manager;
     private DistributedAlarmStore alarmStore;
+    private AlarmProviderService providerService;
+    private TestProvider provider;
+    protected AlarmProviderRegistry registry;
     protected TestListener listener = new TestListener();
 
     @Rule
@@ -75,15 +92,18 @@
         TestUtils.setField(alarmStore, "storageService", new TestStorageService());
         alarmStore.activate();
         manager = new AlarmManager();
+        registry = manager;
         manager.addListener(listener);
         NetTestTools.injectEventDispatcher(manager, new TestEventDispatcher());
         manager.store = alarmStore;
         manager.activate();
+        provider = new TestProvider();
+        providerService = registry.register(provider);
     }
 
     @Test
     public void deactivate() throws Exception {
-        manager.updateAlarms(DEVICE_ID, ImmutableSet.of(ALARM_B, ALARM_A));
+        providerService.updateAlarmList(DEVICE_ID, ImmutableSet.of(ALARM_B, ALARM_A));
         verifyGettingSetsOfAlarms(manager, 2, 2);
         alarmStore.deactivate();
         manager.removeListener(listener);
@@ -110,29 +130,30 @@
         manager.getAlarm(null);
 
         exception.expect(ItemNotFoundException.class);
-        manager.getAlarm(AlarmId.alarmId(1));
+        manager.getAlarm(AlarmId.alarmId(DEVICE_ID, "unique_3"));
     }
 
     @Test
     public void testAlarmUpdates() throws InterruptedException {
 
         assertTrue("No alarms should be present", manager.getAlarms().isEmpty());
-        manager.updateAlarms(DEVICE_ID, ImmutableSet.of());
+        providerService.updateAlarmList(DEVICE_ID, ImmutableSet.of());
         assertTrue("No alarms should be present", manager.getAlarms().isEmpty());
         Map<Alarm.SeverityLevel, Long> zeroAlarms = new CountsMapBuilder().create();
         assertEquals("No alarms count should be present", zeroAlarms, manager.getAlarmCounts());
         assertEquals("No alarms count should be present", zeroAlarms, manager.getAlarmCounts(DEVICE_ID));
 
-        manager.updateAlarms(DEVICE_ID, ImmutableSet.of(ALARM_B, ALARM_A));
+        providerService.updateAlarmList(DEVICE_ID, ImmutableSet.of(ALARM_B, ALARM_A));
         verifyGettingSetsOfAlarms(manager, 2, 2);
         validateEvents(AlarmEvent.Type.CREATED, AlarmEvent.Type.CREATED);
         Map<Alarm.SeverityLevel, Long> critical2 = new CountsMapBuilder().with(CRITICAL, 2L).create();
         assertEquals("A critical should be present", critical2, manager.getAlarmCounts());
         assertEquals("A critical should be present", critical2, manager.getAlarmCounts(DEVICE_ID));
 
-        manager.updateAlarms(DEVICE_ID, ImmutableSet.of(ALARM_A));
+        Alarm updated = manager.updateBookkeepingFields(ALARM_A.id(), true, false, null);
+//        providerService.updateAlarmList(DEVICE_ID, ImmutableSet.of(ALARM_A));
         verifyGettingSetsOfAlarms(manager, 2, 1);
-        validateEvents(AlarmEvent.Type.CREATED);
+        validateEvents(AlarmEvent.Type.UPDATED);
         Map<Alarm.SeverityLevel, Long> critical1cleared1 =
                 new CountsMapBuilder().with(CRITICAL, 1L).with(CLEARED, 1L).create();
         assertEquals("A critical should be present and cleared", critical1cleared1,
@@ -141,7 +162,7 @@
                      manager.getAlarmCounts(DEVICE_ID));
 
         // No change map when same alarms sent
-        manager.updateAlarms(DEVICE_ID, ImmutableSet.of(ALARM_A));
+        providerService.updateAlarmList(DEVICE_ID, ImmutableSet.of(updated));
         verifyGettingSetsOfAlarms(manager, 2, 1);
         validateEvents();
         assertEquals("Map should not be changed for same alarm", critical1cleared1,
@@ -149,18 +170,19 @@
         assertEquals("Map should not be changed for same alarm", critical1cleared1,
                      manager.getAlarmCounts(DEVICE_ID));
 
-        manager.updateAlarms(DEVICE_ID, ImmutableSet.of(ALARM_A, ALARM_A_WITHSRC));
-        verifyGettingSetsOfAlarms(manager, 3, 2);
-        validateEvents(AlarmEvent.Type.CREATED);
+        providerService.updateAlarmList(DEVICE_ID, ImmutableSet.of(updated, ALARM_A_WITHSRC));
+        verifyGettingSetsOfAlarms(manager, 2, 2);
+        validateEvents(AlarmEvent.Type.UPDATED);
         Map<Alarm.SeverityLevel, Long> critical2cleared1 =
-                new CountsMapBuilder().with(CRITICAL, 2L).with(CLEARED, 1L).create();
+                new CountsMapBuilder().with(CRITICAL, 2L).create();
         assertEquals("A critical should be present", critical2cleared1, manager.getAlarmCounts());
         assertEquals("A critical should be present", critical2cleared1, manager.getAlarmCounts(DEVICE_ID));
 
-        manager.updateAlarms(DEVICE_ID, ImmutableSet.of());
-        verifyGettingSetsOfAlarms(manager, 3, 0);
-        validateEvents(AlarmEvent.Type.CREATED, AlarmEvent.Type.CREATED);
-        assertEquals(new CountsMapBuilder().with(CLEARED, 3L).create(), manager.getAlarmCounts(DEVICE_ID));
+        providerService.updateAlarmList(DEVICE_ID, ImmutableSet.of());
+        verifyGettingSetsOfAlarms(manager, 2, 2);
+        validateEvents();
+        assertEquals(new CountsMapBuilder().with(CRITICAL, 2L).create(),
+                     manager.getAlarmCounts(DEVICE_ID));
 
         assertEquals("The counts should be empty for unknown devices", zeroAlarms,
                      manager.getAlarmCounts(DeviceId.NONE));
@@ -207,6 +229,19 @@
     }
 
 
+    private class TestProvider extends AbstractProvider implements AlarmProvider {
+        private DeviceId deviceReceived;
+        private MastershipRole roleReceived;
+
+        public TestProvider() {
+            super(PID);
+        }
+
+        @Override
+        public void triggerProbe(DeviceId deviceId) {
+        }
+    }
+
     /**
      * Test listener class to receive alarm events.
      */
diff --git a/apps/faultmanagement/fmmgr/src/test/java/org/onosproject/faultmanagement/impl/DistributedAlarmStoreTest.java b/apps/faultmanagement/fmmgr/src/test/java/org/onosproject/faultmanagement/impl/DistributedAlarmStoreTest.java
index 34fb320..1aa8a56 100644
--- a/apps/faultmanagement/fmmgr/src/test/java/org/onosproject/faultmanagement/impl/DistributedAlarmStoreTest.java
+++ b/apps/faultmanagement/fmmgr/src/test/java/org/onosproject/faultmanagement/impl/DistributedAlarmStoreTest.java
@@ -20,6 +20,7 @@
 import org.junit.Before;
 import org.junit.Test;
 import org.onosproject.incubator.net.faultmanagement.alarm.Alarm;
+import org.onosproject.incubator.net.faultmanagement.alarm.AlarmId;
 import org.onosproject.incubator.net.faultmanagement.alarm.DefaultAlarm;
 import org.onosproject.net.DeviceId;
 import org.onosproject.store.service.TestStorageService;
@@ -33,7 +34,9 @@
 public class DistributedAlarmStoreTest {
     private DistributedAlarmStore alarmStore;
     private static final DeviceId DEVICE_ID = DeviceId.deviceId("foo:bar");
-    private static final DefaultAlarm ALARM_A = new DefaultAlarm.Builder(
+    private static final String UNIQUE_ID_1 = "unique_id_1";
+    private static final AlarmId A_ID = AlarmId.alarmId(DEVICE_ID, UNIQUE_ID_1);
+    private static final DefaultAlarm ALARM_A = new DefaultAlarm.Builder(A_ID,
             DEVICE_ID, "aaa", Alarm.SeverityLevel.CRITICAL, 0).build();
 
     /**
@@ -61,7 +64,7 @@
      */
     @Test
     public void basics() {
-        alarmStore.setAlarm(ALARM_A);
+        alarmStore.createOrUpdateAlarm(ALARM_A);
         assertTrue("There should be one alarm in the set.",
                    alarmStore.getAlarms().contains(ALARM_A));
         assertTrue("The same alarm should be returned.",
diff --git a/apps/faultmanagement/fmmgr/src/test/java/org/onosproject/faultmanagement/impl/PollingAlarmProviderTest.java b/apps/faultmanagement/fmmgr/src/test/java/org/onosproject/faultmanagement/impl/PollingAlarmProviderTest.java
index b96af16..72d8c3c 100644
--- a/apps/faultmanagement/fmmgr/src/test/java/org/onosproject/faultmanagement/impl/PollingAlarmProviderTest.java
+++ b/apps/faultmanagement/fmmgr/src/test/java/org/onosproject/faultmanagement/impl/PollingAlarmProviderTest.java
@@ -26,6 +26,7 @@
 import org.onosproject.cluster.RoleInfo;
 import org.onosproject.incubator.net.faultmanagement.alarm.Alarm;
 import org.onosproject.incubator.net.faultmanagement.alarm.AlarmConsumer;
+import org.onosproject.incubator.net.faultmanagement.alarm.AlarmId;
 import org.onosproject.incubator.net.faultmanagement.alarm.AlarmProvider;
 import org.onosproject.incubator.net.faultmanagement.alarm.AlarmProviderRegistry;
 import org.onosproject.incubator.net.faultmanagement.alarm.AlarmProviderRegistryAdapter;
@@ -96,8 +97,9 @@
     private final DeviceEvent deviceEvent =
             new DeviceEvent(DeviceEvent.Type.DEVICE_AVAILABILITY_CHANGED, device);
 
-
-    private static final DefaultAlarm ALARM = new DefaultAlarm.Builder(
+    private static final String UNIQUE_ID_1 = "unique_id_1";
+    private static final AlarmId A_ID= AlarmId.alarmId(DEVICE_ID, UNIQUE_ID_1);
+    private static final DefaultAlarm ALARM = new DefaultAlarm.Builder(A_ID,
             DEVICE_ID, "aaa", Alarm.SeverityLevel.CRITICAL, 0).build();
 
     private final Driver driver = new MockDriver();
diff --git a/apps/faultmanagement/fmweb/src/main/java/org/onosproject/faultmanagement/web/AlarmCodec.java b/apps/faultmanagement/fmweb/src/main/java/org/onosproject/faultmanagement/web/AlarmCodec.java
index 079396d..7db32da 100644
--- a/apps/faultmanagement/fmweb/src/main/java/org/onosproject/faultmanagement/web/AlarmCodec.java
+++ b/apps/faultmanagement/fmweb/src/main/java/org/onosproject/faultmanagement/web/AlarmCodec.java
@@ -16,18 +16,17 @@
 package org.onosproject.faultmanagement.web;
 
 import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
 import org.onosproject.codec.CodecContext;
 import org.onosproject.codec.JsonCodec;
-
-import com.fasterxml.jackson.databind.node.ObjectNode;
-
-import static com.google.common.base.Preconditions.checkNotNull;
-import org.onosproject.net.DeviceId;
 import org.onosproject.incubator.net.faultmanagement.alarm.Alarm;
 import org.onosproject.incubator.net.faultmanagement.alarm.AlarmEntityId;
 import org.onosproject.incubator.net.faultmanagement.alarm.AlarmId;
 import org.onosproject.incubator.net.faultmanagement.alarm.DefaultAlarm;
+import org.onosproject.net.DeviceId;
 import org.slf4j.Logger;
+
+import static com.google.common.base.Preconditions.checkNotNull;
 import static org.slf4j.LoggerFactory.getLogger;
 
 /**
@@ -42,12 +41,12 @@
         checkNotNull(alarm, "Alarm cannot be null");
 
         return context.mapper().createObjectNode()
-                .put("id", alarm.id().fingerprint())
+                .put("id", alarm.id().toString())
                 .put("deviceId", alarm.deviceId().toString())
                 .put("description", alarm.description())
                 .put("source",
-                        alarm.source() == null ? null
-                                : alarm.source().toString())
+                     alarm.source() == null ? null
+                             : alarm.source().toString())
                 .put("timeRaised", alarm.timeRaised())
                 .put("timeUpdated", alarm.timeUpdated())
                 .put("timeCleared", alarm.timeCleared())
@@ -66,7 +65,7 @@
         }
 
         log.debug("id={}, full json={} ", json.get("id"), json);
-        Long id = json.get("id").asLong();
+        String id = json.get("id").asText();
 
         DeviceId deviceId = DeviceId.deviceId(json.get("deviceId").asText());
         String description = json.get("description").asText();
@@ -86,16 +85,15 @@
         String assignedUser
                 = jsonAssignedUser == null || jsonAssignedUser.isNull() ? null : jsonAssignedUser.asText();
 
-        return new DefaultAlarm.Builder(
-                deviceId, description, severity, timeRaised).forSource(AlarmEntityId.NONE).
-                withId(AlarmId.alarmId(id)).
-                withTimeUpdated(timeUpdated).
-                withTimeCleared(timeCleared).
-                withServiceAffecting(serviceAffecting).
-                withAcknowledged(acknowledged).
-                withManuallyClearable(manuallyClearable).
-                withAssignedUser(assignedUser).
-                build();
+        return new DefaultAlarm.Builder(AlarmId.alarmId(deviceId, id),
+                deviceId, description, severity, timeRaised).forSource(AlarmEntityId.NONE)
+                .withTimeUpdated(timeUpdated)
+                .withTimeCleared(timeCleared)
+                .withServiceAffecting(serviceAffecting)
+                .withAcknowledged(acknowledged)
+                .withManuallyClearable(manuallyClearable)
+                .withAssignedUser(assignedUser)
+                .build();
 
     }
 }
diff --git a/apps/faultmanagement/fmweb/src/main/java/org/onosproject/faultmanagement/web/AlarmsWebResource.java b/apps/faultmanagement/fmweb/src/main/java/org/onosproject/faultmanagement/web/AlarmsWebResource.java
index ef875cd..f1ac052 100644
--- a/apps/faultmanagement/fmweb/src/main/java/org/onosproject/faultmanagement/web/AlarmsWebResource.java
+++ b/apps/faultmanagement/fmweb/src/main/java/org/onosproject/faultmanagement/web/AlarmsWebResource.java
@@ -17,11 +17,14 @@
 
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.node.ObjectNode;
+
 import java.io.IOException;
 import java.io.InputStream;
+
 import org.onosproject.rest.AbstractWebResource;
 
 import javax.ws.rs.core.Response;
+
 import org.onosproject.incubator.net.faultmanagement.alarm.Alarm;
 import org.onosproject.incubator.net.faultmanagement.alarm.AlarmId;
 
@@ -34,10 +37,12 @@
 import javax.ws.rs.Produces;
 import javax.ws.rs.QueryParam;
 import javax.ws.rs.core.MediaType;
+
 import org.apache.commons.lang.StringUtils;
 import org.onosproject.incubator.net.faultmanagement.alarm.AlarmService;
 import org.onosproject.net.DeviceId;
 import org.slf4j.Logger;
+
 import static org.slf4j.LoggerFactory.getLogger;
 
 /**
@@ -54,13 +59,13 @@
      * Get alarms. Returns a list of alarms
      *
      * @param includeCleared (optional) include recently cleared alarms in response
-     * @param devId (optional) include only for specified device
+     * @param devId          (optional) include only for specified device
      * @return JSON encoded set of alarms
      */
     @GET
     @Produces(MediaType.APPLICATION_JSON)
     public Response getAlarms(@DefaultValue("false") @QueryParam("includeCleared") boolean includeCleared,
-            @DefaultValue("") @QueryParam("devId") String devId
+                              @DefaultValue("") @QueryParam("devId") String devId
     ) {
 
         log.debug("Requesting all alarms, includeCleared={}", includeCleared);
@@ -92,7 +97,7 @@
     public Response getAlarm(@PathParam("id") String id) {
         log.debug("HTTP GET alarm for id={}", id);
 
-        AlarmId alarmId = toAlarmId(id);
+        AlarmId alarmId = AlarmId.alarmId(id);
         Alarm alarm = get(AlarmService.class).getAlarm(alarmId);
 
         ObjectNode result = new ObjectMapper().createObjectNode();
@@ -105,7 +110,7 @@
      * been updated since the REST client last retrieved the alarm being updated.
      *
      * @param alarmIdPath alarm id path
-     * @param stream input JSON
+     * @param stream      input JSON
      * @return updated JSON encoded alarm
      */
     @PUT
@@ -123,9 +128,9 @@
 
             AlarmService service = get(AlarmService.class);
 
-            if (Long.parseLong(alarmIdPath) != alarm.id().fingerprint()) {
-                throw new IllegalArgumentException("id in path is " + Long.parseLong(alarmIdPath)
-                        + " but payload uses id=" + alarm.id().fingerprint());
+            if (!alarmIdPath.equals(alarm.id().toString())) {
+                throw new IllegalArgumentException("id in path is " + alarmIdPath
+                                                           + " but payload uses id=" + alarm.id().toString());
 
             }
             Alarm updated = service.updateBookkeepingFields(
@@ -139,13 +144,4 @@
         }
     }
 
-    private AlarmId toAlarmId(String id) {
-        try {
-            return AlarmId.alarmId(Long.parseLong(id));
-        } catch (NumberFormatException ex) {
-            throw new IllegalArgumentException("Alarm id should be numeric", ex);
-        }
-
-    }
-
 }
diff --git a/apps/faultmanagement/fmweb/src/test/java/org/onosproject/faultmanagement/web/AlarmCodecTest.java b/apps/faultmanagement/fmweb/src/test/java/org/onosproject/faultmanagement/web/AlarmCodecTest.java
index f922fb0..c341838 100644
--- a/apps/faultmanagement/fmweb/src/test/java/org/onosproject/faultmanagement/web/AlarmCodecTest.java
+++ b/apps/faultmanagement/fmweb/src/test/java/org/onosproject/faultmanagement/web/AlarmCodecTest.java
@@ -17,36 +17,37 @@
 
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.node.ObjectNode;
-import java.io.IOException;
-import java.io.InputStream;
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.is;
-import static org.hamcrest.Matchers.notNullValue;
-import static org.hamcrest.Matchers.nullValue;
-
 import org.junit.Test;
 import org.onosproject.codec.JsonCodec;
-import static org.onosproject.faultmanagement.web.AlarmJsonMatcher.matchesAlarm;
-import org.onosproject.net.DeviceId;
 import org.onosproject.incubator.net.faultmanagement.alarm.Alarm;
 import org.onosproject.incubator.net.faultmanagement.alarm.AlarmEntityId;
 import org.onosproject.incubator.net.faultmanagement.alarm.AlarmId;
 import org.onosproject.incubator.net.faultmanagement.alarm.DefaultAlarm;
+import org.onosproject.net.DeviceId;
+
+import java.io.IOException;
+import java.io.InputStream;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.*;
+import static org.onosproject.faultmanagement.web.AlarmJsonMatcher.matchesAlarm;
 
 public class AlarmCodecTest {
 
     private final AlarmCodecContext context = new AlarmCodecContext();
-    private static final AlarmId ALARM_ID = AlarmId.alarmId(44);
+    private static final DeviceId DEVICE_ID = DeviceId.deviceId("foo:bar");
+    private static final String UNIQUE_ID_1 = "unique_id_1";
+    private static final AlarmId ALARM_ID = AlarmId.alarmId(DEVICE_ID, UNIQUE_ID_1);
 
     // Use this to check handling for miminal Alarm
-    private final Alarm alarmMinimumFields = new DefaultAlarm.Builder(
+    private final Alarm alarmMinimumFields = new DefaultAlarm.Builder(ALARM_ID,
             DeviceId.deviceId("of:2222000000000000"), "NE unreachable", Alarm.SeverityLevel.CLEARED, 1
-    ).withId(ALARM_ID).build();
+    ).build();
 
     // Use this to check handling for fully populated Alarm
-    private final Alarm alarmWithSource = new DefaultAlarm.Builder(
+    private final Alarm alarmWithSource = new DefaultAlarm.Builder(ALARM_ID,
             DeviceId.deviceId("of:2222000000000000"), "NE unreachable", Alarm.SeverityLevel.CLEARED, 1
-    ).withId(ALARM_ID).forSource(AlarmEntityId.alarmEntityId("port:1/2/3/4")).withTimeUpdated(2).withTimeCleared(3L).
+    ).forSource(AlarmEntityId.alarmEntityId("port:1/2/3/4")).withTimeUpdated(2).withTimeCleared(3L).
             withServiceAffecting(true).withAcknowledged(true).withManuallyClearable(true).
             withAssignedUser("the assigned user").build();
 
@@ -97,7 +98,8 @@
     }
 
     private void assertCommon(Alarm alarm) {
-        assertThat(alarm.id(), is(AlarmId.alarmId(10L)));
+        assertThat(alarm.id(), is(AlarmId.alarmId(DeviceId.deviceId("of:123"),
+                                                  String.valueOf(10))));
         assertThat(alarm.description(), is("NE is not reachable"));
         assertThat(alarm.source(), is(AlarmEntityId.NONE));
         assertThat(alarm.timeRaised(), is(999L));
diff --git a/apps/faultmanagement/fmweb/src/test/java/org/onosproject/faultmanagement/web/AlarmJsonMatcher.java b/apps/faultmanagement/fmweb/src/test/java/org/onosproject/faultmanagement/web/AlarmJsonMatcher.java
index 037a9a1..6fee782 100644
--- a/apps/faultmanagement/fmweb/src/test/java/org/onosproject/faultmanagement/web/AlarmJsonMatcher.java
+++ b/apps/faultmanagement/fmweb/src/test/java/org/onosproject/faultmanagement/web/AlarmJsonMatcher.java
@@ -15,12 +15,11 @@
  */
 package org.onosproject.faultmanagement.web;
 
+import com.fasterxml.jackson.databind.JsonNode;
 import org.hamcrest.Description;
 import org.hamcrest.TypeSafeDiagnosingMatcher;
 import org.onosproject.incubator.net.faultmanagement.alarm.Alarm;
 
-import com.fasterxml.jackson.databind.JsonNode;
-
 /**
  * Hamcrest matcher for alarms.
  */
@@ -35,7 +34,7 @@
     @Override
     public boolean matchesSafely(JsonNode jsonAlarm, Description description) {
         String jsonAlarmId = jsonAlarm.get("id").asText();
-        String alarmId = Long.toString(alarm.id().fingerprint());
+        String alarmId = alarm.id().toString();
         if (!jsonAlarmId.equals(alarmId)) {
             description.appendText("alarm id was " + jsonAlarmId);
             return false;
diff --git a/drivers/lumentum/src/main/java/org/onosproject/drivers/lumentum/LumentumAlarmConsumer.java b/drivers/lumentum/src/main/java/org/onosproject/drivers/lumentum/LumentumAlarmConsumer.java
index 9b2c3e8..6e0ec2f 100644
--- a/drivers/lumentum/src/main/java/org/onosproject/drivers/lumentum/LumentumAlarmConsumer.java
+++ b/drivers/lumentum/src/main/java/org/onosproject/drivers/lumentum/LumentumAlarmConsumer.java
@@ -71,7 +71,7 @@
                     alarms.add(new DefaultAlarm.Builder(deviceId, getMessage(alarmId),
                                                         getSeverity(alarmId),
                                                         System.currentTimeMillis())
-                                       .withId(AlarmId.alarmId(alarmId))
+                                       .withId(AlarmId.alarmId(deviceId, String.valueOf(alarmId)))
                                        .build());
                 }));
         return ImmutableList.copyOf(alarms);
diff --git a/incubator/api/src/main/java/org/onosproject/incubator/net/faultmanagement/alarm/Alarm.java b/incubator/api/src/main/java/org/onosproject/incubator/net/faultmanagement/alarm/Alarm.java
index 301e8d3..1a7ec9f 100644
--- a/incubator/api/src/main/java/org/onosproject/incubator/net/faultmanagement/alarm/Alarm.java
+++ b/incubator/api/src/main/java/org/onosproject/incubator/net/faultmanagement/alarm/Alarm.java
@@ -121,6 +121,17 @@
     boolean acknowledged();
 
     /**
+     * Returns a flag to indicate if this alarm has been cleared. All
+     * alarms are not cleared until and unless an ONOS user or app takes action to
+     * indicate so.
+     *
+     * @return whether alarm is currently cleared (true indicates it is)
+     */
+    default boolean cleared() {
+        return false;
+    }
+
+    /**
      * Returns a flag to indicate if this alarm is manually-cleared by a user action within ONOS. Some stateless events
      * e.g. backup-failure or upgrade-failure, may be mapped by ONOS to alarms, and these may be deemed manually-
      * clearable. The more typical case is that an alarm represents a persistent fault on or related to a device and
diff --git a/incubator/api/src/main/java/org/onosproject/incubator/net/faultmanagement/alarm/AlarmEvent.java b/incubator/api/src/main/java/org/onosproject/incubator/net/faultmanagement/alarm/AlarmEvent.java
index 9481b01..167d7f6 100644
--- a/incubator/api/src/main/java/org/onosproject/incubator/net/faultmanagement/alarm/AlarmEvent.java
+++ b/incubator/api/src/main/java/org/onosproject/incubator/net/faultmanagement/alarm/AlarmEvent.java
@@ -33,6 +33,10 @@
          */
         CREATED,
         /**
+         * Individual alarm updated.
+         */
+        UPDATED,
+        /**
          * Alarm set updated for a given device.
          */
         REMOVED,
diff --git a/incubator/api/src/main/java/org/onosproject/incubator/net/faultmanagement/alarm/AlarmId.java b/incubator/api/src/main/java/org/onosproject/incubator/net/faultmanagement/alarm/AlarmId.java
index db26a7e..bc84e8a 100644
--- a/incubator/api/src/main/java/org/onosproject/incubator/net/faultmanagement/alarm/AlarmId.java
+++ b/incubator/api/src/main/java/org/onosproject/incubator/net/faultmanagement/alarm/AlarmId.java
@@ -17,48 +17,64 @@
 
 import com.google.common.annotations.Beta;
 import org.onlab.util.Identifier;
+import org.onosproject.net.DeviceId;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+
 /**
  * Alarm identifier suitable as an external key.
  * <p>
  * This class is immutable.</p>
  */
 @Beta
-public final class AlarmId extends Identifier<Long> {
-
-    public static final AlarmId NONE = new AlarmId();
+public final class AlarmId extends Identifier<String> {
 
     /**
      * Instantiates a new Alarm id.
      *
-     * @param id the id
+     * @param id               the device id
+     * @param uniqueIdentifier the unique identifier of the Alarm on that device
      */
-    private AlarmId(long id) {
-        super(id);
-        checkArgument(id != 0L, "id must be non-zero");
-    }
-
-    private AlarmId() {
-        super(0L);
+    private AlarmId(DeviceId id, String uniqueIdentifier) {
+        super(id.toString() + ":" + uniqueIdentifier);
+        checkNotNull(id, "device id must not be null");
+        checkNotNull(uniqueIdentifier, "unique identifier must not be null");
+        checkArgument(!uniqueIdentifier.isEmpty(), "unique identifier must not be empty");
     }
 
     /**
-     * Creates an alarm identifier from the specified long representation.
+     * Instantiates a new Alarm id, primarly meant for lookup.
      *
-     * @param value long value
-     * @return intent identifier
+     * @param globallyUniqueIdentifier the globally unique identifier of the Alarm,
+     *                                 device Id + local unique identifier on the device
      */
-    public static AlarmId alarmId(long value) {
-        return new AlarmId(value);
+    private AlarmId(String globallyUniqueIdentifier) {
+        super(globallyUniqueIdentifier);
+        checkArgument(!globallyUniqueIdentifier.isEmpty(), "unique identifier must not be empty");
     }
 
     /**
-     * Returns the backing integer index.
+     * Creates an alarm identifier from the specified device id and
+     * unique identifier provided representation.
      *
-     * @return backing integer index
+     * @param id               device id
+     * @param uniqueIdentifier per device unique identifier of the alarm
+     * @return alarm identifier
      */
-    public long fingerprint() {
-        return identifier;
+    public static AlarmId alarmId(DeviceId id, String uniqueIdentifier) {
+        return new AlarmId(id, uniqueIdentifier);
     }
+
+    /**
+     * Creates an alarm identifier from the specified globally unique identifier.
+     *
+     * @param globallyUniqueIdentifier the globally unique identifier of the Alarm,
+     *                                 device Id + local unique identifier on the device
+     * @return alarm identifier
+     */
+    public static AlarmId alarmId(String globallyUniqueIdentifier) {
+        return new AlarmId(globallyUniqueIdentifier);
+    }
+
 }
diff --git a/incubator/api/src/main/java/org/onosproject/incubator/net/faultmanagement/alarm/AlarmService.java b/incubator/api/src/main/java/org/onosproject/incubator/net/faultmanagement/alarm/AlarmService.java
index 4cee1ca..cba84f4 100644
--- a/incubator/api/src/main/java/org/onosproject/incubator/net/faultmanagement/alarm/AlarmService.java
+++ b/incubator/api/src/main/java/org/onosproject/incubator/net/faultmanagement/alarm/AlarmService.java
@@ -36,10 +36,30 @@
      * @param isAcknowledged new acknowledged state
      * @param assignedUser   new assigned user, null clear
      * @return updated alarm (including any recent device-derived changes)
+     * @deprecated 1.10.0 Kingfisher
      */
+    @Deprecated
     Alarm updateBookkeepingFields(AlarmId id, boolean isAcknowledged, String assignedUser);
 
     /**
+     * Update book-keeping (ie administrative) fields for the alarm matching the specified identifier.
+     *
+     * @param id             alarm identifier
+     * @param clear          ture if the alarm has to be cleared
+     * @param isAcknowledged new acknowledged state
+     * @param assignedUser   new assigned user, null clear
+     * @return updated alarm (including any recent device-derived changes)
+     */
+    Alarm updateBookkeepingFields(AlarmId id, boolean clear, boolean isAcknowledged, String assignedUser);
+
+    /**
+     * Remove an alarm from ONOS.
+     *
+     * @param id alarm
+     */
+    void remove(AlarmId id);
+
+    /**
      * Returns summary of alarms on a given device.
      *
      * @param deviceId the device
diff --git a/incubator/api/src/main/java/org/onosproject/incubator/net/faultmanagement/alarm/DefaultAlarm.java b/incubator/api/src/main/java/org/onosproject/incubator/net/faultmanagement/alarm/DefaultAlarm.java
index 162b3df..15d6292 100644
--- a/incubator/api/src/main/java/org/onosproject/incubator/net/faultmanagement/alarm/DefaultAlarm.java
+++ b/incubator/api/src/main/java/org/onosproject/incubator/net/faultmanagement/alarm/DefaultAlarm.java
@@ -25,7 +25,6 @@
 /**
  * Default implementation of an alarm.
  */
-//TODO simpler creation and updating.
 public final class DefaultAlarm implements Alarm {
 
     private final AlarmId id;
@@ -34,14 +33,16 @@
     private final String description;
     private final AlarmEntityId source;
     private final long timeRaised;
-    private final long timeUpdated;
-    private final Long timeCleared;
-    private final SeverityLevel severity;
     private final boolean isServiceAffecting;
     private final boolean isAcknowledged;
     private final boolean isManuallyClearable;
     private final String assignedUser;
 
+    private final SeverityLevel severity;
+    private final long timeUpdated;
+    private final Long timeCleared;
+
+
     //Only for Kryo
     DefaultAlarm() {
         id = null;
@@ -151,6 +152,11 @@
     }
 
     @Override
+    public boolean cleared() {
+        return severity.equals(SeverityLevel.CLEARED);
+    }
+
+    @Override
     public boolean manuallyClearable() {
         return isManuallyClearable;
     }
@@ -230,6 +236,9 @@
                 .toString();
     }
 
+    /**
+     * Builder for the DefaultAlarm object.
+     */
     public static class Builder {
 
         // Manadatory fields when constructing alarm ...
@@ -248,8 +257,13 @@
         private boolean isManuallyClearable = false;
         private String assignedUser = null;
 
+        /**
+         * Constructs a Builder to create a Default Alarm based on another alarm.
+         *
+         * @param alarm the other alarm
+         */
         public Builder(final Alarm alarm) {
-            this(alarm.deviceId(), alarm.description(), alarm.severity(), alarm.timeRaised());
+            this(alarm.id(), alarm.deviceId(), alarm.description(), alarm.severity(), alarm.timeRaised());
             this.source = alarm.source();
             this.timeUpdated = alarm.timeUpdated();
             this.timeCleared = alarm.timeCleared();
@@ -260,10 +274,41 @@
 
         }
 
+        /**
+         * Constructs a Builder to create a Default Alarm.
+         *
+         * @param deviceId    the device ID
+         * @param description the Alarm description
+         * @param severity    the severity
+         * @param timeRaised  when the alarm was raised
+         * @deprecated 1.10.0 - Kingfisher
+         */
+        @Deprecated
         public Builder(final DeviceId deviceId,
                        final String description, final SeverityLevel severity, final long timeRaised) {
             super();
-            this.id = AlarmId.NONE;
+            this.deviceId = deviceId;
+            this.description = description;
+            this.severity = severity;
+            this.timeRaised = timeRaised;
+            // Unless specified time-updated is same as raised.
+            this.timeUpdated = timeRaised;
+            this.id = AlarmId.alarmId(deviceId, Long.toString(timeRaised));
+        }
+
+        /**
+         * Constructs a Builder to create a Default Alarm.
+         *
+         * @param id          the AlarmId
+         * @param deviceId    the device ID
+         * @param description the Alarm description
+         * @param severity    the severity
+         * @param timeRaised  when the alarm was raised
+         */
+        public Builder(final AlarmId id, final DeviceId deviceId,
+                       final String description, final SeverityLevel severity, final long timeRaised) {
+            super();
+            this.id = id;
             this.deviceId = deviceId;
             this.description = description;
             this.severity = severity;
@@ -272,52 +317,112 @@
             this.timeUpdated = timeRaised;
         }
 
+        /**
+         * Sets the new alarm source.
+         *
+         * @param source the source
+         * @return self for chaining
+         */
         public Builder forSource(final AlarmEntityId source) {
             this.source = source;
             return this;
         }
 
+        /**
+         * Sets the new alarm time updated.
+         *
+         * @param timeUpdated the time
+         * @return self for chaining
+         */
         public Builder withTimeUpdated(final long timeUpdated) {
             this.timeUpdated = timeUpdated;
             return this;
         }
 
+        /**
+         * Sets the new alarm time cleared.
+         *
+         * @param timeCleared the time
+         * @return self for chaining
+         */
         public Builder withTimeCleared(final Long timeCleared) {
             this.timeCleared = timeCleared;
             return this;
         }
 
+        /**
+         * Sets the new alarm Id.
+         *
+         * @param id the id
+         * @return self for chaining
+         * @deprecated 1.10.0- Kingfisher
+         */
+        @Deprecated
         public Builder withId(final AlarmId id) {
             this.id = id;
             return this;
         }
 
+        /**
+         * Clears the alarm that is being created.
+         *
+         * @return self for chaining
+         */
         public Builder clear() {
             this.severity = SeverityLevel.CLEARED;
             final long now = System.currentTimeMillis();
             return withTimeCleared(now).withTimeUpdated(now);
         }
 
+        /**
+         * Sets the new alarm service affecting flag.
+         *
+         * @param isServiceAffecting the service affecting flag
+         * @return self for chaining
+         */
         public Builder withServiceAffecting(final boolean isServiceAffecting) {
             this.isServiceAffecting = isServiceAffecting;
             return this;
         }
 
+        /**
+         * Sets the new alarm acknowledged flag.
+         *
+         * @param isAcknowledged the acknowledged flag
+         * @return self for chaining
+         */
         public Builder withAcknowledged(final boolean isAcknowledged) {
             this.isAcknowledged = isAcknowledged;
             return this;
         }
 
+        /**
+         * Sets the new alarm the manually clearable flag.
+         *
+         * @param isManuallyClearable the manually clearable flag
+         * @return self for chaining
+         */
         public Builder withManuallyClearable(final boolean isManuallyClearable) {
             this.isManuallyClearable = isManuallyClearable;
             return this;
         }
 
+        /**
+         * Sets the new alarm assigned user.
+         *
+         * @param assignedUser the user
+         * @return self for chaining
+         */
         public Builder withAssignedUser(final String assignedUser) {
             this.assignedUser = assignedUser;
             return this;
         }
 
+        /**
+         * Builds the alarm.
+         *
+         * @return self for chaining
+         */
         public DefaultAlarm build() {
             checkNotNull(id, "Must specify an alarm id");
             checkNotNull(deviceId, "Must specify a device");
diff --git a/incubator/api/src/test/java/org/onosproject/incubator/net/faultmanagement/alarm/AlarmIdTest.java b/incubator/api/src/test/java/org/onosproject/incubator/net/faultmanagement/alarm/AlarmIdTest.java
index bfb56d6..e9522c4 100644
--- a/incubator/api/src/test/java/org/onosproject/incubator/net/faultmanagement/alarm/AlarmIdTest.java
+++ b/incubator/api/src/test/java/org/onosproject/incubator/net/faultmanagement/alarm/AlarmIdTest.java
@@ -16,14 +16,13 @@
 package org.onosproject.incubator.net.faultmanagement.alarm;
 
 import com.google.common.testing.EqualsTester;
-import static org.hamcrest.Matchers.containsString;
 import org.junit.Test;
+import org.onosproject.net.DeviceId;
 
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.not;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThat;
-import static org.junit.Assert.fail;
 import static org.onlab.junit.ImmutableClassChecker.assertThatClassIsImmutable;
 
 /**
@@ -31,9 +30,16 @@
  */
 public class AlarmIdTest {
 
-    private static final long ID_A = 1L;
-    private static final long ID_B = 2L;
-    private static final long ID_Z = 987654321L;
+    private static final DeviceId DEVICE_ID = DeviceId.deviceId("foo:bar");
+    private static final String UNIQUE_ID_1 = "unique_id_1";
+    private static final AlarmId ID_A = AlarmId.alarmId(DEVICE_ID, UNIQUE_ID_1);
+
+    private static final String UNIQUE_ID_2 = "unique_id_2";
+
+    private static final String UNIQUE_ID_3 = "unique_id_3";
+    private static final AlarmId ID_Z = AlarmId.alarmId(DEVICE_ID, UNIQUE_ID_3);
+
+    private static final String ID_STRING = "foo:bar:unique_id_3";
 
     /**
      * Tests the immutability of {@link AlarmId}.
@@ -48,8 +54,8 @@
      */
     @Test
     public void testEquality() {
-        final AlarmId id1 = AlarmId.alarmId(ID_A);
-        final AlarmId id2 = AlarmId.alarmId(ID_A);
+        final AlarmId id1 = AlarmId.alarmId(DEVICE_ID, UNIQUE_ID_1);
+        final AlarmId id2 = AlarmId.alarmId(DEVICE_ID, UNIQUE_ID_1);
 
         assertThat(id1, is(id2));
     }
@@ -59,16 +65,16 @@
      */
     @Test
     public void testNonEquality() {
-        final AlarmId id1 = AlarmId.alarmId(ID_A);
-        final AlarmId id2 = AlarmId.alarmId(ID_B);
+        final AlarmId id1 = AlarmId.alarmId(DEVICE_ID, UNIQUE_ID_1);
+        final AlarmId id2 = AlarmId.alarmId(DEVICE_ID, UNIQUE_ID_2);
 
         assertThat(id1, is(not(id2)));
     }
 
     @Test
     public void valueOf() {
-        final AlarmId id = AlarmId.alarmId(0xdeadbeefL);
-        assertEquals("incorrect valueOf", id, AlarmId.alarmId(0xdeadbeefL));
+        final AlarmId id = AlarmId.alarmId(DEVICE_ID, UNIQUE_ID_1);
+        assertEquals("incorrect valueOf", id, ID_A);
     }
 
     /**
@@ -76,9 +82,9 @@
      */
     @Test
     public void testEquals() {
-        final AlarmId id1 = AlarmId.alarmId(11111L);
-        final AlarmId sameAsId1 = AlarmId.alarmId(11111L);
-        final AlarmId id2 = AlarmId.alarmId(22222L);
+        final AlarmId id1 = AlarmId.alarmId(DEVICE_ID, UNIQUE_ID_1);
+        final AlarmId sameAsId1 = AlarmId.alarmId(DEVICE_ID, UNIQUE_ID_1);
+        final AlarmId id2 = AlarmId.alarmId(DEVICE_ID, UNIQUE_ID_2);
 
         new EqualsTester()
                 .addEqualityGroup(id1, sameAsId1)
@@ -91,18 +97,11 @@
      */
     @Test
     public void testConstruction() {
-        final AlarmId id1 = AlarmId.alarmId(ID_Z);
-        assertEquals(id1.fingerprint(), ID_Z);
+        final AlarmId id1 = AlarmId.alarmId(DEVICE_ID, UNIQUE_ID_3);
+        assertEquals(id1.toString(), ID_Z.toString());
 
-        // No default constructor so no need to test it !
-        assertEquals(0L, AlarmId.NONE.fingerprint());
-        try {
-            final AlarmId bad = AlarmId.alarmId(0L);
-            fail("0 is a Reserved value but we created " + bad);
-        } catch (IllegalArgumentException ex) {
-            assertThat(ex.getMessage(),
-                    containsString("id must be non-zero"));
-        }
+        final AlarmId idString = AlarmId.alarmId(ID_STRING);
+        assertEquals(id1, idString);
 
     }
 }
diff --git a/incubator/api/src/test/java/org/onosproject/incubator/net/faultmanagement/alarm/DefaultAlarmTest.java b/incubator/api/src/test/java/org/onosproject/incubator/net/faultmanagement/alarm/DefaultAlarmTest.java
index 60bb330..7e4c3a9 100644
--- a/incubator/api/src/test/java/org/onosproject/incubator/net/faultmanagement/alarm/DefaultAlarmTest.java
+++ b/incubator/api/src/test/java/org/onosproject/incubator/net/faultmanagement/alarm/DefaultAlarmTest.java
@@ -15,17 +15,23 @@
  */
 package org.onosproject.incubator.net.faultmanagement.alarm;
 
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.is;
-import static org.hamcrest.Matchers.notNullValue;
-import static org.hamcrest.Matchers.greaterThan;
 import org.junit.Test;
-import static org.junit.Assert.*;
-import static org.onlab.junit.ImmutableClassChecker.assertThatClassIsImmutable;
 import org.onosproject.net.DeviceId;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.*;
+import static org.junit.Assert.*;
+import static org.onlab.junit.ImmutableClassChecker.assertThatClassIsImmutable;
+
 public class DefaultAlarmTest {
 
+    private static final AlarmEntityId ALARM_ENTITY_ID = AlarmEntityId.alarmEntityId("port:bar");
+    private static final DeviceId DEVICE_ID = DeviceId.deviceId("foo:bar");
+    private static final String UNIQUE_ID_1 = "unique_id_1";
+    private static final AlarmId ALARM_ID = AlarmId.alarmId(DEVICE_ID, UNIQUE_ID_1);
+    private static final String UNIQUE_ID_2 = "unique_id_1";
+    private static final AlarmId ALARM_ID_2 = AlarmId.alarmId(DEVICE_ID, UNIQUE_ID_2);
+
     @Test
     public void testImmutability() {
         assertThatClassIsImmutable(DefaultAlarm.class);
@@ -44,11 +50,11 @@
 
     @Test
     public void testEquals() {
-        final DefaultAlarm a = new DefaultAlarm.Builder(
+        final DefaultAlarm a = new DefaultAlarm.Builder(ALARM_ID_2,
                 DeviceId.NONE, "desc", Alarm.SeverityLevel.MINOR, 3).build();
-        final DefaultAlarm b = new DefaultAlarm.Builder(
-                DeviceId.NONE, "desc", Alarm.SeverityLevel.MINOR, a.timeRaised() + 1).
-                withId(ALARM_ID).withTimeUpdated(a.timeUpdated() + 1).build();
+        final DefaultAlarm b = new DefaultAlarm.Builder(ALARM_ID,
+                DeviceId.NONE, "desc", Alarm.SeverityLevel.MINOR, a.timeRaised() + 1)
+                .withTimeUpdated(a.timeUpdated() + 1).build();
         assertEquals("id or timeRaised or timeUpdated may differ", a, b);
 
         assertNotEquals(a, new DefaultAlarm.Builder(a).withAcknowledged(!a.acknowledged()).build());
@@ -73,20 +79,17 @@
     @Test
     public void testId() {
         final DefaultAlarm a = generate();
-        assertThat(a.id(), is(AlarmId.NONE));
-        final DefaultAlarm b = new DefaultAlarm.Builder(a).withId(ALARM_ID).build();
+        final DefaultAlarm b = new DefaultAlarm.Builder(a).build();
 
         assertEquals("id ignored in equals", a, b);
-        assertNotEquals(ALARM_ID, a.id());
+        assertEquals(ALARM_ID, a.id());
         assertEquals(ALARM_ID, b.id());
         assertEquals(ALARM_ENTITY_ID, b.source());
 
     }
-    private static final AlarmEntityId ALARM_ENTITY_ID = AlarmEntityId.alarmEntityId("port:bar");
-    private static final AlarmId ALARM_ID = AlarmId.alarmId(888L);
 
     private static DefaultAlarm generate() {
-        return new DefaultAlarm.Builder(
+        return new DefaultAlarm.Builder(ALARM_ID,
                 DeviceId.NONE, "desc", Alarm.SeverityLevel.MINOR, 3).forSource(ALARM_ENTITY_ID).build();
     }
 }