Empty string support (#23)

darrenklein · web-flow · commit bbfc392c3787 · 2020-07-05T20:49:07.000-04:00
* Allow empty string attributes to be written to the DB.

* Add credo, address credo suggestions, add additional documentation.

* Add additional configuration documentation to README.
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -1,4 +1,14 @@
 Contributing
 ============
 
-Contributions to ExAws are always welcome! For contributions to this particular service, feel free to just open a PR or an issue. For larger scale contributions see: https://github.com/ex-aws/ex_aws/blob/master/CONTRIBUTING.md
+Contributions to ExAws.Dynamo are always welcome! For contributions to this particular service, feel free to just open a PR or an issue. For larger scale contributions see: https://github.com/ex-aws/ex_aws/blob/master/CONTRIBUTING.md
+
+Before submitting any PR, please make sure that the code is adequately tested, formatted, and checked for other issues... in other words, please run
+
+```bash
+mix format
+mix credo
+mix test
+```
+
+and respond to any issues that are raised accordingly.
diff --git a/README.md b/README.md
@@ -27,7 +27,33 @@ Documentation for **ExAwsDynamo** can be found at [https://hexdocs.pm/ex_aws_dyn
 
 ### DynamoDB Local
 
-If you are running this module against a local development instance of DynamoDB, you'll want to make sure that you have installed the latest version, `1.11.478` (released 2020-01-16). You can find links to download the latest version [here](https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/DynamoDBLocal.DownloadingAndRunning.html).
+If you are running this module against a local development instance of DynamoDB, you'll want to make sure that you have installed the latest version, `1.13.1` (released 2020-05-29). You can find links to download the latest version [here](https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/DynamoDBLocal.DownloadingAndRunning.html).
+
+## Configuration
+
+### **ExAws**
+
+In order to use **ExAws.Dynamo** against a production instance of DynamoDB, your app will need to provide the following configuration for `:ex_aws`:
+
+Example configuration:
+
+```elixir
+config :ex_aws,
+  access_key_id: [{:system, "AWS_ACCESS_KEY_ID"}, :instance_role],
+  secret_access_key: [{:system, "AWS_SECRET_ACCESS_KEY"}, :instance_role]
+```
+
+**:access_key_id** :: string
+
+The ID portion of a set of AWS credentials.
+
+**:secret_access_key** :: string
+
+The secret portion of a set of AWS credentials.
+
+Please see the [ExAws README](https://github.com/ex-aws/ex_aws) for additional reference.
+
+## Testing
 
 ### Integration tests (optional)
 
diff --git a/config/prod.exs b/config/prod.exs
@@ -0,0 +1 @@
+
diff --git a/lib/ex_aws/dynamo.ex b/lib/ex_aws/dynamo.ex
@@ -3,7 +3,7 @@ defmodule ExAws.Dynamo do
   Operations on the AWS Dynamo service.
 
   NOTE: When Mix.env in [:test, :dev] dynamo clients will run by default against
-  Dynamodb local.
+  DynamoDB local.
 
   ## Basic usage
   ```elixir
@@ -769,22 +769,19 @@ defmodule ExAws.Dynamo do
           {:condition_expression, binary}
           | {:expression_attribute_names, expression_attribute_names_vals}
           | {:expression_attribute_values, expression_attribute_values_vals}
-          | {:return_values_on_condition_check_failure,
-             return_values_on_condition_check_failure_vals}
+          | {:return_values_on_condition_check_failure, return_values_on_condition_check_failure_vals}
         ]
 
   @type transact_update_item_opts :: [
           {:condition_expression, binary}
           | {:expression_attribute_names, expression_attribute_names_vals}
           | {:expression_attribute_values, expression_attribute_values_vals}
-          | {:return_values_on_condition_check_failure,
-             return_values_on_condition_check_failure_vals}
+          | {:return_values_on_condition_check_failure, return_values_on_condition_check_failure_vals}
           | {:update_expression, binary}
         ]
 
   @type transact_write_item ::
-          {:condition_check,
-           {table_name :: binary, key :: primary_key, transact_standard_item_opts}}
+          {:condition_check, {table_name :: binary, key :: primary_key, transact_standard_item_opts}}
           | {:delete, {table_name :: binary, key :: primary_key, transact_standard_item_opts}}
           | {:put, {table_name :: binary, item :: map(), transact_standard_item_opts}}
           | {:update, {table_name :: binary, key :: primary_key, transact_update_item_opts}}
diff --git a/lib/ex_aws/dynamo/decoder.ex b/lib/ex_aws/dynamo/decoder.ex
@@ -1,5 +1,5 @@
 defmodule ExAws.Dynamo.Decoder do
-  @doc """
+  @moduledoc """
   Decodes a dynamo response into a struct.
 
   If Dynamo.Decodable is implemented for the struct it will be called
@@ -48,7 +48,7 @@ defmodule ExAws.Dynamo.Decoder do
   def decode(%{"N" => value}) when is_binary(value), do: binary_to_number(value)
   def decode(%{"N" => value}) when value |> is_integer or value |> is_float, do: value
 
-  def decode(item = %{}) do
+  def decode(%{} = item) do
     item
     |> Enum.reduce(%{}, fn {k, v}, map ->
       Map.put(map, k, decode(v))
diff --git a/lib/ex_aws/dynamo/encodable.ex b/lib/ex_aws/dynamo/encodable.ex
@@ -79,9 +79,6 @@ defimpl ExAws.Dynamo.Encodable, for: Map do
 
   def do_encode(map) do
     Enum.reduce(map, %{}, fn
-      {_, ""}, map ->
-        map
-
       {k, v}, map when is_binary(k) ->
         Map.put(map, k, ExAws.Dynamo.Encodable.encode(v, []))
 
diff --git a/lib/ex_aws/dynamo/lazy.ex b/lib/ex_aws/dynamo/lazy.ex
@@ -1,5 +1,6 @@
 defmodule ExAws.Dynamo.Lazy do
   @moduledoc false
+
   ## Implimentation of the lazy functions surfaced by ExAws.Dynamo.Client
 
   @doc "Generates a scan stream"
diff --git a/mix.exs b/mix.exs
@@ -43,10 +43,11 @@ defmodule ExAws.Dynamo.Mixfile do
   # Run "mix help deps" to learn about dependencies.
   defp deps do
     [
+      {:credo, "~> 1.4", only: [:dev, :test], runtime: false},
       {:ex_doc, ">= 0.0.0", only: :dev},
       {:hackney, ">= 0.0.0", only: [:dev, :test]},
-      {:sweet_xml, ">= 0.0.0", only: [:dev, :test]},
       {:jason, ">= 0.0.0", only: [:dev, :test]},
+      {:sweet_xml, ">= 0.0.0", only: [:dev, :test]},
       ex_aws()
     ]
   end
diff --git a/mix.lock b/mix.lock
@@ -1,5 +1,7 @@
 %{
+  "bunt": {:hex, :bunt, "0.2.0", "951c6e801e8b1d2cbe58ebbd3e616a869061ddadcc4863d0a2182541acae9a38", [:mix], [], "hexpm", "7af5c7e09fe1d40f76c8e4f9dd2be7cebd83909f31fee7cd0e9eadc567da8353"},
   "certifi": {:hex, :certifi, "2.5.2", "b7cfeae9d2ed395695dd8201c57a2d019c0c43ecaf8b8bcb9320b40d6662f340", [:rebar3], [{:parse_trans, "~>3.3", [hex: :parse_trans, repo: "hexpm", optional: false]}], "hexpm", "3b3b5f36493004ac3455966991eaf6e768ce9884693d9968055aeeeb1e575040"},
+  "credo": {:hex, :credo, "1.4.0", "92339d4cbadd1e88b5ee43d427b639b68a11071b6f73854e33638e30a0ea11f5", [:mix], [{:bunt, "~> 0.2.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "1fd3b70dce216574ce3c18bdf510b57e7c4c85c2ec9cad4bff854abaf7e58658"},
   "earmark": {:hex, :earmark, "1.3.2", "b840562ea3d67795ffbb5bd88940b1bed0ed9fa32834915125ea7d02e35888a5", [:mix], [], "hexpm", "e3be2bc3ae67781db529b80aa7e7c49904a988596e2dbff897425b48b3581161"},
   "ex_aws": {:hex, :ex_aws, "2.1.3", "26b6f036f0127548706aade4a509978fc7c26bd5334b004fba9bfe2687a525df", [:mix], [{:configparser_ex, "~> 4.0", [hex: :configparser_ex, repo: "hexpm", optional: true]}, {:hackney, "~> 1.9", [hex: :hackney, repo: "hexpm", optional: true]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: true]}, {:jsx, "~> 2.8", [hex: :jsx, repo: "hexpm", optional: true]}, {:sweet_xml, "~> 0.6", [hex: :sweet_xml, repo: "hexpm", optional: true]}], "hexpm", "0bdbe2aed9f326922fc5a6a80417e32f0c895f4b3b2b0b9676ebf23dd16c5da4"},
   "ex_doc": {:hex, :ex_doc, "0.20.2", "1bd0dfb0304bade58beb77f20f21ee3558cc3c753743ae0ddbb0fd7ba2912331", [:mix], [{:earmark, "~> 1.3", [hex: :earmark, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.10", [hex: :makeup_elixir, repo: "hexpm", optional: false]}], "hexpm", "8e24fc8ff9a50b9f557ff020d6c91a03cded7e59ac3e0eec8a27e771430c7d27"},
diff --git a/test/lib/dynamo/encoder_test.exs b/test/lib/dynamo/encoder_test.exs
@@ -21,9 +21,12 @@ defmodule ExAws.Dynamo.EncoderTest do
     assert Encoder.encode(0.4) == %{"N" => "0.4"}
   end
 
-  test "Encoder removes empty strings from a map" do
+  test "Encoder allows empty strings in a map" do
     assert Encoder.encode(%{"data" => "value", "nodata" => ""}) == %{
-             "M" => %{"data" => %{"S" => "value"}}
+             "M" => %{
+               "data" => %{"S" => "value"},
+               "nodata" => %{"S" => ""}
+             }
            }
   end
 
diff --git a/test/lib/dynamo/integration_test.exs b/test/lib/dynamo/integration_test.exs
@@ -58,7 +58,7 @@ defmodule ExAws.DynamoIntegrationTest do
         user = %Test.User{
           email: "foo@bar.com",
           name: %{first: "bob", last: "bubba"},
-          age: 23,
+          age: "",
           admin: false
         }
 
diff --git a/test/support/ddb_local.ex b/test/support/ddb_local.ex
@@ -18,13 +18,13 @@ defmodule DDBLocal do
   def try_connect() do
     port = get_port()
 
-    if !is_nil(port) do
+    if is_nil(port) do
+      {:error, "No value provided for :port in config/ddb_local_test.exs."}
+    else
       case :gen_tcp.connect('localhost', port, []) do
         {:ok, _} -> :ok
         {:error, error} -> {:error, error}
       end
-    else
-      {:error, "No value provided for :port in config/ddb_local_test.exs."}
     end
   end
 
diff --git a/test/support/mock_models.ex b/test/support/mock_models.ex
@@ -1,4 +1,5 @@
 defmodule Test.User do
+  @moduledoc false
   @derive ExAws.Dynamo.Encodable
   defstruct [:email, :name, :age, :admin]
 
@@ -12,6 +13,7 @@ defmodule Test.User do
 end
 
 defmodule Test.Nested do
+  @moduledoc false
   @derive {ExAws.Dynamo.Encodable, only: [:items]}
   defstruct items: [], secret: nil
 end
diff --git a/upgrade_guides/v4.md b/upgrade_guides/v4.md
@@ -0,0 +1,7 @@
+# Upgrading from version 3.X.X to version 4.X.X
+
+## Empty string/binary values allowed
+
+In May, 2020, DynamoDB was updated to allow storage of empty strings and binaries in non-key fields - [documentation](https://aws.amazon.com/about-aws/whats-new/2020/05/amazon-dynamodb-now-supports-empty-values-for-non-key-string-and-binary-attributes-in-dynamodb-tables/). In earlier versions of this app, fields with empty string attributes were stripped off prior to writing to the DB by `ExAws.Dynamo.Encodable.do_encode/1`; however, in light of these developments, the default behavior of this app will now be to write empty string values.
+
+Please check your application thoroughly and make sure you are prepared for dealing with the possibility of having empty strings in fields that may previously have been expected to return `nil` (since, in the past, they wouldn't have been written at all). To be clear, existing data is not going to be changed - but if, prior to upgrading to version 4, you wrote a record with the values `%{id: "1", optional_string: ""}`, that record will still be returned as `%{id: 1, optional_string: nil}`; if you write a record after upgrading, say `%{id: "2", optional_string: ""}`, that record will be returned exactly as written, `%{id: "2", optional_string: ""}`.

Original file line number	Diff line number	Diff line change
`@@ -43,10 +43,11 @@ defmodule ExAws.Dynamo.Mixfile do`
`43`	`43`	`# Run "mix help deps" to learn about dependencies.`
`44`	`44`	`defp deps do`
`45`	`45`	`[`
	`46`	`+ {:credo, "~> 1.4", only: [:dev, :test], runtime: false},`
`46`	`47`	`{:ex_doc, ">= 0.0.0", only: :dev},`
`47`	`48`	`{:hackney, ">= 0.0.0", only: [:dev, :test]},`
`48`		`- {:sweet_xml, ">= 0.0.0", only: [:dev, :test]},`
`49`	`49`	`{:jason, ">= 0.0.0", only: [:dev, :test]},`
	`50`	`+ {:sweet_xml, ">= 0.0.0", only: [:dev, :test]},`
`50`	`51`	`ex_aws()`
`51`	`52`	`]`
`52`	`53`	`end`