Skip to content

Commit

Permalink
Dbf 71 delete syntactically incorrect objs (#403)
Browse files Browse the repository at this point in the history 
* Basic fix for missing auth attribute in old mntner objects causing deletion problem

* now uses a property map instead of single named properties

* fixed property

* property missing from test properties file

* fixed possible null pointer exception

* missed a final

* do not need to substitute '-' by using literals as keys in properties
  • Loading branch information
Rob Miller authored Sep 13, 2016
1 parent 506285d commit f5b3615
Show file tree
Hide file tree
Showing 4 changed files with 134 additions and 13 deletions.
73 changes: 64 additions & 9 deletions whois-api/src/main/java/net/ripe/db/whois/api/rest/ReferencesService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@
import net.ripe.db.whois.common.dao.RpslObjectDao;
import net.ripe.db.whois.common.dao.RpslObjectInfo;
import net.ripe.db.whois.common.dao.RpslObjectUpdateDao;
import net.ripe.db.whois.common.rpsl.AttributeTemplate;
import net.ripe.db.whois.common.rpsl.AttributeType;
import net.ripe.db.whois.common.rpsl.ObjectTemplate;
import net.ripe.db.whois.common.rpsl.ObjectType;
import net.ripe.db.whois.common.rpsl.RpslAttribute;
import net.ripe.db.whois.common.rpsl.RpslObject;
Expand Down Expand Up @@ -98,6 +100,7 @@ public class ReferencesService {
private final WhoisService whoisService;
private final LoggerContext loggerContext;
private final WhoisObjectMapper whoisObjectMapper;
private final Map dummyMap;
private final String dummyRole;

@Autowired
Expand All @@ -110,7 +113,7 @@ public ReferencesService(
final WhoisService whoisService,
final LoggerContext loggerContext,
final WhoisObjectMapper whoisObjectMapper,
@Value("${whois.dummy_role.nichdl}") final String dummyRole) {
final @Value("#{${whois.dummy}}") Map<String, String> dummyMap) {

this.rpslObjectDao = rpslObjectDao;
this.rpslObjectUpdateDao = rpslObjectUpdateDao;
Expand All @@ -120,7 +123,8 @@ public ReferencesService(
this.whoisService = whoisService;
this.loggerContext = loggerContext;
this.whoisObjectMapper = whoisObjectMapper;
this.dummyRole = dummyRole;
this.dummyMap = dummyMap;
this.dummyRole = dummyMap.get(AttributeType.ADMIN_C.toString());
}

/**
Expand Down Expand Up @@ -425,8 +429,13 @@ public Response delete(

// update the maintainer to point to a dummy person / role

final RpslObjectWithReplacements tmpMntnerWithReplacements = replaceReferencesInMntner(allObjects);
// replace any possible referenes with dummy values
RpslObjectWithReplacements tmpMntnerWithReplacements = replaceReferencesInMntner(allObjects);

// check objects are correctly formed, modify rpsl if not
tmpMntnerWithReplacements.rpslObject = correctAnySyntaxErrors(tmpMntnerWithReplacements.rpslObject);

// modify maintainer
actionRequests.add(new ActionRequest(tmpMntnerWithReplacements.rpslObject, Action.MODIFY));

// delete the person / role objects
Expand Down Expand Up @@ -540,7 +549,6 @@ private Response performUpdate(
private void validateReferences(final RpslObject primaryObject, final Map<RpslObjectInfo, RpslObject> references) {

// make sure that primary object, and all references, are of a valid type

if (primaryObject.getType().equals(ObjectType.MNTNER)) {

// references must be of person / role only
Expand Down Expand Up @@ -625,10 +633,10 @@ private RpslObjectWithReplacements replaceReferencesInMntner(final Collection<Rp
throw new IllegalStateException("No maintainer found");
}

private RpslObjectWithReplacements replaceReferences(final RpslObject mntner, final Collection<RpslObject> references) {
private RpslObjectWithReplacements replaceReferences(final RpslObject object, final Collection<RpslObject> references) {
final Map<RpslAttribute, RpslAttribute> replacements = Maps.newHashMap();

for (final RpslAttribute rpslAttribute : mntner.getAttributes()) {
for (final RpslAttribute rpslAttribute : object.getAttributes()) {
for (final RpslObject reference : references) {
if (rpslAttribute.getCleanValue().equals(reference.getKey()) &&
rpslAttribute.getType().getReferences().contains(ObjectType.PERSON) ||
Expand All @@ -639,16 +647,63 @@ private RpslObjectWithReplacements replaceReferences(final RpslObject mntner, fi
}

if (replacements.isEmpty()) {
return new RpslObjectWithReplacements(mntner, replacements);
return new RpslObjectWithReplacements(object, replacements);
}

final RpslObjectBuilder builder = new RpslObjectBuilder(mntner);
final RpslObjectBuilder builder = new RpslObjectBuilder(object);
for (Map.Entry<RpslAttribute, RpslAttribute> entry : replacements.entrySet()) {
builder.replaceAttribute(entry.getKey(), entry.getValue());
}
return new RpslObjectWithReplacements(builder.get(), replacements);
}

public RpslObject correctAnySyntaxErrors(RpslObject rpslObject) {
final ObjectType rpslObjectType = rpslObject.getType();
final ObjectTemplate objectTemplate = ObjectTemplate.getTemplate(rpslObjectType);

final Map<AttributeType, Integer> attributeCount = Maps.newEnumMap(AttributeType.class);
final List<AttributeTemplate> attributeTemplates = objectTemplate.getAttributeTemplates();

// determine possible attributes for object type
for (final AttributeTemplate attributeTemplate : objectTemplate.getAttributeTemplates()) {
attributeCount.put(attributeTemplate.getAttributeType(), 0);
}

// count instances of each attribute
for (final RpslAttribute attribute : rpslObject.getAttributes()) {
final AttributeType attributeType = attribute.getType();

if (attributeType != null) {
attributeCount.put(attributeType, attributeCount.get(attributeType) + 1);
}
}

// iterate through possible object attributes and check against what we have in the submitted object
for (final AttributeTemplate attributeTemplate : attributeTemplates) {
final AttributeType attributeType = attributeTemplate.getAttributeType();
final int attributeTypeCount = attributeCount.get(attributeType);

// if we are missing any mandatory attributes add a dummy attribute so the object is valid
if (attributeTemplate.getRequirement() == AttributeTemplate.Requirement.MANDATORY && attributeTypeCount == 0) {
rpslObject = addDummyAttribute(rpslObject, attributeType);
}
}
return rpslObject;
}

public RpslObject addDummyAttribute(RpslObject rpslObject, final AttributeType attributeType) {
final RpslObjectBuilder builder = new RpslObjectBuilder(rpslObject);
final Object dummyValue = dummyMap.get(attributeType.toString());
if(dummyValue != null)
{
final RpslAttribute attr = new RpslAttribute(attributeType, dummyValue.toString());
return builder.addAttributeSorted(attr).get();
}
else {
return rpslObject;
}
}

private boolean referenceMatches(final RpslObjectInfo reference, final RpslObject rpslObject) {
return rpslObject.getKey().equals(reference.getKey()) &&
rpslObject.getType().equals(reference.getObjectType());
Expand Down Expand Up @@ -717,7 +772,7 @@ private Response ok(final Object message) {
// model classes

static class RpslObjectWithReplacements {
private final RpslObject rpslObject;
private RpslObject rpslObject;
private final Map<RpslAttribute, RpslAttribute> replacements;

RpslObjectWithReplacements(final RpslObject rpslObject, final Map<RpslAttribute, RpslAttribute> replacements) {
Expand Down
64 changes: 64 additions & 0 deletions whois-api/src/test/java/net/ripe/db/whois/api/rest/ReferencesServiceTestIntegration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import net.ripe.db.whois.common.IntegrationTest;
import net.ripe.db.whois.common.Message;
import net.ripe.db.whois.common.Messages;
import net.ripe.db.whois.common.dao.jdbc.DatabaseHelper;
import net.ripe.db.whois.common.domain.CIString;
import net.ripe.db.whois.common.domain.User;
import net.ripe.db.whois.common.rpsl.AttributeType;
Expand Down Expand Up @@ -1086,6 +1087,69 @@ public void delete_role_mnter_pair_with_override() {

}

@Test
public void delete_role_mnter_pair_with_override_missing_mandatory_attribute() {
databaseHelper.addObject(
"mntner: ANOTHER-MNT\n" +
"upd-to: [email protected]\n" +
"mnt-by: ANOTHER-MNT\n" +
"source: TEST");
databaseHelper.addObject(
"role: Test Role2\n" +
"nic-hdl: TR2-TEST\n" +
"mnt-by: ANOTHER-MNT\n" +
"source: TEST");
databaseHelper.updateObject(
"mntner: ANOTHER-MNT\n" +
"upd-to: [email protected]\n" +
"admin-c: TR2-TEST\n" +
"mnt-by: ANOTHER-MNT\n" +
"source: TEST");

assertThat(objectExists(ObjectType.MNTNER, "ANOTHER-MNT"), is(true));
assertThat(objectExists(ObjectType.ROLE, "TR2-TEST"), is(true));

RestTest.target(getPort(), "whois/references/TEST/mntner/ANOTHER-MNT")
.queryParam("override", "personadmin,secret,reason")
.request()
.delete();

assertThat(objectExists(ObjectType.MNTNER, "ANOTHER-MNT"), is(false));
assertThat(objectExists(ObjectType.ROLE, "TR2-TEST"), is(false));

}

@Test
public void delete_role_mnter_pair_with_override_missing_mandatory_attribute_not_in_map() {
// upd-to: not in map. no maintainers in db missing this mandatory attr, always been mandatory so should never be missing?
databaseHelper.addObject(
"mntner: ANOTHER-MNT\n" +
"mnt-by: ANOTHER-MNT\n" +
"source: TEST");
databaseHelper.addObject(
"role: Test Role2\n" +
"nic-hdl: TR2-TEST\n" +
"mnt-by: ANOTHER-MNT\n" +
"source: TEST");
databaseHelper.updateObject(
"mntner: ANOTHER-MNT\n" +
"admin-c: TR2-TEST\n" +
"mnt-by: ANOTHER-MNT\n" +
"source: TEST");

assertThat(objectExists(ObjectType.MNTNER, "ANOTHER-MNT"), is(true));
assertThat(objectExists(ObjectType.ROLE, "TR2-TEST"), is(true));

RestTest.target(getPort(), "whois/references/TEST/mntner/ANOTHER-MNT")
.queryParam("override", "personadmin,secret,reason")
.request()
.delete();

assertThat(objectExists(ObjectType.MNTNER, "ANOTHER-MNT"), is(true));
assertThat(objectExists(ObjectType.ROLE, "TR2-TEST"), is(true));

}

@Test
public void delete_mntner_fails_person_referenced_from_another_mntner_with_override() {
databaseHelper.addObject(
Expand Down
5 changes: 3 additions & 2 deletions whois-commons/src/main/resources/whois.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,6 @@ whois.maintainers.alloc=RIPE-NCC-HM-MNT,RIPE-NCC-HM-PI-MNT
whois.maintainers.enum=RIPE-GII-MNT,RIPE-NCC-MNT
whois.maintainers.dbm=RIPE-DBM-MNT,RIPE-NCC-LOCKED-MNT,RIPE-DBM-STARTUP-MNT,RIPE-DBM-UNREFERENCED-CLEANUP-MNT,RIPE-ERX-MNT

whois.dummy_role.nichdl = DR1-TEST

# Source aware data sources
whois.db.driver=org.mariadb.jdbc.Driver

Expand Down Expand Up @@ -106,3 +104,6 @@ internals.database.password=

#Feature Toggles
feature.toggle.changed.attr.available=true

#Dummy attributes
whois.dummy={'auth':'MD5-PW $1$SaltSalt$DummifiedMD5HashValue.', 'tech-c':'DR1-TEST', 'admin-c':'DR1-TEST'}
5 changes: 3 additions & 2 deletions whois-endtoend/src/test/resources/whois.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,6 @@ whois.maintainers.alloc=RIPE-NCC-HM-MNT,RIPE-NCC-HM-PI-MNT
whois.maintainers.enum=RIPE-GII-MNT,RIPE-NCC-MNT
whois.maintainers.dbm=RIPE-DBM-MNT,RIPE-NCC-LOCKED-MNT,RIPE-DBM-STARTUP-MNT,RIPE-DBM-UNREFERENCED-CLEANUP-MNT,RIPE-ERX-MNT

whois.dummy_role.nichdl = DR1-TEST

# Source aware data sources
whois.db.driver=com.mysql.jdbc.Driver

Expand Down Expand Up @@ -105,3 +103,6 @@ internals.database.password=

#Feature Toggles
feature.toggle.changed.attr.available=true

#Dummy attributes
whois.dummy={'auth':'MD5-PW $1$SaltSalt$DummifiedMD5HashValue.', 'tech-c':'DR1-TEST', 'admin-c':'DR1-TEST'}

0 comments on commit f5b3615

Please sign in to comment.