Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement tables.metadata list & patch RPC endpoint #3646

Merged
merged 11 commits into from
Jul 2, 2024
3 changes: 2 additions & 1 deletion config/settings/common_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ def pipe_delim(pipe_string):
'mathesar.rpc.columns',
'mathesar.rpc.columns.metadata',
'mathesar.rpc.schemas',
'mathesar.rpc.tables'
'mathesar.rpc.tables',
'mathesar.rpc.tables.metadata'
]

TEMPLATES = [
Expand Down
10 changes: 10 additions & 0 deletions docs/docs/api/rpc.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,16 @@ To use an RPC function:
- TableInfo
- SettableTableInfo

## Table Metadata

::: tables.metadata
options:
members:
- list_
- patch
- TableMetaData
- SettableTableMetaData

## Columns

::: columns
Expand Down
53 changes: 53 additions & 0 deletions mathesar/migrations/0009_add_column_metadata_model.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# Generated by Django 4.2.11 on 2024-06-28 08:35

from django.db import migrations, models
import django.db.models.deletion


class Migration(migrations.Migration):

dependencies = [
('mathesar', '0008_add_metadata_models'),
]

operations = [
migrations.AlterField(
model_name='columnmetadata',
name='attnum',
field=models.SmallIntegerField(),
),
migrations.AlterField(
model_name='columnmetadata',
name='num_max_frac_digits',
field=models.PositiveIntegerField(default=20),
),
migrations.AlterField(
model_name='columnmetadata',
name='num_min_frac_digits',
field=models.PositiveIntegerField(default=0),
),
migrations.AlterField(
model_name='columnmetadata',
name='table_oid',
field=models.PositiveBigIntegerField(),
),
migrations.CreateModel(
name='TableMetaData',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('created_at', models.DateTimeField(auto_now_add=True)),
('updated_at', models.DateTimeField(auto_now=True)),
('schema_oid', models.PositiveBigIntegerField()),
('table_oid', models.PositiveBigIntegerField()),
('import_verified', models.BooleanField(default=False)),
('column_order', models.JSONField(default=list)),
('preview_customized', models.BooleanField(default=False)),
('preview_template', models.CharField(blank=True, max_length=255)),
('database', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='mathesar.database')),
],
),
migrations.AddConstraint(
model_name='tablemetadata',
constraint=models.UniqueConstraint(fields=('database', 'schema_oid', 'table_oid'), name='unique_table_metadata'),
),
]
26 changes: 22 additions & 4 deletions mathesar/models/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,16 +84,16 @@ def connection(self):

class ColumnMetaData(BaseModel):
database = models.ForeignKey('Database', on_delete=models.CASCADE)
table_oid = models.PositiveIntegerField()
attnum = models.PositiveIntegerField()
table_oid = models.PositiveBigIntegerField()
attnum = models.SmallIntegerField()
bool_input = models.CharField(
choices=[("dropdown", "dropdown"), ("checkbox", "checkbox")],
blank=True
)
bool_true = models.CharField(default='True')
bool_false = models.CharField(default='False')
num_min_frac_digits = models.PositiveIntegerField(blank=True)
num_max_frac_digits = models.PositiveIntegerField(blank=True)
num_min_frac_digits = models.PositiveIntegerField(default=0)
num_max_frac_digits = models.PositiveIntegerField(default=20)
num_show_as_perc = models.BooleanField(default=False)
mon_currency_symbol = models.CharField(default="$")
mon_currency_location = models.CharField(
Expand Down Expand Up @@ -121,3 +121,21 @@ class Meta:
name="frac_digits_integrity"
)
]


class TableMetaData(BaseModel):
database = models.ForeignKey('Database', on_delete=models.CASCADE)
schema_oid = models.PositiveBigIntegerField()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not expect a schema_oid field here. I was expecting that we'd follow the patterns we decided on for explorations wherein we'd return all the metadata for the whole database and let the front end filter it out by schema as needed.

table_oid = models.PositiveBigIntegerField()
import_verified = models.BooleanField(default=False)
column_order = models.JSONField(default=list)
preview_customized = models.BooleanField(default=False)
preview_template = models.CharField(max_length=255, blank=True)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I'd like these fields to be renamed:

    • preview_customizedrecord_summary_customized
    • preview_templaterecord_summary_template

    These changes will serve to make our terminology more consistent across the stack.

  2. As a side note: I'm not sure we need preview_customized. I did a little digging on this and couldn't figure out why it's necessary. Perhaps we do need it. I don't think we ought to try removing it in this PR, but I just want to mention this as a heads up. I might want to remove this later once we get deeper into the record summary work before beta. If anyone seeing this comment happens to have a clear understanding of why we need this field, please comment here. A code comment explaining it would be nice too! But not worth spending much time clarifying right now.


class Meta:
constraints = [
models.UniqueConstraint(
fields=["database", "schema_oid", "table_oid"],
name="unique_table_metadata"
)
]
2 changes: 1 addition & 1 deletion mathesar/rpc/columns/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class ColumnMetaData(TypedDict):

Attributes:
database_id: The Django id of the database containing the table.
table_oid: The OID of the table containing the column
table_oid: The OID of the table containing the column.
attnum: The attnum of the column in the table.
bool_input: How the input for a boolean column should be shown.
bool_true: A string to display for `true` values.
Expand Down
1 change: 1 addition & 0 deletions mathesar/rpc/tables/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
from .base import * # noqa
File renamed without changes.
105 changes: 105 additions & 0 deletions mathesar/rpc/tables/metadata.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
"""
Classes and functions exposed to the RPC endpoint for managing table metadata.
"""
from typing import Optional, TypedDict

from modernrpc.core import rpc_method
from modernrpc.auth.basic import http_basic_auth_login_required

from mathesar.rpc.exceptions.handlers import handle_rpc_exceptions
from mathesar.utils.tables import get_tables_meta_data, patch_table_meta_data


class TableMetaData(TypedDict):
"""
Metadata for a table in a database.

Only the `database`, `schema_oid`, and `table_oid` keys are required.

Attributes:
id: The Django id of the TableMetaData object.
database_id: The Django id of the database containing the table.
schema_oid: The OID of the schema containing the table.
table_oid: The OID of the table in the database.
import_verified: Specifies whether a file has been successfully imported into a table.
column_order: The order in which columns of a table are displayed.
preview_customized: Specifies whether the preview has been customized.
preview_template: Preview template for a referent column.
"""
id: int
database_id: int
schema_oid: int
table_oid: int
import_verified: Optional[bool]
column_order: Optional[list[int]]
preview_customized: Optional[bool]
preview_template: Optional[str]

@classmethod
def from_model(cls, model):
return cls(
id=model.id,
database_id=model.database.id,
schema_oid=model.schema_oid,
table_oid=model.table_oid,
import_verified=model.import_verified,
column_order=model.column_order,
preview_customized=model.preview_customized,
preview_template=model.preview_template,
)


class SettableTableMetaData(TypedDict):
"""
Settable metadata fields for a table in a database.

Attributes:
import_verified: Specifies whether a file has been successfully imported into a table.
column_order: The order in which columns of a table are displayed.
preview_customized: Specifies whether the preview has been customized.
preview_template: Preview template for a referent column.
"""
import_verified: Optional[bool]
column_order: Optional[list[int]]
preview_customized: Optional[bool]
preview_template: Optional[str]


@rpc_method(name="tables.metadata.list")
@http_basic_auth_login_required
@handle_rpc_exceptions
def list_(*, schema_oid: int, database_id: int, **kwargs) -> list[TableMetaData]:
"""
List metadata associated with tables for a schema.

Args:
schema_oid: Identity of the schema in the user's database.
database_id: The Django id of the database containing the table.

Returns:
Metadata object for a given table oid.
"""
table_meta_data = get_tables_meta_data(schema_oid, database_id)
return [
TableMetaData.from_model(model) for model in table_meta_data
]


@rpc_method(name="tables.metadata.patch")
@http_basic_auth_login_required
@handle_rpc_exceptions
def patch(
*, metadata_id: int, metadata_dict: SettableTableMetaData, **kwargs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect to pass the table OID here, not the metadata id. I don't mind the fact that this model ends up with a Django id, because presumably that's just the way that Django works. But I think our metadata APIs would be more straightforward for consumers if they didn't need to worry about the Django ids at all. Sticking solely to OIDs eliminates the need for an API consumer to maintain a mapping between table OIDs and metadata IDs.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with that. However, if we go ahead with table_oid, we'd also need database_id to uniquely identify tables across multiple databases. But I do agree that this is more consistent with how our other APIs are currently set up.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we'd also need database_id

Agreed

) -> TableMetaData:
"""
Alter metadata settings associated with a table for a schema.

Args:
metadata_id: Identity of the table metadata in the user's database.
metadata_dict: The dict describing desired table metadata alterations.

Returns:
Altered metadata object.
"""
table_meta_data = patch_table_meta_data(metadata_id, metadata_dict)
return TableMetaData.from_model(table_meta_data)
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ def mock_table_info(_schema_oid, conn):
'description': None
}
]
monkeypatch.setattr(tables, 'connect', mock_connect)
monkeypatch.setattr(tables, 'get_table_info', mock_table_info)
monkeypatch.setattr(tables.base, 'connect', mock_connect)
monkeypatch.setattr(tables.base, 'get_table_info', mock_table_info)
expect_table_list = [
{
'oid': 17408,
Expand Down Expand Up @@ -90,8 +90,8 @@ def mock_table_get(_table_oid, conn):
'schema': 2200,
'description': 'a description on the authors table.'
}
monkeypatch.setattr(tables, 'connect', mock_connect)
monkeypatch.setattr(tables, 'get_table', mock_table_get)
monkeypatch.setattr(tables.base, 'connect', mock_connect)
monkeypatch.setattr(tables.base, 'get_table', mock_table_get)
expect_table_list = {
'oid': table_oid,
'name': 'Authors',
Expand Down Expand Up @@ -123,8 +123,8 @@ def mock_drop_table(_table_oid, conn, cascade):
raise AssertionError('incorrect parameters passed')
return 'public."Table 0"'

monkeypatch.setattr(tables, 'connect', mock_connect)
monkeypatch.setattr(tables, 'drop_table_from_database', mock_drop_table)
monkeypatch.setattr(tables.base, 'connect', mock_connect)
monkeypatch.setattr(tables.base, 'drop_table_from_database', mock_drop_table)
deleted_table = tables.delete(table_oid=1964474, database_id=11, request=request)
assert deleted_table == 'public."Table 0"'

Expand All @@ -149,8 +149,8 @@ def mock_table_add(table_name, _schema_oid, conn, column_data_list, constraint_d
if _schema_oid != schema_oid:
raise AssertionError('incorrect parameters passed')
return 1964474
monkeypatch.setattr(tables, 'connect', mock_connect)
monkeypatch.setattr(tables, 'create_table_on_database', mock_table_add)
monkeypatch.setattr(tables.base, 'connect', mock_connect)
monkeypatch.setattr(tables.base, 'create_table_on_database', mock_table_add)
actual_table_oid = tables.add(table_name='newtable', schema_oid=2200, database_id=11, request=request)
assert actual_table_oid == 1964474

Expand Down Expand Up @@ -180,8 +180,8 @@ def mock_table_patch(_table_oid, _table_data_dict, conn):
if _table_oid != table_oid and _table_data_dict != table_data_dict:
raise AssertionError('incorrect parameters passed')
return 'newtabname'
monkeypatch.setattr(tables, 'connect', mock_connect)
monkeypatch.setattr(tables, 'alter_table_on_database', mock_table_patch)
monkeypatch.setattr(tables.base, 'connect', mock_connect)
monkeypatch.setattr(tables.base, 'alter_table_on_database', mock_table_patch)
altered_table_name = tables.patch(
table_oid=1964474,
table_data_dict={
Expand Down Expand Up @@ -216,8 +216,8 @@ def mock_table_import(_data_file_id, table_name, _schema_oid, conn, comment):
if _schema_oid != schema_oid and _data_file_id != data_file_id:
raise AssertionError('incorrect parameters passed')
return 1964474
monkeypatch.setattr(tables, 'connect', mock_connect)
monkeypatch.setattr(tables, 'import_csv', mock_table_import)
monkeypatch.setattr(tables.base, 'connect', mock_connect)
monkeypatch.setattr(tables.base, 'import_csv', mock_table_import)
imported_table_oid = tables.import_(
data_file_id=10,
table_name='imported_table',
Expand Down Expand Up @@ -253,8 +253,8 @@ def mock_table_preview(_table_oid, columns, conn, limit):
{'id': 3, 'length': Decimal('4.0')},
{'id': 4, 'length': Decimal('5.22')}
]
monkeypatch.setattr(tables, 'connect', mock_connect)
monkeypatch.setattr(tables, 'get_preview', mock_table_preview)
monkeypatch.setattr(tables.base, 'connect', mock_connect)
monkeypatch.setattr(tables.base, 'get_preview', mock_table_preview)
records = tables.get_import_preview(
table_oid=1964474,
columns=[{'attnum': 2, 'type': {'name': 'numeric', 'options': {'precision': 3, 'scale': 2}}}],
Expand Down
69 changes: 69 additions & 0 deletions mathesar/tests/rpc/tables/test_t_metadata.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
"""
This file tests the table metadata RPC functions.

Fixtures:
monkeypatch(pytest): Lets you monkeypatch an object for testing.
"""
from mathesar.models.base import TableMetaData, Database, Server
from mathesar.rpc.tables import metadata


def test_tables_meta_data_list(monkeypatch):
database_id = 2
schema_oid = 123456

def mock_get_tables_meta_data(_schema_oid, _database_id):
server_model = Server(id=2, host='example.com', port=5432)
db_model = Database(id=_database_id, name='mymathesardb', server=server_model)
return [
TableMetaData(
id=1, database=db_model, schema_oid=_schema_oid, table_oid=1234,
import_verified=True, column_order=[8, 9, 10], preview_customized=False,
preview_template="{5555}"
),
TableMetaData(
id=2, database=db_model, schema_oid=_schema_oid, table_oid=4567,
import_verified=False, column_order=[], preview_customized=True,
preview_template="{5512} {1223}"
)
]
monkeypatch.setattr(metadata, "get_tables_meta_data", mock_get_tables_meta_data)

expect_metadata_list = [
metadata.TableMetaData(
id=1, database_id=database_id, schema_oid=schema_oid, table_oid=1234,
import_verified=True, column_order=[8, 9, 10], preview_customized=False,
preview_template="{5555}"
),
metadata.TableMetaData(
id=2, database_id=database_id, schema_oid=schema_oid, table_oid=4567,
import_verified=False, column_order=[], preview_customized=True,
preview_template="{5512} {1223}"
)
]
actual_metadata_list = metadata.list_(schema_oid=schema_oid, database_id=database_id)
assert actual_metadata_list == expect_metadata_list


def test_tables_meta_data_patch(monkeypatch):
database_id = 2
schema_oid = 123456
metadata_dict = {'import_verified': True, 'column_order': [1, 4, 12]}

def mock_patch_tables_meta_data(metadata_id, metadata_dict):
server_model = Server(id=2, host='example.com', port=5432)
db_model = Database(id=database_id, name='mymathesardb', server=server_model)
return TableMetaData(
id=1, database=db_model, schema_oid=schema_oid, table_oid=1234,
import_verified=True, column_order=[1, 4, 12], preview_customized=False,
preview_template="{5555}"
)
monkeypatch.setattr(metadata, "patch_table_meta_data", mock_patch_tables_meta_data)

expect_metadata_object = metadata.TableMetaData(
id=1, database_id=database_id, schema_oid=schema_oid, table_oid=1234,
import_verified=True, column_order=[1, 4, 12], preview_customized=False,
preview_template="{5555}"
)
actual_metadata_object = metadata.patch(metadata_id=1, metadata_dict=metadata_dict)
assert actual_metadata_object == expect_metadata_object
Loading
Loading