-
Notifications
You must be signed in to change notification settings - Fork 107
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.
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
Add versioning #692
base: master
Are you sure you want to change the base?
Add versioning #692
Changes from all commits
f3a0f53
f77edf7
faf7514
33e9cc0
033ceca
ba0b216
4f10cd7
a04f012
fe249ad
676fa48
46d3886
51915a9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,151 @@ | ||
package edu.wpi.grip.core; | ||
|
||
import edu.wpi.grip.core.sockets.InputSocket; | ||
import edu.wpi.grip.core.sockets.OutputSocket; | ||
import edu.wpi.grip.core.sockets.Socket; | ||
|
||
import com.google.common.annotations.VisibleForTesting; | ||
import com.google.common.base.CaseFormat; | ||
|
||
import org.apache.commons.lang.RandomStringUtils; | ||
|
||
import java.util.Arrays; | ||
import java.util.HashMap; | ||
import java.util.HashSet; | ||
import java.util.NoSuchElementException; | ||
import java.util.Set; | ||
|
||
import static com.google.common.base.Preconditions.checkNotNull; | ||
|
||
/** | ||
* A partial implementation of {@code PipelineEntry} that implements the removal and ID methods. | ||
*/ | ||
public abstract class AbstractPipelineEntry implements PipelineEntry { | ||
|
||
@VisibleForTesting | ||
static final IdPool idPool = IdPool.INSTANCE; | ||
|
||
protected final Object removedLock = new Object(); | ||
private boolean removed = false; | ||
private String id; | ||
|
||
/** | ||
* Creates a unique ID string for the given subclass. | ||
* | ||
* @param entryClass the subclass | ||
* | ||
* @return an ID string for the subclass. | ||
*/ | ||
protected static String makeId(Class<? extends PipelineEntry> entryClass) { | ||
String id; | ||
do { | ||
id = CaseFormat.UPPER_CAMEL.to(CaseFormat.LOWER_HYPHEN, entryClass.getSimpleName()) | ||
+ "." + RandomStringUtils.randomAlphanumeric(8); | ||
} while (idPool.checkId(entryClass, id)); | ||
return id; | ||
} | ||
|
||
/** | ||
* Creates a new pipeline entry. | ||
* | ||
* @param id the ID of the new entry. This must be unique for all instances of the concrete class. | ||
* | ||
* @throws NullPointerException if the ID is null | ||
* @throws IllegalArgumentException if the ID is already taken | ||
*/ | ||
protected AbstractPipelineEntry(String id) { | ||
idPool.get(getClass()).add(id); | ||
this.id = id; | ||
} | ||
|
||
@Override | ||
public final Socket getSocketByUid(String uid) throws NoSuchElementException { | ||
checkNotNull(uid, "UID"); | ||
for (InputSocket in : getInputSockets()) { | ||
if (in.getUid().equals(uid)) { | ||
return in; | ||
} | ||
} | ||
for (OutputSocket out : getOutputSockets()) { | ||
if (out.getUid().equals(uid)) { | ||
return out; | ||
} | ||
} | ||
throw new NoSuchElementException(uid); | ||
} | ||
|
||
/** | ||
* Cleans up this entry, such as by freeing resources or disabling callbacks. | ||
*/ | ||
protected abstract void cleanUp(); | ||
|
||
@Override | ||
public final void setRemoved() { | ||
synchronized (removedLock) { | ||
cleanUp(); | ||
idPool.removeId(this); | ||
removed = true; | ||
} | ||
} | ||
|
||
@Override | ||
public final boolean removed() { | ||
synchronized (removedLock) { | ||
return removed; | ||
} | ||
} | ||
|
||
@Override | ||
public final void setId(String id) throws NullPointerException, IllegalArgumentException { | ||
checkNotNull(id, "The ID cannot be null"); | ||
boolean inDeserialization = Arrays.stream(new Exception().getStackTrace()) | ||
.map(e -> e.getClassName()) | ||
.anyMatch(n -> n.matches(".*(Step|Source)Converter")); | ||
if (!inDeserialization) { | ||
throw new IllegalStateException( | ||
"This method may only be called during project deserialization"); | ||
} | ||
idPool.get(getClass()).add(id); | ||
this.id = id; | ||
} | ||
|
||
@Override | ||
public final String getId() { | ||
return id; | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return getId(); | ||
} | ||
|
||
/** | ||
* Pool of used IDs. | ||
*/ | ||
@VisibleForTesting | ||
static class IdPool extends HashMap<Class<? extends PipelineEntry>, Set<String>> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Guava has a type for this: |
||
private static final IdPool INSTANCE = new IdPool(); | ||
|
||
/** | ||
* Checks if an ID is already used by an instance of a pipeline entry class. | ||
*/ | ||
public boolean checkId(Class<? extends PipelineEntry> clazz, String id) { | ||
return get(clazz).contains(id); | ||
} | ||
|
||
/** | ||
* Removes the ID of the given entry. | ||
*/ | ||
public void removeId(PipelineEntry e) { | ||
get(e.getClass()).remove(e.getId()); | ||
} | ||
|
||
@Override | ||
@SuppressWarnings("unchecked") | ||
public Set<String> get(Object key) { | ||
return computeIfAbsent((Class<? extends PipelineEntry>) key, k -> new HashSet<>()); | ||
} | ||
|
||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,18 @@ | ||
package edu.wpi.grip.core; | ||
|
||
import edu.wpi.grip.core.events.OperationAddedEvent; | ||
import edu.wpi.grip.core.sockets.Socket; | ||
|
||
import com.google.common.collect.ImmutableList; | ||
import com.google.common.eventbus.Subscribe; | ||
|
||
import java.util.Collection; | ||
import java.util.LinkedHashMap; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Optional; | ||
import java.util.function.Function; | ||
import java.util.stream.Collectors; | ||
|
||
import javax.inject.Singleton; | ||
|
||
|
@@ -24,10 +29,54 @@ public class Palette { | |
|
||
@Subscribe | ||
public void onOperationAdded(OperationAddedEvent event) { | ||
final OperationMetaData operation = event.getOperation(); | ||
map(operation.getDescription().name(), operation); | ||
for (String alias : operation.getDescription().aliases()) { | ||
map(alias, operation); | ||
final OperationMetaData operationData = event.getOperation(); | ||
map(operationData.getDescription().name(), operationData); | ||
for (String alias : operationData.getDescription().aliases()) { | ||
map(alias, operationData); | ||
} | ||
// Validate that every input and output socket has a unique name and UID | ||
Operation operation = operationData.getOperationSupplier().get(); | ||
try { | ||
final List<? extends Socket> sockets = new ImmutableList.Builder<Socket>() | ||
.addAll(operation.getInputSockets()) | ||
.addAll(operation.getOutputSockets()) | ||
.build(); | ||
checkDuplicates( | ||
operationData, | ||
"input socket names", | ||
operation.getInputSockets(), s -> s.getSocketHint().getIdentifier() | ||
); | ||
checkDuplicates( | ||
operationData, | ||
"output socket names", | ||
operation.getOutputSockets(), s -> s.getSocketHint().getIdentifier() | ||
); | ||
checkDuplicates(operationData, "socket IDs", sockets, Socket::getUid); | ||
} finally { | ||
operation.cleanUp(); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not a huge fan of creating an operation when the user hasn't asked the UUID to create one. Perhaps put this into a test that loads every operation? |
||
} | ||
|
||
private static <T, U> void checkDuplicates(OperationMetaData operationMetaData, | ||
String type, | ||
List<T> list, | ||
Function<T, U> extractionFunction) { | ||
List<U> duplicates = list.stream() | ||
.map(extractionFunction) | ||
.collect(Collectors.toList()); | ||
list.stream() | ||
.map(extractionFunction) | ||
.distinct() | ||
.forEach(duplicates::remove); | ||
if (!duplicates.isEmpty()) { | ||
throw new IllegalArgumentException( | ||
String.format( | ||
"Duplicate %s found in operation %s: %s", | ||
type, | ||
operationMetaData.getDescription().name(), | ||
duplicates | ||
) | ||
); | ||
} | ||
} | ||
|
||
|
@@ -36,6 +85,7 @@ public void onOperationAdded(OperationAddedEvent event) { | |
* | ||
* @param key The key the operation should be mapped to | ||
* @param operation The operation to map the key to | ||
* | ||
* @throws IllegalArgumentException if the key is already in the {@link #operations} map. | ||
*/ | ||
private void map(String key, OperationMetaData operation) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
package edu.wpi.grip.core; | ||
|
||
import edu.wpi.grip.core.sockets.InputSocket; | ||
import edu.wpi.grip.core.sockets.OutputSocket; | ||
import edu.wpi.grip.core.sockets.Socket; | ||
|
||
import java.util.List; | ||
import java.util.NoSuchElementException; | ||
|
||
/** | ||
* An entry in the pipeline. | ||
*/ | ||
public interface PipelineEntry { | ||
|
||
/** | ||
* Gets a read-only list of the input sockets to this entry. | ||
* This may be empty, but may never be null. | ||
*/ | ||
List<InputSocket> getInputSockets(); | ||
|
||
/** | ||
* Gets a read-only list of the output sockets from this entry. | ||
* This may be empty, but may never be null. | ||
*/ | ||
List<OutputSocket> getOutputSockets(); | ||
|
||
/** | ||
* Gets the socket with the given ID in this entry. | ||
* | ||
* @param uid the UID of the socket to get | ||
* | ||
* @throws NoSuchElementException if there is no socket with the given UID, | ||
*/ | ||
Socket getSocketByUid(String uid) throws NoSuchElementException; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Never mind. I understand now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Part of the contract is that the socket ID has to be unique within its parent There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add a unit test that guarantees this contract for all operations? |
||
|
||
/** | ||
* Sets this entry as removed from the pipeline. | ||
*/ | ||
void setRemoved(); | ||
|
||
/** | ||
* Checks if this entry has been removed from the pipeline. | ||
*/ | ||
boolean removed(); | ||
|
||
/** | ||
* Sets the ID of this entry. This should only be used by deserialization. | ||
* | ||
* @param newId the new ID | ||
* | ||
* @throws NullPointerException if the ID is null | ||
* @throws IllegalArgumentException if the ID is already taken | ||
*/ | ||
void setId(String newId) throws NullPointerException, IllegalArgumentException; | ||
|
||
/** | ||
* Gets the ID of this entry. This ID should be unique for all instances of the concrete class. | ||
*/ | ||
String getId(); | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just make this package private? Or hide it in some other way?