Merge branch '0.2.x' into develop

Changelog.md

@@ -1,3 +1,13 @@
+# 0.2.7
+
+## Features
+
+* Add `remote_photo_path` to `AccountMigration` entity [#119](https://github.com/diaspora/diaspora_federation/pull/119)
+
+## Bug fixes
+
+* Only parse each nested element name once from the XML [#118](https://github.com/diaspora/diaspora_federation/pull/118)
+
 # 0.2.6
 
 ## Bug fixes

Gemfile.lock

@@ -1,18 +1,18 @@
 PATH
   remote: .
   specs:
-    diaspora_federation (0.2.6)
+    diaspora_federation (0.2.7)
       faraday (~> 1.0)
       faraday_middleware (~> 1.0)
       nokogiri (~> 1.6, >= 1.6.8)
       typhoeus (~> 1.0)
       valid (~> 1.0)
-    diaspora_federation-json_schema (0.2.6)
-    diaspora_federation-rails (0.2.6)
+    diaspora_federation-json_schema (0.2.7)
+    diaspora_federation-rails (0.2.7)
       actionpack (>= 5.2, < 7)
-      diaspora_federation (= 0.2.6)
-    diaspora_federation-test (0.2.6)
-      diaspora_federation (= 0.2.6)
+      diaspora_federation (= 0.2.7)
+    diaspora_federation-test (0.2.7)
+      diaspora_federation (= 0.2.7)
       fabrication (~> 2.16)
       uuid (~> 2.3, >= 2.3.8)
 

docs/_entities/account_migration.md

@@ -9,14 +9,15 @@ This entity is sent when a person changes their diaspora* ID (e.g. when a user m
 | Property    | Type                         | Description                                                                          |
 | ----------- | ---------------------------- | ------------------------------------------------------------------------------------ |
 | `author`    | [diaspora\* ID][diaspora-id] | The diaspora\* ID of the sender of the entity. The entity may be sent by either old user identity or new user identity. |
-| `person`    | [Profile][profile]           | New profile of a person. |
+| `profile`   | [Profile][profile]           | New profile of a person.                                                             |
 | `signature` | [Signature][signature]       | Signature that validates original and target diaspora* IDs with the private key of the second identity, other than the entity author. So if the author is the old identity then this signature is made with the new identity key, and vice versa. |
 
 ## Optional Properties
 
-| Property    | Type                         | Description                                                                          |
-| ----------- | ---------------------------- | ------------------------------------------------------------------------------------ |
-| `old_identity`    | [diaspora\* ID][diaspora-id] | The diaspora\* ID of the closed account. This field is mandatory if the author of the entity is the new identity. |
+| Property            | Type                         | Description                                                                          |
+| ------------------- | ---------------------------- | ------------------------------------------------------------------------------------ |
+| `old_identity`      | [diaspora\* ID][diaspora-id] | The diaspora\* ID of the closed account. This field is mandatory if the author of the entity is the new identity. |
+| `remote_photo_path` | [URL][url]                   | The URL to the path (without filenames) of the migrated photos on the new pod.       |
 
 
 ### Signature
@@ -59,9 +60,11 @@ AccountMigration:old-diaspora-id@example.org:new-diaspora-id@example.com
     07b1OIY6sTUQwV5pbpgFK0uz6W4cu+oQnlg410Q4uISUOdNOlBdYqhZJm62VFhgvzt4TZXfiJgoupFkRjP0BsaVaZuP2zKMNvO3ngWOeJRf2oRK4Ub5cEA/g7yijkRc+7y8r1iLJ31MFb1czyeCsLxw9Ol8SvAJddogGiLHDhjE=
   </signature>
   <old_identity>alice@example.org</old_identity>
+  <remote_photo_path>https://newpod.example.net/uploads/images/</remote_photo_path>
 </account_migration>
 ~~~
 
 [diaspora-id]: {{ site.baseurl }}/federation/types.html#diaspora-id
 [profile]: {{ site.baseurl }}/entities/profile.html
 [signature]: {{ site.baseurl }}/federation/types.html#signature
+[url]: {{ site.baseurl }}/federation/types.html#url

lib/diaspora_federation/entities/account_migration.rb

@@ -35,6 +35,11 @@ module DiasporaFederation
       #   @return [String] old identity
       property :old_identity, :string, default: nil
 
+      # @!attribute [r] remote_photo_path
+      #   The url to the path of the photos on the new pod. Can be empty if photos weren't migrated.
+      #   @return [String] remote photo path
+      property :remote_photo_path, :string, optional: true
+
       # Returns diaspora* ID of the old person identity.
       # @return [String] diaspora* ID of the old person identity
       def old_identity

lib/diaspora_federation/parsers/xml_parser.rb

@@ -14,7 +14,7 @@ module DiasporaFederation
       def parse(root_node)
         from_xml_sanity_validation(root_node)
 
-        hash = root_node.element_children.map {|child|
+        hash = root_node.element_children.uniq(&:name).map {|child|
           property, type = find_property_for(child.name)
           if property
             value = parse_element_from_node(child.name, type, root_node)

lib/diaspora_federation/test/factories.rb

@@ -45,6 +45,7 @@ module DiasporaFederation
         author { Fabricate.sequence(:diaspora_id) }
         profile {|attrs| Fabricate(:profile_entity, author: attrs[:author]) }
         old_identity { Fabricate.sequence(:diaspora_id) }
+        remote_photo_path "https://diaspora.example.tld/uploads/images/"
       end
 
       Fabricator(:person_entity, class_name: DiasporaFederation::Entities::Person) do

lib/diaspora_federation/validators/account_migration_validator.rb

@@ -11,6 +11,8 @@ module DiasporaFederation
       rule :profile, :not_nil
 
       rule :old_identity, :diaspora_id
+
+      rule :remote_photo_path, URI: [:path]
     end
   end
 end

lib/diaspora_federation/validators/photo_validator.rb

@@ -12,7 +12,7 @@ module DiasporaFederation
 
       rule :public, :boolean
 
-      rule :remote_photo_path, :not_empty
+      rule :remote_photo_path, [:not_empty, URI: [:path]]
 
       rule :remote_photo_name, :not_empty
 

lib/diaspora_federation/version.rb

@@ -2,5 +2,5 @@
 
 module DiasporaFederation
   # the gem version
-  VERSION = "0.2.6"
+  VERSION = "0.2.7"
 end

spec/lib/diaspora_federation/entities/account_migration_spec.rb

@@ -8,10 +8,11 @@ module DiasporaFederation
     let(:new_diaspora_id) { new_user.diaspora_id }
 
     let(:data) {
-      hash.tap {|hash|
+      hash.dup.tap {|data|
         properties = described_class.new(hash).send(:enriched_properties)
-        hash[:signature] = properties[:signature]
-        hash[:profile] = Entities::Profile.new(hash[:profile].to_h.tap {|profile| profile[:edited_at] = nil })
+        data[:signature] = properties[:signature]
+        data[:profile] = Entities::Profile.new(hash[:profile].to_h.tap {|profile| profile[:edited_at] = nil })
+        data[:remote_photo_path] = "http://localhost:3000/uploads/images/"
       }
     }
     let(:signature_data) { "AccountMigration:#{old_diaspora_id}:#{new_diaspora_id}" }
@@ -128,6 +129,7 @@ module DiasporaFederation
           </profile>
           <signature>#{data[:signature]}</signature>
           <old_identity>#{data[:old_identity]}</old_identity>
+          <remote_photo_path>#{data[:remote_photo_path]}</remote_photo_path>
         </account_migration>
       XML
 
@@ -167,6 +169,7 @@ module DiasporaFederation
           </profile>
           <signature>#{data[:signature]}</signature>
           <old_identity>#{data[:old_identity]}</old_identity>
+          <remote_photo_path>#{data[:remote_photo_path]}</remote_photo_path>
         </account_migration>
       XML
 
@@ -220,5 +223,50 @@ module DiasporaFederation
         }.to raise_error Entity::ValidationError
       end
     end
+
+    context "optional values" do
+      let(:hash) {
+        {
+          author:  old_diaspora_id,
+          profile: Entities::Profile.new(author: new_diaspora_id)
+        }
+      }
+
+      it "uses default values when parsing" do
+        minimal_xml = <<~XML
+          <account_migration>
+            <author>#{data[:author]}</author>
+            <profile>
+              <author>#{data[:profile].author}</author>
+            </profile>
+            <signature>#{data[:signature]}</signature>
+          </account_migration>
+        XML
+
+        parsed_xml = Nokogiri::XML(minimal_xml).root
+        parsed_instance = Entity.entity_class(parsed_xml.name).from_xml(parsed_xml)
+        expect(parsed_instance.old_identity).to eq(data[:author])
+        expect(parsed_instance.remote_photo_path).to be_nil
+      end
+
+      it "adds old_identity when author is the old identity" do
+        expected_xml = <<~XML
+          <account_migration>
+            <author>#{data[:author]}</author>
+            <profile>
+              <author>#{data[:profile].author}</author>
+              <searchable>true</searchable>
+              <public>false</public>
+              <nsfw>false</nsfw>
+            </profile>
+            <signature>#{data[:signature]}</signature>
+            <old_identity>#{data[:author]}</old_identity>
+          </account_migration>
+        XML
+
+        entity = Entities::AccountMigration.new(hash)
+        expect(entity.to_xml.to_s.strip).to eq(expected_xml.strip)
+      end
+    end
   end
 end

spec/lib/diaspora_federation/parsers/xml_parser_spec.rb

@@ -123,6 +123,14 @@ module DiasporaFederation
           expect(parsed[0][:multi].first.to_h).to eq(child_entity2.to_h)
           expect(parsed[0][:asdf]).to eq("QWERT")
         end
+
+        it "parses array entities only once" do
+          expect(Entities::OtherEntity).to receive(:from_xml).twice.and_call_original
+
+          parsed = Parsers::XmlParser.new(Entities::TestNestedEntity).parse(nested_payload)
+
+          expect(parsed[0][:multi]).to have(2).items
+        end
       end
 
       it "doesn't drop extra properties" do

spec/lib/diaspora_federation/validators/account_migration_validator_spec.rb

@@ -23,5 +23,16 @@ module DiasporaFederation
         let(:property) { :old_identity }
       end
     end
+
+    describe "#remote_photo_path" do
+      let(:property) { :remote_photo_path }
+
+      it_behaves_like "a property with a value validation/restriction" do
+        let(:wrong_values) { [] }
+        let(:correct_values) { [nil] }
+      end
+
+      it_behaves_like "a url path validator"
+    end
   end
 end

spec/lib/diaspora_federation/validators/photo_validator_spec.rb

@@ -26,11 +26,17 @@ module DiasporaFederation
       let(:property) { :public }
     end
 
-    %i[remote_photo_name remote_photo_path].each do |prop|
-      describe "##{prop}" do
-        it_behaves_like "a property that mustn't be empty" do
-          let(:property) { prop }
-        end
+    describe "#remote_photo_path" do
+      let(:property) { :remote_photo_path }
+
+      it_behaves_like "a property that mustn't be empty"
+
+      it_behaves_like "a url path validator"
+    end
+
+    describe "#remote_photo_name" do
+      it_behaves_like "a property that mustn't be empty" do
+        let(:property) { :remote_photo_name }
       end
     end
 

test/gemfiles/no-rails.Gemfile.lock

@@ -1,15 +1,15 @@
 PATH
   remote: ../..
   specs:
-    diaspora_federation (0.2.6)
+    diaspora_federation (0.2.7)
       faraday (~> 1.0)
       faraday_middleware (~> 1.0)
       nokogiri (~> 1.6, >= 1.6.8)
       typhoeus (~> 1.0)
       valid (~> 1.0)
-    diaspora_federation-json_schema (0.2.6)
-    diaspora_federation-test (0.2.6)
-      diaspora_federation (= 0.2.6)
+    diaspora_federation-json_schema (0.2.7)
+    diaspora_federation-test (0.2.7)
+      diaspora_federation (= 0.2.7)
       fabrication (~> 2.16)
       uuid (~> 2.3, >= 2.3.8)