Skip to content

Commit

Permalink
Fix test, remove subjectId from HashedDataFeedKey
Browse files Browse the repository at this point in the history
  • Loading branch information
@at055612
at055612 committed Jan 28, 2025
1 parent 3c9b2d2 commit 631d4e4
Show file tree
Hide file tree
Showing 11 changed files with 98 additions and 79 deletions.
10 changes: 4 additions & 6 deletions stroom-feed/stroom-feed-impl/src/main/java/stroom/feed/impl/FeedResourceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,12 @@ class FeedResourceImpl implements FeedResource {

private final Provider<FeedStore> feedStoreProvider;
private final Provider<DocumentResourceHelper> documentResourceHelperProvider;
private final List<String> supportedEncodings;

@Inject
FeedResourceImpl(final Provider<FeedStore> feedStoreProvider,
final Provider<DocumentResourceHelper> documentResourceHelperProvider) {
this.feedStoreProvider = feedStoreProvider;
this.documentResourceHelperProvider = documentResourceHelperProvider;
final List<String> encodings = new ArrayList<>(feedStoreProvider.get().fetchSupportedEncodings());
// Allow user to select no encoding
encodings.add("");
supportedEncodings = Collections.unmodifiableList(encodings);
}

@Override
Expand All @@ -64,6 +59,9 @@ public FeedDoc update(final String uuid, final FeedDoc doc) {

@Override
public List<String> fetchSupportedEncodings() {
return supportedEncodings;
final List<String> encodings = new ArrayList<>(feedStoreProvider.get().fetchSupportedEncodings());
// Allow user to select no encoding
encodings.add("");
return Collections.unmodifiableList(encodings);
}
}
15 changes: 10 additions & 5 deletions ...ve/stroom-receive-common/src/main/java/stroom/receive/common/CachedHashedDataFeedKey.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,18 @@ public String getHashAlgorithmId() {
}

@NotBlank
public String getSubjectId() {
return hashedDataFeedKey.getSubjectId();
public String getAccountId() {
return hashedDataFeedKey.getAccountId();
}

public String getDisplayName() {
return hashedDataFeedKey.getDisplayName();
}
// @NotBlank
// public String getSubjectId() {
// return hashedDataFeedKey.getSubjectId();
// }
//
// public String getDisplayName() {
// return hashedDataFeedKey.getDisplayName();
// }

public Map<String, String> getStreamMetaData() {
return hashedDataFeedKey.getStreamMetaData();
Expand Down
2 changes: 1 addition & 1 deletion ...receive/stroom-receive-common/src/main/java/stroom/receive/common/DataFeedKeyService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public interface DataFeedKeyService extends AuthenticatorFilter {
Optional<HashedDataFeedKey> getDataFeedKey(final HttpServletRequest request,
final AttributeMap attributeMap);

Optional<HashedDataFeedKey> getDataFeedKey(final String subjectId);
Optional<HashedDataFeedKey> getLatestDataFeedKey(final String accountId);

void addDataFeedKeys(HashedDataFeedKeys hashedDataFeedKeys,
Path sourceFile);
Expand Down
65 changes: 42 additions & 23 deletions ...ive/stroom-receive-common/src/main/java/stroom/receive/common/DataFeedKeyServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,16 @@
import org.apache.commons.lang3.StringUtils;

import java.nio.file.Path;
import java.util.Comparator;
import java.util.EnumMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.Optional;
import java.util.Timer;
import java.util.TimerTask;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Predicate;
import java.util.regex.Pattern;
Expand All @@ -46,9 +48,10 @@ public class DataFeedKeyServiceImpl implements DataFeedKeyService, Managed {
// Holds all the keys read from the data feed key files, entries are evicted when
// the DataFeedKey has passed its expiry date.
private final Map<CacheKey, CachedHashedDataFeedKey> cacheKeyToDataFeedKeyMap = new ConcurrentHashMap<>();
private final Map<String, CachedHashedDataFeedKey> subjectIdToDataFeedKeyMap = new ConcurrentHashMap<>();
// An account will likely have >1 CachedHashedDataFeedKey due to the overlap of keys when
// new keys are being supplied
private final Map<String, List<CachedHashedDataFeedKey>> accountIdToDataFeedKeyMap = new ConcurrentHashMap<>();

// TODO replace with cache
// Cache of the un-hashed key to validated DataFeedKey, to save us the hashing cost
private final LoadingStroomCache<String, Optional<HashedDataFeedKey>> unHashedKeyToDataFeedKeyCache;

Expand Down Expand Up @@ -96,11 +99,23 @@ public Optional<HashedDataFeedKey> getDataFeedKey(final HttpServletRequest reque
}

@Override
public Optional<HashedDataFeedKey> getDataFeedKey(final String subjectId) {
return Optional.ofNullable(subjectIdToDataFeedKeyMap.get(subjectId))
.map(CachedHashedDataFeedKey::getDataFeedKey);
public Optional<HashedDataFeedKey> getLatestDataFeedKey(final String accountId) {
if (accountId == null) {
return Optional.empty();
} else {
return Optional.ofNullable(accountIdToDataFeedKeyMap.get(accountId))
.flatMap(cachedKeys -> cachedKeys.stream()
.max(Comparator.comparing(CachedHashedDataFeedKey::getExpiryDate)))
.map(CachedHashedDataFeedKey::getDataFeedKey);
}
}

// @Override
// public Optional<HashedDataFeedKey> getDataFeedKey(final String subjectId) {
// return Optional.ofNullable(subjectIdToDataFeedKeyMap.get(subjectId))
// .map(CachedHashedDataFeedKey::getDataFeedKey);
// }

@Override
public void addDataFeedKeys(final HashedDataFeedKeys hashedDataFeedKeys,
final Path sourceFile) {
Expand All @@ -122,26 +137,30 @@ private void addDataFeedKey(final CachedHashedDataFeedKey cachedHashedDataFeedKe
final DataFeedKeyHashAlgorithm hashAlgorithm = DataFeedKeyHashAlgorithm.fromUniqueId(hashAlgorithmId);
final CacheKey cacheKey = new CacheKey(hashAlgorithm, hash);
cacheKeyToDataFeedKeyMap.put(cacheKey, cachedHashedDataFeedKey);
subjectIdToDataFeedKeyMap.put(cachedHashedDataFeedKey.getSubjectId(), cachedHashedDataFeedKey);
// Use CopyOnWriteArrayList as write are very infrequent
accountIdToDataFeedKeyMap.computeIfAbsent(
cachedHashedDataFeedKey.getAccountId(),
k -> new CopyOnWriteArrayList<>())
.add(cachedHashedDataFeedKey);
}
}

@Override
public void evictExpired() {
LOGGER.debug("Evicting expired dataFeedKeys");
final AtomicInteger counter = new AtomicInteger();
final Predicate<Entry<?, CachedHashedDataFeedKey>> removeIfPredicate = entry ->
entry.getValue().isExpired();

counter.set(0);
cacheKeyToDataFeedKeyMap.entrySet().removeIf(
PredicateUtil.countingPredicate(counter, removeIfPredicate));
PredicateUtil.countingPredicate(counter, entry -> entry.getValue().isExpired()));
LOGGER.debug("Removed {} cacheKeyToDataFeedKeyMap entries", counter);

counter.set(0);
subjectIdToDataFeedKeyMap.entrySet().removeIf(
PredicateUtil.countingPredicate(counter, removeIfPredicate));
LOGGER.debug("Removed {} subjectIdToDataFeedKeyMap entries", counter);
accountIdToDataFeedKeyMap.forEach((accountId, cachedHashedDataFeedKeys) -> {
cachedHashedDataFeedKeys.removeIf(
PredicateUtil.countingPredicate(counter, CachedHashedDataFeedKey::isExpired));
});
LOGGER.debug("Removed {} cachedHashedDataFeedKey items from subjectIdToDataFeedKeyMap", counter);

counter.set(0);

Expand All @@ -158,20 +177,20 @@ public void removeKeysForFile(final Path sourceFile) {
if (sourceFile != null) {
LOGGER.info("Evicting dataFeedKeys for sourceFile {}", sourceFile);
final AtomicInteger counter = new AtomicInteger();
final Predicate<Entry<?, CachedHashedDataFeedKey>> removeIfPredicate = entry -> {
final boolean doRemove = Objects.equals(
sourceFile, entry.getValue().getSourceFile());
if (doRemove) {
counter.incrementAndGet();
}
return doRemove;
};
final Predicate<CachedHashedDataFeedKey> removeIfPredicate = cachedKey -> Objects.equals(
sourceFile, cachedKey.getSourceFile());

cacheKeyToDataFeedKeyMap.entrySet().removeIf(removeIfPredicate);
cacheKeyToDataFeedKeyMap.entrySet().removeIf(PredicateUtil.countingPredicate(
counter,
entry -> removeIfPredicate.test(entry.getValue())));
LOGGER.debug("Removed {} cacheKeyToDataFeedKeyMap entries", counter);
LOGGER.info("Evicted {} dataFeedKeys for sourceFile {}", counter, sourceFile);
counter.set(0);
subjectIdToDataFeedKeyMap.entrySet().removeIf(removeIfPredicate);
accountIdToDataFeedKeyMap.forEach((accountId, cachedHashedDataFeedKeys) -> {
cachedHashedDataFeedKeys.removeIf(PredicateUtil.countingPredicate(
counter, removeIfPredicate));
});

LOGGER.debug("Removed {} subjectIdToDataFeedKeyMap entries", counter);
}
}
Expand Down
6 changes: 4 additions & 2 deletions ...ve/stroom-receive-common/src/main/java/stroom/receive/common/DataFeedKeyUserIdentity.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@
*/
public class DataFeedKeyUserIdentity implements UserIdentity {

public static final String SUBJECT_ID_PREFIX = "data-feed-key-";

private final String subjectId;
private final String displayName;

public DataFeedKeyUserIdentity(final HashedDataFeedKey hashedDataFeedKey) {
Objects.requireNonNull(hashedDataFeedKey);
this.subjectId = hashedDataFeedKey.getSubjectId();
this.displayName = hashedDataFeedKey.getDisplayName();
this.subjectId = SUBJECT_ID_PREFIX + hashedDataFeedKey.getAccountId();
this.displayName = subjectId;
}

@Override
Expand Down
58 changes: 31 additions & 27 deletions ...-receive/stroom-receive-common/src/main/java/stroom/receive/common/HashedDataFeedKey.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@
import java.util.Map;
import java.util.Objects;

/**
* Represents the hashed form of a Data Feed Key, i.e. where we only have the
* hash of the key and not the key itself.
*/
@JsonPropertyOrder(alphabetic = true)
public class HashedDataFeedKey {

Expand All @@ -25,13 +29,13 @@ public class HashedDataFeedKey {
@JsonPropertyDescription("The hash algorithm ID used to hash the datafeed key. A zero padded 3 digit number.")
private final String hashAlgorithmId;

@JsonProperty
@JsonPropertyDescription("The unique subject ID of the user associated with the datafeed key.")
private final String subjectId;

@JsonProperty
@JsonPropertyDescription("A more human friendly display form of the user identity. May be null.")
private final String displayName;
// @JsonProperty
// @JsonPropertyDescription("The unique subject ID of the user associated with the datafeed key.")
// private final String subjectId;
//
// @JsonProperty
// @JsonPropertyDescription("A more human friendly display form of the user identity. May be null.")
// private final String displayName;

@JsonProperty
@JsonPropertyDescription("The unique ID for the account sending the data to stroom. " +
Expand All @@ -51,15 +55,15 @@ public class HashedDataFeedKey {
@JsonCreator
public HashedDataFeedKey(@JsonProperty("hash") final String hash,
@JsonProperty("hashAlgorithmId") final String hashAlgorithmId,
@JsonProperty("subjectId") final String subjectId,
@JsonProperty("displayName") final String displayName,
// @JsonProperty("subjectId") final String subjectId,
// @JsonProperty("displayName") final String displayName,
@JsonProperty("accountId") final String accountId,
@JsonProperty("streamMetaData") final Map<String, String> streamMetaData,
@JsonProperty("expiryDateEpochMs") final long expiryDateEpochMs) {
this.hash = hash;
this.hashAlgorithmId = hashAlgorithmId;
this.subjectId = subjectId;
this.displayName = displayName;
// this.subjectId = subjectId;
// this.displayName = displayName;
this.accountId = accountId;
this.streamMetaData = streamMetaData;
this.expiryDateEpochMs = expiryDateEpochMs;
Expand All @@ -75,17 +79,17 @@ public String getHashAlgorithmId() {
return hashAlgorithmId;
}

@NotBlank
public String getSubjectId() {
return subjectId;
}
// @NotBlank
// public String getSubjectId() {
// return subjectId;
// }

/**
* May be null.
*/
public String getDisplayName() {
return displayName;
}
// /**
// * May be null.
// */
// public String getDisplayName() {
// return displayName;
// }

@NotBlank
public String getAccountId() {
Expand Down Expand Up @@ -123,8 +127,8 @@ public boolean equals(final Object object) {
return expiryDateEpochMs == that.expiryDateEpochMs
&& Objects.equals(hash, that.hash)
&& Objects.equals(hashAlgorithmId, that.hashAlgorithmId)
&& Objects.equals(subjectId, that.subjectId)
&& Objects.equals(displayName, that.displayName)
// && Objects.equals(subjectId, that.subjectId)
// && Objects.equals(displayName, that.displayName)
&& Objects.equals(accountId, that.accountId)
&& Objects.equals(streamMetaData, that.streamMetaData);
}
Expand All @@ -133,8 +137,8 @@ public boolean equals(final Object object) {
public int hashCode() {
return Objects.hash(hash,
hashAlgorithmId,
subjectId,
displayName,
// subjectId,
// displayName,
accountId,
streamMetaData,
expiryDateEpochMs);
Expand All @@ -145,8 +149,8 @@ public String toString() {
return "DataFeedKey{" +
"hash='" + hash + '\'' +
", hashAlgorithmId='" + hashAlgorithmId + '\'' +
", subjectId='" + subjectId + '\'' +
", displayName='" + displayName + '\'' +
// ", subjectId='" + subjectId + '\'' +
// ", displayName='" + displayName + '\'' +
", accountId='" + accountId + '\'' +
", streamMetaData=" + streamMetaData +
", expiryDateEpochMs=" + expiryDateEpochMs +
Expand Down
3 changes: 3 additions & 0 deletions ...receive/stroom-receive-common/src/main/java/stroom/receive/common/HashedDataFeedKeys.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
import java.util.List;
import java.util.Objects;

/**
* A collection of {@link HashedDataFeedKey}
*/
@JsonPropertyOrder(alphabetic = true)
public class HashedDataFeedKeys {

Expand Down
6 changes: 0 additions & 6 deletions ...ceive/stroom-receive-common/src/test/java/stroom/receive/common/DataFeedKeyGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ public static KeyWithHash generateRandomKey(final String subjectId,
return new KeyWithHash(key, new HashedDataFeedKey(
hasher.hash(key),
hasher.getAlgorithm().getUniqueId(),
subjectId,
displayName,
accountId,
attributeMap,
expiry.toEpochMilli()));
Expand All @@ -45,8 +43,6 @@ public static KeyWithHash generateFixedTestKey1() {
final HashedDataFeedKey hashedDataFeedKey1 = new HashedDataFeedKey(
hasher.hash(key1),
hasher.getAlgorithm().getUniqueId(),
"datafeed-key-user1",
"user 1",
"1234",
Map.of(
"key1", "val1",
Expand All @@ -67,8 +63,6 @@ public static KeyWithHash generateFixedTestKey2() {
final HashedDataFeedKey hashedDataFeedKey2 = new HashedDataFeedKey(
hasher.hash(key2),
hasher.getAlgorithm().getUniqueId(),
"datafeed-key-user2",
"user 2",
"6789",
Map.of(
"key3", "val3",
Expand Down
4 changes: 2 additions & 2 deletions ...stroom-receive-common/src/test/java/stroom/receive/common/TestDataFeedKeyServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,9 @@ void authenticate_validAndKnownKey() {

final UserIdentity userIdentity = optUserIdentity.get();
assertThat(userIdentity.getSubjectId())
.isEqualTo(hashedDataFeedKey.getSubjectId());
.isEqualTo(DataFeedKeyUserIdentity.SUBJECT_ID_PREFIX + hashedDataFeedKey.getAccountId());
assertThat(userIdentity.getDisplayName())
.isEqualTo(hashedDataFeedKey.getDisplayName());
.isEqualTo(userIdentity.getSubjectId());
}

@Test
Expand Down
4 changes: 0 additions & 4 deletions ...ive/stroom-receive-common/src/test/java/stroom/receive/common/TestHashedDataFeedKeys.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,6 @@ void testSerde() throws IOException {
final HashedDataFeedKey hashedDataFeedKey1 = new HashedDataFeedKey(
hasher.hash(key1),
hasher.getAlgorithm().getUniqueId(),
"user1",
"user 1",
"system 1",
Map.of(
"key1", "val1",
Expand All @@ -73,8 +71,6 @@ void testSerde() throws IOException {
final HashedDataFeedKey hashedDataFeedKey2 = new HashedDataFeedKey(
hasher.hash(key2),
hasher.getAlgorithm().getUniqueId(),
"user2",
"user 2",
"system 2",
Map.of(
"key3", "val3",
Expand Down
4 changes: 1 addition & 3 deletions ...room-receive-common/src/test/java/stroom/receive/common/TestRequestAuthenticatorImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,7 @@ Stream<DynamicTest> authenticate_tokenEnabled() {
final UserIdentity dataFeedKeyUser = new DataFeedKeyUserIdentity(new HashedDataFeedKey(
"my hash",
DataFeedKeyHashAlgorithm.ARGON2.getDisplayValue(),
"MySubjectId",
"My Display Name",
"My System Name",
"MyAccountId",
null,
Long.MAX_VALUE));

Expand Down

0 comments on commit 631d4e4

Please sign in to comment.