Finish Cassandra DAO

[thibaultcha]

• Don't rely on models anymore, use it in a more functional way, such as:

factory.apis:insert {
  name = "api 1",
  public_dns = "hello.com",
  target_url = "http://httpconsole.org"
}

• Static queries, prepared on start
• Cassandra constraints: unique checks, exists checks for unique and foreign entities since not natively supported by Cassandra.
• INSERT, UPDATE, SELECT, DELETE + tests
• Dynamic SELECT (allowed fields)
• deserialize SELECT output (json encoded/type)
• Metrics INCREMENT + DELETE
• Check all UNIQUE and FOREIGN fields from schema are written into __exists, __unique (tests or in :prepare)
• Paginated SELECT
• Proper error types
• immutable property for values that cannot be updated

[thibaultcha]

@thefosk

• Why would an application not be attached to an account? Is an application possibly only linked to an API? Why so? I understand a plugin can only be linked to an API but not an application.

Attaching an application to an account_id could allow us to use account_id as a clustering key for better performance.

• Why would an account's secret_key have to be unique?

Right now we have to maintain an INDEX on it only for this unique field. I understand the use case for an key authentication with an auto-generated api key (and we can just generate unique values). But isn't each user free to chose any password they want in the case the provider is using BASIC auth?

• Why do we have to maintain an index on API's target_url? This is a very, very high cardinality value that will not be queried. (I don't see a use case)
• Also we should RTFM more about all these options on tables:

DESCRIBE table metrics;

CREATE TABLE apenode.metrics (
    api_id uuid,
    application_id uuid,
    origin_ip text,
    name text,
    period text,
    "timestamp" timestamp,
    value counter,
    PRIMARY KEY ((api_id, application_id, origin_ip, name, period, "timestamp"))
) WITH bloom_filter_fp_chance = 0.01
    AND caching = '{"keys":"ALL", "rows_per_partition":"NONE"}'
    AND comment = ''
    AND compaction = {'min_threshold': '4', 'class': 'org.apache.cassandra.db.compaction.SizeTieredCompactionStrategy', 'max_threshold': '32'}
    AND compression = {'sstable_compression': 'org.apache.cassandra.io.compress.LZ4Compressor'}
    AND dclocal_read_repair_chance = 0.1
    AND default_time_to_live = 0
    AND gc_grace_seconds = 864000
    AND max_index_interval = 2048
    AND memtable_flush_period_in_ms = 0
    AND min_index_interval = 128
    AND read_repair_chance = 0.0
    AND speculative_retry = '99.0PERCENTILE';

• Why not enforcing map<string, string> on plugin's value column and make sure we store strings in it. Another good solution is we can have 2 columns: map<string, number>, and map<string, string> which is perfectly fine since cassandra is a column family DB. This unset map will not be set, and appear as null on CQL

TODO

We should start replacing the controllers code with the new DAO, as the main features are now finished.

[thibaultcha]

Note for me Monday:

• Pagination is supported by the driver in my fork no need to wait for PR yet, we duplicated the driver in Kong's code.
• Fix this test by adding an api entity to a generated plugin. Best is to cache the Faker's inserted entities and randomly select one from there. Faster than random DB select.
• Expose the page_state option to the base_dao and write tests for it.

Then just need to finish updating the controllers code.

[thibaultcha]

We need a way to distinguish server errors from client errors in the DAO.

Cassandra error != Non existing FOREIGN key or failing UNIQUE check

[subnetmarco]

The easiest solution I can think of is a response like:

local res, err, client_err, server_err = dao:do_something()

By having both err and client_err, server_err, it's up to the caller to decide if he needs a better drill down or not on the error. This will duplicate data.

It's up to the caller to parse it, for example if the caller doesn't care about the separation, he will just invoke the method the usual way:

local res, err = dao:do_something()

but if he does care, then he will add the other two variables:

local res, err, client_err, server_err  = dao:do_something()

So it's only going to be verbose only when the separation is required, not always.

[subnetmarco]

Or err could be a table like:

local err = {
    client = {},
    server = {}
}

Or error messages could include a flag to tell if they're client or server errors, like:

local err = { public_dns = { message = "Invalid public dns", client = true } }

[thibaultcha]

I think it's better practise to standardise the errors with a code. So all the errors wou'll be getting from the DAO will be

{
  type = "client",
  error = some_table
}

But sometimes the error can also be a string, is that an issue?

[subnetmarco]

@thibaultcha the only problem that I see with your solutions (and with my last one, with the client flag), is that if we have both client and server errors then the caller will need to sanitize the error table before logging it or showing it to the user.

[thibaultcha]

local error_codes = {
    [0x0000]= "Server error",
    [0x000A]= "Protocol error",
    [0x0100]= "Bad credentials",
    [0x1000]= "Unavailable exception",
    [0x1001]= "Overloaded",
    [0x1002]= "Is_bootstrapping",
    [0x1003]= "Truncate_error",
    [0x1100]= "Write_timeout",
    [0x1200]= "Read_timeout",
    [0x2000]= "Syntax_error",
    [0x2100]= "Unauthorized",
    [0x2200]= "Invalid",
    [0x2300]= "Config_error",
    [0x2400]= "Already_exists",
    [0x2500]= "Unprepared"
}

• Those are Cassandra error codes. It is possible to receive a Cassandra error but it's actually a client error (passing an invalid UUID for example will be an "Invalid" error). I need to parse the error before deciding if client or server.
• The only case when we could have both a client and a server error is in the UNIQUE and EXISTS checks (Cassandra.......................). Basically there might be a lot of UNIQUE and EXISTS checks at once (2 or 3 for some entities) and if an error is encountered, it adds it to the final error object. I need to interrupt the checks and return as soon as such an error is encountered. This way, no more aggregated errors.

[subnetmarco]

@thibaultcha so what is the conclusion?

[thibaultcha]

[subnetmarco]

Excuse me?

[thibaultcha]

Okay okay, this is now all done! Just need to finish to update the code and we can switch to #4 and #8 😄

[thibaultcha]

At this point the DAO is finished and the refactor TODOs are now moved to #12