Skip to content

Commit 7e88191

Browse files
authored
Fix incorrect resource ownership transmission in schema retrieval C functions (#756)
## Usage and product changes Fix a crash caused by an incorrect database's ownership acquisition in the C layer of the `schema` and `type_schema` functions, affecting Python and Java drivers. Add respective BDD steps implementations for these functions in Rust, Python, and Java. ## Implementation Borrow instead of taking ownership of the referenced database resource.
1 parent 71adee4 commit 7e88191

File tree

12 files changed

+135
-15
lines changed

12 files changed

+135
-15
lines changed

c/src/database.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,13 @@ pub extern "C" fn database_delete(database: *const Database) {
4949
/// A full schema text as a valid TypeQL define query string.
5050
#[no_mangle]
5151
pub extern "C" fn database_schema(database: *const Database) -> *mut c_char {
52-
try_release_string(take_arc(database).schema())
52+
try_release_string(borrow(database).schema())
5353
}
5454

5555
/// The types in the schema as a valid TypeQL define query string.
5656
#[no_mangle]
5757
pub extern "C" fn database_type_schema(database: *const Database) -> *mut c_char {
58-
try_release_string(take_arc(database).type_schema())
58+
try_release_string(borrow(database).type_schema())
5959
}
6060

6161
// /// Iterator over the <code>ReplicaInfo</code> corresponding to each replica of a TypeDB cloud database.

dependencies/typedb/artifacts.bzl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ def typedb_artifact():
2525
artifact_name = "typedb-all-{platform}-{version}.{ext}",
2626
tag_source = deployment["artifact"]["release"]["download"],
2727
commit_source = deployment["artifact"]["snapshot"]["download"],
28-
commit = "fed272ee65df1e854c72c4066fdb2c4241565bb0"
28+
commit = "16eca5f4238a772532a6824966e2fc78437467dc"
2929
)
3030

3131
#def typedb_cloud_artifact():

dependencies/typedb/repositories.bzl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,5 @@ def typedb_behaviour():
3535
git_repository(
3636
name = "typedb_behaviour",
3737
remote = "https://github.com/typedb/typedb-behaviour",
38-
commit = "569b59cad1355bfaaf1ca18a8f4adf40e5d1da4f", # sync-marker: do not remove this comment, this is used for sync-dependencies by @typedb_behaviour
38+
commit = "8f9345de853ad7d0ae66e7afefd16be2cfa3dced", # sync-marker: do not remove this comment, this is used for sync-dependencies by @typedb_behaviour
3939
)

java/test/behaviour/connection/database/DatabaseSteps.java

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
package com.typedb.driver.test.behaviour.connection.database;
2121

2222
import com.typedb.driver.api.Driver;
23+
import com.typedb.driver.api.Transaction;
2324
import com.typedb.driver.api.database.Database;
2425
import com.typedb.driver.test.behaviour.config.Parameters;
2526
import io.cucumber.java.en.Then;
@@ -53,6 +54,15 @@ public void deleteDatabases(Driver driver, List<String> names) {
5354
}
5455
}
5556

57+
public String createTemporaryDatabaseWithSchema(Driver driver, String schemaQuery) {
58+
String name = "temp-" + (int) (Math.random() * 10000);
59+
createDatabases(driver, list(name));
60+
Transaction transaction = driver.transaction(name, Transaction.Type.SCHEMA);
61+
transaction.query(schemaQuery).resolve();
62+
transaction.commit();
63+
return name;
64+
}
65+
5666
@When("connection create database: {non_semicolon}{may_error}")
5767
public void connection_create_database(String name, Parameters.MayError mayError) {
5868
mayError.check(() -> createDatabases(driver, list(name)));
@@ -133,4 +143,34 @@ public void connection_does_not_have_databases(List<String> names) {
133143
assertFalse(databases.contains(databaseName));
134144
}
135145
}
146+
147+
@Then("connection get database\\({word}) has schema:")
148+
public void connection_get_database_has_schema(String name, String schema) {
149+
String expectedSchema = schema.strip();
150+
String expectedSchemaRetrieved;
151+
if (expectedSchema.isEmpty()) {
152+
expectedSchemaRetrieved = "";
153+
} else {
154+
String tempDatabaseName = createTemporaryDatabaseWithSchema(driver, expectedSchema);
155+
expectedSchemaRetrieved = driver.databases().get(tempDatabaseName).schema();
156+
}
157+
158+
String realSchema = driver.databases().get(name).schema();
159+
assertEquals(expectedSchemaRetrieved, realSchema);
160+
}
161+
162+
@Then("connection get database\\({word}) has type schema:")
163+
public void connection_get_database_has_type_schema(String name, String schema) {
164+
String expectedSchema = schema.strip();
165+
String expectedSchemaRetrieved;
166+
if (expectedSchema.isEmpty()) {
167+
expectedSchemaRetrieved = "";
168+
} else {
169+
String tempDatabaseName = createTemporaryDatabaseWithSchema(driver, expectedSchema);
170+
expectedSchemaRetrieved = driver.databases().get(tempDatabaseName).typeSchema();
171+
}
172+
173+
String realSchema = driver.databases().get(name).typeSchema();
174+
assertEquals(expectedSchemaRetrieved, realSchema);
175+
}
136176
}

java/test/behaviour/debug/BUILD

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ typedb_java_test(
2828
deps = [
2929
# Internal package dependencies
3030
"//java/test/behaviour:behaviour",
31+
"//java/test/behaviour/query:steps",
32+
"//java/test/behaviour/util:steps",
3133

3234
# TODO: Add your addition debugging dependencies here
3335
# e.g. "//java/test/behaviour/connection/steps:connection-database",

java/test/integration/ExampleTest.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
* under the License.
1818
*/
1919

20+
// EXAMPLE START MARKER
2021
// EXAMPLE START MARKER
2122
// EXAMPLE START MARKER
2223
package com.typedb.driver.test.integration;
@@ -48,6 +49,8 @@
4849
import java.util.concurrent.atomic.AtomicInteger;
4950
import java.util.stream.Collectors;
5051

52+
53+
// EXAMPLE END MARKER
5154
import static org.junit.Assert.assertEquals;
5255
import static org.junit.Assert.assertFalse;
5356
import static org.junit.Assert.assertNotEquals;
@@ -56,6 +59,7 @@
5659
import static org.junit.Assert.fail;
5760

5861
@SuppressWarnings("Duplicates")
62+
// EXAMPLE START MARKER
5963
public class ExampleTest {
6064
// EXAMPLE END MARKER
6165
@BeforeClass

python/tests/behaviour/connection/database/database_steps.py

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
# specific language governing permissions and limitations
1616
# under the License.
1717

18+
import random
1819
from concurrent.futures.thread import ThreadPoolExecutor
1920
from functools import partial
2021

@@ -23,6 +24,7 @@
2324
from tests.behaviour.config.parameters import MayError, parse_list
2425
from tests.behaviour.context import Context
2526
from tests.behaviour.util.util import assert_collections_equal
27+
from typedb.api.connection.transaction import TransactionType
2628
from typedb.driver import *
2729

2830

@@ -125,15 +127,34 @@ def step_impl(context: Context):
125127
does_not_have_databases(context, names=parse_list(context))
126128

127129

130+
def create_temporary_database_with_schema(context: Context, schema_query: str) -> str:
131+
name = "temp-" + str(random.randint(0, 100000))
132+
create_databases(context.driver, [name])
133+
transaction = context.driver.transaction(name, TransactionType.SCHEMA)
134+
transaction.query(schema_query).resolve()
135+
transaction.commit()
136+
return name
137+
138+
128139
@step("connection get database({name}) has schema")
129140
def step_impl(context: Context, name: str):
130-
expected_schema = context.text
141+
expected_schema = context.text.strip()
142+
if expected_schema:
143+
temp_database_name = create_temporary_database_with_schema(context, expected_schema)
144+
expected_schema_retrieved = context.driver.databases.get(temp_database_name).schema()
145+
else:
146+
expected_schema_retrieved = ""
131147
real_schema = context.driver.databases.get(name).schema()
132-
assert_that(real_schema, is_(expected_schema))
148+
assert_that(real_schema, is_(expected_schema_retrieved))
133149

134150

135151
@step("connection get database({name}) has type schema")
136152
def step_impl(context: Context, name: str):
137-
expected_schema = context.text
138-
real_schema = context.driver.databases.get(name).type_schema()
139-
assert_that(real_schema, is_(expected_schema))
153+
expected_type_schema = context.text.strip()
154+
if expected_type_schema:
155+
temp_database_name = create_temporary_database_with_schema(context, expected_type_schema)
156+
expected_type_schema_retrieved = context.driver.databases.get(temp_database_name).type_schema()
157+
else:
158+
expected_type_schema_retrieved = ""
159+
real_type_schema = context.driver.databases.get(name).type_schema()
160+
assert_that(real_type_schema, is_(expected_type_schema_retrieved))

rust/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ use typedb_driver::{
7272
ConceptRow, QueryAnswer,
7373
},
7474
concept::{Concept, ValueType},
75-
Credentials, DriverOptions, Error, TransactionOptions, TransactionType, TypeDBDriver, QueryOptions,
75+
Credentials, DriverOptions, Error, QueryOptions, TransactionOptions, TransactionType, TypeDBDriver,
7676
};
7777

7878
fn typedb_example() {

rust/example.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use typedb_driver::{
99
ConceptRow, QueryAnswer,
1010
},
1111
concept::{Concept, ValueType},
12-
Credentials, DriverOptions, Error, TransactionOptions, TransactionType, TypeDBDriver, QueryOptions,
12+
Credentials, DriverOptions, Error, QueryOptions, TransactionOptions, TransactionType, TypeDBDriver,
1313
};
1414

1515
fn typedb_example() {

rust/tests/behaviour/steps/connection/database.rs

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,13 @@ use futures::{
2626
};
2727
use macro_rules_attribute::apply;
2828
use tokio::time::sleep;
29-
use typedb_driver::{Database, DatabaseManager, Result as TypeDBResult, TypeDBDriver};
29+
use typedb_driver::{Database, DatabaseManager, Result as TypeDBResult, TransactionType, TypeDBDriver};
30+
use uuid::Uuid;
3031

31-
use crate::{assert_with_timeout, generic_step, in_oneshot_background, params, util::iter_table, Context};
32+
use crate::{
33+
assert_with_timeout, connection::transaction::open_transaction_for_database, generic_step, in_oneshot_background,
34+
params, query::run_query, util::iter_table, Context,
35+
};
3236

3337
async fn create_database(driver: &TypeDBDriver, name: String, may_error: params::MayError) {
3438
may_error.check(driver.databases().create(name).await);
@@ -38,6 +42,14 @@ async fn delete_database(driver: &TypeDBDriver, name: &str, may_error: params::M
3842
may_error.check(driver.databases().get(name).and_then(Database::delete).await);
3943
}
4044

45+
async fn database_schema(driver: &TypeDBDriver, name: &str) -> String {
46+
driver.databases().get(name).await.expect("Expected database").schema().await.expect("Expected schema")
47+
}
48+
49+
async fn database_type_schema(driver: &TypeDBDriver, name: &str) -> String {
50+
driver.databases().get(name).await.expect("Expected database").type_schema().await.expect("Expected type schema")
51+
}
52+
4153
async fn has_database(driver: &TypeDBDriver, name: &str) -> bool {
4254
driver.databases().contains(name.clone()).await.unwrap()
4355
}
@@ -155,3 +167,44 @@ async fn connection_does_not_have_databases(context: &mut Context, step: &Step)
155167
);
156168
}
157169
}
170+
171+
async fn create_temporary_database_with_schema(driver: &TypeDBDriver, schema_query: String) -> String {
172+
let name = format!("temp-{}", Uuid::new_v4());
173+
create_database(driver, name.clone(), params::MayError::False).await;
174+
let transaction = open_transaction_for_database(driver, &name, TransactionType::Schema, None)
175+
.await
176+
.expect("Expected transaction");
177+
run_query(&transaction, &schema_query, None).await.expect("Expected successful query");
178+
transaction.commit().await.expect("Expected successful commit");
179+
name
180+
}
181+
182+
#[apply(generic_step)]
183+
#[step(expr = r"connection get database\({word}\) has schema:")]
184+
async fn connection_get_database_has_schema(context: &mut Context, name: String, step: &Step) {
185+
let expected_schema = step.docstring.as_ref().unwrap().trim().to_string();
186+
let driver = context.driver.as_ref().unwrap();
187+
let expected_schema_retrieved = if expected_schema.is_empty() {
188+
String::new()
189+
} else {
190+
let temp_database_name = create_temporary_database_with_schema(driver, expected_schema).await;
191+
database_schema(driver, &temp_database_name).await
192+
};
193+
let schema = database_schema(driver, &name).await;
194+
assert_eq!(expected_schema_retrieved, schema);
195+
}
196+
197+
#[apply(generic_step)]
198+
#[step(expr = r"connection get database\({word}\) has type schema:")]
199+
async fn connection_get_database_has_type_schema(context: &mut Context, name: String, step: &Step) {
200+
let expected_type_schema = step.docstring.as_ref().unwrap().trim().to_string();
201+
let driver = context.driver.as_ref().unwrap();
202+
let expected_type_schema_retrieved = if expected_type_schema.is_empty() {
203+
String::new()
204+
} else {
205+
let temp_database_name = create_temporary_database_with_schema(driver, expected_type_schema).await;
206+
database_type_schema(driver, &temp_database_name).await
207+
};
208+
let type_schema = database_type_schema(driver, &name).await;
209+
assert_eq!(expected_type_schema_retrieved, type_schema);
210+
}

rust/tests/behaviour/steps/connection/transaction.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ use crate::{
2828
generic_step, in_background, in_oneshot_background, params, params::check_boolean, util::iter_table, Context,
2929
};
3030

31-
async fn open_transaction_for_database(
31+
pub(crate) async fn open_transaction_for_database(
3232
driver: &TypeDBDriver,
3333
database_name: impl AsRef<str>,
3434
transaction_type: TransactionType,

rust/tests/behaviour/steps/query.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ use crate::{
3838
BehaviourTestOptionalError, Context,
3939
};
4040

41-
async fn run_query(
41+
pub(crate) async fn run_query(
4242
transaction: &Transaction,
4343
query: impl AsRef<str>,
4444
query_options: Option<QueryOptions>,

0 commit comments

Comments
 (0)