nicmx/rdap-data-access-api

Interfaces a Red Dog's data access implementation should inherit from.

Red Dog's Data Access API

Set of interfaces from which Data Access Implementations are expected to implement. An implementation of the data access API will wrap whatever data storage is configured to the RDAP server.

Documentation

The general documentation can be seen here, within the docs there's more specific documentation that can be seen at Installation/Configuration > Option 1 - Full Data Access Implementation.

Contact

Any comment or feedback is welcomed, issues can be reported at Red Dog's Github corner.

Visit our contact page to get more information.

License

Red Dog is licensed under Apache License 2.0.

rdap-data-access-api's Issues

Why are the IP address arguments Strings

rdap-data-access-api/src/main/java/mx/nic/rdap/db/spi/IpNetworkDAO.java

Line 22 in c225a08

public IpNetwork getByInetAddress(String ipAddress) throws RdapDataAccessException;

Java has a standard IP implementation called InetAddress. I don't think there is a reason why we should be torturing every single implementation with validating and parsing this string. User input parsing is being deferred to the model. This goes against every self-respecting software architectural pattern. Parsing and validating user input is the rdap-server's responsibility; please move these validations to the server so implementations only have to worry about data access.

`useNsAsAttribute` should not belong to the API

Every method in DomainDAO requires a boolean argument named useNsAsAttribute.

There are several reasons why I feel this argument should be removed from the API:

1. Its value remains constant through the lifetime of an RDAP server. Given an RDAP server, its value is either always true or always false. And it's always the same value for all the functions.
2. The argument is rather short-sighted. RFC 7483 states that "The nameserver object class accommodates both models and degrees of variation in between." Using a boolean value, we can provide support for both models, but not variations in between.
3. I expect other data-access implementations to not care for both models. From what I understand, An implementation coming from a RIR will, for example, have no use for the object model, and "other DNRs model nameservers" will not care for the attribute model. So really, the flag is none of the RDAP Server's business; it is data-access-implementation-specific configuration.
4. The argument is really just SQL-provider-specific; because it's trying to fulfil both models. The data-access API should not be polluted with the idiosyncrasies of one single implementation. If the SQL provider needs the flag, it would be better off getting it from its own configuration.

The nameserver_as_domain_attribute property should be moved to data-access.properties and nameserverAsDomainAttribute should be moved to the SQL provider, as a private member of DomainDAOImpl.

Rename/delete dir "src/main/test"

In the mentioned directory "src/main/test" there are some classes used for unit tests, I have 2 observations for this:

1. This classes test objects of the core (rdap-core), so, they don't belong to that project?
2. The directory is placed at the same level that the main code "src/main/java"; looking at the other projects (rdap-sql-provider and rdap-server) they have the test classes at the directory "src/test/java/", which is a nice approach, so why not use the same approach in the test classes of this project?

Conclusion: if the classes are moved to the project rdap-core the 2nd point doesn't apply (or only applies if there are going to be some test classes for this project), if they are going to stay in this project they could be placed as mentioned in the 2nd point.

MANIFEST is really necessary?

The manifest file placed at src/main/resources/META-INF seems to be useless, since it doesn't have any special data/information in it. Beside this, maven adds the default manifest file when the application is built. I suggest that this file shouldn't exist, it should be deleted just to be clear.

Taking this in consideration, the whole folder resources could be deleted since is going to be empty, but of course, this is something to consider by who's going to solve the issue (assuming that my opinion is useful).

The API wants me to implement updates

I created a class that's trying to implement mx.nic.rdap.db.spi.DomainSpi. The interface wants me to implement a method called storeToDatabase.

This looks like rdap-migrator debris. It has no place in an API called "rdap-data-ACCESS-api". And I get that I can return a dummy value to silence the compiler, but this is bad API design, bound to get in the way forevermore and to annoy our users.

Please refactor all these out-of-place methods out of the way.

Should we unbox primitives that will never be null?

The boxing is inconsistent through the API. Most primitive arguments to the API methods are boxed, but some are not.

The tendency seems to be to box everything, but I find this practice annoying. The resultLimit argument, for example, is present in every search function, and as such every single search function needs to include a remark stating that null is not an option by contract. With no gain, as far as I can see.

What is the rationale that led to boxing most of these variables?

Not specifying expected return values

This:

public SearchResultStruct searchByName(...

Should read like this:

public SearchResultStruct<Domain> searchByName(...

To prevent users from doing whatever.

Because it's not like we're allowing them to return anything. rdap-server crashes if it gets something that's not Domains.

This was just an example. This issue applies to every use of SearchResultStruct.