From 851685c40eb89428fb6582d713dedea5cca38f3e Mon Sep 17 00:00:00 2001
From: Santiago Palladino <spalladino@gmail.com>
Date: Fri, 9 Mar 2018 13:25:45 -0300
Subject: [PATCH] Add default implementation for metadata URI

This allows token implementation to be non-abstract
---
 contracts/mocks/ERC721TokenMock.sol    | 27 +++--------------------
 contracts/token/ERC721/ERC721Token.sol | 30 ++++++++++++++++++++++++++
 test/token/ERC721/ERC721Token.test.js  | 24 ++++++++++++++++++---
 3 files changed, 54 insertions(+), 27 deletions(-)

diff --git a/contracts/mocks/ERC721TokenMock.sol b/contracts/mocks/ERC721TokenMock.sol
index 0f11448ab..091093cf5 100644
--- a/contracts/mocks/ERC721TokenMock.sol
+++ b/contracts/mocks/ERC721TokenMock.sol
@@ -5,7 +5,7 @@ import "../token/ERC721/ERC721Token.sol";
 /**
  * @title ERC721TokenMock
  * This mock just provides a public mint and burn functions for testing purposes,
- * and a mock metadata URI implementation
+ * and a public setter for metadata URI
  */
 contract ERC721TokenMock is ERC721Token {
   function ERC721TokenMock(string name, string symbol)
@@ -21,28 +21,7 @@ contract ERC721TokenMock is ERC721Token {
     doBurn(ownerOf(_tokenId), _tokenId);
   }
 
-  // Mock implementation for testing.
-  // Do not use this code in production!
-  function tokenURI(uint256 _tokenId) public view returns (string) {
-    require(exists(_tokenId));
-    
-    bytes memory uri = new bytes(78 + 7);
-    
-    uint256 i;
-    uint256 value = _tokenId;
-    
-    uri[0] = "m";
-    uri[1] = "o";
-    uri[2] = "c";
-    uri[3] = "k";
-    uri[4] = ":";
-    uri[5] = "/";
-    uri[6] = "/";
-    for (i = 0; i < 78; i++) {
-      uri[6 + 78 - i] = byte(value % 10 + 48);
-      value = value / 10;
-    }
-
-    return string(uri);
+  function setTokenURI(uint256 _tokenId, string _uri) public {
+    doSetTokenURI(_tokenId, _uri);
   }
 }
diff --git a/contracts/token/ERC721/ERC721Token.sol b/contracts/token/ERC721/ERC721Token.sol
index d57ad0645..aec1a3673 100644
--- a/contracts/token/ERC721/ERC721Token.sol
+++ b/contracts/token/ERC721/ERC721Token.sol
@@ -30,6 +30,9 @@ contract ERC721Token is ERC721, ERC721BasicToken {
   // Mapping from token id to position in the allTokens array
   mapping(uint256 => uint256) internal allTokensIndex;
 
+  // Optional mapping for token URIs 
+  mapping(uint256 => string) internal tokenURIs;
+
   /**
   * @dev Constructor function
   */
@@ -63,6 +66,27 @@ contract ERC721Token is ERC721, ERC721BasicToken {
     return symbol_;
   }
 
+  /**
+  * @dev Returns an URI for a given token ID
+  * @dev Throws if the token ID does not exist. May return an empty string.
+  * @param _tokenId uint256 ID of the token to query
+  */
+  function tokenURI(uint256 _tokenId) public view returns (string) {
+    require(exists(_tokenId));
+    return tokenURIs[_tokenId];
+  }
+
+  /**
+  * @dev Internal function to set the token URI for a given token
+  * @dev Reverts if the token ID does not exist
+  * @param _tokenId uint256 ID of the token to set its URI
+  * @param _uri string URI to assign
+  */
+  function doSetTokenURI(uint256 _tokenId, string _uri) internal {
+    require(exists(_tokenId));
+    tokenURIs[_tokenId] = _uri;
+  }
+
   /**
   * @dev Gets the token ID at a given index of the tokens list of the requested owner
   * @param _owner address owning the tokens list to be accessed
@@ -152,6 +176,12 @@ contract ERC721Token is ERC721, ERC721BasicToken {
   function doBurn(address _owner, uint256 _tokenId) internal {
     super.doBurn(_owner, _tokenId);
 
+    // Clear metadata (if any)
+    if (bytes(tokenURIs[_tokenId]).length == 0) {
+      delete tokenURIs[_tokenId];
+    }
+
+    // Reorg all tokens array
     uint256 tokenIndex = allTokensIndex[_tokenId];
     uint256 lastTokenIndex = allTokens.length.sub(1);
     uint256 lastToken = allTokens[lastTokenIndex];
diff --git a/test/token/ERC721/ERC721Token.test.js b/test/token/ERC721/ERC721Token.test.js
index 77ec11fbf..a1aabc241 100644
--- a/test/token/ERC721/ERC721Token.test.js
+++ b/test/token/ERC721/ERC721Token.test.js
@@ -77,6 +77,8 @@ contract('ERC721Token', function (accounts) {
     });
     
     describe('metadata', function () {
+      const sampleUri = 'mock://mytoken';
+
       it('has a name', async function () {
         const name = await this.token.name();
         name.should.be.equal(name);
@@ -87,10 +89,26 @@ contract('ERC721Token', function (accounts) {
         symbol.should.be.equal(symbol);
       });
 
-      it('returns metadata for a token id', async function () {
+      it('sets and returns metadata for a token id', async function () {
+        await this.token.setTokenURI(firstTokenId, sampleUri);
         const uri = await this.token.tokenURI(firstTokenId);
-        const expected = `mock://${firstTokenId.toString().padStart(78, 0)}`;
-        uri.should.be.equal(expected);
+        uri.should.be.equal(sampleUri);
+      });
+
+      it('can burn token with metadata', async function () {
+        await this.token.setTokenURI(firstTokenId, sampleUri);
+        await this.token.burn(firstTokenId);
+        const exists = await this.token.exists(firstTokenId);
+        exists.should.be.false;
+      });
+
+      it('returns empty metadata for token', async function () {
+        const uri = await this.token.tokenURI(firstTokenId);
+        uri.should.be.equal('');
+      });
+
+      it('reverts when querying metadata for non existant token id', async function () {
+        await assertRevert(this.token.tokenURI(500));
       });
     });