adding mongoose_subdomain_core #3116
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3116 +/- ##
==========================================
+ Coverage 79.23% 79.24% +0.01%
==========================================
Files 388 389 +1
Lines 31850 31957 +107
==========================================
+ Hits 25235 25325 +90
- Misses 6615 6632 +17
Continue to review full report at Codecov.
|
| -type subdomain_item() :: #subdomain_item{}. | ||
|
|
||
| %% corresponds to the fields in #subdomain_item{} record | ||
| -type subdomain_info() :: #{host_type := host_type(), |
There was a problem hiding this comment.
The two types: record and map look a bit odd to me. I understand that the record is used for ETS operations and the map is easier to use in the tests but... I would only have the record here.
Applying the Occam's razor makes me think that having only one type here would have the following benefits:
- Easier to learn how it works
- Easier to debug - one data structure is easier to follow
- Less code, no unnecessary conversions
- Easier to maintain in case the type needs to be extended
There was a problem hiding this comment.
I didn't want to expose this record to the external code (not in this PR), it's not only about tests. I would say it was done not for the tests at all.
There was a problem hiding this comment.
Not exposing the record beyond a a module would make more sense to me if there were a lot of data in the record taht we could hide. Otherwise it's just a lightweight and efficient data structure that you can just use directly instead of converting to a new structure that does not offer anything extra.
I don't want to start a long discussion here... it can stay as it is, in my view it's just unnecessary.
There was a problem hiding this comment.
Let's keep it as it is for now.
| %%------------------------------------------ | ||
| ?assertEqual(ok, mongoose_subdomain_core:register_subdomain(?DYNAMIC_HOST_TYPE1, | ||
| Pattern1, Handler)), | ||
| mongoose_domain_core:insert(<<"example.net">>, ?DYNAMIC_HOST_TYPE1, dummy_src), |
There was a problem hiding this comment.
How about checking return values in these tests? Would it be too verbose?
… more deterministic and predictable
| -type subdomain_item() :: #subdomain_item{}. | ||
|
|
||
| %% corresponds to the fields in #subdomain_item{} record | ||
| -type subdomain_info() :: #{host_type := host_type(), |
There was a problem hiding this comment.
Let's keep it as it is for now.
tested with the following command:
./rebar3 ct --suite mongoose_subdomain_core_SUITE --repeat 100
Initial implementation of subdomains management subsystem.
Note that it detects but doesn't solve subdomains or domain/subdomain names collisions:
for more information see comment in the corresponding test cases: