Uploaded image for project: 'OpenNMS'
  1. OpenNMS
  2. NMS-9278

Finishing the implementation of the missing endpoints for ReST v2

    Details

    • Type: Enhancement
    • Status: In Progress (View Workflow)
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: Meridian-2016.1.4, 19.0.1
    • Fix Version/s: 20.0.1, Meridian-2017.1.0
    • Component/s: REST
    • Security Level: Default (Default Security Scheme)
    • Labels:
      None

      Description

      The current ReST API (i.e. /opennms/rest or /opennms/api/v1) is the official API.

      There is some work on /optnnms/api/v2 for some endpoints, but lots of them are missing.

      There are some features that don't have a v1 alternative, and only provide v2 alternative. This is fine, but as v2 is not documented and/or completed, but it seems reasonable to provide an implementation for the missing endpoints, just to be able to have v2 as an alternative of v1.

      The best feature of the v2 API, is the ability to specify powerful filters using FIQL. This by itself is enough reason to have a v1 alternative to be able to implement reach interfaces that can have filters as powerful as the filters you can currently use on the OpenNMS WebUI, and even have the ability to make more powerful filters.

      Another feature of FIQL in Apache CXF is the ability to create aliases. This is not used at the moment, but could provide human readable alternatives for some attributes, in order to define filters using the same names you see on the JSON/XML output, instead of defining the filters as HQL expects them (which is how the current implementation works not only on v2 but also on v1).

      Here is the list of the currently available endpoints in v2:

      /api/v2/applications
      /api/v2/minions
      /api/v2/monitoringLocations
      /api/v2/discovery
      /api/v2/scanreports
      /api/v2/nodes (not fully implemented compared with v1)
      /api/v2/outages
      /api/v2/notifications
      /api/v2/geolocation
      

      Here is the list of the missing endpoints (exist on v1 but not on v2 that can benefit from FIQL):

      /events
      /alarms
      /nodes/{ID}/ipinterfaces
      /nodes/{ID}/ipinterfaces/{IP}/services
      /nodes/{ID}/snmpinterfaces
      /nodes/{ID}/hardwareInventory
      /nodes/{ID}/categories
      /ifservices
      /acks
      /categories
      /remotelocations
      

      Note: I'm not 100% sure if the last 3 requires a v2 implementation.

      Here is the list of v1 end-points that may not have v2 counterparts, because those are not expected to have filtering capabilities (at the moment):

      /requisitions
      /requisitionNames
      /foreignSources
      /foreignSourcesConfig
      /snmpConfig
      /measurements
      /availability
      /config*
      /info
      /graphs
      /graphml
      /groups
      /users
      /heatmap
      /info
      /ksc
      /timeline
      /sched-outages
      

        Issue Links

          Activity

          Hide
          agalue Alejandro Galue added a comment -

          On v1, we have our own handler to parse the query parameters and build the HQL filter. This allows us to specify filters like:

          /rest/outages?ifRegainedService=null
          

          With FIQL, handling null is not part of the specification, and imposes hard code changes in order to cleanly handle it. For this reason, each FIQL operation requires a valid value. For strings and dates, we already have some custom values that can be used to "emulate" the effect of having null on the database for certain columns.

          Here are some examples:

          Outstanding Outages
          /api/v2/outages?_s=ifRegainedService==1970-01-01T00:00:00.000-0000
          
          Acknowledged Notifications
          /api/v2/notifications?_s=answeredBy!=\u0000
          

          As shown before, there are ways to specify nullable comparators without explicitly using null on the FIQL expression. So far, that seems to be reasonable for the typical use cases.

          Show
          agalue Alejandro Galue added a comment - On v1, we have our own handler to parse the query parameters and build the HQL filter. This allows us to specify filters like: / rest /outages?ifRegainedService= null With FIQL, handling null is not part of the specification, and imposes hard code changes in order to cleanly handle it. For this reason, each FIQL operation requires a valid value. For strings and dates, we already have some custom values that can be used to "emulate" the effect of having null on the database for certain columns. Here are some examples: Outstanding Outages /api/v2/outages?_s=ifRegainedService==1970-01-01T00:00:00.000-0000 Acknowledged Notifications /api/v2/notifications?_s=answeredBy!=\u0000 As shown before, there are ways to specify nullable comparators without explicitly using null on the FIQL expression. So far, that seems to be reasonable for the typical use cases.
          Hide
          agalue Alejandro Galue added a comment -

          An interesting consequence of the current data parser is that dates are returned as timestamps in milliseconds when generating JSON, while when generating a XML the data is returned on a human readable format (similar but not equal to the format expected by FIQL).

          When using FIQL filter against date fields, it is mandatory that the date follows the following pattern to avoid runtime exceptions:

          https://github.com/OpenNMS/opennms/blob/develop/opennms-webapp-rest/src/main/webapp/WEB-INF/applicationContext-cxf-rest-v2.xml#L41

          That means, any integration against v2, should have a proper handler for XML and JSON. For XML, to properly specify the timezone, and for JSON to generate the appropriate data according to the expected format.

          Show
          agalue Alejandro Galue added a comment - An interesting consequence of the current data parser is that dates are returned as timestamps in milliseconds when generating JSON, while when generating a XML the data is returned on a human readable format (similar but not equal to the format expected by FIQL). When using FIQL filter against date fields, it is mandatory that the date follows the following pattern to avoid runtime exceptions: https://github.com/OpenNMS/opennms/blob/develop/opennms-webapp-rest/src/main/webapp/WEB-INF/applicationContext-cxf-rest-v2.xml#L41 That means, any integration against v2, should have a proper handler for XML and JSON. For XML, to properly specify the timezone, and for JSON to generate the appropriate data according to the expected format.
          Hide
          agalue Alejandro Galue added a comment - - edited

          The current design is very useful, but it makes hard the implementation of nested resources like ip-interfaces, snmp-interfaces which are accessed through the node end-point.

          There are 2 reasons for that:

          1) The criteria builder is expected to be constructed using the node-criteria from the URL path. To re-use the methods from AbstractDaoRestService, this node criteria should be extracted from the UrlInfo that should be injected somehow.

          2) The GET method from AbstractDaoRestService is intended to use the primary key of the element. This is not the case of the embedded resources, because for IP addresses the key is the IP address, and for SNMP interfaces the key is the ifIndex, and those are different from the primary key on each case.

          In fact, [2] affects the node endpoint itself, as it doesn't seem to be possible to use the foreignSource:foreignID combination to identify a node.

          That will makes v2 very different from v1. You'll gain the flexibility of FIQL at expenses of complex queries based on dynamic primary keys that might require additional queries to ReST.

          Show
          agalue Alejandro Galue added a comment - - edited The current design is very useful, but it makes hard the implementation of nested resources like ip-interfaces, snmp-interfaces which are accessed through the node end-point. There are 2 reasons for that: 1) The criteria builder is expected to be constructed using the node-criteria from the URL path. To re-use the methods from AbstractDaoRestService , this node criteria should be extracted from the UrlInfo that should be injected somehow. 2) The GET method from AbstractDaoRestService is intended to use the primary key of the element. This is not the case of the embedded resources, because for IP addresses the key is the IP address, and for SNMP interfaces the key is the ifIndex, and those are different from the primary key on each case. In fact, [2] affects the node endpoint itself, as it doesn't seem to be possible to use the foreignSource:foreignID combination to identify a node. That will makes v2 very different from v1. You'll gain the flexibility of FIQL at expenses of complex queries based on dynamic primary keys that might require additional queries to ReST.
          Hide
          agalue Alejandro Galue added a comment -

          I've refactored AbstractDaoRestService to make it more generic and have better defaults methods.

          The current state of the branch has full implementations for all the descendent end-points from api/v2/nodes, events and alarms.

          I found that JSON is now the default when retrieving data, but JSON is not a good format when creating data using the v2 API. Guess the entities require some JSON annotations in order to translate elements like NodeType, NodeLabelSource, etc.

          Show
          agalue Alejandro Galue added a comment - I've refactored AbstractDaoRestService to make it more generic and have better defaults methods. The current state of the branch has full implementations for all the descendent end-points from api/v2/nodes, events and alarms. I found that JSON is now the default when retrieving data, but JSON is not a good format when creating data using the v2 API. Guess the entities require some JSON annotations in order to translate elements like NodeType, NodeLabelSource, etc.
          Hide
          agalue Alejandro Galue added a comment -

          I found on my tests that the end-points are returning duplicates. Of course adding a distinct to the criteria fixes the problem, but raises another one: you lose flexibility in ordering due to NMS-7830.

          I would prefer to have a usable ReST v2 with strong filtering capabilities, and not flexible ordering capabilities.

          Show
          agalue Alejandro Galue added a comment - I found on my tests that the end-points are returning duplicates. Of course adding a distinct to the criteria fixes the problem, but raises another one: you lose flexibility in ordering due to NMS-7830 . I would prefer to have a usable ReST v2 with strong filtering capabilities, and not flexible ordering capabilities.
          Hide
          agalue Alejandro Galue added a comment -

          I found on my tests that filtering by IP address doesn't work, due to a conversion failure by CXF.

          Show
          agalue Alejandro Galue added a comment - I found on my tests that filtering by IP address doesn't work, due to a conversion failure by CXF.
          Hide
          mvr Markus von Rüden added a comment -

          The custom type conversion I was able to solve:

          /*******************************************************************************
           * This file is part of OpenNMS(R).
           *
           * Copyright (C) 2017-2017 The OpenNMS Group, Inc.
           * OpenNMS(R) is Copyright (C) 1999-2017 The OpenNMS Group, Inc.
           *
           * OpenNMS(R) is a registered trademark of The OpenNMS Group, Inc.
           *
           * OpenNMS(R) is free software: you can redistribute it and/or modify
           * it under the terms of the GNU Affero General Public License as published
           * by the Free Software Foundation, either version 3 of the License,
           * or (at your option) any later version.
           *
           * OpenNMS(R) is distributed in the hope that it will be useful,
           * but WITHOUT ANY WARRANTY; without even the implied warranty of
           * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
           * GNU Affero General Public License for more details.
           *
           * You should have received a copy of the GNU Affero General Public License
           * along with OpenNMS(R).  If not, see:
           *      http://www.gnu.org/licenses/
           *
           * For more information contact:
           *     OpenNMS(R) Licensing <license@opennms.org>
           *     http://www.opennms.org/
           *     http://www.opennms.com/
           *******************************************************************************/
          
          package org.opennms.web.rest.support;
          
          import java.net.InetAddress;
          import java.util.HashMap;
          import java.util.Map;
          
          import org.apache.cxf.jaxrs.ext.search.Beanspector;
          import org.apache.cxf.jaxrs.ext.search.SearchParseException;
          import org.apache.cxf.jaxrs.ext.search.fiql.FiqlParser;
          import org.opennms.core.utils.InetAddressUtils;
          
          import com.google.common.base.Strings;
          
          /**
           * This FiqlParser is a custom version of the original {@link FiqlParser} from apache cxf.
           *
           * The default implementation lacks the possibility to map string values to certain types before setting them on the search bean, e.g. String conversion to InetAddress.
           * The custom implementation should address this issue.
           *
           * @param <T>
           * @author mvrueden
           */
          public class CustomFiqlParser<T> extends FiqlParser<T> {
          
              interface TypeMapper<T> {
                  T map(String value);
              }
          
              private final Map<String, TypeMapper<?>> customMappers = new HashMap<>();
          
              public CustomFiqlParser(Class<T> tclass, Map<String, String> contextProperties, Map<String, String> beanProperties) {
                  super(tclass, contextProperties, beanProperties);
          
                  // Add mapping from String to InetAddress
                  this.customMappers.put(InetAddress.class.getName(), value -> {
                      if (value == null || Strings.isNullOrEmpty(value)) {
                          return null;
                      }
                      return InetAddressUtils.getInetAddress(value);
                  });
              }
          
              @Override
              protected Object parseType(String originalPropName, Object ownerBean, Object lastCastedValue, String setter, Beanspector.TypeInfo typeInfo, String value) throws SearchParseException {
                  // Custom mapper
                  final TypeMapper<?> typeMapper = customMappers.get(typeInfo.getTypeClass().getName());
                  if (typeMapper != null) {
                      if (ownerBean != null) {
                          throw new IllegalStateException("You cannot provide a ownerBean and have a typeMapper defined. Something is wrong.");
                      }
                      return typeMapper.map(value);
                  }
                  // No custom mapping, fall back to default behaviour
                  return super.parseType(originalPropName, ownerBean, lastCastedValue, setter, typeInfo, value);
              }
          }
          
          

          However it is not possible to do a wildcard query, such as: 127.0.0.*

          Show
          mvr Markus von Rüden added a comment - The custom type conversion I was able to solve: /******************************************************************************* * This file is part of OpenNMS(R). * * Copyright (C) 2017-2017 The OpenNMS Group, Inc. * OpenNMS(R) is Copyright (C) 1999-2017 The OpenNMS Group, Inc. * * OpenNMS(R) is a registered trademark of The OpenNMS Group, Inc. * * OpenNMS(R) is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as published * by the Free Software Foundation, either version 3 of the License, * or (at your option) any later version. * * OpenNMS(R) is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU Affero General Public License for more details. * * You should have received a copy of the GNU Affero General Public License * along with OpenNMS(R). If not, see: * http: //www.gnu.org/licenses/ * * For more information contact: * OpenNMS(R) Licensing <license@opennms.org> * http: //www.opennms.org/ * http: //www.opennms.com/ *******************************************************************************/ package org.opennms.web. rest .support; import java.net.InetAddress; import java.util.HashMap; import java.util.Map; import org.apache.cxf.jaxrs.ext.search.Beanspector; import org.apache.cxf.jaxrs.ext.search.SearchParseException; import org.apache.cxf.jaxrs.ext.search.fiql.FiqlParser; import org.opennms.core.utils.InetAddressUtils; import com.google.common.base.Strings; /** * This FiqlParser is a custom version of the original {@link FiqlParser} from apache cxf. * * The default implementation lacks the possibility to map string values to certain types before setting them on the search bean, e.g. String conversion to InetAddress. * The custom implementation should address this issue. * * @param <T> * @author mvrueden */ public class CustomFiqlParser<T> extends FiqlParser<T> { interface TypeMapper<T> { T map( String value); } private final Map< String , TypeMapper<?>> customMappers = new HashMap<>(); public CustomFiqlParser( Class <T> tclass, Map< String , String > contextProperties, Map< String , String > beanProperties) { super (tclass, contextProperties, beanProperties); // Add mapping from String to InetAddress this .customMappers.put(InetAddress.class.getName(), value -> { if (value == null || Strings.isNullOrEmpty(value)) { return null ; } return InetAddressUtils.getInetAddress(value); }); } @Override protected Object parseType( String originalPropName, Object ownerBean, Object lastCastedValue, String setter, Beanspector.TypeInfo typeInfo, String value) throws SearchParseException { // Custom mapper final TypeMapper<?> typeMapper = customMappers.get(typeInfo.getTypeClass().getName()); if (typeMapper != null ) { if (ownerBean != null ) { throw new IllegalStateException( "You cannot provide a ownerBean and have a typeMapper defined. Something is wrong." ); } return typeMapper.map(value); } // No custom mapping, fall back to default behaviour return super .parseType(originalPropName, ownerBean, lastCastedValue, setter, typeInfo, value); } } However it is not possible to do a wildcard query, such as: 127.0.0.*

            People

            • Assignee:
              agalue Alejandro Galue
              Reporter:
              agalue Alejandro Galue
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:

                Development