Skip to content

Commit

Permalink
fix for config errors and added missing rollback for constraint testing
Browse files Browse the repository at this point in the history
  • Loading branch information
@datomo
datomo committed Apr 7, 2024
1 parent 1f4ba2f commit 509ff33
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 92 deletions.
3 changes: 1 addition & 2 deletions config/src/main/java/org/polypheny/db/config/Config.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -959,8 +959,7 @@ protected void notifyConfigListeners() {
boolean validate( final Object i ) {
if ( this.validationMethod != null ) {
return this.validationMethod.validate( i );
} //else if (this.validationMethod == null ) {
else {
} else {
return true;
}
}
Expand Down
6 changes: 4 additions & 2 deletions config/src/main/java/org/polypheny/db/config/Feedback.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@
package org.polypheny.db.config;

import lombok.AllArgsConstructor;
import lombok.Value;

@AllArgsConstructor
@Value
public class Feedback {

public final boolean successful;
public final String message;
public boolean successful;
public String message;


public static Feedback of( boolean successful ) {
Expand Down
178 changes: 98 additions & 80 deletions config/src/main/java/org/polypheny/db/webui/ConfigService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,96 +84,114 @@ private void configRoutes( final Javalin http ) {
StringBuilder feedback = new StringBuilder();
boolean allValid = true;
for ( Map.Entry<String, Object> entry : changes.entrySet() ) {
Config c = cm.getConfig( entry.getKey() );
switch ( c.getConfigType() ) {
case "ConfigInteger":
Double d = (Double) entry.getValue();
if ( !c.setInt( d.intValue() ) ) {
allValid = false;
appendError( feedback, entry, c );
}
break;
case "ConfigDouble":
if ( !c.setDouble( (double) entry.getValue() ) ) {
allValid = false;
appendError( feedback, entry, c );
}
break;
case "ConfigDecimal":
if ( !c.setDecimal( (BigDecimal) entry.getValue() ) ) {
allValid = false;
appendError( feedback, entry, c );
}
break;
case "ConfigLong":
if ( !c.setLong( (long) entry.getValue() ) ) {
allValid = false;
appendError( feedback, entry, c );
}
case "ConfigString":
if ( !c.setString( (String) entry.getValue() ) ) {
allValid = false;
appendError( feedback, entry, c );
}
break;
case "ConfigBoolean":
if ( !c.setBoolean( (boolean) entry.getValue() ) ) {
allValid = false;
appendError( feedback, entry, c );
}
break;
case "ConfigClazz":
case "ConfigEnum":
if ( !c.parseStringAndSetValue( (String) entry.getValue() ) ) {
allValid = false;
appendError( feedback, entry, c );
}
break;
case "ConfigClazzList":
case "ConfigEnumList":
try {
if ( !c.parseStringAndSetValue( mapper.writeValueAsString( entry.getValue() ) ) ) {
allValid = false;
appendError( feedback, entry, c );
}
} catch ( JsonProcessingException e ) {
allValid = false;
appendError( feedback, entry, c );
}

break;
case "ConfigList":
Feedback res = c.setConfigObjectList( (List<Object>) entry.getValue(), c.getTemplateClass() );
if ( !res.successful ) {
allValid = false;
if ( res.message.trim().isEmpty() ) {
appendError( feedback, entry, c );
} else {
feedback.append( "Could not set " )
.append( c.getKey() )
.append( " due to: " )
.append( res.message )
.append( " " );
}

}
break;
default:
allValid = false;
feedback.append( "Config with type " ).append( c.getConfigType() ).append( " is not supported yet." );
log.error( "Config with type {} is not supported yet.", c.getConfigType() );
try {
allValid = trySetConfig( entry, cm, allValid, feedback );
} catch ( Exception e ) {
allValid = false;
feedback.append( "Could not set " )
.append( entry.getKey() )
.append( " to " )
.append( entry.getValue() )
.append( " because of: " )
.append( e.getMessage() )
.append( " " );
}

}
if ( allValid ) {
ctx.result( "{\"success\":1}" );
ctx.json( new Feedback( true, "All values were saved." ) );
} else {
feedback.append( "All other values were saved." );
ctx.result( "{\"warning\": \"" + feedback + "\"}" );
ctx.json( new Feedback( false, feedback.toString() ) );
}
} );
}


private boolean trySetConfig( Entry<String, Object> entry, ConfigManager cm, boolean allValid, StringBuilder feedback ) {
Config c = cm.getConfig( entry.getKey() );
switch ( c.getConfigType() ) {
case "ConfigInteger":
Double d = (Double) entry.getValue();
if ( !c.setInt( d.intValue() ) ) {
allValid = false;
appendError( feedback, entry, c );
}
break;
case "ConfigDouble":
if ( !c.setDouble( (double) entry.getValue() ) ) {
allValid = false;
appendError( feedback, entry, c );
}
break;
case "ConfigDecimal":
if ( !c.setDecimal( (BigDecimal) entry.getValue() ) ) {
allValid = false;
appendError( feedback, entry, c );
}
break;
case "ConfigLong":
if ( !c.setLong( (long) entry.getValue() ) ) {
allValid = false;
appendError( feedback, entry, c );
}
case "ConfigString":
if ( !c.setString( (String) entry.getValue() ) ) {
allValid = false;
appendError( feedback, entry, c );
}
break;
case "ConfigBoolean":
if ( !c.setBoolean( (boolean) entry.getValue() ) ) {
allValid = false;
appendError( feedback, entry, c );
}
break;
case "ConfigClazz":
case "ConfigEnum":
if ( !c.parseStringAndSetValue( (String) entry.getValue() ) ) {
allValid = false;
appendError( feedback, entry, c );
}
break;
case "ConfigClazzList":
case "ConfigEnumList":
try {
if ( !c.parseStringAndSetValue( mapper.writeValueAsString( entry.getValue() ) ) ) {
allValid = false;
appendError( feedback, entry, c );
}
} catch ( JsonProcessingException e ) {
allValid = false;
appendError( feedback, entry, c );
}

break;
case "ConfigList":
Feedback res = c.setConfigObjectList( (List<Object>) entry.getValue(), c.getTemplateClass() );
if ( !res.successful ) {
allValid = false;
if ( res.message.trim().isEmpty() ) {
appendError( feedback, entry, c );
} else {
feedback.append( "Could not set " )
.append( c.getKey() )
.append( " due to: " )
.append( res.message )
.append( " " );
}

}
break;
default:
allValid = false;
feedback.append( "Config with type " ).append( c.getConfigType() ).append( " is not supported yet." );
log.error( "Config with type {} is not supported yet.", c.getConfigType() );
}
return allValid;
}


private static void appendError( StringBuilder feedback, Entry<String, Object> entry, Config c ) {
feedback.append( "Could not set " )
.append( c.getKey() )
Expand Down
28 changes: 20 additions & 8 deletions dbms/src/main/java/org/polypheny/db/processing/ConstraintEnforceAttacher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.util.stream.IntStream;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.NotImplementedException;
import org.jetbrains.annotations.Nullable;
import org.polypheny.db.PolyImplementation;
import org.polypheny.db.algebra.AlgNode;
import org.polypheny.db.algebra.AlgRoot;
Expand Down Expand Up @@ -85,6 +86,7 @@
import org.polypheny.db.transaction.TransactionException;
import org.polypheny.db.transaction.TransactionManager;
import org.polypheny.db.type.entity.PolyValue;
import org.polypheny.db.util.Pair;

@Slf4j
public class ConstraintEnforceAttacher {
Expand Down Expand Up @@ -615,23 +617,25 @@ public ConstraintTracker( TransactionManager manager ) {

@Override
public void onConfigChange( Config c ) {
if ( !testConstraintsValid() ) {
Pair<Boolean, String> validError = testConstraintsValid();
if ( !validError.getKey() ) {
c.setBoolean( !c.getBoolean() );
throw new GenericRuntimeException( "Could not change the constraints." );
throw new GenericRuntimeException( "Could not change the constraints. \n" + validError.getValue() );
}
}


@Override
public void restart( Config c ) {
if ( !testConstraintsValid() ) {
Pair<Boolean, String> validError = testConstraintsValid();
if ( !validError.getKey() ) {
c.setBoolean( !c.getBoolean() );
throw new GenericRuntimeException( "After restart the constraints where not longer enforceable." );
throw new GenericRuntimeException( "After restart the constraints where not longer enforceable. \n" + validError.getValue() );
}
}


private boolean testConstraintsValid() {
private Pair<Boolean, @Nullable String> testConstraintsValid() {
if ( RuntimeConfig.FOREIGN_KEY_ENFORCEMENT.getBoolean() || RuntimeConfig.UNIQUE_CONSTRAINT_ENFORCEMENT.getBoolean() ) {
try {
List<LogicalTable> tables = Catalog
Expand All @@ -658,20 +662,28 @@ private boolean testConstraintsValid() {

if ( !rows.isEmpty() ) {
int index = rows.get( 0 ).get( 0 ).get( 1 ).asNumber().intValue();
if ( statement.getTransaction() != null ) {
statement.getTransaction().rollback();
}

throw new TransactionException( infos.get( 0 ).errorMessages().get( index ) + "\nThere are violated constraints, the transaction was rolled back!" );
}
try {
statement.getTransaction().commit();
} catch ( TransactionException e ) {
throw new GenericRuntimeException( "Error while committing constraint enforcement check." );
if ( statement.getTransaction() != null ) {
statement.getTransaction().rollback();
}

throw new GenericRuntimeException( "Error while committing constraint enforcement check, the transaction was rolled back!" );
}


} catch ( TransactionException e ) {
return false;
return Pair.of( false, e.getMessage() );
}
}
return true;
return Pair.of( true, null );
}

}
Expand Down

0 comments on commit 509ff33

Please sign in to comment.