Fix patch bug where attribute descriptions were incorrectly copied ba…

…ck to the dictionary. Drop enum sibling validation issues from error to warning. Improve UI of enum attributes and enum sibling attributes. (#133)

lib/schema.ex

@@ -550,7 +550,13 @@ defmodule Schema do
   end
 
   defp reduce_data(object) do
-    delete_links(object) |> Map.drop([:_source, :_source_patched])
+    Map.drop(object, internal_keys(object))
+  end
+
+  defp internal_keys(map) do
+    Enum.filter(Map.keys(map), fn key ->
+      String.starts_with?(to_string(key), "_")
+    end)
   end
 
   defp reduce_attributes(data) do

lib/schema/cache.ex

@@ -1306,8 +1306,19 @@ defmodule Schema.Cache do
   defp update_dictionary(dictionary) do
     types = get_in(dictionary, [:types, :attributes])
 
+    # Enum attributes point to their enum sibling through the :sibling attribute,
+    # however the siblings do _not_ refer back to the related enum attribute, so let's build that.
+    sibling_of_map =
+      Enum.reduce(dictionary[:attributes], %{}, fn {attribute_key, attribute}, acc ->
+        if Map.has_key?(attribute, :sibling) do
+          Map.put(acc, String.to_atom(attribute[:sibling]), attribute_key)
+        else
+          acc
+        end
+      end)
+
     Map.update!(dictionary, :attributes, fn attributes ->
-      Enum.into(attributes, %{}, fn {name, attribute} ->
+      Enum.into(attributes, %{}, fn {attribute_key, attribute} ->
         type = attribute[:type] || "object_t"
 
         attribute =
@@ -1321,7 +1332,16 @@ defmodule Schema.Cache do
               attribute
           end
 
-        {name, attribute}
+        attribute =
+          case sibling_of_map[attribute_key] do
+            nil ->
+              attribute
+
+            enum_attribute_key ->
+              Map.put(attribute, :_sibling_of, enum_attribute_key)
+          end
+
+        {attribute_key, attribute}
       end)
     end)
   end

lib/schema/utils.ex

@@ -159,38 +159,38 @@ defmodule Schema.Utils do
   end
 
   defp update_data_types(attributes, types, objects) do
-    Enum.into(attributes, %{}, fn {name, value} ->
-      data =
-        if value[:type] == "object_t" do
-          update_object_type(name, value, objects)
+    Enum.into(attributes, %{}, fn {attribute_key, attribute} ->
+      attribute_update =
+        if attribute[:type] == "object_t" do
+          update_object_type(attribute_key, attribute, objects)
         else
-          update_data_type(name, value, types)
+          update_data_type(attribute_key, attribute, types)
         end
 
-      {name, data}
+      {attribute_key, attribute_update}
     end)
   end
 
-  defp update_object_type(name, value, objects) do
-    key = value[:object_type]
+  defp update_object_type(attribute_key, attribute, objects) do
+    object_key = attribute[:object_type]
 
-    case find_entity(objects, value, key) do
-      {key, nil} ->
-        Logger.warning("Undefined object type: #{key}, for #{name}")
-        Map.put(value, :object_name, "_undefined_")
+    case find_entity(objects, attribute, object_key) do
+      {object_key, nil} ->
+        Logger.warning("Undefined object type: #{object_key}, for #{attribute_key}")
+        Map.put(attribute, :object_name, "_undefined_")
 
-      {key, object} ->
-        value
+      {object_key, object} ->
+        attribute
         |> Map.put(:object_name, object[:caption])
-        |> Map.put(:object_type, Atom.to_string(key))
+        |> Map.put(:object_type, Atom.to_string(object_key))
     end
   end
 
-  defp update_data_type(name, value, types) do
+  defp update_data_type(attribute_key, attribute, types) do
     type =
-      case value[:type] do
+      case attribute[:type] do
         nil ->
-          Logger.warning("Missing data type for: #{name}, will use string_t type")
+          Logger.warning("Missing data type for: #{attribute_key}, will use string_t type")
           "string_t"
 
         t ->
@@ -199,11 +199,11 @@ defmodule Schema.Utils do
 
     case types[String.to_atom(type)] do
       nil ->
-        Logger.warning("Undefined data type: #{name}: #{type}")
-        value
+        Logger.warning("Undefined data type: #{attribute_key}: #{type}")
+        attribute
 
       type ->
-        Map.put(value, :type_name, type[:caption])
+        Map.put(attribute, :type_name, type[:caption])
     end
   end
 
@@ -292,6 +292,9 @@ defmodule Schema.Utils do
 
         case find_entity(dictionary_attributes, item, item_attribute_key) do
           {_, nil} ->
+            # Special fix-up is needed for attributes from extension classes and objects.
+            # In this special-case, the dictionary ends up missing important details.
+            # TODO: Figure out where this happens and why.
             case String.split(Atom.to_string(item_attribute[:_source]), "/") do
               [ext, _] ->
                 ext_key = String.to_atom("#{ext}/#{item_attribute_key}")
@@ -306,7 +309,8 @@ defmodule Schema.Utils do
                       # back to the dictionary
                       clean_item_attribute = Map.delete(item_attribute, :observable)
 
-                      deep_merge(dictionary_attribute, clean_item_attribute)
+                      # Merge attribute from extension class or object to the dictionary attribute
+                      deep_merge(clean_item_attribute, dictionary_attribute)
                       |> update_links_fn.(link)
                   end
 

lib/schema/validator2.ex

@@ -838,31 +838,54 @@ defmodule Schema.Validator2 do
          attribute_name,
          attribute_details
        ) do
-    if event_enum_value == 99 do
-      # Enum value is the integer 99 (Other). The enum sibling, if present, can be anything.
-      response
-    else
-      sibling_name = attribute_details[:sibling]
+    sibling_name = attribute_details[:sibling]
 
-      if Map.has_key?(event_item, sibling_name) do
-        # Sibling is present - make sure the string value matches up
-        enum_caption = attribute_details[:enum][event_enum_value_atom][:caption]
-        sibling_value = event_item[sibling_name]
+    if Map.has_key?(event_item, sibling_name) do
+      # Sibling is present - make sure the string value matches up
+      enum_caption = attribute_details[:enum][event_enum_value_atom][:caption]
+      sibling_value = event_item[sibling_name]
 
+      if event_enum_value == 99 do
+        # Enum value is the integer 99 (Other). The enum sibling should _not_ match the
+        if enum_caption == sibling_value do
+          enum_attribute_path = make_attribute_path(parent_attribute_path, attribute_name)
+          sibling_attribute_path = make_attribute_path(parent_attribute_path, sibling_name)
+
+          add_warning(
+            response,
+            "attribute_enum_sibling_suspicous_other",
+            "Attribute \"#{sibling_attribute_path}\" enum sibling value" <>
+              " #{inspect(sibling_value)} suspiciously matches the caption of" <>
+              " enum \"#{enum_attribute_path}\" value 99 (#{inspect(enum_caption)})." <>
+              " Note: the recommendation is to use the original source value for" <>
+              " 99 (#{inspect(enum_caption)}), so this should only match in the edge case" <>
+              " where #{inspect(sibling_value)} is actually the original source value.",
+            %{
+              attribute_path: sibling_attribute_path,
+              attribute: sibling_name,
+              value: sibling_value
+            }
+          )
+        else
+          # The 99 (Other) sibling value looks good
+          response
+        end
+      else
         if enum_caption == sibling_value do
           # Sibling has correct value
           response
         else
           enum_attribute_path = make_attribute_path(parent_attribute_path, attribute_name)
           sibling_attribute_path = make_attribute_path(parent_attribute_path, sibling_name)
 
-          add_error(
+          add_warning(
             response,
             "attribute_enum_sibling_incorrect",
             "Attribute \"#{sibling_attribute_path}\" enum sibling value" <>
-              " #{inspect(sibling_value)} is incorrect for" <>
+              " #{inspect(sibling_value)} does not match the caption of" <>
               " enum \"#{enum_attribute_path}\" value #{inspect(event_enum_value)};" <>
-              " expected \"#{enum_caption}\", got #{inspect(sibling_value)}.",
+              " expected \"#{enum_caption}\", got #{inspect(sibling_value)}." <>
+              " Note: matching is recommended but not required.",
             %{
               attribute_path: sibling_attribute_path,
               attribute: sibling_name,
@@ -871,10 +894,10 @@ defmodule Schema.Validator2 do
             }
           )
         end
-      else
-        # Sibling not present, which is OK
-        response
       end
+    else
+      # Sibling not present, which is OK
+      response
     end
   end
 

lib/schema_web/controllers/page_controller.ex

@@ -54,7 +54,7 @@ defmodule SchemaWeb.PageController do
   """
   @spec data_types(Plug.Conn.t(), any) :: Plug.Conn.t()
   def data_types(conn, params) do
-    data = Schema.data_types() |> sort_attributes()
+    data = Schema.data_types() |> sort_attributes_by_key()
 
     render(conn, "data_types.html",
       extensions: Schema.extensions(),
@@ -84,18 +84,19 @@ defmodule SchemaWeb.PageController do
         render(conn, "profile.html",
           extensions: Schema.extensions(),
           profiles: data,
-          data: sort_attributes(profile)
+          data: sort_attributes_by_key(profile)
         )
     end
   end
 
   def profiles(conn, params) do
-    data = SchemaController.get_profiles(params)
+    profiles = SchemaController.get_profiles(params)
+    sorted_profiles = sort_by_key(profiles)
 
     render(conn, "profiles.html",
       extensions: Schema.extensions(),
-      profiles: data,
-      data: data
+      profiles: profiles,
+      data: sorted_profiles
     )
   end
 
@@ -138,7 +139,7 @@ defmodule SchemaWeb.PageController do
   """
   @spec dictionary(Plug.Conn.t(), any) :: Plug.Conn.t()
   def dictionary(conn, params) do
-    data = SchemaController.dictionary(params) |> sort_attributes()
+    data = SchemaController.dictionary(params) |> sort_attributes_by_key()
 
     render(conn, "dictionary.html",
       extensions: Schema.extensions(),
@@ -169,7 +170,7 @@ defmodule SchemaWeb.PageController do
       data ->
         data =
           data
-          |> sort_attributes()
+          |> sort_attributes_by_key()
           |> Map.put(:key, Schema.Utils.to_uid(extension, id))
 
         render(conn, "class.html",
@@ -202,7 +203,7 @@ defmodule SchemaWeb.PageController do
       data ->
         data =
           data
-          |> sort_attributes()
+          |> sort_attributes_by_key()
           |> Map.put(:key, Schema.Utils.to_uid(params["extension"], id))
 
         render(conn, "object.html",
@@ -214,7 +215,7 @@ defmodule SchemaWeb.PageController do
   end
 
   def objects(conn, params) do
-    data = SchemaController.objects(params) |> sort_by_name()
+    data = SchemaController.objects(params) |> sort_by_key()
 
     render(conn, "objects.html",
       extensions: Schema.extensions(),
@@ -231,19 +232,19 @@ defmodule SchemaWeb.PageController do
     end)
   end
 
-  defp sort_attributes(map) do
-    sort_attributes(map, :caption)
-  end
-
   defp sort_attributes(map, key) do
     Map.update!(map, :attributes, &sort_by(&1, key))
   end
 
-  defp sort_by_name(map) do
-    sort_by(map, :caption)
+  defp sort_by_key(map) do
+    Enum.sort(map)
   end
 
   defp sort_by(map, key) do
     Enum.sort(map, fn {_, v1}, {_, v2} -> v1[key] <= v2[key] end)
   end
+
+  defp sort_attributes_by_key(map) do
+    Map.update!(map, :attributes, &Enum.sort/1)
+  end
 end

lib/schema_web/templates/layout/app.html.eex

@@ -24,12 +24,11 @@ limitations under the License.
   <link href="https://fonts.googleapis.com/css2?family=Bai+Jamjuree&display=swap" rel="stylesheet">
 
   <!-- Bootstrap CSS -->
-  <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/[email protected].1/dist/css/bootstrap.min.css"
-      integrity="sha384-zCbKRCUGaJDkqS1kPbPd7TveP5iyJE0EjAuZQTgFLD2ylzuqKfdKlfG/eSrtxUkn"
+  <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/[email protected].2/dist/css/bootstrap.min.css"
+      integrity="sha384-xOolHFLEh07PJGoPkLv1IbcEPTNtaed2xpHsD9ESMhqIYd0nLMwNLD69Npy4HI+N"
       crossorigin="anonymous">
 
-  <link rel="stylesheet"
-        href="https://cdn.jsdelivr.net/npm/[email protected]/dist/css/bootstrap-select.min.css">
+  <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/[email protected]/dist/css/bootstrap-select.min.css">
 
   <link rel="stylesheet" href='<%= Routes.static_path(@conn, "/css/app.css") %>'/>
 
@@ -249,12 +248,12 @@ limitations under the License.
     crossorigin="anonymous">
 </script>
 
-<script src="https://cdn.jsdelivr.net/npm/[email protected].1/dist/js/bootstrap.min.js"
-    integrity="sha384-VHvPCCyXqtD5DqJeNxl2dtTyhF78xXNXdkwX1CZeRusQfRKp+tA7hAShOK/B/fQ2"
+<script src="https://cdn.jsdelivr.net/npm/[email protected].2/dist/js/bootstrap.bundle.min.js"
+    integrity="sha384-Fy6S3B9q64WdZWQUiU+q4/2Lc9npb8tCaSX9FK7E8HnRr0Jz8D6OP9dO5Vg3Q9ct"
     crossorigin="anonymous">
 </script>
 
-<script src="https://cdn.jsdelivr.net/npm/[email protected].14/dist/js/bootstrap-select.min.js"></script>
+<script src="https://cdn.jsdelivr.net/npm/[email protected].18/dist/js/bootstrap-select.min.js"></script>
 <script type="text/javascript" src='<%= Routes.static_path(@conn, "/js/sorttable.js") %>'></script>
 
 <script>

lib/schema_web/templates/page/class.html.eex

@@ -94,8 +94,8 @@ limitations under the License.
   <table id="data-table" class="table table-bordered sortable">
     <thead >
     <tr class="thead-color">
-      <th style="width: 10%">Caption</th>
       <th style="width: 10%">Name</th>
+      <th style="width: 10%">Caption</th>
       <th style="width: 10%">Group</th>
       <th style="width: 10%">Requirement</th>
       <th style="width: 10%">Type</th>
@@ -104,9 +104,9 @@ limitations under the License.
     </thead>
     <tbody class="searchable">
       <%= for {key, field} <- @data[:attributes] do %>
-        <tr class="<%= field_classes(field)%>">
-          <td class="name"><%= raw format_attribute_caption(@conn, key, field) %></td>
-          <td data-toggle="tooltip" title="<%= format_class_attribute_source(@data[:key], field) %>"><%= raw format_attribute_name(key) %></td>
+        <tr class="<%= field_classes(field) %>">
+          <td class="name" data-toggle="tooltip" title="<%= format_class_attribute_source(@data[:key], field) %>"><%= format_attribute_name(key) %></td>
+          <td><%= raw format_attribute_caption(@conn, key, field) %></td>
           <td class="capitalize"><%= field[:group] %></td>
           <td><%= raw format_requirement(constraints, key, field) %></td>
           <td class="extensions"><%= raw format_type(@conn, field) %></td>

lib/schema_web/templates/page/classes.html.eex

@@ -31,8 +31,8 @@ limitations under the License.
   <table class="table table-striped table-bordered sortable">
     <thead>
       <tr class="thead-color">
+        <th style="width: 20%">Name</th>
         <th style="width: 20%">Caption</th>
-        <th style="width: 15%">Name</th>
         <th style="width: 10%">ID</th>
         <th>Description</th>
       </tr>
@@ -42,8 +42,8 @@ limitations under the License.
         <% name = Atom.to_string(key) %>
         <% path = Routes.static_path(@conn, "/classes/" <> name) %>
         <tr class="ocsf-class" <%= raw format_profiles(map[:profiles])%>>
-          <td class="name"><%= raw format_caption(name, map) %></td>
-          <td class="extensions"><a href="<%= path %>"><%= name %></a></td>
+          <td class="name"><a href="<%= path %>"><%= name %></a></td>
+          <td><%= raw format_caption(name, map) %></td>
           <% uid = map[:uid] %>
           <%= if uid != nil do %>
             <td><%= uid %></td>