Skip to content

Commit

Permalink
Java: Connectors - Memory - Unit test coverage microsoft#1598 (micros…
Browse files Browse the repository at this point in the history
…oft#1795)

### Motivation and Context
<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  2. What problem does it solve?
  3. What scenario does it contribute to?
  4. If it fixes an open issue, please link to the issue here.
-->
Java: Connectors - Memory - Unit test coverage microsoft#1598 

### Description
<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->
Run unit tests with coverage and add additional tests to try to increase
coverage.
Added positive tests cases for AzureCognitiveSearchMemory. Negative
tests and coverage need to be addressed.
Because this is somewhat time consuming, I suggest that be done in a
separate PR rather than hold this PR.
I have created [Java: Increase coverage of
AzureCognitiveSearchMemoryTests
](https://github.com/microsoft/semantic-kernel/issues/1797) for that
purpose.

### Contribution Checklist
<!-- Before submitting this PR, please make sure: -->
- [ x] The code builds clean without any errors or warnings
- [x] The PR follows SK Contribution Guidelines
(https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
- [x] The code follows the .NET coding conventions
(https://learn.microsoft.com/dotnet/csharp/fundamentals/coding-style/coding-conventions)
verified with `dotnet format`
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄

---------

Co-authored-by: Luigi Montoya <yayodelta@gmail.com>
Co-authored-by: joe-braley <joebraley@microsoft.com>
Co-authored-by: Luigi96 <luiseduardom@microsoft.com>
Co-authored-by: Mark Wallace <127216156+markwallace-microsoft@users.noreply.github.com>
Co-authored-by: John Oliver <1615532+johnoliver@users.noreply.github.com>
  • Loading branch information
6 people authored Jul 5, 2023
1 parent 64e27b8 commit 93f1773
Show file tree
Hide file tree
Showing 11 changed files with 950 additions and 74 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,4 @@ public ZonedDateTime getTimestamp() {
return timestamp;
}

/**
* Checks if the data has a timestamp.
*
* @return {@code true} if the data has a timestamp; otherwise, {@code false}.
*/
public boolean hasTimestamp() {
return timestamp != null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public static MemoryRecord localRecord(
@Nullable String key,
@Nullable ZonedDateTime timestamp) {

boolean isReference = true;
boolean isReference = false;
String emptyString = "";
MemoryRecordMetadata metadata =
new MemoryRecordMetadata(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import reactor.util.function.Tuple2;

import java.util.Collection;
import java.util.List;

import javax.annotation.Nonnull;

Expand All @@ -26,7 +27,7 @@ public interface MemoryStore {
*
* @return An unmodifiable group of collection names.
*/
Mono<Collection<String>> getCollectionsAsync();
Mono<List<String>> getCollectionsAsync();

/**
* Determines if a collection exists in the data store.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,18 @@
<artifactId>jakarta.inject-api</artifactId>
<version>2.0.1.MR</version>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter</artifactId>
</dependency>
</dependencies>

</project>
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,13 @@ public AzureCognitiveSearchMemory(String endpoint, TokenCredential credentials)
.buildAsyncClient();
}

private AzureCognitiveSearchMemory(AzureCognitiveSearchMemory other) {
this._adminClient = other._adminClient;
AzureCognitiveSearchMemory(SearchIndexAsyncClient adminClient) {
this._adminClient = adminClient;
}

@Override
public AzureCognitiveSearchMemory copy() {
return new AzureCognitiveSearchMemory(this);
return new AzureCognitiveSearchMemory(this._adminClient);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public Mono<List<MemoryQueryResult>> searchAsync(

@Override
public Mono<List<String>> getCollectionsAsync() {
return Mono.error(new NotSupportedException("Pending implementation"));
return _storage.getCollectionsAsync();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,9 @@ public Mono<Boolean> doesCollectionExistAsync(@Nonnull String collectionName) {
}

@Override
public Mono<Collection<String>> getCollectionsAsync() {
return Mono.just(Collections.unmodifiableCollection(this._store.keySet()));
public Mono<List<String>> getCollectionsAsync() {
List<String> keys = new ArrayList<>(this._store.keySet());
return Mono.just(Collections.unmodifiableList(keys));
}

@Override
Expand Down Expand Up @@ -256,29 +257,4 @@ public MemoryStore build() {
}
}

/*
protected boolean TryGetCollection(
String name,
[NotNullWhen(true)] out ConcurrentDictionary<String,
MemoryRecord>? collection,
boolean create)
{
if (this._store.TryGetValue(name, out collection))
{
return true;
}
if (create)
{
collection = new ConcurrentDictionary<String, MemoryRecord>();
return this._store.TryAdd(name, collection);
}
collection = null;
return false;
}
private readonly ConcurrentDictionary<String,
ConcurrentDictionary<String, MemoryRecord>> _store = new();
*/
}
Loading

0 comments on commit 93f1773

Please sign in to comment.