Skip to content

Remove unused Resource methods, add tests for existing methods #69

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.

Sign up for GitHub

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

Merged
EdwardLarson merged 12 commits into from
Oct 17, 2022
Merged

Remove unused Resource methods, add tests for existing methods #69

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
f0256ac
Remove unused, untested methods from ofrak.Resource
whyitfor Oct 7, 2022
f1c04c2
Fix formatting issue
whyitfor Oct 7, 2022
199b535
Add tests for Resource.{is_modified, summarize, get_data_range_within…
whyitfor Oct 7, 2022
bc00f5b
Remove Resource.get_offset_within_root; add test for Resource.identify
whyitfor Oct 7, 2022
2e06b60
Fix bug introduced during refactoring
whyitfor Oct 8, 2022
53a5cc5
Remove singe-line type variables
whyitfor Oct 8, 2022
f05c567
Remove Resource.get_siblings_as_view and Resource.get_siblings
whyitfor Oct 8, 2022
7f8dbfe
Add tests for uncovered functions
whyitfor Oct 8, 2022
6a1ced4
Merge branch 'redballoonsecurity:master' into maintenance/resource
whyitfor Oct 11, 2022
8b418db
Test Resource.__repr__ directly
whyitfor Oct 11, 2022
1c20c3c
Add test for Resource.remove_attributes.
whyitfor Oct 11, 2022
e542cfa
Bump function coverage to 83
whyitfor Oct 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 1 addition & 42 deletions docs/user-guide/resource_view.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,48 +76,7 @@ print(my_sym.name) # >> "foo"
print(hex(my_sym.vaddr)) # >> "0x1000200"
```

## Advanced ResourceView Usage

Now that we have the basics out of the way, let's jump forward a few steps:

```python
@dataclass
class Symbol(ResourceView):
name: str
vaddr: int

@index
def Vaddr(self) -> int:
return self.vaddr

async def get_alternative_names(self) -> Iterable[Symbol]:
"""
This symbol is one possible name for a particular address. This method finds other names for
the same address. It assumes all symbols are children of one parent resource (all symbols are
siblings)
"""
return self.resource.get_siblings_as_view(
Symbol,
r_filter=ResourceFilter(
tags=(Symbol,),
attribute_filters=(
ResourceAttributeValueFilter(Symbol.Vaddr, self.vaddr),
)
)
)
```

Now `Symbol` has been expanded to show off some of the other uses of a view:

* Indexes can be added
to views just like they can be added to `ResourceAttributes` classes. Adding an index allows us
to filter and sort resources with these attributes by some value.

* Views can have methods, and these methods have direct access to the fields of the view (e.g.
`self.vaddr`).

* Views provide a way to access the underlying resource (if it exists). The `get_alternative_names`
method accesses `self.resource` to make a query to fetch more resources. `.resource` returns a
ResourceViews provide a way to access the underlying resource (if it exists). `.resource` returns a
`Resource` and is how you should access the resource when you need it. If the view does
not have an underlying resource, a `ValueError` is raised:

Expand Down
4 changes: 2 additions & 2 deletions examples/ex2_simple_code_modification.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,9 @@ async def main(ofrak_context: OFRAKContext, file_path: str, output_file_name: st
)

# Patch in the modified bytes
ret_instruction_offset = await ret_instruction.resource.get_offset_within_root()
range_in_root = await ret_instruction.resource.get_data_range_within_root()
binary_injector_config = BinaryPatchConfig(
ret_instruction_offset,
range_in_root.start,
new_instruction_bytes,
)
await binary_resource.run(BinaryPatchModifier, binary_injector_config)
Expand Down
2 changes: 1 addition & 1 deletion ofrak_core/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ inspect:
.PHONY: test
test: inspect
$(PYTHON) -m pytest test_ofrak --cov=ofrak --cov-report=term-missing
fun-coverage --cov-fail-under=79
fun-coverage --cov-fail-under=83

.PHONY: dependencies
dependencies:
169 changes: 9 additions & 160 deletions ofrak_core/ofrak/resource.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

from ofrak.component.interface import ComponentInterface
from ofrak.model.component_model import ComponentContext, CC, ComponentRunResult
from ofrak.model.data_model import DataPatch, DataMove
from ofrak.model.data_model import DataPatch
from ofrak.model.job_model import (
JobRunContext,
)
Expand Down Expand Up @@ -184,21 +184,6 @@ async def get_data_length(self) -> int:
)
return await self._data_service.get_data_length(self._resource.data_id)

async def get_data_index_within_parent(self) -> int:
"""
Data is stored as a tree structure. Each data ID corresponds to a node; nodes's children
are sorted by offset. The index of a node in their parent's list of children indicates
the relative ordering of the child resources which correspond to those child nodes.

:return: The relative position of this resource's data node in the parent
"""
if self._resource.data_id is None:
raise ValueError(
"Resource does not have a data_id. Cannot get data index from a "
"resource with no data."
)
return await self._data_service.get_index_within_parent(self._resource.data_id)

async def get_data_range_within_parent(self) -> Range:
"""
If this resource is "mapped," i.e. its underlying data is defined as a range of its parent's
Expand Down Expand Up @@ -229,54 +214,6 @@ async def get_data_range_within_root(self) -> Range:
)
return await self._data_service.get_data_range_within_root(self._resource.data_id)

async def get_offset_within_root(self) -> int:
"""
Does the same thing as `get_data_range_within_root`, except it returns the start offset of
the relative range to the root.

:return: The start offset of the root node's data which this resource represents
"""
root_range = await self.get_data_range_within_root()
return root_range.start

async def get_data_unmapped_range(self, offset: int) -> Range:
"""
This resource may have children mapped in at particular ranges of this resource's
underlying binary data. This method gets a range starting at an ``offset`` and ending at
the start of the next range mapped by a child.

:param offset: An offset from the start of this resource's binary data where the unmapped
range should start

:raises OutOfBoundError: If the provided offset is not a valid offset within the resource
:raises AmbiguousOrderError: If there is unmapped data directly before the given offset

:return: A range starting at ``offset`` and ending at the the offset of the start of the
next range mapped by a child or, if the ``offset`` is within a mapped range,
ending at ``offset`` to create a 0-length range
"""
if self._resource.data_id is None:
raise ValueError(
"Resource does not have a data_id. Cannot get data range from a "
"resource with no data."
)
return await self._data_service.get_unmapped_range(self._resource.data_id, offset)

async def set_data_alignment(self, alignment: int):
"""
Set the alignment constraint for the data node associated with this resource. This method
does not modify the resource's data, but sets an alignment value that can be used to
ensure that unpackers and modifiers do not make changes that violate the set alignment.

:param alignment: The new alignment value
"""
if self._resource.data_id is None:
raise ValueError(
"Resource does not have a data_id. Cannot set data alignment for a "
"resource with no data."
)
return await self._data_service.set_alignment(self._resource.data_id, alignment)

async def set_data_overlaps_enabled(self, enable_overlaps: bool):
"""
Enable or disable allowing overlaps for the data node associated with this resource. If
Expand Down Expand Up @@ -323,6 +260,13 @@ async def save(self):
return

async def _fetch(self, resource: MutableResourceModel):
"""
Update the local model with the latest version from the resource service. This will fail
if this resource has been modified.

:raises InvalidStateError: If the local resource model has been modified
:raises NotFoundError: If the resource service does not have a model for this resource's ID
"""
if resource.is_modified and not resource.is_deleted:
raise InvalidStateError(
f"Cannot fetch dirty resource {resource.id.hex()} (resource "
Expand Down Expand Up @@ -360,16 +304,6 @@ async def _update_views(self, component_result: ComponentRunResult):
for view in views_in_context.values():
view.set_deleted()

async def fetch(self):
"""
Update the local model with the latest version from the resource service. This will fail
if this resource has been modified.

:raises InvalidStateError: If the local resource model has been modified
:raises NotFoundError: If the resource service does not have a model for this resource's ID
"""
return await self._fetch(self._resource)

async def run(
self,
component_type: Type[ComponentInterface[CC]],
Expand Down Expand Up @@ -664,7 +598,6 @@ async def create_child(
:param data_before: The sibling resource whose data is sequentially before the new resource
:return:
"""
data_model_id: Optional[bytes]
if data_range is not None:
if self._resource.data_id is None:
raise ValueError(
Expand Down Expand Up @@ -805,8 +738,7 @@ def add_view(self, view: ResourceViewInterface):

:param view: An instance of a view
"""
attributes: ResourceAttributes
for attributes in view.get_attributes_instances().values():
for attributes in view.get_attributes_instances().values(): # type: ignore
self.add_attributes(attributes)
self.add_tag(type(view))

Expand Down Expand Up @@ -841,12 +773,6 @@ def get_tags(self, inherit: bool = True) -> Iterable[ResourceTag]:
"""
return self._resource.get_tags(inherit)

def get_related_tags(self, tag: RT) -> List[RT]:
"""
Get all tags associated with the resource which inherit from the given tag (if any).
"""
return self._resource.get_specific_tags(tag)

def has_tag(self, tag: ResourceTag, inherit: bool = True) -> bool:
"""
Determine if the resource is associated with the provided tag.
Expand Down Expand Up @@ -934,13 +860,6 @@ def get_attributes(self, attributes_type: Type[RA]) -> RA:
)
return attributes

def get_all_attributes(self) -> Iterable[ResourceAttributes]:
"""
Get values for all the attributes this resource has.
:return:
"""
return list(self._resource.attributes.values())

def remove_attributes(self, attributes_type: Type[ResourceAttributes]):
Comment thread
whyitfor marked this conversation as resolved.
"""
Remove the value of a given attributes type from this resource, if there is such a value.
Expand Down Expand Up @@ -1017,27 +936,6 @@ def has_component_run(self, component_id: bytes, desired_version: Optional[int]
return True
return version == desired_version

def move(
self,
range: Range,
after: Optional["Resource"] = None,
before: Optional["Resource"] = None,
):
if not self._component_context:
raise InvalidStateError(
f"Cannot remap resource {self._resource.id.hex()} outside of a modifier component"
)
if self._resource.data_id is None:
raise ValueError("Cannot create a data move for a resource with no data")
self._component_context.modification_trackers[self._resource.id].data_moves.append(
DataMove(
range,
self._resource.data_id,
after_data_id=after.get_data_id() if after is not None else None,
before_data_id=before.get_data_id() if before is not None else None,
)
)

def queue_patch(
self,
patch_range: Range,
Expand Down Expand Up @@ -1273,55 +1171,6 @@ async def get_only_descendant(
)
return await self._create_resource(models[0])

async def get_siblings_as_view(
self,
v_type: Type[RV],
r_filter: ResourceFilter = None,
r_sort: ResourceSort = None,
) -> Iterable[RV]:
"""
Get all the siblings (resources which share a parent) of this resource. May optionally
filter the siblings so only those matching certain parameters are returned. May optionally
sort the siblings by an indexable attribute value key. The siblings
will be returned as an instance of the given
[viewable tag][ofrak.model.viewable_tag_model.ViewableResourceTag].

:param v_type: The type of [view][ofrak.resource] to get the siblings as
:param r_filter: Contains parameters which resources must match to be returned, including
any tags it must have and/or values of indexable attributes
:param r_sort: Specifies which indexable attribute to use as the key to sort and the
direction to sort
:return:

:raises NotFoundError: If a filter was provided and no resources match the provided filter
"""
siblings = await self.get_siblings(r_filter, r_sort)
view_tasks = [r.view_as(v_type) for r in siblings]
return await asyncio.gather(*view_tasks)

async def get_siblings(
self,
r_filter: ResourceFilter = None,
r_sort: ResourceSort = None,
) -> Iterable["Resource"]:
"""
Get all the siblings (resources which share a parent) of this resource. May optionally
sort the siblings by an indexable attribute value key. May optionally
filter the siblings so only those matching certain parameters are returned.

:param r_filter: Contains parameters which resources must match to be returned, including
any tags it must have and/or values of indexable attributes
:param r_sort: Specifies which indexable attribute to use as the key to sort and the
direction to sort
:return:

:raises NotFoundError: If a filter was provided and no resources match the provided filter
"""
models = await self._resource_service.get_siblings_by_id(
self._resource.id, r_filter=r_filter, r_sort=r_sort
)
return await self._create_resources(models)

async def get_only_sibling_as_view(
self,
v_type: Type[RV],
Expand Down
Loading