Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't Read IO Objects in Attribute Value Marshal #832

Merged
merged 2 commits into from
Jun 3, 2015
Merged

Conversation

awood45
Copy link
Member

@awood45 awood45 commented Jun 2, 2015

Resolves Issue #831

Running the following code exercised the problem, and now works with
this change:

dynamodb.put_item(
  table_name: "foo",
  item: {
    id: 1,
    contents: StringIO.new("bar")
  }
)

The issue appeared to be that #read was called twice on the StringIO or
IO object, and the second call would fail as String#read does not
exist.

One thing I'd like to review further before merging is the fact that maps and collections do this same #read call but do not have this problem when tested.

Resolves Issue #831

Running the following code exercised the problem, and now works with
this change:

```
dynamodb.put_item(
  table_name: "foo",
  item: {
    id: 1,
    contents: StringIO.new("bar")
  }
)
```

The issue appeared to be that #read was called twice on the StringIO or
IO object, and the second call would fail as String#read does not
exist.
@trevorrowe
Copy link
Member

The change looks good. Maps and lists definitely are effected, but I suspect what you meant to say is that they are not tested?

dynamodb.put_item(
  table_name: "foo",
  item: {
    id: 1,
    contents: { # map
      data: StringIO.new("bar")
    }
  }
)
#=> raises NoMethodError: undefined method `read' for "bar":String
dynamodb.put_item(
  table_name: "foo",
  item: {
    id: 1,
    contents: [ # list
      StringIO.new("bar"),
    ]
  }
)
#=> raises NoMethodError: undefined method `read' for "bar":String

The hash and array logic used the format method changed in the previous
diff, so those worked correctly from the outset. Sets, however, had
special logic which required a second change.
@awood45
Copy link
Member Author

awood45 commented Jun 3, 2015

Per our earlier discussion, Hashes and Arrays worked fine from the first change, the second use of #read was isolated to Set types only. This change fixes the appropriate test and code.

trevorrowe added a commit that referenced this pull request Jun 3, 2015
Don't Read IO Objects in Attribute Value Marshal
@trevorrowe trevorrowe merged commit 00fbe63 into master Jun 3, 2015
@trevorrowe trevorrowe deleted the issue831 branch June 11, 2015 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants