Skip to content
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

added create and delete methods for indices, these should be moved in… #8

Merged
merged 34 commits into from
Dec 19, 2018

Conversation

lauraerinmann
Copy link
Collaborator

…to a refactor for CORE.

import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a biggie, but I'd rather just use Map.Entry rather than importing it and using it as Entry. We already imported Map.

public interface ProjectIndex {

void create(String index) throws IOException;

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also maybe add an update method.

this.client = client;
}

@Override
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these overriding anything?

import org.springframework.stereotype.Component;

@Component
public class CommitElasticDAOImpl extends BaseElasticDAOImpl implements CommitIndexDAO {


public List<Map<String, Object>> findByIndexIds(Set<String> indexIds) {
return null;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's start adding TODOs to these so we can find them easier.

public void deleteAll(String index, Collection<? extends BaseJson> jsons) throws IOException {
BulkRequest bulkIndex = new BulkRequest();
for (BaseJson json : jsons) {
bulkIndex.add(new DeleteRequest(index, null, json.getId()));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

json.getIndexId() instead of getId()

public void indexAll(String index, Collection<? extends BaseJson> jsons) throws IOException {
BulkRequest bulkIndex = new BulkRequest();
for (BaseJson json : jsons) {
bulkIndex.add(new IndexRequest((index)).source(json));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IndexRequest needs type and id param, add "_doc" and json.getIndexId()


public void index(String index, BaseJson json) throws IOException {
client.index(new IndexRequest(index).source(json), RequestOptions.DEFAULT);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add "_doc" and json.getIndexId() to IndexRequest

}

public void deleteById(String index, String indexId) throws IOException {
client.delete(new DeleteRequest(index, null, indexId), RequestOptions.DEFAULT);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think all the type arguments still needs an actual type, elastic suggests just use "_doc"

@Override
public void create(String index) throws IOException {
CreateIndexRequest commitIndex = new CreateIndexRequest(index + "_commit");
commitIndex.mapping(getCommitMapAsString(), XContentType.JSON);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs the type, use "_doc" as first argument

}

private String getCommitMapAsString() {
return "{\"properties\":{\"added\":{\"properties\":{\"id\":{\"type\":\"keyword\"},\"_indexId\":{\"type\":\"keyword\"},\"type\":{\"type\":\"keyword\"}}},\"updated\":{\"properties\":{\"id\":{\"type\":\"keyword\"},\"_indexId\":{\"type\":\"keyword\"},\"previousIndexId\":{\"type\":\"keyword\"},\"type\":{\"type\":\"keyword\"}}},\"deleted\":{\"properties\":{\"id\":{\"type\":\"keyword\"},\"previousIndexId\":{\"type\":\"keyword\"},\"type\":{\"type\":\"keyword\"}}},\"id\":{\"type\":\"keyword\"},\"_refId\":{\"type\":\"keyword\"},\"_creator\":{\"type\":\"keyword\"},\"_created\":{\"type\":\"date\",\"format\":\"yyyy-MM-dd'T'HH:mm:ss.SSSZ\"},\"_projectId\":{\"type\":\"keyword\"},\"_indexId\":{\"type\":\"keyword\"}}}";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add {"_doc": to the front and } to the back

@HuiJun HuiJun merged commit 06b3bc6 into develop Dec 19, 2018
@HuiJun HuiJun deleted the feature/MMS-1003 branch December 19, 2018 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants