From a55cc3653de22d868ade5303918280a38e8e9fe8 Mon Sep 17 00:00:00 2001
From: Azul <azul@riseup.net>
Date: Wed, 31 Jan 2018 12:27:55 +0100
Subject: [PATCH] keys: store type and rev in hash rather than serialized

Since the old keys used to be strings i started out by
json serializing the new keys with type, value, rev.

However storing serialized json in couch (json) does
not really make sense. So now we do not serialize but
instead have one json document. The lookup for a key of
type pgp may still return a string but for everything
that uses the new api it will return a hash with type
and revision.

This data structure is way easier to handle also on the
nickserver side.
---
 app/controllers/api/keys_controller.rb  | 7 ++-----
 app/controllers/api/users_controller.rb | 2 +-
 app/models/identity.rb                  | 7 ++++---
 app/models/keyring.rb                   | 9 ++++++---
 features/step_definitions/key_steps.rb  | 2 +-
 test/unit/keyring_test.rb               | 4 ++--
 6 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/app/controllers/api/keys_controller.rb b/app/controllers/api/keys_controller.rb
index 7eb76ee6..32ccc197 100644
--- a/app/controllers/api/keys_controller.rb
+++ b/app/controllers/api/keys_controller.rb
@@ -5,14 +5,11 @@ class Api::KeysController < ApiController
 
   # get /keys
   def index
-    keys = identity.keys.map do |k,v|
-      [k, JSON.parse(v)]
-    end
-    render json: keys.to_h
+    render json: identity.keys
   end
 
   def show
-    render json: JSON.parse(identity.keys[params[:id]])
+    render json: identity.keys[params[:id]]
   end
 
   def create
diff --git a/app/controllers/api/users_controller.rb b/app/controllers/api/users_controller.rb
index 65b80c7d..eda37e8b 100644
--- a/app/controllers/api/users_controller.rb
+++ b/app/controllers/api/users_controller.rb
@@ -112,7 +112,7 @@ module Api
       PgpKey.new(key).tap do |key|
         if key.valid?
           identity = Identity.for(@user)
-          identity.set_key(:pgp, key)
+          identity.set_key(:pgp, key.to_s)
           identity.save
         end
       end
diff --git a/app/models/identity.rb b/app/models/identity.rb
index b8c22456..13ce124c 100644
--- a/app/models/identity.rb
+++ b/app/models/identity.rb
@@ -131,9 +131,10 @@ class Identity < CouchRest::Model::Base
     read_attribute('keys') || HashWithIndifferentAccess.new
   end
 
-  def set_key(type, key)
-    return if keys[type] == key.to_s
-    write_attribute('keys', keys.merge(type => key.to_s))
+  def set_key(type, key_hash)
+    key_hash.stringify_keys! if key_hash.respond_to? :stringify_keys!
+    return if keys[type] == key_hash
+    write_attribute('keys', keys.merge(type => key_hash))
   end
 
   def delete_key(type)
diff --git a/app/models/keyring.rb b/app/models/keyring.rb
index 66f7bfdb..a5320c2b 100644
--- a/app/models/keyring.rb
+++ b/app/models/keyring.rb
@@ -20,13 +20,13 @@ class Keyring
 
   def create(type, value)
     raise Error, "key already exists" if storage.keys[type].present?
-    storage.set_key type, {type: type, value: value, rev: new_rev}.to_json
+    storage.set_key type, {type: type, value: value, rev: new_rev}
     storage.save
   end
 
   def update(type, rev:, value:)
     check_rev type, rev
-    storage.set_key type, {type: type, value: value, rev: new_rev}.to_json
+    storage.set_key type, {type: type, value: value, rev: new_rev}
     storage.save
   end
 
@@ -37,7 +37,7 @@ class Keyring
   end
 
   def key_of_type(type)
-    JSON.parse(storage.keys[type]) if storage.keys[type]
+    storage.keys[type]
   end
 
   protected
@@ -46,6 +46,9 @@ class Keyring
   def check_rev(type, rev)
     old = key_of_type(type)
     raise NotFound, type unless old
+    # We used to store plain strings. It's deprecated now.
+    # If we happen to run into them do not check their revision.
+    return if old.is_a? String
     raise Error, "wrong revision: #{rev}" unless old['rev'] == rev
   end
 
diff --git a/features/step_definitions/key_steps.rb b/features/step_definitions/key_steps.rb
index 3d5e0154..ad3fac65 100644
--- a/features/step_definitions/key_steps.rb
+++ b/features/step_definitions/key_steps.rb
@@ -16,7 +16,7 @@ Then /^I should have published an? "([^"]*)" key(?: with value "([^"]*)")?$/ do
   identity = Identity.for(@user)
   keys = identity.keys
   assert_includes keys.keys, type
-  assert_equal value, JSON.parse(keys[type])['value'] if value
+  assert_equal value, keys[type]['value'] if value
 end
 
 Then /^I should not have published an? "([^"]*)" key$/ do |type|
diff --git a/test/unit/keyring_test.rb b/test/unit/keyring_test.rb
index c7df63e9..48bab295 100644
--- a/test/unit/keyring_test.rb
+++ b/test/unit/keyring_test.rb
@@ -82,8 +82,8 @@ class KeyringTest < ActiveSupport::TestCase
 
   def teststorage
     @teststorage ||= Hash.new.tap do |dummy|
-      def dummy.set_key(type, value)
-        self[type] = value
+      def dummy.set_key(type, hash)
+        self[type] = hash.stringify_keys
       end
 
       def dummy.keys
-- 
GitLab