Skip to content

Commit

Permalink
Fix #242 - Update dates in new changed: lines when override is not us…
Browse files Browse the repository at this point in the history
…ed. (#244)

* Add field type for changed lines.
* Add RPSLObject support for overwriting new changed lines.
* Update dates in new changed: lines when override is not used.
  • Loading branch information
mxsasha authored Jul 8, 2019
1 parent 4a949b7 commit d68ce2a
Show file tree
Hide file tree
Showing 11 changed files with 250 additions and 107 deletions.
2 changes: 1 addition & 1 deletion irrd/integration_tests/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
admin-c: PERSON-TEST
notify: notify@example.com
mnt-by: TEST-MNT
changed: 2017-05-19T12:22:08Z
changed: changed@example.com 20190701 # comment
source: TEST
remarks: remark
"""
Expand Down
23 changes: 23 additions & 0 deletions irrd/rpsl/fields.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import datetime
import re
from typing import List, Type, Optional

Expand Down Expand Up @@ -328,6 +329,28 @@ def parse(self, value: str, messages: RPSLParserMessages, strict_validation=True
return RPSLFieldParseResult(value)


class RPSLChangedField(RPSLTextField):
"""Field for an changed line. Only performs basic validation for email."""
def parse(self, value: str, messages: RPSLParserMessages, strict_validation=True) -> Optional[RPSLFieldParseResult]:
date: Optional[str]
try:
email, date = value.split(' ')
except ValueError:
email = value
date = None

if not re_email.match(email):
messages.error(f'Invalid e-mail address: {email}')
return None
if date:
try:
datetime.datetime.strptime(date, '%Y%m%d')
except ValueError as ve:
messages.error(f'Invalid changed date: {date}: {ve}')
return None
return RPSLFieldParseResult(value)


class RPSLDNSNameField(RPSLTextField):
"""Field for a DNS name, as used in e.g. inet-rtr names."""
keep_case = False
Expand Down
40 changes: 40 additions & 0 deletions irrd/rpsl/parser.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import datetime
import json
import re
from collections import OrderedDict, Counter
Expand Down Expand Up @@ -397,6 +398,45 @@ def _update_attribute_value(self, attribute, new_values):
self._object_data.insert(insert_idx, (attribute, new_value, []))
insert_idx += 1

def overwrite_date_new_changed_attributes(self, existing_obj=None) -> None:
"""
Overwrite the date in any newly added changed: attributes per #242.
Which changed: lines are new is determined by comparing to existing_obj,
which should be another RPSLObject, or None if all changed: lines
should be considered new.
"""
parsed_values_to_overwrite = set(self.parsed_data['changed'])
if existing_obj:
parsed_values_to_overwrite -= set(existing_obj.parsed_data['changed'])

# As the value is already validated by RPSLChangedField,
# we can safely make assumptions on the format.
new_object_data = []
removed_values_with_comment: List[Tuple[int, str]] = []
for idx, (attr_name, attr_value, continuation_chars) in enumerate(self._object_data):
if attr_name == 'changed':
attr_value_clean = attr_value.split('#')[0].strip()
if attr_value_clean in parsed_values_to_overwrite:
removed_values_with_comment.append((idx, attr_value))
continue
new_object_data.append((attr_name, attr_value, continuation_chars))
self._object_data = new_object_data

current_date = datetime.datetime.now().strftime('%Y%m%d')
for idx, value in removed_values_with_comment:
try:
content, comment = map(str.strip, value.split('#'))
except ValueError:
content = value.strip()
comment = ''
email = content.split(' ')[0] # Ignore existing date
if comment:
new_value = f'{email} {current_date} # {comment}'
else:
new_value = f'{email} {current_date}'
self._object_data.insert(idx, ('changed', new_value, []))
self.messages.info(f'Set date in changed line "{value}" to today.')

def __repr__(self):
source = self.parsed_data.get('source', '')
return f'{self.rpsl_object_class}/{self.pk()}/{source}'
Expand Down
36 changes: 18 additions & 18 deletions irrd/rpsl/rpsl_objects.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from .fields import (RPSLTextField, RPSLIPv4PrefixField, RPSLIPv4PrefixesField, RPSLIPv6PrefixField,
RPSLIPv6PrefixesField, RPSLIPv4AddressRangeField, RPSLASNumberField, RPSLASBlockField,
RPSLSetNameField, RPSLEmailField, RPSLDNSNameField, RPSLGenericNameField, RPSLReferenceField,
RPSLReferenceListField, RPSLAuthField, RPSLRouteSetMembersField)
RPSLReferenceListField, RPSLAuthField, RPSLRouteSetMembersField, RPSLChangedField)
from .parser import RPSLObject, UnknownRPSLObjectClassException


Expand All @@ -31,7 +31,7 @@ class RPSLAsBlock(RPSLObject):
('remarks', RPSLTextField(optional=True, multiple=True)),
('notify', RPSLEmailField(optional=True, multiple=True)),
('mnt-by', RPSLReferenceListField(lookup_key=True, multiple=True, referring=['mntner'])),
('changed', RPSLTextField(multiple=True)),
('changed', RPSLChangedField(multiple=True)),
('source', RPSLGenericNameField()),
])

Expand All @@ -47,7 +47,7 @@ class RPSLAsSet(RPSLObject):
('remarks', RPSLTextField(optional=True, multiple=True)),
('notify', RPSLEmailField(optional=True, multiple=True)),
('mnt-by', RPSLReferenceListField(lookup_key=True, multiple=True, referring=['mntner'])),
('changed', RPSLTextField(multiple=True)),
('changed', RPSLChangedField(multiple=True)),
('source', RPSLGenericNameField()),
])

Expand All @@ -71,7 +71,7 @@ class RPSLAutNum(RPSLObject):
('remarks', RPSLTextField(optional=True, multiple=True)),
('notify', RPSLEmailField(optional=True, multiple=True)),
('mnt-by', RPSLReferenceListField(lookup_key=True, optional=True, multiple=True, referring=['mntner'])),
('changed', RPSLTextField(multiple=True)),
('changed', RPSLChangedField(multiple=True)),
('source', RPSLGenericNameField()),
])

Expand All @@ -90,7 +90,7 @@ class RPSLDomain(RPSLObject):
('remarks', RPSLTextField(optional=True, multiple=True)),
('notify', RPSLEmailField(optional=True, multiple=True)),
('mnt-by', RPSLReferenceListField(lookup_key=True, optional=True, multiple=True, referring=['mntner'])),
('changed', RPSLTextField(multiple=True)),
('changed', RPSLChangedField(multiple=True)),
('source', RPSLGenericNameField()),
])

Expand All @@ -106,7 +106,7 @@ class RPSLFilterSet(RPSLObject):
('remarks', RPSLTextField(optional=True, multiple=True)),
('notify', RPSLEmailField(optional=True, multiple=True)),
('mnt-by', RPSLReferenceListField(lookup_key=True, multiple=True, referring=['mntner'])),
('changed', RPSLTextField(multiple=True)),
('changed', RPSLChangedField(multiple=True)),
('source', RPSLGenericNameField()),
])

Expand All @@ -129,7 +129,7 @@ class RPSLInetRtr(RPSLObject):
('remarks', RPSLTextField(optional=True, multiple=True)),
('notify', RPSLEmailField(optional=True, multiple=True)),
('mnt-by', RPSLReferenceListField(lookup_key=True, multiple=True, referring=['mntner'])),
('changed', RPSLTextField(multiple=True)),
('changed', RPSLChangedField(multiple=True)),
('source', RPSLGenericNameField()),
])

Expand All @@ -147,7 +147,7 @@ class RPSLInet6Num(RPSLObject):
('remarks', RPSLTextField(optional=True, multiple=True)),
('notify', RPSLEmailField(optional=True, multiple=True)),
('mnt-by', RPSLReferenceListField(lookup_key=True, multiple=True, referring=['mntner'])),
('changed', RPSLTextField(multiple=True)),
('changed', RPSLChangedField(multiple=True)),
('source', RPSLGenericNameField()),
])

Expand All @@ -165,7 +165,7 @@ class RPSLInetnum(RPSLObject):
('remarks', RPSLTextField(optional=True, multiple=True)),
('notify', RPSLEmailField(optional=True, multiple=True)),
('mnt-by', RPSLReferenceListField(lookup_key=True, multiple=True, referring=['mntner'])),
('changed', RPSLTextField(multiple=True)),
('changed', RPSLChangedField(multiple=True)),
('source', RPSLGenericNameField()),
])

Expand All @@ -182,7 +182,7 @@ class RPSLKeyCert(RPSLObject):
('tech-c', RPSLReferenceField(lookup_key=True, optional=True, multiple=True, referring=['role', 'person'])),
('notify', RPSLEmailField(optional=True, multiple=True)),
('mnt-by', RPSLReferenceListField(lookup_key=True, multiple=True, referring=['mntner'])),
('changed', RPSLTextField(multiple=True)),
('changed', RPSLChangedField(multiple=True)),
('source', RPSLGenericNameField()),
])

Expand Down Expand Up @@ -264,7 +264,7 @@ class RPSLMntner(RPSLObject):
('remarks', RPSLTextField(optional=True, multiple=True)),
('notify', RPSLEmailField(optional=True, multiple=True)),
('mnt-by', RPSLReferenceListField(lookup_key=True, multiple=True, referring=['mntner'])),
('changed', RPSLTextField(multiple=True)),
('changed', RPSLChangedField(multiple=True)),
('source', RPSLGenericNameField()),
])

Expand Down Expand Up @@ -342,7 +342,7 @@ class RPSLPeeringSet(RPSLObject):
('remarks', RPSLTextField(optional=True, multiple=True)),
('notify', RPSLEmailField(optional=True, multiple=True)),
('mnt-by', RPSLReferenceListField(lookup_key=True, multiple=True, referring=['mntner'])),
('changed', RPSLTextField(multiple=True)),
('changed', RPSLChangedField(multiple=True)),
('source', RPSLGenericNameField()),
])

Expand All @@ -358,7 +358,7 @@ class RPSLPerson(RPSLObject):
('remarks', RPSLTextField(optional=True, multiple=True)),
('notify', RPSLEmailField(optional=True, multiple=True)),
('mnt-by', RPSLReferenceListField(lookup_key=True, multiple=True, referring=['mntner'])),
('changed', RPSLTextField(multiple=True)),
('changed', RPSLChangedField(multiple=True)),
('source', RPSLGenericNameField()),
])

Expand All @@ -377,7 +377,7 @@ class RPSLRole(RPSLObject):
('remarks', RPSLTextField(optional=True, multiple=True)),
('notify', RPSLEmailField(optional=True, multiple=True)),
('mnt-by', RPSLReferenceListField(lookup_key=True, multiple=True, referring=['mntner'])),
('changed', RPSLTextField(multiple=True)),
('changed', RPSLChangedField(multiple=True)),
('source', RPSLGenericNameField()),
])

Expand All @@ -401,7 +401,7 @@ class RPSLRoute(RPSLObject):
('remarks', RPSLTextField(optional=True, multiple=True)),
('notify', RPSLEmailField(optional=True, multiple=True)),
('mnt-by', RPSLReferenceListField(lookup_key=True, multiple=True, referring=['mntner'])),
('changed', RPSLTextField(multiple=True)),
('changed', RPSLChangedField(multiple=True)),
('source', RPSLGenericNameField()),
])

Expand All @@ -418,7 +418,7 @@ class RPSLRouteSet(RPSLObject):
('remarks', RPSLTextField(optional=True, multiple=True)),
('notify', RPSLEmailField(optional=True, multiple=True)),
('mnt-by', RPSLReferenceListField(lookup_key=True, multiple=True, referring=['mntner'])),
('changed', RPSLTextField(multiple=True)),
('changed', RPSLChangedField(multiple=True)),
('source', RPSLGenericNameField()),
])

Expand All @@ -442,7 +442,7 @@ class RPSLRoute6(RPSLObject):
('remarks', RPSLTextField(optional=True, multiple=True)),
('notify', RPSLEmailField(optional=True, multiple=True)),
('mnt-by', RPSLReferenceListField(lookup_key=True, multiple=True, referring=['mntner'])),
('changed', RPSLTextField(multiple=True)),
('changed', RPSLChangedField(multiple=True)),
('source', RPSLGenericNameField()),
])

Expand All @@ -459,7 +459,7 @@ class RPSLRtrSet(RPSLObject):
('remarks', RPSLTextField(optional=True, multiple=True)),
('notify', RPSLEmailField(optional=True, multiple=True)),
('mnt-by', RPSLReferenceListField(lookup_key=True, multiple=True, referring=['mntner'])),
('changed', RPSLTextField(multiple=True)),
('changed', RPSLChangedField(multiple=True)),
('source', RPSLGenericNameField()),
])

Expand Down
17 changes: 16 additions & 1 deletion irrd/rpsl/tests/test_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from ..fields import (RPSLIPv4PrefixField, RPSLIPv4PrefixesField, RPSLIPv6PrefixField,
RPSLIPv6PrefixesField, RPSLIPv4AddressRangeField, RPSLASNumberField, RPSLASBlockField,
RPSLSetNameField, RPSLEmailField, RPSLDNSNameField, RPSLGenericNameField, RPSLReferenceField,
RPSLReferenceListField, RPSLTextField, RPSLAuthField, RPSLRouteSetMemberField)
RPSLReferenceListField, RPSLTextField, RPSLAuthField, RPSLRouteSetMemberField, RPSLChangedField)
from ..parser_state import RPSLParserMessages


Expand Down Expand Up @@ -285,6 +285,21 @@ def test_validate_email_field():
assert_validation_err('Invalid e-mail', field.parse, 'a@[192.0.2.2.2]')


def test_validate_changed_field():
field = RPSLChangedField()
messages = RPSLParserMessages()
assert field.parse('foo.bar@example.asia', messages).value == 'foo.bar@example.asia'
assert field.parse('foo.bar@[192.0.2.1] 20190701', messages).value == 'foo.bar@[192.0.2.1] 20190701'
assert field.parse('foo.bar@[2001:db8::1] 19980101', messages).value == 'foo.bar@[2001:db8::1] 19980101'
assert not messages.errors()

assert_validation_err('Invalid e-mail', field.parse, 'foo.bar+baz@')
assert_validation_err('Invalid changed date', field.parse, 'foo.bar@example.com 20191301')
assert_validation_err('Invalid e-mail', field.parse, '\nfoo.bar@example.com \n20190701')
assert_validation_err('Invalid changed date', field.parse, 'foo.bar@example.com \n20190701')
assert_validation_err('Invalid changed date', field.parse, 'foo.bar@example.com 20190701\n')


def test_validate_dns_name_field():
field = RPSLDNSNameField()
messages = RPSLParserMessages()
Expand Down
35 changes: 35 additions & 0 deletions irrd/rpsl/tests/test_rpsl_objects.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import datetime

import pytest
from IPy import IP
from pytest import raises
Expand Down Expand Up @@ -478,3 +480,36 @@ def test_parse(self):
]
assert obj.references_strong_inbound() == set()
assert obj.render_rpsl_text() == rpsl_text


class TestOverwriteDateNewChangedAttributes:
expected_date = datetime.datetime.now().strftime('%Y%m%d')

# This applies to all objects identically - only one test needed
def test_changed_line_overwrite_with_date_and_comment(self):
new_rpsl_text = self._generate_old_new_object('changed: new1@example.com 19980101 # comment')
assert 'changed: changed@example.com 20190701 # comment' in new_rpsl_text
assert f'changed: new1@example.com {self.expected_date} # comment' in new_rpsl_text

def test_changed_line_overwrite_without_comment(self):
new_rpsl_text = self._generate_old_new_object('changed: new1@example.com 19980101')
assert 'changed: changed@example.com 20190701 # comment' in new_rpsl_text
assert f'changed: new1@example.com {self.expected_date}' in new_rpsl_text

def test_changed_line_overwrite_without_date_with_comment(self):
new_rpsl_text = self._generate_old_new_object('changed: new1@example.com#comment')
assert 'changed: changed@example.com 20190701 # comment' in new_rpsl_text
assert f'changed: new1@example.com {self.expected_date} # comment' in new_rpsl_text

def _generate_old_new_object(self, new_changed_line):
rpsl_text = object_sample_mapping[RPSLRouteSet().rpsl_object_class]
obj_current = rpsl_object_from_text(rpsl_text, strict_validation=True)

lines = rpsl_text.splitlines()
lines.insert(4, new_changed_line)
rpsl_text = '\n'.join(lines)
obj_new = rpsl_object_from_text(rpsl_text, strict_validation=True)

obj_new.overwrite_date_new_changed_attributes(obj_current)
assert f'Set date in changed line' in obj_new.messages.infos()[1]
return obj_new.render_rpsl_text()
6 changes: 3 additions & 3 deletions irrd/scripts/tests/test_rpsl_read.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
descr: TEST ASN block
remarks: test remark
mnt-by: TEST-MNT
changed: 2014-02-24T13:15:13Z
changed: changed@example.com 20190701 # comment
tech-c: PERSON-TEST
admin-c: PERSON-TEST
source: TEST
Expand All @@ -22,7 +22,7 @@
remarks: test remark
mnt-by: TEST-MNT
unknown-obj: unknown value should be caught in strict validation
changed: 2014-02-24T13:15:13Z
changed: changed@example.com 20190701 # comment
tech-c: PERSON-TEST
admin-c: PERSON-TEST
source: TEST
Expand All @@ -31,7 +31,7 @@
descr: TEST ASN block
remarks: test remark
mnt-by: TEST-MNT
changed: 2014-02-24T13:15:13Z
changed: changed@example.com 20190701
tech-c: PERSON-TEST
admin-c: PERSON-TEST
source: TEST
Expand Down
20 changes: 19 additions & 1 deletion irrd/updates/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@ def save(self, database_handler: DatabaseHandler) -> None:
logger.info(f'{id(self)}: Saving change for {self.rpsl_obj_new}: deleting current object')
database_handler.delete_rpsl_object(self.rpsl_obj_current)
else:
if not self.used_override:
self.rpsl_obj_new.overwrite_date_new_changed_attributes(self.rpsl_obj_current)
# This call may have emitted a new info message.
self._import_new_rpsl_obj_info_messages()
logger.info(f'{id(self)}: Saving change for {self.rpsl_obj_new}: inserting/updating current object')
database_handler.upsert_rpsl_object(self.rpsl_obj_new)
self.status = UpdateRequestStatus.SAVED
Expand All @@ -138,7 +142,10 @@ def submitter_report(self) -> str:

report = f'{self.request_type_str().title()} {status}: [{self.object_class_str()}] {self.object_pk_str()}\n'
if self.info_messages or self.error_messages:
report += '\n' + self.rpsl_text_submitted + '\n'
if not self.rpsl_obj_new or self.error_messages:
report += '\n' + self.rpsl_text_submitted + '\n'
else:
report += '\n' + self.rpsl_obj_new.render_rpsl_text() + '\n'
report += ''.join([f'ERROR: {e}\n' for e in self.error_messages])
report += ''.join([f'INFO: {e}\n' for e in self.info_messages])
return report
Expand Down Expand Up @@ -251,6 +258,17 @@ def _check_references(self) -> bool:
logger.debug(f'{id(self)}: Reference check succeeded')
return True

def _import_new_rpsl_obj_info_messages(self):
"""
Import new info messages from self.rpsl_obj_new.
This is used after overwrite_date_new_changed_attributes()
is called, as it's called just before saving, but may
emit a new info message.
"""
for info_message in self.rpsl_obj_new.messages.infos():
if info_message not in self.info_messages:
self.info_messages.append(info_message)


def parse_change_requests(requests_text: str,
database_handler: DatabaseHandler,
Expand Down
Loading

0 comments on commit d68ce2a

Please sign in to comment.