Skip to content

Commit ffce998

Browse files
More code review fixed
1 parent cad91fb commit ffce998

18 files changed

Lines changed: 87 additions & 79 deletions

File tree

docs/internal/resources-framework.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ Generic, `R`-parameterized contracts in `use_cases/`: `ApplyResourceUseCase<R>`
333333
```rust
334334
pub struct ResourceManifest {
335335
#[serde(rename = "$schema")]
336-
pub schema: String, // required — canonical schema URL
336+
pub schema: ResourceSchemaId, // required — canonical schema URL
337337
pub headers: ResourceManifestHeaders,
338338
pub spec: serde_json::Value, // desired state; kind-specific shape
339339
}

src/adapter/graphql/src/queries/resources/models.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ pub struct ResourceAccountSelectorInput {
5656
}
5757

5858
impl ResourceAccountSelectorInput {
59-
pub fn into_manifest_account(self) -> kamu_resources::ResourceManifestAccount {
60-
kamu_resources::ResourceManifestAccount {
59+
pub fn into_manifest_account(self) -> kamu_resources::ResourceAccountRef {
60+
kamu_resources::ResourceAccountRef {
6161
id: self.by_id.map(Into::into),
6262
name: self.by_name.map(|name| name.to_string()),
6363
}

src/app/cli/src/commands/apply_command.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -726,7 +726,7 @@ impl ApplyPrinter<'_> {
726726
"{}: {} -> {}/{}",
727727
style,
728728
source,
729-
kamu_resources::ResourceSchema::display_name(&result.resource.schema),
729+
kamu_resources::ResourceSchemaId::display_name(&result.resource.schema),
730730
result.resource.headers.name
731731
)
732732
}

src/app/cli/src/error.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -461,14 +461,16 @@ impl From<ResourceLookupProblem> for ResourceLookupCliError {
461461
match problem {
462462
ResourceLookupProblem::IDNotFound(err) => Self::IDNotFound(err.0),
463463
ResourceLookupProblem::NameNotFound(err) => Self::NameNotFound {
464-
kind: kamu_resources::ResourceSchema::display_name(&err.kind).to_string(),
464+
kind: kamu_resources::ResourceSchemaId::display_name(&err.kind).to_string(),
465465
name: err.name,
466466
},
467467
ResourceLookupProblem::SchemaMismatch(err) => Self::SchemaMismatch {
468468
id: err.id,
469-
expected_schema: kamu_resources::ResourceSchema::display_name(&err.expected_schema)
470-
.to_string(),
471-
actual_schema: kamu_resources::ResourceSchema::display_name(&err.actual_schema)
469+
expected_schema: kamu_resources::ResourceSchemaId::display_name(
470+
&err.expected_schema,
471+
)
472+
.to_string(),
473+
actual_schema: kamu_resources::ResourceSchemaId::display_name(&err.actual_schema)
472474
.to_string(),
473475
},
474476
}

src/domain/resources/domain/src/manifests/resource_manifest.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,15 @@ use serde::de::{MapAccess, SeqAccess, Visitor};
1313
use serde::ser::SerializeMap;
1414
use serde::{Deserialize, Deserializer, Serialize, Serializer};
1515

16-
use crate::{ResourceID, ResourceName};
16+
use crate::{ResourceID, ResourceName, ResourceSchemaId};
1717

1818
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
1919

2020
#[derive(Debug, Clone, Serialize, Deserialize)]
2121
#[serde(rename_all = "camelCase", deny_unknown_fields)]
2222
pub struct ResourceManifest {
2323
#[serde(rename = "$schema")]
24-
pub schema: String,
24+
pub schema: ResourceSchemaId,
2525
pub headers: ResourceManifestHeaders,
2626
pub spec: serde_json::Value,
2727
}
@@ -32,7 +32,7 @@ pub struct ResourceManifest {
3232
#[serde(rename_all = "camelCase", deny_unknown_fields)]
3333
pub struct ResourceManifestHeaders {
3434
pub id: Option<ResourceID>,
35-
pub account: Option<ResourceManifestAccount>,
35+
pub account: Option<ResourceAccountRef>,
3636
pub name: ResourceName,
3737
pub description: Option<String>,
3838
#[serde(
@@ -53,7 +53,7 @@ pub struct ResourceManifestHeaders {
5353

5454
#[derive(Debug, Clone, Serialize, Deserialize)]
5555
#[serde(rename_all = "camelCase", deny_unknown_fields)]
56-
pub struct ResourceManifestAccount {
56+
pub struct ResourceAccountRef {
5757
pub name: Option<String>,
5858
pub id: Option<odf::AccountID>,
5959
}

src/domain/resources/domain/src/values/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,13 @@ mod resource_headers;
1212
mod resource_headers_input;
1313
mod resource_id;
1414
mod resource_phase;
15-
mod resource_schema;
15+
mod resource_schema_id;
1616
mod resource_warning;
1717

1818
pub use resource_condition::*;
1919
pub use resource_headers::*;
2020
pub use resource_headers_input::*;
2121
pub use resource_id::*;
2222
pub use resource_phase::*;
23-
pub use resource_schema::*;
23+
pub use resource_schema_id::*;
2424
pub use resource_warning::*;

src/domain/resources/domain/src/values/resource_schema.rs renamed to src/domain/resources/domain/src/values/resource_schema_id.rs

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,15 @@ use url::Url;
1515
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
1616

1717
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
18-
pub struct ResourceSchema {
18+
pub struct ResourceSchemaId {
1919
schema: String,
2020
base: String,
2121
context: String,
2222
version: String,
2323
name: String,
2424
}
2525

26-
impl ResourceSchema {
26+
impl ResourceSchemaId {
2727
pub fn parse(schema: &str) -> Result<Self, ParseResourceSchemaError> {
2828
if schema.trim() != schema || schema.is_empty() {
2929
return Err(ParseResourceSchemaError::new(schema));
@@ -104,21 +104,21 @@ impl ResourceSchema {
104104
}
105105
}
106106

107-
impl std::fmt::Display for ResourceSchema {
107+
impl std::fmt::Display for ResourceSchemaId {
108108
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
109109
f.write_str(&self.schema)
110110
}
111111
}
112112

113-
impl FromStr for ResourceSchema {
113+
impl FromStr for ResourceSchemaId {
114114
type Err = ParseResourceSchemaError;
115115

116116
fn from_str(s: &str) -> Result<Self, Self::Err> {
117117
Self::parse(s)
118118
}
119119
}
120120

121-
impl Serialize for ResourceSchema {
121+
impl Serialize for ResourceSchemaId {
122122
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
123123
where
124124
S: Serializer,
@@ -127,7 +127,7 @@ impl Serialize for ResourceSchema {
127127
}
128128
}
129129

130-
impl<'de> Deserialize<'de> for ResourceSchema {
130+
impl<'de> Deserialize<'de> for ResourceSchemaId {
131131
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
132132
where
133133
D: Deserializer<'de>,
@@ -147,9 +147,10 @@ mod tests {
147147

148148
#[test]
149149
fn parses_schema_with_path_base() {
150-
let schema =
151-
ResourceSchema::parse("https://opendatafabric.org/schemas/config/v1alpha1/VariableSet")
152-
.unwrap();
150+
let schema = ResourceSchemaId::parse(
151+
"https://opendatafabric.org/schemas/config/v1alpha1/VariableSet",
152+
)
153+
.unwrap();
153154

154155
assert_eq!(
155156
schema.as_str(),
@@ -166,7 +167,7 @@ mod tests {
166167
#[test]
167168
fn parses_schema_with_host_only_base() {
168169
let schema =
169-
ResourceSchema::parse("https://schemas.partner.org/billing/v1/Invoice").unwrap();
170+
ResourceSchemaId::parse("https://schemas.partner.org/billing/v1/Invoice").unwrap();
170171

171172
assert_eq!(
172173
schema.as_str(),
@@ -195,7 +196,7 @@ mod tests {
195196
"https://schemas.partner.org/billing//v1/Invoice",
196197
] {
197198
assert!(
198-
ResourceSchema::parse(invalid_schema).is_err(),
199+
ResourceSchemaId::parse(invalid_schema).is_err(),
199200
"schema should be rejected: {invalid_schema}"
200201
);
201202
}

src/domain/resources/facade-tests/tests/contract/account_scoping.rs

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
// by the Apache License, Version 2.0.
99

1010
use database_common::PaginationOpts;
11-
use kamu_resources::{ResourceID, ResourceManifestAccount};
11+
use kamu_resources::{ResourceAccountRef, ResourceID};
1212
use kamu_resources_facade::{
1313
ApplyManifestError,
1414
ApplyManifestRequest,
@@ -59,7 +59,7 @@ async fn create_with_account_selector(
5959
h: &impl FacadeContractHarness,
6060
facade_account: TestAccount,
6161
name: &str,
62-
selector: ResourceManifestAccount,
62+
selector: ResourceAccountRef,
6363
) -> ResourceID {
6464
apply_manifest_and_get_id(
6565
h,
@@ -69,10 +69,7 @@ async fn create_with_account_selector(
6969
.await
7070
}
7171

72-
fn variable_set_manifest_json_with_account(
73-
name: &str,
74-
account: &ResourceManifestAccount,
75-
) -> String {
72+
fn variable_set_manifest_json_with_account(name: &str, account: &ResourceAccountRef) -> String {
7673
let mut account_fields = Vec::new();
7774
if let Some(n) = &account.name {
7875
account_fields.push(format!(r#""name": "{n}""#));
@@ -98,42 +95,42 @@ fn variable_set_manifest_json_with_account(
9895
)
9996
}
10097

101-
fn account_by_name(name: &odf::AccountName) -> ResourceManifestAccount {
102-
ResourceManifestAccount {
98+
fn account_by_name(name: &odf::AccountName) -> ResourceAccountRef {
99+
ResourceAccountRef {
103100
name: Some(name.to_string()),
104101
id: None,
105102
}
106103
}
107104

108-
fn account_by_id(id: odf::AccountID) -> ResourceManifestAccount {
109-
ResourceManifestAccount {
105+
fn account_by_id(id: odf::AccountID) -> ResourceAccountRef {
106+
ResourceAccountRef {
110107
name: None,
111108
id: Some(id),
112109
}
113110
}
114111

115-
fn account_by_name_and_id(name: &odf::AccountName, id: odf::AccountID) -> ResourceManifestAccount {
116-
ResourceManifestAccount {
112+
fn account_by_name_and_id(name: &odf::AccountName, id: odf::AccountID) -> ResourceAccountRef {
113+
ResourceAccountRef {
117114
name: Some(name.to_string()),
118115
id: Some(id),
119116
}
120117
}
121118

122-
fn unknown_account_by_name() -> ResourceManifestAccount {
123-
ResourceManifestAccount {
119+
fn unknown_account_by_name() -> ResourceAccountRef {
120+
ResourceAccountRef {
124121
name: Some("unknown-resource-contract-account".to_string()),
125122
id: None,
126123
}
127124
}
128125

129-
fn unknown_account_by_id() -> ResourceManifestAccount {
130-
ResourceManifestAccount {
126+
fn unknown_account_by_id() -> ResourceAccountRef {
127+
ResourceAccountRef {
131128
name: None,
132129
id: Some(odf::AccountID::new_generated_ed25519().1),
133130
}
134131
}
135132

136-
fn selector_by_name(name: &str, account: Option<ResourceManifestAccount>) -> ResourceSelector {
133+
fn selector_by_name(name: &str, account: Option<ResourceAccountRef>) -> ResourceSelector {
137134
ResourceSelector {
138135
account,
139136
kind: VARIABLE_SET_KIND.to_string(),
@@ -143,7 +140,7 @@ fn selector_by_name(name: &str, account: Option<ResourceManifestAccount>) -> Res
143140

144141
fn batch_selector_by_name(
145142
name: &str,
146-
account: Option<ResourceManifestAccount>,
143+
account: Option<ResourceAccountRef>,
147144
) -> ResourceBatchSelector {
148145
ResourceBatchSelector {
149146
account,

src/domain/resources/facade-tests/tests/contract/batch_ops.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
// the Business Source License, use of this software will be governed
88
// by the Apache License, Version 2.0.
99

10-
use kamu_resources::{ApplyResourceOutcome, ResourceManifestAccount};
10+
use kamu_resources::{ApplyResourceOutcome, ResourceAccountRef};
1111
use kamu_resources_facade::{
1212
ApplyManifestRequest,
1313
BatchResourceError,
@@ -241,7 +241,7 @@ pub async fn test_get_many_empty_refs_validates_bad_account(h: &impl FacadeContr
241241
let result = facade
242242
.get_many(
243243
ResourceBatchSelector {
244-
account: Some(ResourceManifestAccount {
244+
account: Some(ResourceAccountRef {
245245
name: Some("unknown-resource-contract-account".to_string()),
246246
id: None,
247247
}),
@@ -270,7 +270,7 @@ contract_test!(
270270
pub async fn test_batch_empty_refs_validation_is_consistent(h: &impl FacadeContractHarness) {
271271
let facade = h.facade_for(TestAccount::Alice);
272272
let bad_kind = "NoSuchResourceKindXYZ";
273-
let bad_account = Some(ResourceManifestAccount {
273+
let bad_account = Some(ResourceAccountRef {
274274
name: Some("unknown-resource-contract-account".to_string()),
275275
id: None,
276276
});

src/domain/resources/facade-tests/tests/contract/error_taxonomy.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
//! account-accepting API (not demoted to internal errors).
1717
1818
use database_common::PaginationOpts;
19-
use kamu_resources::{ApplyResourceOutcome, ResourceManifestAccount};
19+
use kamu_resources::{ApplyResourceOutcome, ResourceAccountRef};
2020
use kamu_resources_facade::{
2121
ApplyManifestError,
2222
ApplyManifestRequest,
@@ -343,7 +343,7 @@ contract_test!(bad_account_taxonomy, super::test_bad_account_taxonomy);
343343
pub async fn test_bad_account_taxonomy(h: &impl FacadeContractHarness) {
344344
let facade = h.facade_for(TestAccount::Alice);
345345

346-
let unknown_account = ResourceManifestAccount {
346+
let unknown_account = ResourceAccountRef {
347347
name: Some("unknown-resource-contract-account".to_string()),
348348
id: None,
349349
};

0 commit comments

Comments
 (0)