Skip to content

Commit b1e1765

Browse files
AlonNaor22claude
andcommitted
Support field lookup by field.id as fallback to field.label
Extend load_dict and load to resolve fields by field.id when a field.label match is not found. The lookup order is: label first, then id, then raise an error. Section lookup is also extended to support both section.label and section.id. Closes #50 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent e053b06 commit b1e1765

File tree

2 files changed

+210
-23
lines changed

2 files changed

+210
-23
lines changed

src/onepasswordconnectsdk/config.py

Lines changed: 84 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,56 @@ def _vault_uuid_for_field(field: str, vault_tag: dict):
205205
)
206206

207207

208+
def _match_field(field, field_identifier, section_path, sections):
209+
"""Check if a field matches the given identifier and section.
210+
211+
Args:
212+
field: The item field to check.
213+
field_identifier (str): The value to match against (label or id).
214+
section_path (str): The section part of the field path.
215+
sections (dict): Mapping of section labels/ids to section ids.
216+
217+
Returns:
218+
bool: True if the field matches the identifier and section.
219+
"""
220+
try:
221+
section_id = field.section.id
222+
except AttributeError:
223+
section_id = None
224+
225+
return (
226+
section_id is None
227+
or (section_id == sections.get(section_path))
228+
or section_path in sections.values()
229+
)
230+
231+
232+
def _find_field(item_fields, field_identifier, section_path, sections, match_attr):
233+
"""Search for a field by a given attribute (label or id).
234+
235+
Args:
236+
item_fields (list): The list of fields on the item.
237+
field_identifier (str): The value to match against.
238+
section_path (str): The section part of the field path.
239+
sections (dict): Mapping of section labels/ids to section ids.
240+
match_attr (str): The field attribute to compare against
241+
(e.g. "label" or "id").
242+
243+
Returns:
244+
Field or None: The matching field, or None if not found.
245+
"""
246+
for field in item_fields:
247+
if getattr(field, match_attr, None) == field_identifier:
248+
if _match_field(field, field_identifier, section_path, sections):
249+
return field
250+
return None
251+
252+
253+
# The order in which field attributes are checked during lookup.
254+
# Label is tried first; if no match, id is tried as a fallback.
255+
FIELD_MATCH_ATTRIBUTES = ("label", "id")
256+
257+
208258
def _set_values_for_item(
209259
client: "Client",
210260
parsed_item: ParsedItem,
@@ -230,38 +280,49 @@ def _set_values_for_item(
230280
{parsed_field.name}"
231281
)
232282

233-
value_found = False
234-
for field in item.fields:
235-
try:
236-
section_id = field.section.id
237-
except AttributeError:
238-
section_id = None
239-
240-
if field.label == path_parts[1]:
241-
if (
242-
section_id is None
243-
or (section_id == sections.get(path_parts[0]))
244-
or path_parts[0] in sections.values()
245-
):
246-
value_found = True
247-
248-
if config_object:
249-
setattr(config_object, parsed_field.name, field.value)
250-
else:
251-
config_dict[parsed_field.name] = field.value
252-
break
253-
if not value_found:
283+
section_path = path_parts[0]
284+
field_identifier = path_parts[1]
285+
286+
matched_field = None
287+
for attr in FIELD_MATCH_ATTRIBUTES:
288+
matched_field = _find_field(
289+
item.fields, field_identifier, section_path, sections, attr
290+
)
291+
if matched_field:
292+
break
293+
294+
if matched_field:
295+
if config_object:
296+
setattr(config_object, parsed_field.name, matched_field.value)
297+
else:
298+
config_dict[parsed_field.name] = matched_field.value
299+
else:
254300
raise UnknownSectionAndFieldTag(
255-
f"There is no section {path_parts[0]} \
256-
for field {path_parts[1]}"
301+
f"There is no section {section_path} \
302+
for field {field_identifier}"
257303
)
258304

259305

260306
def _convert_sections_to_dict(sections: List[Section]):
307+
"""Convert a list of sections into a lookup dict.
308+
309+
Builds a mapping that supports lookup by both section label and
310+
section id. Label-based keys take priority — an id-based key is
311+
only added when it does not collide with an existing label key.
312+
313+
Args:
314+
sections (List[Section]): The sections from a 1Password item.
315+
316+
Returns:
317+
dict: A mapping of section label/id to section id.
318+
"""
261319
if not sections:
262320
return {}
263321

264322
section_dict = {section.label: section.id for section in sections}
323+
for section in sections:
324+
if section.id not in section_dict:
325+
section_dict[section.id] = section.id
265326
return section_dict
266327

267328

src/tests/test_config.py

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
USERNAME_VALUE = "new_user"
1515
PASSWORD_VALUE = "password"
1616
HOST_VALUE = "http://somehost"
17+
API_KEY_VALUE = "sk-test-abc123"
18+
DB_PORT_VALUE = "5432"
1719

1820

1921
class Config:
@@ -118,3 +120,127 @@ def test_load_dict(respx_mock):
118120
}
119121
]
120122
}
123+
124+
# Item with field ids that differ from their labels, used to test
125+
# the field.id fallback lookup path.
126+
ITEM_NAME3 = "Service Credentials"
127+
ITEM_ID3 = "wepiqdxdzncjtnvmv5fegud4q3"
128+
129+
item_with_distinct_ids = {
130+
"id": ITEM_ID3,
131+
"title": ITEM_NAME3,
132+
"vault": {
133+
"id": VAULT_ID
134+
},
135+
"category": "LOGIN",
136+
"sections": [
137+
{
138+
"id": "Section_A1B2C3",
139+
"label": "api_settings"
140+
}
141+
],
142+
"fields": [
143+
{
144+
"id": "Field_X9Y8Z7",
145+
"label": "API Key",
146+
"value": API_KEY_VALUE,
147+
"section": {
148+
"id": "Section_A1B2C3"
149+
}
150+
},
151+
{
152+
"id": "Field_D4E5F6",
153+
"label": "Database Port",
154+
"value": DB_PORT_VALUE
155+
}
156+
]
157+
}
158+
159+
160+
def test_load_dict_by_field_id(respx_mock):
161+
"""load_dict should resolve fields by id when label doesn't match."""
162+
config_dict = {
163+
"api_key": {
164+
"opitem": ITEM_NAME3,
165+
"opfield": "api_settings.Field_X9Y8Z7",
166+
"opvault": VAULT_ID
167+
},
168+
"db_port": {
169+
"opitem": ITEM_NAME3,
170+
"opfield": ".Field_D4E5F6",
171+
"opvault": VAULT_ID
172+
}
173+
}
174+
175+
respx_mock.get(
176+
f"v1/vaults/{VAULT_ID}/items?filter=title eq \"{ITEM_NAME3}\""
177+
).mock(return_value=Response(200, json=[item_with_distinct_ids]))
178+
respx_mock.get(
179+
f"v1/vaults/{VAULT_ID}/items/{ITEM_ID3}"
180+
).mock(return_value=Response(200, json=item_with_distinct_ids))
181+
182+
config_values = onepasswordconnectsdk.load_dict(SS_CLIENT, config_dict)
183+
184+
assert config_values["api_key"] == API_KEY_VALUE
185+
assert config_values["db_port"] == DB_PORT_VALUE
186+
187+
188+
def test_load_by_field_id(respx_mock):
189+
"""load should resolve fields by id when label doesn't match."""
190+
191+
class ConfigById:
192+
api_key: f'opitem:"{ITEM_NAME3}" opfield:api_settings.Field_X9Y8Z7 opvault:{VAULT_ID}' = None
193+
db_port: f'opitem:"{ITEM_NAME3}" opfield:.Field_D4E5F6 opvault:{VAULT_ID}' = None
194+
195+
respx_mock.get(
196+
f"v1/vaults/{VAULT_ID}/items?filter=title eq \"{ITEM_NAME3}\""
197+
).mock(return_value=Response(200, json=[item_with_distinct_ids]))
198+
respx_mock.get(
199+
f"v1/vaults/{VAULT_ID}/items/{ITEM_ID3}"
200+
).mock(return_value=Response(200, json=item_with_distinct_ids))
201+
202+
config_obj = onepasswordconnectsdk.load(SS_CLIENT, ConfigById())
203+
204+
assert config_obj.api_key == API_KEY_VALUE
205+
assert config_obj.db_port == DB_PORT_VALUE
206+
207+
208+
def test_load_dict_label_takes_priority(respx_mock):
209+
"""When both label and id could match, label should win."""
210+
ambiguous_item = {
211+
"id": "wepiqdxdzncjtnvmv5fegud4q4",
212+
"title": "Ambiguous Item",
213+
"vault": {"id": VAULT_ID},
214+
"category": "LOGIN",
215+
"fields": [
216+
{
217+
"id": "shared_ref",
218+
"label": "wrong_label",
219+
"value": "value_from_id_match"
220+
},
221+
{
222+
"id": "other_id",
223+
"label": "shared_ref",
224+
"value": "value_from_label_match"
225+
}
226+
]
227+
}
228+
229+
config_dict = {
230+
"result": {
231+
"opitem": "Ambiguous Item",
232+
"opfield": ".shared_ref",
233+
"opvault": VAULT_ID
234+
}
235+
}
236+
237+
respx_mock.get(
238+
f"v1/vaults/{VAULT_ID}/items?filter=title eq \"Ambiguous Item\""
239+
).mock(return_value=Response(200, json=[ambiguous_item]))
240+
respx_mock.get(
241+
f"v1/vaults/{VAULT_ID}/items/wepiqdxdzncjtnvmv5fegud4q4"
242+
).mock(return_value=Response(200, json=ambiguous_item))
243+
244+
config_values = onepasswordconnectsdk.load_dict(SS_CLIENT, config_dict)
245+
246+
assert config_values["result"] == "value_from_label_match"

0 commit comments

Comments
 (0)