Add dedicated Astarte FDO library#1819
Add dedicated Astarte FDO library#1819mizzet1 wants to merge 3 commits intoastarte-platform:masterfrom
Conversation
8c6037f to
baa50c7
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1819 +/- ##
==========================================
- Coverage 85.96% 85.80% -0.17%
==========================================
Files 559 561 +2
Lines 9757 9747 -10
==========================================
- Hits 8388 8363 -25
- Misses 1369 1384 +15
... and 15 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
818b75e to
8845121
Compare
92aaab0 to
da1285f
Compare
|
Points addressed from #1801 :
Missing: |
a4725f9 to
f63fdb8
Compare
| sevk blob, | ||
| svk blob, | ||
| sek blob, | ||
| sevk session_key, |
There was a problem hiding this comment.
now that the feature branch has been merged, we actually do need to create new migrations and treat existing ones as read only
libs/astarte_fdo/lib/astarte_fdo.ex
Outdated
| @@ -0,0 +1,18 @@ | |||
| defmodule AstarteFdo do | |||
libs/astarte_fdo/lib/cbor.ex
Outdated
| # limitations under the License. | ||
| # | ||
|
|
||
| defmodule Astarte.FDO.CBOR do |
There was a problem hiding this comment.
this is astarte data access material, it has nothing to do with astarte_fdo
| @moduledoc """ | ||
| This module defines the structure of an error message in | ||
| the FDO protocol, including the error code, previous message ID, | ||
| error message, timestamp, and correlation ID. | ||
| It also provides functions for encoding the error message | ||
| into a CBOR for transmission in the FDO protocol. | ||
| """ |
There was a problem hiding this comment.
why have we removed moduledocs?
| {:ok, public_key} <- decode(cbor_list) do | ||
| {:ok, public_key} |
There was a problem hiding this comment.
this should trigger a failure in credo, why isn't it failing?
apps/astarte_pairing/mix.exs
Outdated
| {:httpoison, "~> 2.2", override: true} | ||
| {:astarte_fdo, path: astarte_lib("astarte_fdo")}, | ||
| {:astarte_generators, path: astarte_lib("astarte_generators"), only: [:dev, :test]}, | ||
| {:stream_data, "~> 1.1", only: [:dev, :test]} |
| alias COSE.Messages.Sign1 | ||
| alias COSE.Messages.Sign1 |
| end | ||
| end | ||
|
|
||
| def fetch(realm_name, guid) do |
There was a problem hiding this comment.
in general, we use core modules to place implementations of functions which are complex or long enough and internal logic makes sense on its own
for instance, we may have a function in the "outer" module with validation and the function in the core module which expects that its preconditions are met, and the outer calls core after having made its validations.
in general, you never want other modules to use Core directly (except maybe tests), but all functionality should be available from the outer OwnershipVoucher module
personally, I would leave these functions there
|
|
||
| def from_db(%{"alg" => alg, "k" => k, "kty" => kty}) do | ||
| %Symmetric{ | ||
| alg: String.to_existing_atom(alg), |
| } | ||
| ) | ||
| when is_empty(service_info) do | ||
| when service_info.module == nil and service_info.key == nil and service_info.value == nil do |
9da63f2 to
64d8fd9
Compare
Signed-off-by: Riccardo Nalgi <riccardo.nalgi@secomind.com>
Signed-off-by: Riccardo Nalgi <riccardo.nalgi@secomind.com>
- remove unused astarte_fdo and gitignore files from fdo library - create new migrations and treat existing ones as read only - move all Ecto related FDO modules to astarte_data_access - add missing moduledocs - place "fdo" scope under libraries - delete github workflow files from fdo library - add missing copyright header - use guard to check emptyness of service info - remove data_access dependency from fdo library Signed-off-by: Riccardo Nalgi <riccardo.nalgi@secomind.com>

to avoid circular dependencies (and having libraries depend on apps), this pr aim to create a new library astarte_fdo to move dedicated fdo modules there